Skip to content

Commit

Permalink
Impose a more sensible ordering for animation graph evaluation. (#15589)
Browse files Browse the repository at this point in the history
This is an updated version of #15530. Review comments were addressed.

This commit changes the animation graph evaluation to be operate in a
more sensible order and updates the semantics of blend nodes to conform
to [the animation composition RFC]. Prior to this patch, a node graph
like this:

```
	    ┌─────┐
	    │     │
	    │  1  │
	    │     │
	    └──┬──┘
	       │
       ┌───────┴───────┐
       │               │
       ▼               ▼
    ┌─────┐         ┌─────┐
    │     │         │     │
    │  2  │         │  3  │
    │     │         │     │
    └──┬──┘         └──┬──┘
       │               │
   ┌───┴───┐       ┌───┴───┐
   │       │       │       │
   ▼       ▼       ▼       ▼
┌─────┐ ┌─────┐ ┌─────┐ ┌─────┐
│     │ │     │ │     │ │     │
│  4  │ │  6  │ │  5  │ │  7  │
│     │ │     │ │     │ │     │
└─────┘ └─────┘ └─────┘ └─────┘
```

Would be evaluated as (((4 ⊕ 5) ⊕ 6) ⊕ 7), with the blend (lerp/slerp)
operation notated as ⊕. As quaternion multiplication isn't commutative,
this is very counterintuitive and will especially lead to trouble with
the forthcoming additive blending feature (#15198).

This patch fixes the issue by changing the evaluation order to
postorder, with children of a node evaluated in ascending order by node
index.

To do so, this patch revamps `AnimationCurve` to be based on an
*evaluation stack* and a *blend register*. During target evaluation, the
graph evaluator traverses the graph in postorder. When encountering a
clip node, the evaluator pushes the possibly-interpolated value onto the
evaluation stack. When encountering a blend node, the evaluator pops
values off the stack into the blend register, accumulating weights as
appropriate. When the graph is completely evaluated, the top element on
the stack is *committed* to the property of the component.

A new system, the *graph threading* system, is added in order to cache
the sorted postorder traversal to avoid the overhead of sorting children
at animation evaluation time. Mask evaluation has been moved to this
system so that the graph only has to be traversed at most once per
frame. Unlike the `ActiveAnimation` list, the *threaded graph* is cached
from frame to frame and only has to be regenerated when the animation
graph asset changes.

This patch currently regresses the `animate_target` performance in
`many_foxes` by around 50%, resulting in an FPS loss of about 2-3 FPS.
I'd argue that this is an acceptable price to pay for a much more
intuitive system. In the future, we can mitigate the regression with a
fast path that avoids consulting the graph if only one animation is
playing. However, in the interest of keeping this patch simple, I didn't
do so here.

[the animation composition RFC]:
https://github.com/bevyengine/rfcs/blob/main/rfcs/51-animation-composition.md

# Objective

- Describe the objective or issue this PR addresses.
- If you're fixing a specific issue, say "Fixes #X".

## Solution

- Describe the solution used to achieve the objective above.

## Testing

- Did you test these changes? If so, how?
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Showcase

> This section is optional. If this PR does not include a visual change
or does not add a new feature, you can delete this section.

- Help others understand the result of this PR by showcasing your
awesome work!
- If this PR adds a new feature or public API, consider adding a brief
pseudo-code snippet of it in action
- If this PR includes a visual change, consider adding a screenshot,
GIF, or video
  - If you want, you could even include a before/after comparison!
- If the Migration Guide adequately covers the changes, you can delete
this section

While a showcase should aim to be brief and digestible, you can use a
toggleable section to save space on longer showcases:

<details>
  <summary>Click to view showcase</summary>

```rust
println!("My super cool code.");
```

</details>

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
  • Loading branch information
pcwalton and alice-i-cecile authored Oct 3, 2024
1 parent 67744bb commit ca8dd06
Show file tree
Hide file tree
Showing 5 changed files with 1,030 additions and 202 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_animation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ bevy_ui = { path = "../bevy_ui", version = "0.15.0-dev", features = [
bevy_text = { path = "../bevy_text", version = "0.15.0-dev" }

# other
fixedbitset = "0.5"
petgraph = { version = "0.6", features = ["serde-1"] }
ron = "0.8"
serde = "1"
blake3 = { version = "1.0" }
thiserror = "1"
thread_local = "1"
uuid = { version = "1.7", features = ["v4"] }
smallvec = "1"

[lints]
workspace = true
Expand Down
Loading

0 comments on commit ca8dd06

Please sign in to comment.