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

Unified Transform blend stack #15665

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

mweatherley
Copy link
Contributor

@mweatherley mweatherley commented Oct 5, 2024

Objective

Because curve-based animation (#15434) supports animating things based on arbitrary curves, it would be good to support Curve<Transform>. Unfortunately, the TransformCurve wrapper had to be killed in the migration to blend stacks (#15589, #15598) because Transform would fight with all of its parts over write access to the actual Transform component.

The goal of this PR is to reintroduce TransformCurve by making all the animated parts of Transform (TranslationCurve, RotationCurve, ScaleCurve) share the same blend stack. This allows TransformCurve to function by sharing the same blend stack with all of them, so they simply blend harmoniously instead of fighting over the same component data.

Solution

A new Animatable type TransformParts has been introduced:

/// Basically `Transform`, but with every part optional. Note that this is the true
/// output of `Transform` animation, since individual parts may be unconstrained
/// if they lack an animation curve to control them. 
#[derive(Default, Debug, Clone, Reflect)]
pub(crate) struct TransformParts {
    translation: Option<Vec3>,
    rotation: Option<Quat>,
    scale: Option<Vec3>,
}

Subsequently, TranslationCurve, RotationCurve, ScaleCurve, and TransformCurve all share a single blend stack where the individual elements are TransformParts. For example, when TranslationCurve contributes an element to the blend stack, it sets the rotation and scale components to be None, and you can probably guess how everything else goes from there; the only wrinkle is that when multiple curves are added from the same animation clip, they append to the same element on top of the stack instead of each getting their own element — this is for compatibility with assumptions in graph evaluation.

An example transform_curve_animation has been added which shows a procedural animation defined by a Curve<Transform> with an animation defined by a Curve<Vec3> additively blended on top.

Testing

Tested on many_foxes and found little to no performance difference. Testing on animation_graph helps assure that nothing broke in the migration to the unified blend stack.

Screenshot 2024-10-07 at 8 18 47 AM


Showcase

// Allocate an animation clip.
let mut clip = AnimationClip::default();

// This curve describes the position of the ship over time.
let wobbly_circle_curve =
    function_curve(Interval::new(0.0, std::f32::consts::TAU).unwrap(), |t| {
        vec3(t.sin() * 5.0, t.sin() * 1.5, t.cos() * 5.0)
    });

// This curve uses the position of the ship to make its inward wing point toward
// the center of the platform that we'll position.
let transform_curve = wobbly_circle_curve.map(|position| {
    Transform::from_translation(position).aligned_by(
        Dir3::NEG_X,
        vec3(0.0, -2.0, 0.0) - position,
        Dir3::Y,
        Dir3::Y,
    )
});

clip.add_curve_to_target(animation_target_id, TransformCurve(transform_curve));
Screen.Recording.2024-10-07.at.11.06.52.AM.mov

@mweatherley mweatherley added C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Animation Make things move and change over time S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 5, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 7, 2024
# Objective

Fix a couple of substantial errors found during the development of
#15665:
- `AnimationCurveEvaluator::add` was secretly unreachable. In other
words, additive blending never actually occurred.
- Weights from the animation graph nodes were ignored, and only
`ActiveAnimation`'s weights were used.

## Solution

Made additive blending reachable and included the graph node weight in
the weight of the stack elements appended in the curve application loop
of `animate_targets`.

## Testing

Tested on existing examples and on the new example added in #15665.
@mweatherley mweatherley marked this pull request as ready for review October 7, 2024 15:10
@mweatherley mweatherley added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant