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

Animation Composition #51

Merged
merged 20 commits into from
Jul 18, 2023
Merged

Conversation

james7132
Copy link
Member

@james7132 james7132 commented Feb 22, 2022

RENDERED

This RFC is an extension of #49, detailing the low level details on how to compose the base primitives for more complex animations together via blending. This RFC stops short of the final step of applying the sampled values to the target animated properties, as that is separate and can be handled regardless of how the values are composed or sampled.

This will remain a draft until #49, or some variant of it that covers the same subject matter, gets merged. This looks like it might be easier to implement than the actual storage solution.

@james7132 james7132 changed the title Start #51: Animation Composition Animation Composition Feb 22, 2022
@aunyks
Copy link

aunyks commented May 8, 2022

I’d love to have this feature so that I can blend multiple glTF animations.

@ValorZard
Copy link

Since Bevy doesn't currently have an editor, or any official debug UI for that matter, how would a user manipulate the animation state tree? In Unity and Godot, there is a fancy visual scripting editor that lets you build a animation graph quite easily, but I could see that becoming quite unwieldy in code. This wouldn't be a problem once we have an actual editor, but for the time being, managing an animation state machine could become quite troublesome.

@james7132 james7132 marked this pull request as ready for review November 22, 2022 04:11
@james7132
Copy link
Member Author

james7132 commented Nov 22, 2022

Since Bevy doesn't currently have an editor, or any official debug UI for that matter, how would a user manipulate the animation state tree? In Unity and Godot, there is a fancy visual scripting editor that lets you build a animation graph quite easily, but I could see that becoming quite unwieldy in code. This wouldn't be a problem once we have an actual editor, but for the time being, managing an animation state machine could become quite troublesome.

I agree here. While we're missing the editor tools for this, I would like to add some tooling that is similar to the current AnimationPlayer to make it easy to just use it as is, but ultimately this graph is supposed to be a low level primitive, not a high level interface. We'll need to rip off that band-aid eventually.

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

I think this is the right general direction!

rfcs/51-animation-composition.md Outdated Show resolved Hide resolved
rfcs/51-animation-composition.md Outdated Show resolved Hide resolved

Every node has it's own time value. For clip nodes, this is the time at which the
underlying clip is sampled at. For blend nodes, this value doesn't do anything.
However, an optional setting will allow a blend node to propagate the time value
Copy link
Member

Choose a reason for hiding this comment

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

What scenarios does this enable? Does this pattern exist in prior art? To my knowledge, Godot doesn't allow blend nodes to override time, and given that each animation will be playing at its own cadence, does the propagated value even have any meaning in this context?

Also is this "delta time" or "absolute time". Maybe worth clarifying in the language?

Copy link
Member Author

@james7132 james7132 Dec 14, 2022

Choose a reason for hiding this comment

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

The first common use case that I can think of is "any-time-after" transitions. Where a transition from a source animation is triggered automatically after a fixed time point has passed. That source animation will continue from the same time it had before, but the new animation it's transitioning to will start at zero. Both will advance at their own respective time scales while the transition blends smoothly from the source to the destination. While this is notably very low level, this can be used to build smooth transitions for a state machine like flow at a higher level.

For the propagation, this is largely just a QoL feature for letting you advance an entire subgraph at once.. Micromanaging every clip remotely can get rather crazy, so this is mainly offered as a way to avoid needing to set the tme manually across an entire subgraph. We could remove this given that the intent of this is to provide low level animation blending support, not a high level control.

It should be absolute time for the underlying clip. The delta in how each clip is advanced is scaled based on the global, graph level, and clip level time scales. I'll add this into the actual document itself.

Copy link
Member

Choose a reason for hiding this comment

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

For the propagation, this is largely just a QoL feature for letting you advance an entire subgraph at once..

Yeah I think the propagation part might be too "high level" and opinionated at this stage in the game. Useful for some scenarios and not others, "invalid" in some cases (scrubbing results in going "outside" the duration bounds of one animation but not the other, animations are configured to progress at different rates / times, etc). And the behavior (when needed) can be implemented reasonably easily using existing features (ex: for_each_child_node(parent_node, |child| child.set_time(time))

Copy link
Member

Choose a reason for hiding this comment

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

(or for_each_descendant_node)

Copy link
Contributor

Choose a reason for hiding this comment

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

When reading the preceding paragraph, I had this question: "This seems to try to enable composition, but why a graph system would improve on a simple weight-based approach?"

And this sentence comes in. It gives a feature that is impossible to accomplish without graphs: namely: putting a bunch of animations together and having the capacity to control them exactly like if they were only a single animation.

In this context, I think having time control on blend nodes is really useful.

rfcs/51-animation-composition.md Outdated Show resolved Hide resolved
rfcs/51-animation-composition.md Show resolved Hide resolved
rfcs/51-animation-composition.md Outdated Show resolved Hide resolved
bors bot pushed a commit to bevyengine/bevy that referenced this pull request Jan 9, 2023
# Objective

- Fixes #6338

This PR allows for smooth transitions between different animations.

## Solution

- This PR uses very simple linear blending of animations.
- When starting a new animation, you can give it a duration, and throughout that duration, the previous and the new animation are being linearly blended, until only the new animation is running.
- I'm aware of bevyengine/rfcs#49 and bevyengine/rfcs#51, which are more complete solutions to this problem, but they seem still far from being implemented. Until they're ready, this PR allows for the most basic use case of blending, i.e. smoothly transitioning between different animations.

## Migration Guide

- no bc breaking changes
james7132 pushed a commit to james7132/bevy that referenced this pull request Jan 21, 2023
# Objective

- Fixes bevyengine#6338

This PR allows for smooth transitions between different animations.

## Solution

- This PR uses very simple linear blending of animations.
- When starting a new animation, you can give it a duration, and throughout that duration, the previous and the new animation are being linearly blended, until only the new animation is running.
- I'm aware of bevyengine/rfcs#49 and bevyengine/rfcs#51, which are more complete solutions to this problem, but they seem still far from being implemented. Until they're ready, this PR allows for the most basic use case of blending, i.e. smoothly transitioning between different animations.

## Migration Guide

- no bc breaking changes
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
# Objective

- Fixes bevyengine#6338

This PR allows for smooth transitions between different animations.

## Solution

- This PR uses very simple linear blending of animations.
- When starting a new animation, you can give it a duration, and throughout that duration, the previous and the new animation are being linearly blended, until only the new animation is running.
- I'm aware of bevyengine/rfcs#49 and bevyengine/rfcs#51, which are more complete solutions to this problem, but they seem still far from being implemented. Until they're ready, this PR allows for the most basic use case of blending, i.e. smoothly transitioning between different animations.

## Migration Guide

- no bc breaking changes
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Fixes bevyengine#6338

This PR allows for smooth transitions between different animations.

## Solution

- This PR uses very simple linear blending of animations.
- When starting a new animation, you can give it a duration, and throughout that duration, the previous and the new animation are being linearly blended, until only the new animation is running.
- I'm aware of bevyengine/rfcs#49 and bevyengine/rfcs#51, which are more complete solutions to this problem, but they seem still far from being implemented. Until they're ready, this PR allows for the most basic use case of blending, i.e. smoothly transitioning between different animations.

## Migration Guide

- no bc breaking changes
Copy link
Contributor

@nicopap nicopap left a comment

Choose a reason for hiding this comment

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

This looks good to me. I don't have any particular concerns.

Just a bit of confusion on why sampling is necessarily exclusive.

rfcs/51-animation-composition.md Outdated Show resolved Hide resolved
rfcs/51-animation-composition.md Outdated Show resolved Hide resolved
}
```

The individual curves stored within are indexed by `ClipId`, and each pointer to
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering earlier why we needed ClipId. This explains it.

rfcs/51-animation-composition.md Outdated Show resolved Hide resolved
rfcs/51-animation-composition.md Outdated Show resolved Hide resolved
Comment on lines 158 to 159
assigned a auto-imcrementing `ClipId` much like `NodeId`. However, instead of
storing a `Vec<AnimationClip`, a map of property paths to `Track` is stored.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest adding a "this is explained in the 'Bindings' section". I didn't understand what "map of property paths to Track" meant until I read it (ie: the GraphBindings struct)


### Graph Storage
Two main stores are needed internally for the graph: node storage and clip
storage. Nodes are stored in a flat `Vec<Node>`, and a newtyped `NodeId` is used
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what properties a Node stores. A list of dependencies and weights?

Copy link
Member

Choose a reason for hiding this comment

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

a node is either a blend node or a clip node. they would have dependencies, time setting, a clip

clip C is 0.125. If multiple paths to a end clip are present, the final weight
for the clip is the sum of all path's weights.

After traversing the graph, the weights of all active inputs are clamped and
Copy link
Contributor

Choose a reason for hiding this comment

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

what does "active inputs" mean in this context? Are those the weight field of Influences?

}

struct GraphInfluences {
influence: Vec<Influence> // Indexed by BoneId
Copy link
Contributor

Choose a reason for hiding this comment

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

Indexed by BoneId

Is this true? The paragraph that directly follows say

This is effectively a Vec<f32> indexed on ClipId mapping clips and their respective cumulative weights.

Comment on lines +375 to +376
- maintaining the binding components is going to be a nightmare if there are
any hierarchy or name changes underneath an `AnimationGraph`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The current system has a path cache right? Why would that be impossible to establish in the graph-based system? How is maintaining the cache less of a nightmare? It should have the same limitations.

Copy link
Member Author

Choose a reason for hiding this comment

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

The path cache is just a cache. It's not treated as a source of truth, just a opportunistic way to reduce expensive searching. The binding mentioned here is more akin to relations and would have to be treated as a source of truth to get the full benefits, and that has the same mire of edge cases are still being worked out for the hierarchy.

@james7132 james7132 requested a review from mockersf May 7, 2023 02:04
Comment on lines +205 to +206
clip C is 0.125. If multiple paths to a end clip are present, the final weight
for the clip is the sum of all path's weights.
Copy link
Member

@mockersf mockersf May 13, 2023

Choose a reason for hiding this comment

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

the final weight for the clip is the sum of all path's weights.

Shouldn't it be the average? Is there a risk of a weight greater than 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's not well mentioned here, but the final results are after normalizing the entire weight vector so that the sum of all weights is 1.0, even if a final weight is over 1.0, it's influence will still be proportional to the rest of the clips that are in the graph.

@mockersf
Copy link
Member

is bevyengine/bevy#7130 usable for this graph?

@mockersf
Copy link
Member

@cart are you OK to move forward with this RFC? In my opinion there are a few minor questions, but they should be answered when going for the implementation

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

I'm happy with where this is at. I think this is a solid first step for animation composition!

@cart cart merged commit 658472f into bevyengine:main Jul 18, 2023
@james7132
Copy link
Member Author

I know this is already merged, but I wanted to close out a few last loose ends.

is bevyengine/bevy#7130 usable for this graph?

I haven't fully reviewed that PR yet, but my impression is no. The construction I had in mind does not involve many HashMap lookups of any kind and the traversal really only needed jumping around a few items in a Vec<Node>, which should be exceptionally fast to evaluate multiple times per game tick. Though I don't think this would preclude us from including this variant of a graph in bevy_graph though.

github-merge-queue bot pushed a commit to bevyengine/bevy that referenced this pull request Feb 2, 2024
# Objective
Allow animation of types other than translation, scale, and rotation on
`Transforms`.

## Solution
Add a base trait for all values that can be animated by the animation
system. This provides the basic operations for sampling and blending
animation values for more than just translation, rotation, and scale.

This implements part of bevyengine/rfcs#51, but is missing the
implementations for `Range<T>` and `Color`. This also does not fully
integrate with the existing `AnimationPlayer` yet, just setting up the
trait.

---------

Co-authored-by: Kirillov Kirill <[email protected]>
Co-authored-by: François <[email protected]>
Co-authored-by: irate <[email protected]>
Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: Alice Cecile <[email protected]>
tjamaan pushed a commit to tjamaan/bevy that referenced this pull request Feb 6, 2024
# Objective
Allow animation of types other than translation, scale, and rotation on
`Transforms`.

## Solution
Add a base trait for all values that can be animated by the animation
system. This provides the basic operations for sampling and blending
animation values for more than just translation, rotation, and scale.

This implements part of bevyengine/rfcs#51, but is missing the
implementations for `Range<T>` and `Color`. This also does not fully
integrate with the existing `AnimationPlayer` yet, just setting up the
trait.

---------

Co-authored-by: Kirillov Kirill <[email protected]>
Co-authored-by: François <[email protected]>
Co-authored-by: irate <[email protected]>
Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: Alice Cecile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Responded
Development

Successfully merging this pull request may close these issues.

6 participants