vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
470 stars 75 forks source link

Exception with very small triangles #19

Closed bertt closed 5 years ago

bertt commented 5 years ago

Hi', I've noticed an ArgumentOutOfRangeException (Non-negative number required. Parameter name: count ) occurs when adding very small triangles (area <0.0000002) on model.CreateMeshes(mesh);

As the exception occurs in the final step it's a bit puzzling what's going on. Suggestion: improve error handling for this case?

vpenades commented 5 years ago

I understand that "mesh" is a MeshBuilder, right?

Can you confirm that all the primitives in the MeshBuilder have at least one triangle?

bertt commented 5 years ago

Here some sample code to reproduce:

            var p0 = new Point(4403.12831325084, 5497.3228336684406, -451.62756590108586);
            var p1 = new Point(4403.1283132596873, 5497.3228336591274, -451.62756593199413);
            var p2 = new Point(4392.54991199635, 5483.549242743291, -450.72132376581396);

            var triangle = new Triangle(p0, p1, p2);
            var normal = triangle.GetNormal();
            var material1 = new MaterialBuilder().WithDoubleSide(true).WithMetallicRoughnessShader().WithChannelParam("BaseColor", new Vector4(1, 1, 1, 1));
            var mesh = new MeshBuilder<VertexPositionNormal>("mesh");
            var prim = mesh.UsePrimitive(material1);
            prim.AddTriangle(
            new VertexPositionNormal((float)triangle.GetP0().X, (float)triangle.GetP0().Y, (float)triangle.GetP0().Z, normal.X, normal.Y, normal.Z),
            new VertexPositionNormal((float)triangle.GetP1().X, (float)triangle.GetP1().Y, (float)triangle.GetP1().Z, normal.X, normal.Y, normal.Z),
            new VertexPositionNormal((float)triangle.GetP2().X, (float)triangle.GetP2().Y, (float)triangle.GetP2().Z, normal.X, normal.Y, normal.Z));

            var model = ModelRoot.CreateModel();
            try
            {
                model.CreateMeshes(mesh);
            }
            catch(ArgumentOutOfRangeException e)
            {
                Console.WriteLine(e);
            }
vpenades commented 5 years ago

What is happening is that you're creating points with Double precission.

When you convert P0 and P1 to VertexPositionNormal, P0 and P1 are converted to Single precission, and they become exactly the same point.

So when you try to add the triangle, MeshBuilder detects that two of the three vertices are exactly the same, which would result in a degenerated triangle, and discards the triangle, resulting in an empty mesh.

Finally, CreateMeshes fails because it is not able to create a glTF mesh from an empty MeshBuilder, since empty meshes are not allowed by glTF.

In order to get early warnings, you can do these changes:

For vertices:

var mesh = new MeshBuilder<VertexPositionNormal>("mesh");
// this will tell MeshBuilder to throw exceptions instead
// of silently failing when invalid vertices are introduced.
mesh.VertexPreprocessor.SetDebugPreprocessors();

For triangles:

var triIndices = prim.AddTriangle(....);
// newer versions of the library return a tuple with the indices of the vertices,
// if the triangle is degenerated, it will silently fail and return (-1,-1,-1)
if (triIndices.Item1 < 0) throw new InvalidOperationException("Degenerated triangle");
bertt commented 5 years ago

ok thanks, thats helpful. In my case I want to draw each triangle with some unique color, so have to set mesh.UsePrimitive(material1) for each triangle (degenerated triangle or not). In this case I can't find a way to call model.CreateMeshes(mesh) without exception. If I draw all triangles with same color, all goes well.

