Additive blending within animation graphs #15198
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IQuick143
added
C-Feature
alice-i-cecile
added
S-Needs-Review
Contributor
Godot just calls them blend nodes, so I think it's fine to use that. See
I'm not sure right now; don't have time to think about it. Definitely research into what other engines do would be helpful.
Perhaps a builder API might be better, but I think that should be done as a follow-up.
It'd be nice, but I won't block this PR on it :)
I'd add "Additive" check boxes to the graph nodes in the
Yes, it's a breaking change. I don't think we need to go out of our way to support old |
aecsocket
force-pushed
the
additive-blending
branch
from
51901f7 to
b89fdd9
Compare
Contributor
Author
|
I've added the additive toggle checkboxes to the
I don't like this because we already have a concept called a "blend node", which is an animation graph node without an associated animation clip. If I say "blend node", does this refer to a node without an animation clip (but unspecified additive/not-additive), or does this refer to a non-additive node (which may have an animation clip or not)? |
Contributor
|
@aecsocket I'd call the three types of node "clip nodes", "blend nodes", and "additive blend nodes". I don't think that's particularly ambiguous. If we need to disambiguate between the last two, though, we could call them "interpolating blend nodes" and "additive blend nodes". |
pcwalton
added a commit
to pcwalton/bevy
that referenced
this pull request
Sep 29, 2024
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 ⊕ standing in for the
blend (lerp/slerp) operation. As quaternion multiplication isn't
commutative, this is very counterintuitive and will especially lead to
trouble with the forthcoming additive blending feature (bevyengine#15198).
This patch fixes the issue. It revamps `Keyframes` to be based on a
stack.
TODO write more.
[the animation composition RFC]: https://github.com/bevyengine/rfcs/blob/main/rfcs/51-animation-composition.md
pcwalton
added a commit
to pcwalton/bevy
that referenced
this pull request
Sep 29, 2024
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 (bevyengine#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 `Keyframes` 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 35%, 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
pcwalton
added a commit
to pcwalton/bevy
that referenced
this pull request
Sep 29, 2024
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 (bevyengine#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 `Keyframes` 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
pcwalton
added a commit
to pcwalton/bevy
that referenced
this pull request
Oct 2, 2024
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 (bevyengine#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
pcwalton
added a commit
to pcwalton/bevy
that referenced
this pull request
Oct 2, 2024
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 (bevyengine#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
pcwalton
added a commit
to pcwalton/bevy
that referenced
this pull request
Oct 2, 2024
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 (bevyengine#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
Member
|
@aecsocket #15589 is in the merge queue now, if you'd like to pick this up again :) |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 3, 2024
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>
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Oct 4, 2024
…engine#15589) This is an updated version of bevyengine#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 (bevyengine#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>
robtfm
pushed a commit
to robtfm/bevy
that referenced
this pull request
Oct 4, 2024
…engine#15589) This is an updated version of bevyengine#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 (bevyengine#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>
Contributor
|
Closing as superseded by #15631 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Expands on #15013 by adding the concept of additive animation graph nodes. These nodes do not share weight with other nodes in the same graph, and instead are applied with the raw weight of its node. For example, if an additive node has weight
1.0, its animation clip will always be applied with a weight of1.0, regardless of if there are other nodes which have a weight > 0. This can be used to implement animations which play "on top of" other animations, i.e. a punching animation (additive) on top of movement (shared weight between idle/walk/run/..).Solution
Adds the concept of additive animation nodes, and API to add these nodes to an
AnimationGraph.Testing
I used this implementation of additive blending in my own project, and it works.
Outstanding concerns
Before this exits draft stage:
AnimationGraphNode::additivedocs.AnimationGraphAPI for adding nodes is getting quite complex, with a lot of permutations of:add_clipadd_blend..with_mask..additiveanimation_graphexampleadditiveto serializedanimgraph.ronfiles. Do we need to set a default for this for old animgraph files which don't have this field yet? Does this count as a breaking change?Showcase
The
AnimationGraphAPI now supports additive animation nodes. Additive nodes don't share weight with other nodes in the same graph, so their full weight will be applied to the animation. This is useful for when you want to layer an animation on top of another animation without it reducing the intensity of the animation underneath - for example, playing a punching animation on top of your base idle or movement animation.The
animation_graphexample now includes checkboxes for each animation clip node, which allows toggling whether that node is additive or not. You can use this to preview how additive blending works.Migration Guide
Existing
animgraph.ronfiles must have theadditiveboolean field added to each animation graph node. Setting this field tofalseemulates the existing behavior of sharing weight across all nodes.