Skip to content
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

GLTFLoader: Fix missing associations #22583

Merged
merged 1 commit into from
Sep 28, 2021
Merged

GLTFLoader: Fix missing associations #22583

merged 1 commit into from
Sep 28, 2021

Conversation

ghost
Copy link

@ghost ghost commented Sep 24, 2021

Resolve an issue where association mappings are missing for meshes when several glTF node definitions reference the same mesh-group, and the mesh-group has multiple primitives.

Continued from PR: #22544

Repro this issue:
Load the 2CylinderEngine gltf-sample model.
Walk the node hierarchy and check that each node/mesh has an association.
You will find several objects that do not have associations.

Explanation: Nodes that refer to the same Mesh/Group get cloned, if a Group gets cloned then the association map will not a have record of the clone and needs to be updated, in addition the child meshes of the group also get cloned and those child clones also need to be updated in the association map.

Additional fix:
As a side effect of the cloning there can be several 'source' nodes (the ones getting cloned) that have association mappings but are not actually in the node hierarchy (leaking memory in other words), this PR removes the extraneous mappings.

…several glTF node definitions reference the same mesh data, and the mesh data has multiple primitives.Resolve issue where association mappings are missing for meshes when several glTF node definitions reference the same mesh data, and the mesh data has multiple primitives.
@ghost
Copy link
Author

ghost commented Sep 24, 2021

@takahirox I have patched an issue here with multiple-associations, can you take a look?

@mrdoob
Copy link
Owner

mrdoob commented Sep 28, 2021

Merging this. Hopefully everything looks good @takahirox 🤞

@mrdoob mrdoob added this to the r133 milestone Sep 28, 2021
@mrdoob mrdoob merged commit def04db into mrdoob:dev Sep 28, 2021
@mrdoob
Copy link
Owner

mrdoob commented Sep 28, 2021

Thanks!

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.

2 participants