Sample code, in this case I have 2 triangles, one valid, one degenerated. I think the problem there is now an empty primitive for second degenerated triangle. Maybe there is a way to check for degenerated triangles before creating primitive?

        private static void TestWithColors()
        {
            var valid = "4373.192624189425, 5522.678275192156, -359.8238015332605,4370.978060142137, 5522.723320999183, -359.89184701762827, 4364.615741107147, 5511.510615546256, -359.08922455413233";
            var degenerated = "4374.713581837248, 5519.741978117265, -360.87014389818034,4373.187151107471, 5521.493282925338, -355.70835120644153, 4373.187151107471, 5521.493282925338, -355.70835120644153";
            var triangles = GetTriangles(new List<string>() { valid, degenerated });

            CreateGltf(triangles);
        }

        private static void CreateGltf(List<Triangle> triangles)
        {
            var mesh = new MeshBuilder<VertexPositionNormal>("mesh");
            mesh.VertexPreprocessor.SetDebugPreprocessors();
            var r = new Random();

            foreach (var triangle in triangles)
            {
                var c = (float)r.Next(255);
                var material1 = new MaterialBuilder().WithDoubleSide(true).WithMetallicRoughnessShader().WithChannelParam("BaseColor", new Vector4(c / 255, c / 255, c / 255, 1));
                var prim = mesh.UsePrimitive(material1);

                var normal = triangle.GetNormal();
                try
                {
                    var indices = prim.AddTriangle(
                    new VertexPositionNormal((float)triangle.GetP0().X, (float)triangle.GetP0().Y, (float)triangle.GetP0().Z, normal.X, normal.Y, normal.Z),
                    new VertexPositionNormal((float)triangle.GetP1().X, (float)triangle.GetP1().Y, (float)triangle.GetP1().Z, normal.X, normal.Y, normal.Z),
                    new VertexPositionNormal((float)triangle.GetP2().X, (float)triangle.GetP2().Y, (float)triangle.GetP2().Z, normal.X, normal.Y, normal.Z));
                }
                catch (ArgumentException)
                {
                    // we get here for second triangle, do nothing... ;
                }
            }

            var model = ModelRoot.CreateModel();
            // next line still gives System.ArgumentOutOfRangeException' 
            // Additional information: Non - negative number required.
            model.CreateMeshes(mesh);
            model.UseScene("Default")
                .CreateNode()
                .WithMesh(model.LogicalMeshes[0]);
            model.SaveGLB("building_sharp.glb");
            model.SaveGLTF("building_sharp.gltf");
        }
vpenades commented 5 years ago

The problem is that although it is valid to create empty MeshBuilders, glTF specification considers empty glTF meshes as being invalid, I guess because they're considered a waste of resources.

Typically you use CreateMeshes to convert one or multiple MeshBuilders, to the corresponding counterparts as glTF Meshes, I believe the best thing I can do is that CreateMeshes should return null for every empty MeshBuilder.

And, in the case of MeshBuilder with empty Mesh primitives, to simply skip them.

I'll add the examples you posted as unit tests and see how can I solve the issue.

bertt commented 5 years ago

ok great thanks

bertt commented 5 years ago

update: in my project I remove the degenerated triangles before creating primitives, works good for me. But it would be nice if this library can detect degenerated triangles and skip them.

vpenades commented 5 years ago

MeshBuilder does detect degenerated triangles and removes them.

The problem is that creating a MeshBuilder and adding a degenerated triangle leads to an empty mesh, which causes an error later.

What the library needs to be able to do is to handle empty MeshBuilders.

vpenades commented 5 years ago

I've just published Alpha0011 packages, give them a try.

I also suggest you to use SharpGLTF.Scenes.SceneBuilder class of the Toolkit to build your scenes, you can find some usage examples here.

With SceneBuilder, now it is possible to create a complete glTF scenes, without requiring using the Schema2 namespace, and by using it it brings the possibility of further optimizations that are not possible by working directly with the Schema2 namespace.

bertt commented 5 years ago

I've upgraded to alpha0011 and no longer see the ArgumentOutOfRangeException in above testcode. Next I will take a look at using SharpGLTF.Scenes.SceneBuilder instead of Schema2.