Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using Manifold in Unity causes normals errors #1151

Open
strich opened this issue Feb 19, 2025 · 15 comments
Open

Using Manifold in Unity causes normals errors #1151

strich opened this issue Feb 19, 2025 · 15 comments

Comments

@strich
Copy link

strich commented Feb 19, 2025

I've been testing out Manifold for a usecase in a video game using the Unity engine. I've been able to setup bindings and figure out how to get mesh data to and from Manifold. However I've run into an issue where it looks like the normals are malformed:
Image

I'd love some help in understanding what I might be doing wrong.

Note I've had to convert the verts Y-Up → Z-Up as it appears Unity and Manifold operation in different formats. I also briefly toyed with triangle winding conversions, but it broke things - Could it be one or both of these aren't done correctly?

Prototype code in Unity:

public void Start()
{
    var timer = new System.Diagnostics.Stopwatch();
    timer.Start();
    
    var inputA = Manifold.Create(GetManifoldMeshFromMesh(SplitMeshA));
    var inputB = Manifold.Create(GetManifoldMeshFromMesh(SplitMeshB));

    Debug.Log($"InputA Status: {inputA.Status}.");
    Debug.Log($"InputB Status: {inputB.Status}.");
    (Manifold resultA, Manifold resultB) = Manifold.Split(inputA, inputB);

    Debug.Log($"Manifold Operation Time: {timer.ElapsedMilliseconds}ms.");
    resultB = resultB.AsOriginal();
    SplitMeshA.sharedMesh = GetMeshFromManifold(resultA, SplitMeshA.transform);
    SplitMeshB.sharedMesh = GetMeshFromManifold(resultB, SplitMeshB.transform);

    timer.Stop();
    Debug.Log($"Manifold Total Operation Time: {timer.ElapsedMilliseconds}ms.");
}

ManifoldNET.MeshGL GetManifoldMeshFromMesh(MeshFilter meshFilter)
{
    Dictionary<Vector3, uint> uniqueVerts = new Dictionary<Vector3, uint>();
    List<float> vertProperties = new List<float>();
    List<uint> triVerts = new List<uint>();
    var localToWorld = meshFilter.transform.localToWorldMatrix;
    var mesh = meshFilter.sharedMesh;
    foreach(var triIndex in mesh.triangles)
    {
        Vector3 v = mesh.vertices[triIndex];

        // Convert to world space
        v = localToWorld.MultiplyPoint3x4(v);

        // Convert Unity coordinates to Manifold's coordinate system (Y-Up → Z-Up)
        v = new Vector3(v.x, v.z, -v.y);

        if(!uniqueVerts.TryGetValue(v, out uint index))
        {
            index = (uint)uniqueVerts.Count;
            uniqueVerts[v] = index;
            vertProperties.Add(v.x);
            vertProperties.Add(v.y);
            vertProperties.Add(v.z);
        }
        triVerts.Add(index);
    }

    return new ManifoldNET.MeshGL(vertProperties.ToArray(), triVerts.ToArray());
}


Mesh GetMeshFromManifold(Manifold manifold, Transform originalTransform)
{
    var mesh = new Mesh();
    Vector3[] verts = new Vector3[manifold.MeshGL.VerticesNumber];
    int vertOffset = 0;
    var worldToLocal = originalTransform.worldToLocalMatrix;
    var meshGLVerts = manifold.MeshGL.VerticesProperties;
    var meshGLVertCount = manifold.MeshGL.VerticesNumber;
    for(int i = 0; i < meshGLVertCount; i++)
    {
        verts[i] = new Vector3(
            meshGLVerts[vertOffset],
            -meshGLVerts[vertOffset + 2], // Flip Y back
            meshGLVerts[vertOffset + 1]  // Swap Y and Z back
        );
        // Convert back to local space
        verts[i] = worldToLocal.MultiplyPoint3x4(verts[i]);
        vertOffset += 3;
    }
    mesh.vertices = verts;
    mesh.triangles = manifold.MeshGL.TriangleVertices;
    //mesh.Optimize();
    mesh.RecalculateNormals();
    mesh.RecalculateTangents();
    return mesh;
}
@pca006132
Copy link
Collaborator

I can't see where you use the normal computed in manifold in your prototype code. And I think you should be careful with the vertOffset thing: it should depend on the number of properties you have, e.g. if you want to export normals in vertProperties.

@strich
Copy link
Author

strich commented Feb 19, 2025

I can't see where you use the normal computed in manifold in your prototype code. And I think you should be careful with the vertOffset thing: it should depend on the number of properties you have, e.g. if you want to export normals in vertProperties.

Oh thanks! Can you provide some pointers as to how to retrieve set and retrieve the normals in Manifold? I've been unsuccessful in learning how in the samples/docs sorry.

@strich
Copy link
Author

strich commented Feb 19, 2025

Okay I've resolved most of it now:

Image

I passed in the normals and updated the properties to 6. It seems the cut looks fine, but some of the untouched model on the sides looks broken.

@pca006132
Copy link
Collaborator

Looking at it, the C# fork seems to be a bit outdated (6 months ago). I forgot if we fixed any bug related to this in the last 6 months. I guess it should be possible to patch our current head using the updates in the fork and build a C# library.

@pca006132
Copy link
Collaborator

You probably need to call CalculateNormals to populate the normals before you can export them to MeshGL. And I am not sure what RecalculateNormals in unity does...

@strich
Copy link
Author

strich commented Feb 19, 2025

Looking at it, the C# fork seems to be a bit outdated (6 months ago). I forgot if we fixed any bug related to this in the last 6 months. I guess it should be possible to patch our current head using the updates in the fork and build a C# library.

Actually I took the fork bindings and used the latest build here. So Manifold itself is up to date and luckily the API doesn't appear to have changed.

You probably need to call CalculateNormals to populate the normals before you can export them to MeshGL. And I am not sure what RecalculateNormals in unity does...

RecalculateNormals in Unity is typically used when making meshes at runtime, but its likely unnecessary here.

@strich
Copy link
Author

strich commented Feb 19, 2025

You probably need to call CalculateNormals to populate the normals before you can export them to MeshGL. And I am not sure what RecalculateNormals in unity does...

Okay I see - Yes the bindings didn't have it. But I've added it now. It's not clear to me how its meant to be used - Should I call CalculateNormals before or after the boolean operation?

I've tried both with resultA.CalculateNormals(3, 30); and doesn't seem to change anything.

@pca006132
Copy link
Collaborator

Iirc you should use normalIdx = 0 for this and GetMeshGL, but I am not sure if I am remembering the correct thing. Maybe you can wait for @elalish for the definitive answer, I am not that familiar with this.

@strich
Copy link
Author

strich commented Feb 19, 2025

Iirc you should use normalIdx = 0 for this and GetMeshGL, but I am not sure if I am remembering the correct thing. Maybe you can wait for @elalish for the definitive answer, I am not that familiar with this.

You might be right. I have tried it with 0 to no avail sadly.

@elalish
Copy link
Owner

elalish commented Feb 20, 2025

Now that you've passed in the normals, can you by chance show a render that actually draws your normals as little vectors? I'd guess that you used to have multiple normals per vert and now you don't. You shouldn't recalculate any normals if they're being passed in properly. You'll need to make use of the merge vectors in MeshGL (or the Merge() function if necessary).

@strich
Copy link
Author

strich commented Feb 21, 2025

This is the before cut:

Image

This is after:

Image

I am not currently recalculating any normals with Unity methods.

I can confirm that my code both in and out of Manifold is only applying 1 normal per vert. So that's likely why. I'm not sure how I should be writing to Manifold with multiple normals per vert?

@elalish
Copy link
Owner

elalish commented Feb 22, 2025

Yes, this is the issue, and I've gotten enough questions about how to use MeshGL now that I really need to write some more docs. I just reread what I have and it's pathetically sparse. There's a bit on the wiki: https://github.com/elalish/manifold/wiki/Manifold-Library#vertex-properties

Just like with GL buffers, anywhere you have different properties meeting at the same vertex, that vertex must be duplicated. The only difference is we add the mergeFromVert and mergeToVert vectors that tell us which verts have in fact been duplicated, so we can stitch the manifold back together losslessly. Does that make sense?

@strich
Copy link
Author

strich commented Feb 24, 2025

Okay that makes sense in theory to me. It's still not quite clear to me exactly what should go in the structures having read the code comments:

  /// Optional: A list of only the vertex indicies that need to be merged to
  /// reconstruct the manifold.
  std::vector<I> mergeFromVert;
  /// Optional: The same length as mergeFromVert, and the corresponding value
  /// contains the vertex to merge with. It will have an identical position, but
  /// the other properties may differ.
  std::vector<I> mergeToVert;

Right now per my code, I'm iterating over the triangle indices and giving just those vectors and normals. These same vector indices should go into mergeFromVert?
I suppose my code should be refactoring to iterate over the vertex list (which in Unity is the same length as normals list, so presumably duped), and fill the property list with verts and normals if they're in the triangle indices list. Then any that are not, go into mergeToVert?

Here is my attempt at it below. It is untested because I assume I need to run Merge() after populating the merge arrays, which crashes Unity at the moment - This could be my binding code maybe but I don't know. I noted that without Merge() they error out with NotManifold which is probably expected?

    public static ManifoldNET.MeshGL GetManifoldMeshFromMesh(MeshFilter meshFilter)
    {
        Mesh mesh = meshFilter.sharedMesh;
        Transform transform = meshFilter.transform;

        var vertProperties = new List<float>();
        var triVerts = new List<uint>();
        var mergeFromVert = new List<int>();
        var mergeToVert = new List<int>();

        Matrix4x4 localToWorld = transform.localToWorldMatrix;

        Dictionary<(Vector3 pos, Vector3 normal, Vector2 uv), uint> uniqueVerts = new Dictionary<(Vector3, Vector3, Vector2), uint>();

        for(int i = 0; i < mesh.triangles.Length; i++)
        {
            int triIndex = mesh.triangles[i];
            Vector3 v = mesh.vertices[triIndex];
            Vector3 normal = mesh.normals[triIndex];
            Vector2 uv = mesh.uv.Length > 0 ? mesh.uv[triIndex] : Vector2.zero;

            // Convert to world space
            v = localToWorld.MultiplyPoint3x4(v);
            normal = localToWorld.MultiplyVector(normal).normalized;

            // Convert Unity Y-Up to Manifold Z-Up
            v = new Vector3(v.x, v.z, -v.y);
            normal = new Vector3(normal.x, normal.z, -normal.y);

            var key = (v, normal, uv);
            if(!uniqueVerts.TryGetValue(key, out uint index))
            {
                index = (uint)uniqueVerts.Count;
                uniqueVerts[key] = index;

                vertProperties.Add(v.x);
                vertProperties.Add(v.y);
                vertProperties.Add(v.z);
            }
            else
            {
                // Track duplicate vertices that should be merged
                mergeFromVert.Add((int)index);
                mergeToVert.Add((int)uniqueVerts[key]);
            }

            triVerts.Add(index);
        }

        // Reverse triangle winding for Manifold
        //for(int i = 0; i < triVerts.Count; i += 3)
        //{
        //    (triVerts[i + 1], triVerts[i + 2]) = (triVerts[i + 2], triVerts[i + 1]);
        //}

        var meshGL = new ManifoldNET.MeshGL(vertProperties.ToArray(), triVerts.ToArray());
        meshGL.MergeFromVert = mergeFromVert.ToArray();
        meshGL.MergeToVert = mergeToVert.ToArray();
        meshGL.Merge();
        return meshGL;
    }
    public static Mesh GetMeshFromManifold(Manifold manifold, Transform originalTransform)
    {
        var mesh = new Mesh();
        var meshGL = manifold.GetMeshGL(0);
        
        int vertCount = meshGL.VerticesNumber;
        int propCount = meshGL.PropertiesNumber;
        int vertOffset = 0;
        var worldToLocal = originalTransform.worldToLocalMatrix;
        var verts = new Vector3[vertCount];
        var normals = new Vector3[vertCount];
        var meshGLVerts = meshGL.VerticesProperties;

        // Convert from Manifold back to Unity
        for(int i = 0; i < vertCount; i++)
        {
            verts[i] = new Vector3(
                meshGLVerts[vertOffset],      // X
                -meshGLVerts[vertOffset + 2], // Flip Y back
                meshGLVerts[vertOffset + 1]   // Swap Y and Z back
            );
            verts[i] = worldToLocal.MultiplyPoint3x4(verts[i]);

            if(propCount > 3)
            {
                normals[i] = new Vector3(
                    meshGLVerts[vertOffset + 3],
                    -meshGLVerts[vertOffset + 5], // Flip Y back
                    meshGLVerts[vertOffset + 4]   // Swap Y and Z back
                ).normalized;
            }

            vertOffset += propCount;
        }

        // Apply merging information to remove duplicate vertices
        var mergeFrom = meshGL.MergeFromVert;
        var mergeTo = meshGL.MergeToVert;
        if(mergeFrom.Length > 0 && mergeTo.Length > 0)
        {
            for(int i = 0; i < mergeFrom.Length; i++)
            {
                int fromIndex = (int)mergeFrom[i];
                int toIndex = (int)mergeTo[i];

                // Merge vertex positions and normals
                verts[fromIndex] = verts[toIndex];
                normals[fromIndex] = normals[toIndex];
            }
        }

        // Assign data to Unity Mesh
        mesh.vertices = verts;
        mesh.normals = normals;
        mesh.triangles = meshGL.TriangleVertices.Select(t => (int)t).ToArray();

        // Ensure correct normals and tangents for shading
        mesh.RecalculateBounds();
        mesh.RecalculateTangents(); // Needed for correct lighting in normal-mapped materials

        return mesh;
    }

@elalish
Copy link
Owner

elalish commented Feb 24, 2025

So, it looks like Unity (as with most graphics programs) already stores their info in effectively MeshGL format - simply missing any merging / stitching info since they generally don't care. If you're trying to build that with a dictionary (checking floating point equality of positions), it's already inexact, so you might as well not bother and simply rely on Merge to do it for you.

It looks like your GetMeshFromManifold function is right, but just remove the stuff about the merging - Unity doesn't need it. Ideally there'd be a way to save it as a side-channel to make sending it back to manifold easier, but it's not really necessary.

For GetManifoldMeshFromMesh, just remove all the unique dictionary stuff. But set your meshGL.PropertiesNumber = 8 and fill in vertProperties the same way you read from it in your other function - looping across the length of mesh.vertices, mesh.normals and mesh.uv instead of mesh.triangles. Call Merge() at the end like you do now, and it should work, unless you have separate surfaces whose verts happen to coincide.

@strich
Copy link
Author

strich commented Feb 28, 2025

I am unfortunately running into an issue here where Merge() is causing a crash. Annoyingly I haven't been able to load the crash exception up to review exactly why. I've raised another ticket to review this issue further: #1173 (comment)

Repository owner locked and limited conversation to collaborators Feb 28, 2025
@elalish elalish converted this issue into a discussion Feb 28, 2025
@elalish elalish reopened this Feb 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants