Support for Morphed `Texcoord_n` and `Color_n` Attributes

[ptasev]

Looks like the glTF spec added the optional Texcoord_n, and Color_n morph targets. Should this be supported in IMorphTargetBuilder and IMeshDecoder?

Client implementations SHOULD support at least three attributes — POSITION, NORMAL, and TANGENT — for morphing. Client implementations MAY optionally support morphed TEXCOORD_n and/or COLOR_n attributes. https://www.khronos.org/registry/glTF/specs/2.0/glTF-2.0.html#morph-targets

I have a file format I'm converting to that supports Texcoord_0 and Color_0 morphing, however it's not an often used feature.

[vpenades]

yes, it's a useful feature, and I'm surprised it was not part of the spec since the very beginning.

Although I don't think I'll be able to work on it for the next release, I'll add it to the roadmap, though.

[ptasev]

I noticed a bug with the MorphTargetBuilder. https://github.com/vpenades/SharpGLTF/blob/f34511e1d656c006fa2b68311ed0c4b5de6dbf1b/src/SharpGLTF.Toolkit/Geometry/MorphTargetBuilder.cs#L88-L103

On Line 90, it checks for geometry to be default, but this is bad in the case that material is not default. Should probably check that both geometry and material are default before entering the if block.

[ptasev]

I was trying to create a unit test for a potential bug, but I ran into a different exception with accessors in my code. Do you see an issue with my code or is this another bug?

        [Test]
        public void TestMorphColorTargets2()
        {
            // create material
            var material = new MaterialBuilder("mat1")
                .WithDoubleSide(true)
                .WithMetallicRoughnessShader();
            var material2 = new MaterialBuilder("mat2")
                .WithDoubleSide(true)
                .WithMetallicRoughnessShader();

            // create a mesh with two primitives, one for each material

            var triangle = new MeshBuilder<VertexPosition, VertexColor1>("mesh");

            var prim = triangle.UsePrimitive(material);
            var redColor = new Vector4(1f, 0f, 0f, 1f);
            prim.AddTriangle(new VBColor1(new VertexPosition(-10, 0, 0), redColor),
                new VBColor1(new VertexPosition(10, 0, 0), redColor),
                new VBColor1(new VertexPosition(0, 10, 0), redColor));

            var prim2 = triangle.UsePrimitive(material2);
            prim2.AddTriangle(new VBColor1(new VertexPosition(-10, 0, 0), redColor),
                new VBColor1(new VertexPosition(10, 0, 0), redColor),
                new VBColor1(new VertexPosition(0, 10, 0), redColor));

            // create a morph target that will change the color from red to green only for prim2
            var greenColor = new Vector4(0f, 1f, 0f, 1f);
            foreach (var p in triangle.Primitives)
            {
                for (var i = 0; i < p.Vertices.Count; ++i)
                {
                    var oldVertexPosition = p.Vertices[i];
                    var greenMat = new VertexColor1(greenColor);

                    ((IPrimitiveBuilder)p).SetVertexDelta(0, i, default,
                        ReferenceEquals(p, prim2)
                            ? greenMat.Subtract(oldVertexPosition.Material)
                            : VertexMaterialDelta.Zero);
                }
            }

            // create a scene
            var scene = new Scenes.SceneBuilder();
            scene.AddRigidMesh(triangle, Matrix4x4.Identity);

            // save the model in different formats
            var model = scene.ToGltf2();
            var animation = model.CreateAnimation();

            // create a morph channel
            animation.CreateMorphChannel(model.LogicalNodes[0],
                new Dictionary<float, float[]>
                {
                    { 0f, new[] { 0f } },
                    { 1f, new[] { 1f } }
                }, 1);

            // evaluate triangles at animation 0.5, and get the color of the first pixel of the first triangle
            var triangles = Schema2.Toolkit
                .EvaluateTriangles(model.DefaultScene, null, model.LogicalAnimations[0], 0.5f)
                .ToArray();

            // Assert
            var morphedColor = triangles[1].A.GetMaterial().GetColor(0);
            Assert.AreEqual(0.5f, morphedColor.X);
            Assert.AreEqual(0.5f, morphedColor.Y);
            Assert.AreEqual(0, morphedColor.Z);
            Assert.AreEqual(1, morphedColor.W);

            morphedColor = triangles[0].A.GetMaterial().GetColor(0);
            Assert.AreEqual(redColor, morphedColor);
        }

[vpenades]

@ptasev that exception is a bug.

What's going on is that by default, the library bakes the vertices into strided vertices. But strided vertices are not well suited for meshes that contain morph targets, so the library automatically resolves to split vertices (one accessor per vertex attribute) when it encounters geometry with morph targets.

But for some reason, it's failing to notice the mesh has morph targets.

EDIT: Looking further into the test, you're setting morph targets for one of the primitives, but not for the other. I have code that prevents setting a morph target with zeroes, which is working great for naive use cases, but in meshes with multiple materials this is causing issues

I'll look into it

[vpenades]

@ptasev I've just pushed a small improvement that covers this and lets the test pass; I would suggest you to add your test to SharpGltf.ThirdParty.Tests under your name.cs

anyway, morph targets definitely require more test cases so I'll keep reviewing the code related to it

[ptasev]

Thanks, I'll add the test soon. It seems the test passes now, but if I add the following lines at the end then it fails:

AttachmentInfo
    .From("ColorMorphingMultiPrim.glb")
    .WriteFile(f => model.Save(f.FullName, new WriteSettings() { Validation = ValidationMode.Skip }));
