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

feat: updateBoneMatricesForInstance #7859

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions libs/gltfio/include/gltfio/Animator.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ class UTILS_PUBLIC Animator {
*/
void updateBoneMatrices();

/**
* Updates the bone matrices of the specified instance using the state of this animatior.
* This is useful if you have another instance that has the same skeleton as the asset of this animator,
* and you wish to apply the same animation to those instances (e.g. clothing).
*
* NOTE: In most cases, you only need to use the updateBoneMatrices() method. This method is necessary
* only when you need to synchronize animations across multiple instances with the same skeleton.
*
* @param instance The instance to update.
*/
void updateBoneMatricesForInstance(FilamentInstance* instance);
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like maybe we can formalize addInstance below instead of adding this method?

We can do some validation of the the animation associated with the "new", added instance vs the one that's used to build this animator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought so as well first. However, the "problem" I found with that is that we are again creating all those channels and data for the new instance.
With the solution I have right now we don't need to recreate any data (however I admit that my solution is maybe a bit fragile as it relies on the name of the entities to synchronise entity transforms).

Copy link
Contributor Author

@hannojg hannojg May 16, 2024

Choose a reason for hiding this comment

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

(The solution I am talking about is not visible in this PR but it is what I outlined here)


/**
* Applies a blended transform to the union of nodes affected by two animations.
* Used for cross-fading from a previous skinning-based animation or rigid body animation.
Expand Down
4 changes: 4 additions & 0 deletions libs/gltfio/src/Animator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ void Animator::updateBoneMatrices() {
}
}

void Animator::updateBoneMatricesForInstance(FilamentInstance* instance) {
mImpl->updateBoneMatrices(downcast(instance));
}

float Animator::getAnimationDuration(size_t animationIndex) const {
return mImpl->animations[animationIndex].duration;
}
Expand Down
Loading