Three.js: GLTFLoader geometry.morphAttributes normal/position .name not being set

[Bug-Reaper]

Hey all, this is a bug with the GLTFLoader that's been an issue I keep bumping into.

The geometry.morphAttributes positions and normals never get their name's set. This screenshot shows it off best:

This can be easily confirmed by importing any .gltf/.glb file with morphTargets to a THREE.js scene. Console logging the relevant imported mesh will yield the same results as shown in the screenshot above. I'm including an example .glb file you can use that has a few morphTargets setup on a sphere.

Because the geometry.morphAttributes' normals & positions never get their names set, attempting to add any animations which target the morphInfluence by name will fail.

EX: An animation targeting .morphTargetInfluences[ morphTargetName ] will never animate.

I have a PR that can fix this and I'm going to prepare it for submission right now. Will want to double check with @donmccurdy to decide if my solution's approach is optimal.

Cheers
exampleGLB.zip

Three.js version

• [x] Dev
• [x] r116
• [x] All of them

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] macOS
• [ ] Linux
• [ ] Android
• [ ] iOS

Hardware Requirements (graphics card, VR Device, ...)

[donmccurdy]

I'm surprised that the animation system relies on BufferAttribute instances having particular names, rather than using the morphTargetDictionary? But similarly, calling .updateMorphTargets() on a mesh with unnamed BufferAttribute instances in its morph attributes does cause the dictionary to break.

We can fix this by:

A. Overwriting the names on the attributes in GLTFLoader, possibly losing names the user assigned to those attributes (although I've never seen a user do that)
B. Make the animation system less dependent on the attribute names, and use the dictionary instead.

I'm not sure which of those would be better.

[Bug-Reaper]

Gotto get the PR up. But it looks like we can just assign the names of the BufferAttribute instances of the geometry in the GLTFLoader as it creates the MorphTarget dictionary for the corresponding mesh.

It feels like a surprisingly clean solution, but take a look at the PR when it's up. I simply assumed this was a fuck up in the GLTFLoader so I didn't even look into why the animation system depended on those being set.

When I import the same model as a .fbx, those geometry bufferAttribute instances do get their name's set correctly (Matching the morphTargetDictionary names).

Is there any use case for having the geometry's morphAttribute's bufferAttribute instance names differ from the MorphTargetDictionary names of the geometry's mesh? Is that even allowed for in the GLTF spec?

[donmccurdy]

Is there any use case for having the geometry's morphAttribute's bufferAttribute instance names differ from the MorphTargetDictionary names of the geometry's mesh? Is that even allowed for in the GLTF spec?

Attribute names have nothing to do with morph target names in the glTF spec: the same attribute could be reused by different morph targets, each with different names, perhaps on different meshes.

[Bug-Reaper]

Is there any use case for having the geometry's morphAttribute's bufferAttribute instance names differ from the MorphTargetDictionary names of the geometry's mesh? Is that even allowed for in the GLTF spec?

Attribute names have nothing to do with morph target names in the glTF spec: the same attribute could be reused by different morph targets, each with different names, perhaps on different meshes.

Understood. The way that this PR works should solve the issue while also addressing these concerns.

The name assignment only occurs after we're aware the buffer attributes are being used for the normals/positions of a geometry's morphAttributes. It also inherits the names from the parent mesh's morphTargetDictionary's targetNames. So if the same buffer attribute was used for two meshes, they could be renamed independently in accordance to the mesh they're attached to.

[Bug-Reaper]

Just for reference this issue was discussed here:
https://github.com/mrdoob/three.js/pull/19358
TLDR: The initial proposed solution was not ideal. Naming the buffer attributes at any point would indeed causes issues if they were used in multiple places. Further, there's no real need to have the attribute names match the morphTargetDictionary names.

Issue was ultimately resolved here by removing code that unnecessarily checks the attribute names in propertyBinding.js:
https://github.com/mrdoob/three.js/pull/19362