AttachmentInfo
    .From("ColorMorphingMultiPrim.gltf")
    .WriteFile(f => model.Save(f.FullName, new WriteSettings() { Validation = ValidationMode.Skip }));

[vpenades]

It seems my previous fix didn't provide the expected results.

Forcing the exporter to write a non existing morph target in the first primitive produces this output:

"meshes": [
    {
      "name": "mesh",
      "primitives": [
        {
          "attributes": {
            "POSITION": 0,
            "COLOR_0": 1
          },
          "indices": 2,
          "material": 0,
          "targets": []     <-- the empty array
        },
        {
          "attributes": {
            "POSITION": 3,
            "COLOR_0": 4
          },
          "indices": 5,
          "material": 1,
          "targets": [    <-- morph targets of the 2nd primitive
            {
              "COLOR_0": 6
            }
          ]
        }
      ]
    }

The first primitive has an empty targets array which is what's producing the problem, and the second one is correct.

Now, the question is, should all primitives in a mesh have target matching target entries, even if empty? in which case fixing the array would be trivial.

Otherwise, if all primitives need to have matching morph targets, things will get more complicated... I guess we'll have to dig into the specs

[ptasev]

The validator definitely complains that there needs to be the same number of morph targets in every primitive. This makes sense since the weights array is the same across the entire mesh.

Yes, I'll need to read into the spec for what to do if there is no change for a primitive. Seems wasteful to have to put zeros if empty array (or array of empty objects for each morph target) doesn't work.

[vpenades]

It seems we're not alone: https://github.com/KhronosGroup/glTF/issues/2154

[ptasev]

Here's what I could find in the spec:

So I guess we can make the array with a bunch of empty objects and make sure it's the same number across all primitives, if my understanding is correct:

"targets": [ {} ]

[vpenades]

"targets": [ {} ]

I think the problem is that, not only empty arrays are disallowed, also empty objects ?

[ptasev]

That I'm not sure of, but if the first part I highlighted is correct, then that means we don't need to include all attributes specified by the base mesh, and in that case it will interpreted as using original values. If we don't have to include all attributes, I take that to mean that we don't need to include any if the intent is to make no changes to original values.

But that's all theoretical from reading the spec.

[vpenades]

[ {} ] gives an error in gltf validator.

I would need an example from khronos to see which is the valid way of writing this, because from the specs its not clear at all

[ptasev]

That's contrary to their spec. I posted a comment on the issue in the gltf repo, so let's see what they say.

[vpenades]

I've taken MorphPrimitivesTest.gltf from the gltf sample models and edited out the target from one of the primitives, from:

"targets": [
            {
              "POSITION": 4
            }
          ],

to

"targets": [
            { }
          ],

gltf validator gives this error:

 "code": "EMPTY_ENTITY",
                "message": "Entity cannot be empty.",
                "severity": 0,
                "pointer": "/meshes/0/primitives/0/targets/0"

So it's clear there's some conflicting issues here.

I'm affraid there'll be no other choice than to hack some zeroed accessors in... but we'll wait for clarification

[ptasev]

Not sure if you saw this on Discord, but currently there's no way to get IMaterialTransform from DrawableInstance.

[ptasev]

Looks like if the morph target doesn't have position deltas, then calculating smooth normals fails (line 87). The position deltas is null.

Even if I put a check for null and return XYZ.Zero for position delta, line 88 will fail because Tangents check for normal deltas which is also null. It seems line 87 updates the base normals, but not the normal deltas. That makes sense, but overall, idk if it makes sense to generate normals and tangents for the morph targets. https://github.com/vpenades/SharpGLTF/blob/d9fdfa92b98f70afc579912908ea79b12bf3a03d/src/SharpGLTF.Core/Runtime/MeshDecoder.Schema2.cs#L60-L90

[vpenades]

@ptasev Technically, normals can be derived from the surface triangles, and the tangets can be derived from the surface UVs and the normals... and that is true for both source geometry and morph geometry.

In normal cirumstances, you usually create meshes in two ways: from other formats, where normals and tangents are usually available, or from procedurally generated geometry. In this latter case it's usually very painful to require the developer to generate its own normals (or maybe I am naive by wanting to be too helpful)... but as you noticed, this didn't take into account morph targets.

I'm not sure what to do in this case.... ideally, it could be possible to generate the normals and tangents for the morph targets, but it would require quite a lot of code and development time... so maybe the alternative for now is to simply hack it to not throw exceptions.

What do you think?

[ptasev]

Part of what makes the library so useful is a lot of helper classes on top of glTF to help work with the data. I think this is a good feature to have. MeshDecoder is a huge time saver.

My earlier comment is in regards to the case where morph targets don't have vertex deltas and normal deltas. It doesn't make sense to generate normals, and instead they can be copied from the base mesh if I'm not mistaken.

[vpenades]

Looks like if the morph target doesn't have position deltas, then calculating smooth normals fails (line 87). The position deltas is null.

@ptasev I couldn't replicate that scenario, could you write a test case for it?

I thick after the morph targets and a few other issues are resolved, I'll publish new nugets

[ptasev]

To reproduce, add this line to the end of the test I recently added to the repo: var aaa = model.LogicalMeshes[0].Decode(); https://github.com/vpenades/SharpGLTF/blob/master/tests/SharpGLTF.ThirdParty.Tests/PetarTasevTests.cs#L110

[vpenades]

I believe this issue is completed, so closing it down.