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

fix: fix removeUnnecessaryJoints and combineSkeletons #1557

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

0b5vr
Copy link
Contributor

@0b5vr 0b5vr commented Dec 13, 2024

This PR fixes removeUnnecessaryJoints and combineSkeletons.

It processes the same bone index attribute twice or more, which collapses the mesh.

TODOs

  • fix combineSkeletons

Points need review

  • Is it working properly?

it processes a same bone index attribute twice or more, which collapses the mesh

We are going to fix combineSkeletons in the next commit
@0b5vr 0b5vr added the bug Something isn't working label Dec 13, 2024
@0b5vr 0b5vr added this to the next milestone Dec 13, 2024
@0b5vr 0b5vr self-assigned this Dec 13, 2024
Comment on lines +113 to +114
bones.push(mesh.skeleton.bones[oldIndex]);
boneInverses.push(mesh.skeleton.boneInverses[oldIndex]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considering cases multiple meshes share the same skin index attribute but not the same bones (which is unlikely) or boneInverses (which is more likely), we moved these lines here

It sometimes modified a skin index attribute twice or more, which collapses the skinned mesh rendering

I'm sorry that it became a huge refactor ._.
@0b5vr 0b5vr marked this pull request as ready for review December 13, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant