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

✨ Improve Animator copying (createAnimatorWithAnimationsFrom) by avoiding our patch #47

Open
mrousavy opened this issue Mar 12, 2024 · 2 comments
Assignees

Comments

@mrousavy
Copy link
Member

We currently have a patch that allows the user to use animations from a different entity.

std::shared_ptr<AnimatorWrapper> FilamentAssetWrapper::createAnimatorWithAnimationsFrom(std::shared_ptr<FilamentAssetWrapper> otherAsset) {
#ifdef HAS_FILAMENT_ANIMATOR_PATCH
// TODO(copy-animations): We currently copy animations from an asset onto another instance (different model than the original asset), we
// should replace this with once we found a good solution discussed here: https://github.com/google/filament/issues/7622
Animator* animator = new gltfio::Animator(otherAsset->_asset.get(), _asset->getInstance());
return std::make_shared<AnimatorWrapper>(_asset, animator);
#else
return getAnimator();
#endif
}

For example, Entity A has 30 animations, Entity B has 0 animations, with our patch (``) you can use all of Entity A's 30 animations on Entity B.

The implementation of the patch is a bit rough as we just create a new Animator* pointer - which seems to work fine even in production, but we wanted to avoid the use of a patch in Filament for multiple reasons:

  • Avoiding a patch by having an upstream change is just always better
  • We are abusing the API a bit here, it wasn't really built like this
  • We cannot use Filament from CocoaPods/Maven now, so we have to build it manually and ship it over npm to use our patch

We already created an issue in google/filament and are hoping to find a solution that can be merged upstream there: google/filament#7622

@mrousavy mrousavy added the enhancement New feature or request label Mar 12, 2024
@hannojg
Copy link
Member

hannojg commented Mar 16, 2024

A part from the patch can already be removed as this PR has been merged in filament.
Will put out a update PR once I find the time!

@hannojg hannojg added 🔧 chore and removed enhancement New feature or request labels Apr 20, 2024
@hannojg
Copy link
Member

hannojg commented Jun 3, 2024

We are currently waiting for this PR to be approved:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants