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

Morph target animation fix #4532

Merged
merged 1 commit into from
Aug 12, 2022
Merged

Morph target animation fix #4532

merged 1 commit into from
Aug 12, 2022

Conversation

ellthompson
Copy link
Contributor

@ellthompson ellthompson commented Aug 11, 2022

Morph target animation curves that are named via the glb mesh's extras property should be differentiated from index based curves.

Fixes #4522
Fixes #4520

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@ellthompson ellthompson added bug area: animation Animation related issue labels Aug 11, 2022
@ellthompson ellthompson requested a review from a team August 11, 2022 15:54
@ellthompson ellthompson self-assigned this Aug 11, 2022
@mvaligursky
Copy link
Contributor

Could you please explain what this does? you add the prefix, and later strip it.

if (weightName.indexOf('name.') === 0) {
weightName = weightName.replace('name.', '');
} else {
weightName = Number(weightName);
Copy link
Member

Choose a reason for hiding this comment

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

worth checking for NaN here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will always be the index of the morph target as a string.

@slimbuck
Copy link
Member

(approved, but pls wait for @mvaligursky to also approve before merging)

@slimbuck slimbuck closed this Aug 12, 2022
@slimbuck slimbuck reopened this Aug 12, 2022
@ellthompson
Copy link
Contributor Author

Could you please explain what this does? you add the prefix, and later strip it.

The prefix is only added to morph target curves which are named via the mesh's extras property. This means we can differentiate them at binding time from indexed morph target curves, passing them to the setWeight function as a string instead of a number. Then those named curves will be referenced against the name lookup table, indexed curves will behave as before.

@ellthompson ellthompson merged commit 43b5fb3 into main Aug 12, 2022
@ellthompson ellthompson deleted the named-morph-target-fix branch August 12, 2022 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: animation Animation related issue bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shape key / morph key bug Morph target animation issues in 1.55
3 participants