Skip to content

GLTFLoader: Clone node associations. #31051

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

Merged
merged 1 commit into from
May 13, 2025

Conversation

nkrajina
Copy link
Contributor

@nkrajina nkrajina commented May 5, 2025

Description

GLTFLoader doesn't correctly assign the nodes association when multiple
nodes reference the same mesh. The problem occurs in _loadNodeShallow
where nodes property is added to the shared object describing the node/mesh
association.

For example, in the CesiumMilkTruck model nodes 0 (Wheels) and 2 (Wheels.001)
reference the same mesh 0, but resulting associations for both three.js meshes are
{meshes: 0, primitives: 0, nodes: 2}.

A simple solution is to clone the association object if there are multiple references to
the same mesh.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 8, 2025

GLTFLoader doesn't correctly assign the nodes association when multiple
nodes reference the same mesh.

What are the consequences of this? Does that means certain glTF asset can't be loaded properly? If so, can you share one so it's possible to reproduce the issue?

@nkrajina
Copy link
Contributor Author

nkrajina commented May 9, 2025

GLTF assets will load properly, but the object returned by loader.parse() will have incorrect data in parser.associations map when multiple GLTF nodes map to the same GLTF mesh.

I need a way to reference an arbitrary point in the GLTF model with animations, so I can compute where it is positioned at each animation frame. The point can be uniquely specified by the following:

  • index of the node in the GLTF
  • index of the primitive in the node's mesh
  • indices of vertices of the triangle in the primitive
  • barycentric coordinates of the point inside the triangle
  • optional index of the instance if instancing is used (ignored in the rest of this discussion)

Node and primitive indices should suffice to uniquely determine the generated three.js mesh, from which further computations can be done. But since the nodes property in the associations map is incorrectly stored, incorrect mesh will be addressed.

@Mugen87 Mugen87 requested a review from donmccurdy May 9, 2025 21:13
Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead with this, it sounds like a worthwhile fix to me.

I have been somewhat regretting the complexity of how GLTFLoader unpacks the scene and manages associations – this is a maintenance challenge but I don't have free personal time to work on it, which would need some careful testing. For the changes in output that we probably need to make eventually, see:

@Mugen87 Mugen87 merged commit 7896d65 into mrdoob:dev May 13, 2025
11 checks passed
@Mugen87 Mugen87 added this to the r177 milestone May 13, 2025
@Mugen87 Mugen87 changed the title clone node associations in GLTFLoader GLTFLoader: Clone node associations. May 25, 2025
RuthySheffi pushed a commit to RuthySheffi/three.js that referenced this pull request Jun 5, 2025
RuthySheffi pushed a commit to RuthySheffi/three.js that referenced this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants