vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
458 stars 72 forks source link

Normal vectors disregarded during comparison #92

Closed emackey closed 3 years ago

emackey commented 3 years ago

Apologies if this is "user error," this is my first time using this project.

I think I'm seeing an error where co-located vertices are merged together even if their normal vectors are different. Is this expected?

For example if I supply some flat faces from a cube, the resulting glTF merges the verts and smooths over the sharp corners.

emackey commented 3 years ago

Reproduce case:

using System;
using System.Numerics;

using SharpGLTF.Geometry;
using SharpGLTF.Materials;
using SharpGLTF.Schema2;

namespace Example1
{
    using VERTEX = SharpGLTF.Geometry.VertexTypes.VertexPositionNormal;

    class Program
    {
        static void Main(string[] args)
        {
            var material1 = new MaterialBuilder()
                .WithDoubleSide(true)
                .WithMetallicRoughnessShader()
                .WithChannelParam("BaseColor", new Vector4(1,0,0,1) );

            var mesh = new MeshBuilder<VERTEX>("mesh");

            var v0 = new VERTEX(15, 5, 0, 1, 0, 0);
            var v1 = new VERTEX(15, 5, 10, 1, 0, 0);
            var v2 = new VERTEX(15, 0, 10, 1, 0, 0);
            var v3 = new VERTEX(15, 0, 0, 1, 0, 0);

            var prim = mesh.UsePrimitive(material1);
            prim.AddTriangle(v0, v1, v3);
            prim.AddTriangle(v3, v1, v2);

            v0 = new VERTEX(15, 5, 10, 0, 0, 1);
            v1 = new VERTEX(0, 5, 10, 0, 0, 1);
            v2 = new VERTEX(0, 0, 10, 0, 0, 1);
            v3 = new VERTEX(15, 0, 10, 0, 0, 1);

            prim.AddTriangle(v0, v1, v3);
            prim.AddTriangle(v3, v1, v2);

            // create a scene

            var scene = new SharpGLTF.Scenes.SceneBuilder();

            scene.AddRigidMesh(mesh, Matrix4x4.Identity);

            // save the model in different formats

            var model = scene.ToGltf2();
            model.SaveGLB("mesh.glb");
            model.SaveGLTF("mesh.gltf");
        }
    }
}
vpenades commented 3 years ago

Ouch, you're right, there's a bug in this line:

https://github.com/vpenades/SharpGLTF/blob/0a1da6761fc2b88ec74c355b3e54da441f0577b5/src/SharpGLTF.Toolkit/Geometry/VertexBuilder.cs#L289

It should be fixed with this:

return a.Geometry.Equals(b.Geometry) && a.Material.Equals(b.Material) && a.Skinning.Equals(b.Skinning); 

I'll fix it in the code and upload new packages ASAP

emackey commented 3 years ago

That was amazingly fast, thank you!

vpenades commented 3 years ago

I happen to be around here today, actually, I'm in the middle of adding the mesh instancing to the library.

vpenades commented 3 years ago

@emackey I've added a third party tests projects, and I included your code as an unit test. If you ever want to add AGI extensions tests, that would be a good place to add them too.