Rework animation to be done in two phases.

[pcwalton]

Objective

Bevy's animation system currently does tree traversals based on Name that aren't necessary. Not only do they require in unsafe code because tree traversals are awkward with parallelism, but they are also somewhat slow, brittle, and complex, which manifested itself as way too many queries in #11670.

Solution

Divide animation into two phases: animation advancement and animation evaluation, which run after one another. Advancement operates on the AnimationPlayer and sets the current animation time to match the game time. Evaluation operates on all animation bones in the scene in parallel and sets the transforms and/or morph weights based on the time and the clip.

To do this, we introduce a new component, AnimationTarget, which the asset loader places on every bone. It contains the ID of the entity containing the AnimationPlayer, as well as a UUID that identifies which bone in the animation the target corresponds to. In the case of glTF, the UUID is derived from the full path name to the bone. The rule that AnimationTargets are descendants of the entity containing AnimationPlayer is now just a convention, not a requirement; this allows us to eliminate the unsafe code.

Migration guide

• AnimationClip now uses UUIDs instead of hierarchical paths based on the Name component to refer to bones. This has several consequences:
  • A new component, AnimationTarget, should be placed on each bone that you wish to animate, in order to specify its UUID and the associated AnimationPlayer. The glTF loader automatically creates these components as necessary, so most uses of glTF rigs shouldn't need to change.
  • Moving a bone around the tree, or renaming it, no longer prevents an AnimationPlayer from affecting it.
  • Dynamically changing the AnimationPlayer component will likely require manual updating of the AnimationTarget components.
• Entities with AnimationPlayer components may now possess descendants that also have AnimationPlayer components. They may not, however, animate the same bones.
• As they aren't specific to TypeIds, bevy_reflect::utility::NoOpTypeIdHash and bevy_reflect::utility::NoOpTypeIdHasher have been renamed to bevy_reflect::utility::NoOpHash and bevy_reflect::utility::NoOpHasher respectively.

[mockersf]

on the many_foxes examples, this PR is slower than main

[pcwalton]

I would expect the problem to be #11711. I also haven't looked at performance at all.

[pcwalton]

By the way, I don't know how to implement the Animatable trait, which is an approved RFC, without something like this or an explosion in unsafe code.

[pcwalton]

I've verified that with #11711 applied on top of this, the many_foxes example is faster than main with this PR. Without #11711, this PR alone is indeed slower because of all the hashing.

[pcwalton]

I think this is ready for review.

[github-actions[bot]]

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy? It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

[pcwalton]

I addressed review comments, but want to add more documentation.

[pcwalton]

@james7132 I added more documentation to call out the caveats you mentioned. Let me know how you like it.

[pcwalton]

Benchmark on Windows 11, Intel(R) Core(TM) i9-10980HK CPU @ 2.40GHz, NVIDIA GeForce GTX 1650 Ti with Max-Q Design: main FPS: avg 40.78, stddev 1.18 two-phase-animation FPS: avg 42.14, stddev 1.07

Benchmark on M2 Mac mini, 2023: main FPS: avg 89.37, stddev 4.93 two-phase-animation FPS: avg 88.52, stddev 4.50

I think that this is overall a bit faster, but on the Mac it's in the noise (.18σ).

[djeedai]

Next time I suggest measuring times (ms) rather than FPS. FPS is a logarithmic value which is hard to reason about. Also I think Tracy does very good A/B perf comparison distribution graphs, the rendering team generally uses that on their PRs.

[pcwalton]

@djeedai

Benchmark on Windows 11, Intel(R) Core(TM) i9-10980HK CPU @ 2.40GHz, NVIDIA GeForce GTX 1650 Ti with Max-Q Design: main: frame time avg 24.60 ms, stddev 0.79 ms two-phase-animation: frame time avg 23.79 ms, stddev 0.62 ms

Benchmark on M2 Mac mini, 2023: main: frame time avg 15.06 ms, stddev 1.44 ms two-phase-animation: 15.59 ms, stddev 1.11 ms

[djeedai]

@pcwalton thanks a lot for the timings! Is that the foxes demo? Frame time over 15ms looks quite slow no? That means we're under 60FPS? I know this PR improves things but looks like there's still a lot of work to do then 😅

[pcwalton]

@djeedai Yeah, it's the foxes demo. This is a pretty low-end GPU and there's a lot of skinning going on in that demo. I don't think it's indicative of a problem.

[pcwalton]

I believe I addressed all comments.

[pcwalton]

@NthTensor I got rid of the warning.

[mockersf]

example morph_targets is not working with this PR. https://github.com/pcwalton/bevy/pull/1 fixes it but breaks one of the assumptions of the PR

[mockersf]

https://github.com/KhronosGroup/glTF-Sample-Assets/tree/main/Models/InterpolationTest doesn't work either with this PR

[pcwalton]

OK, I allowed AnimationTarget and AnimationPlayer to coexist on the same entity. The fact that they couldn't is a relic of when I was using EntityMut on AnimationTargets, so they would lock out the AnimationPlayers. This is no longer necessary.

[NthTensor]

Both animated_transform and morph_targets seem to be working. @mockersf

[mockersf]

Both animated_transform and morph_targets seem to be working. @mockersf

animated_transform doesn't behave as before and https://github.com/KhronosGroup/glTF-Sample-Assets/tree/main/Models/InterpolationTest doesn't work

[pcwalton]

@mockersf You were running InterpolationTest through scene_viewer, right? Because the way that scene_viewer plays animations is pretty broken, and the fact that we now use multiple AnimationPlayers in this PR broke it. I'll fix it as part of this PR.

Both of those issues are problems with the examples, not with the Bevy logic.

[pcwalton]

Those should be working now and should be ready for review and merging afterward now that 0.13 has shipped.

[mockersf]

@mockersf You were running InterpolationTest through scene_viewer, right?

Yup!

Because the way that scene_viewer plays animations is pretty broken, and the fact that we now use multiple AnimationPlayers in this PR broke it. I'll fix it as part of this PR.

Thanks! It's good practice to fix the examples with the PR that breaks them, it avoids having to investigate without the context.