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

Allow animation clips to animate arbitrary properties. #15282

Merged
merged 11 commits into from
Sep 23, 2024

Conversation

pcwalton
Copy link
Contributor

Currently, Bevy restricts animation clips to animating Transform::translation, Transform::rotation, Transform::scale, or MorphWeights, which correspond to the properties that glTF can animate. This is insufficient for many use cases such as animating UI, as the UI layout systems expect to have exclusive control over UI elements' Transforms and therefore the Style properties must be animated instead.

This commit fixes this, allowing for AnimationClips to animate arbitrary properties. The Keyframes structure has been turned into a low-level trait that can be implemented to achieve arbitrary animation behavior. Along with Keyframes, this patch adds a higher-level trait, AnimatableProperty, that simplifies the task of animating single interpolable properties. Built-in Keyframes implementations exist for translation, rotation, scale, and morph weights. For the most part, you can migrate by simply changing your code from
Keyframes::Translation(...) to TranslationKeyframes(...), and likewise for rotation, scale, and morph weights.

An example AnimatableProperty implementation for the font size of a text section follows:

 #[derive(Reflect)]
 struct FontSizeProperty;

 impl AnimatableProperty for FontSizeProperty {
     type Component = Text;
     type Property = f32;
     fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> {
         Some(&mut component.sections.get_mut(0)?.style.font_size)
     }
 }

In order to keep this patch relatively small, this patch doesn't include an implementation of AnimatableProperty on top of the reflection system. That can be a follow-up.

This patch builds on top of the new EntityMutExcept<> type in order to widen the AnimationTarget query to include write access to all components. Because EntityMutExcept<> has some performance overhead over an explicit query, we continue to explicitly query Transform in order to avoid regressing the performance of skeletal animation, such as the many_foxes benchmark. I've measured the performance of that benchmark and have found no significant regressions.

A new example, animated_ui, has been added. This example shows how to use Bevy's built-in animation infrastructure to animate font size and color, which wasn't possible before this patch.

Showcase

BevyAnimatedUI.mp4

Migration Guide

  • Animation keyframes are now an extensible trait, not an enum. Replace Keyframes::Translation(...), Keyframes::Scale(...), Keyframes::Rotation(...), and Keyframes::Weights(...) with Box::new(TranslationKeyframes(...)), Box::new(ScaleKeyframes(...)), Box::new(RotationKeyframes(...)), and Box::new(MorphWeightsKeyframes(...)) respectively.

@pcwalton pcwalton added the A-Animation Make things move and change over time label Sep 18, 2024
@pcwalton pcwalton added C-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 18, 2024
@pcwalton pcwalton added this to the 0.15 milestone Sep 18, 2024
Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@pcwalton pcwalton force-pushed the animatable-keyframes branch 2 times, most recently from b0eb301 to 38498f2 Compare September 18, 2024 06:50
Currently, Bevy restricts animation clips to animating
`Transform::translation`, `Transform::rotation`, `Transform::scale`, or
`MorphWeights`, which correspond to the properties that glTF can
animate. This is insufficient for many use cases such as animating UI,
as the UI layout systems expect to have exclusive control over UI
elements' `Transform`s and therefore the `Style` properties must be
animated instead.

This commit fixes this, allowing for `AnimationClip`s to animate
arbitrary properties. The `Keyframes` structure has been turned into a
low-level trait that can be implemented to achieve arbitrary animation
behavior. Along with `Keyframes`, this patch adds a higher-level trait,
`AnimatableProperty`, that simplifies the task of animating single
interpolable properties. Built-in `Keyframes` implementations exist for
translation, rotation, scale, and morph weights. For the most part, you
can migrate by simply changing your code from
`Keyframes::Translation(...)` to `TranslationKeyframes(...)`, and
likewise for rotation, scale, and morph weights.

An example `AnimatableProperty` implementation for the font size of a
text section follows:

     #[derive(Reflect)]
     struct FontSizeProperty;

     impl AnimatableProperty for FontSizeProperty {
         type Component = Text;
         type Property = f32;
         fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> {
             Some(&mut component.sections.get_mut(0)?.style.font_size)
         }
     }

In order to keep this patch relatively small, this patch doesn't include
an implementation of `AnimatableProperty` on top of the reflection
system. That can be a follow-up.

This patch builds on top of the new `EntityMutExcept<>` type in order to
widen the `AnimationTarget` query to include write access to all
components. Because `EntityMutExcept<>` has some performance overhead
over an explicit query, we continue to explicitly query `Transform` in
order to avoid regressing the performance of skeletal animation, such as
the `many_foxes` benchmark. I've measured the performance of that
benchmark and have found no significant regressions.

A new example, `animated_ui`, has been added. This example shows how to
use Bevy's built-in animation infrastructure to animate font size and
color, which wasn't possible before this patch.
Copy link
Contributor

@notmd notmd left a comment

Choose a reason for hiding this comment

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

Some unnecessary derefs.

crates/bevy_animation/src/keyframes.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/keyframes.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/keyframes.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/keyframes.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

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

Love the functionality this adds. It's a shame you had to hand-implement Reflect for VariableCurve, but I don't see any reasonable workaround short of improving the derive macro.

crates/bevy_animation/src/animatable.rs Show resolved Hide resolved
examples/animation/animated_transform.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

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

Tiny suggestion around a comment in the example, otherwise looks great!

examples/animation/animated_ui.rs Outdated Show resolved Hide resolved
examples/animation/animated_ui.rs Outdated Show resolved Hide resolved
@james7132 james7132 requested review from james7132 and removed request for notmd September 20, 2024 19:07
Copy link
Contributor

@kristoff3r kristoff3r left a comment

Choose a reason for hiding this comment

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

Looks very good to me, I really like the level of documentation and examples. I tested all of the animation related examples.

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/lib.rs Show resolved Hide resolved
Copy link
Contributor

@atornity atornity left a comment

Choose a reason for hiding this comment

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

Look good to me. I do think it would be cool if we could call the VariableCurve constructors using arrays, like so:

let curve = VariableCurve::linear::<TranslationKeyframes>(
    [0.0, 1.0, 2.0],
    [
        Vec3::new(1.0, 0.0, 1.0),
        Vec3::new(-1.0, 0.0, 1.0),
        Vec3::new(-1.0, 0.0, -1.0),
    ],
);

crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/keyframes.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/keyframes.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/keyframes.rs Outdated Show resolved Hide resolved
@pcwalton
Copy link
Contributor Author

Comments addressed.

examples/animation/animated_transform.rs Outdated Show resolved Hide resolved
@pcwalton pcwalton added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 22, 2024
Comment on lines +72 to +75
+ GetTypeRegistration
+ Reflect
+ TypePath
+ Typed
Copy link
Member

Choose a reason for hiding this comment

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

Note: This should now be replaceable by Reflectable (on main, see #5772).

@alice-i-cecile alice-i-cecile added M-Needs-Release-Note Work that should be called out in the blog due to impact X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers labels Sep 23, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 23, 2024
@alice-i-cecile alice-i-cecile added the A-Reflection Runtime information about types label Sep 23, 2024
Merged via the queue into bevyengine:main with commit 8154164 Sep 23, 2024
34 checks passed
pub fn animate_targets(
clips: Res<Assets<AnimationClip>>,
graphs: Res<Assets<AnimationGraph>>,
players: Query<(&AnimationPlayer, &Handle<AnimationGraph>)>,
mut targets: Query<(
Entity,
&AnimationTarget,
Copy link

@pyhinox pyhinox Sep 24, 2024

Choose a reason for hiding this comment

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

Will it faster if we keep the &AnimationTarget here instead of checking if entity has AnimationTarget at line 1114?I think it could help reduce the search scope.

github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2024
# Objective

This PR extends and reworks the material from #15282 by allowing
arbitrary curves to be used by the animation system to animate arbitrary
properties. The goals of this work are to:
- Allow far greater flexibility in how animations are allowed to be
defined in order to be used with `bevy_animation`.
- Delegate responsibility over keyframe interpolation to `bevy_math` and
the `Curve` libraries and reduce reliance on keyframes in animation
definitions generally.
- Move away from allowing the glTF spec to completely define animations
on a mechanical level.

## Solution

### Overview

At a high level, curves have been incorporated into the animation system
using the `AnimationCurve` trait (closely related to what was
`Keyframes`). From the top down:

1. In `animate_targets`, animations are driven by `VariableCurve`, which
is now a thin wrapper around a `Box<dyn AnimationCurve>`.
2. `AnimationCurve` is something built out of a `Curve`, and it tells
the animation system how to use the curve's output to actually mutate
component properties. The trait looks like this:
```rust
/// A low-level trait that provides control over how curves are actually applied to entities
/// by the animation system.
///
/// Typically, this will not need to be implemented manually, since it is automatically
/// implemented by [`AnimatableCurve`] and other curves used by the animation system
/// (e.g. those that animate parts of transforms or morph weights). However, this can be
/// implemented manually when `AnimatableCurve` is not sufficiently expressive.
///
/// In many respects, this behaves like a type-erased form of [`Curve`], where the output
/// type of the curve is remembered only in the components that are mutated in the
/// implementation of [`apply`].
///
/// [`apply`]: AnimationCurve::apply
pub trait AnimationCurve: Reflect + Debug + Send + Sync {
    /// Returns a boxed clone of this value.
    fn clone_value(&self) -> Box<dyn AnimationCurve>;

    /// The range of times for which this animation is defined.
    fn domain(&self) -> Interval;

    /// Write the value of sampling this curve at time `t` into `transform` or `entity`,
    /// as appropriate, interpolating between the existing value and the sampled value
    /// using the given `weight`.
    fn apply<'a>(
        &self,
        t: f32,
        transform: Option<Mut<'a, Transform>>,
        entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle<AnimationGraph>)>,
        weight: f32,
    ) -> Result<(), AnimationEvaluationError>;
}
```
3. The conversion process from a `Curve` to an `AnimationCurve` involves
using wrappers which communicate the intent to animate a particular
property. For example, here is `TranslationCurve`, which wraps a
`Curve<Vec3>` and uses it to animate `Transform::translation`:
```rust
/// This type allows a curve valued in `Vec3` to become an [`AnimationCurve`] that animates
/// the translation component of a transform.
pub struct TranslationCurve<C>(pub C);
```

### Animatable Properties

The `AnimatableProperty` trait survives in the transition, and it can be
used to allow curves to animate arbitrary component properties. The
updated documentation for `AnimatableProperty` explains this process:
<details>
  <summary>Expand AnimatableProperty example</summary

An `AnimatableProperty` is a value on a component that Bevy can animate.

You can implement this trait on a unit struct in order to support
animating
custom components other than transforms and morph weights. Use that type
in
conjunction with `AnimatableCurve` (and perhaps
`AnimatableKeyframeCurve`
to define the animation itself). For example, in order to animate font
size of a
text section from 24 pt. to 80 pt., you might use:

```rust
#[derive(Reflect)]
struct FontSizeProperty;

impl AnimatableProperty for FontSizeProperty {
    type Component = Text;
    type Property = f32;
    fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> {
        Some(&mut component.sections.get_mut(0)?.style.font_size)
    }
}
```

You can then create an `AnimationClip` to animate this property like so:

```rust
let mut animation_clip = AnimationClip::default();
animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableKeyframeCurve::new(
        [
            (0.0, 24.0),
            (1.0, 80.0),
        ]
    )
    .map(AnimatableCurve::<FontSizeProperty, _>::from_curve)
    .expect("Failed to create font size curve")
);
```

Here, the use of `AnimatableKeyframeCurve` creates a curve out of the
given keyframe time-value
pairs, using the `Animatable` implementation of `f32` to interpolate
between them. The
invocation of `AnimatableCurve::from_curve` with `FontSizeProperty`
indicates that the `f32`
output from that curve is to be used to animate the font size of a
`Text` component (as
configured above).


</details>

### glTF Loading

glTF animations are now loaded into `Curve` types of various kinds,
depending on what is being animated and what interpolation mode is being
used. Those types get wrapped into and converted into `Box<dyn
AnimationCurve>` and shoved inside of a `VariableCurve` just like
everybody else.

### Morph Weights

There is an `IterableCurve` abstraction which allows sampling these from
a contiguous buffer without allocating. Its only reason for existing is
that Rust disallows you from naming function types, otherwise we would
just use `Curve` with an iterator output type. (The iterator involves
`Map`, and the name of the function type would have to be able to be
named, but it is not.)

A `WeightsCurve` adaptor turns an `IterableCurve` into an
`AnimationCurve`, so it behaves like everything else in that regard.

## Testing

Tested by running existing animation examples. Interpolation logic has
had additional tests added within the `Curve` API to replace the tests
in `bevy_animation`. Some kinds of out-of-bounds errors have become
impossible.

Performance testing on `many_foxes` (`animate_targets`) suggests that
performance is very similar to the existing implementation. Here are a
couple trace histograms across different runs (yellow is this branch,
red is main).
<img width="669" alt="Screenshot 2024-09-27 at 9 41 50 AM"
src="https://github.com/user-attachments/assets/5ba4e9ac-3aea-452e-aaf8-1492acc2d7fc">
<img width="673" alt="Screenshot 2024-09-27 at 9 45 18 AM"
src="https://github.com/user-attachments/assets/8982538b-04cf-46b5-97b2-164c6bc8162e">

---

## Migration Guide

Most user code that does not directly deal with `AnimationClip` and
`VariableCurve` will not need to be changed. On the other hand,
`VariableCurve` has been completely overhauled. If you were previously
defining animation curves in code using keyframes, you will need to
migrate that code to use curve constructors instead. For example, a
rotation animation defined using keyframes and added to an animation
clip like this:
```rust
animation_clip.add_curve_to_target(
    animation_target_id,
    VariableCurve {
        keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
        keyframes: Keyframes::Rotation(vec![
            Quat::IDENTITY,
            Quat::from_axis_angle(Vec3::Y, PI / 2.),
            Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
            Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
            Quat::IDENTITY,
        ]),
        interpolation: Interpolation::Linear,
    },
);
```

would now be added like this:
```rust
animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableKeyframeCurve::new([0.0, 1.0, 2.0, 3.0, 4.0].into_iter().zip([
        Quat::IDENTITY,
        Quat::from_axis_angle(Vec3::Y, PI / 2.),
        Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
        Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
        Quat::IDENTITY,
    ]))
    .map(RotationCurve)
    .expect("Failed to build rotation curve"),
);
```

Note that the interface of `AnimationClip::add_curve_to_target` has also
changed (as this example shows, if subtly), and now takes its curve
input as an `impl AnimationCurve`. If you need to add a `VariableCurve`
directly, a new method `add_variable_curve_to_target` accommodates that
(and serves as a one-to-one migration in this regard).

### For reviewers

The diff is pretty big, and the structure of some of the changes might
not be super-obvious:
- `keyframes.rs` became `animation_curves.rs`, and `AnimationCurve` is
based heavily on `Keyframes`, with the adaptors also largely following
suite.
- The Curve API adaptor structs were moved from `bevy_math::curve::mod`
into their own module `adaptors`. There are no functional changes to how
these adaptors work; this is just to make room for the specialized
reflection implementations since `mod.rs` was getting kind of cramped.
- The new module `gltf_curves` holds the additional curve constructions
that are needed by the glTF loader. Note that the loader uses a mix of
these and off-the-shelf `bevy_math` curve stuff.
- `animatable.rs` no longer holds logic related to keyframe
interpolation, which is now delegated to the existing abstractions in
`bevy_math::curve::cores`.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com>
robtfm pushed a commit to robtfm/bevy that referenced this pull request Oct 4, 2024
# Objective

This PR extends and reworks the material from bevyengine#15282 by allowing
arbitrary curves to be used by the animation system to animate arbitrary
properties. The goals of this work are to:
- Allow far greater flexibility in how animations are allowed to be
defined in order to be used with `bevy_animation`.
- Delegate responsibility over keyframe interpolation to `bevy_math` and
the `Curve` libraries and reduce reliance on keyframes in animation
definitions generally.
- Move away from allowing the glTF spec to completely define animations
on a mechanical level.

## Solution

### Overview

At a high level, curves have been incorporated into the animation system
using the `AnimationCurve` trait (closely related to what was
`Keyframes`). From the top down:

1. In `animate_targets`, animations are driven by `VariableCurve`, which
is now a thin wrapper around a `Box<dyn AnimationCurve>`.
2. `AnimationCurve` is something built out of a `Curve`, and it tells
the animation system how to use the curve's output to actually mutate
component properties. The trait looks like this:
```rust
/// A low-level trait that provides control over how curves are actually applied to entities
/// by the animation system.
///
/// Typically, this will not need to be implemented manually, since it is automatically
/// implemented by [`AnimatableCurve`] and other curves used by the animation system
/// (e.g. those that animate parts of transforms or morph weights). However, this can be
/// implemented manually when `AnimatableCurve` is not sufficiently expressive.
///
/// In many respects, this behaves like a type-erased form of [`Curve`], where the output
/// type of the curve is remembered only in the components that are mutated in the
/// implementation of [`apply`].
///
/// [`apply`]: AnimationCurve::apply
pub trait AnimationCurve: Reflect + Debug + Send + Sync {
    /// Returns a boxed clone of this value.
    fn clone_value(&self) -> Box<dyn AnimationCurve>;

    /// The range of times for which this animation is defined.
    fn domain(&self) -> Interval;

    /// Write the value of sampling this curve at time `t` into `transform` or `entity`,
    /// as appropriate, interpolating between the existing value and the sampled value
    /// using the given `weight`.
    fn apply<'a>(
        &self,
        t: f32,
        transform: Option<Mut<'a, Transform>>,
        entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle<AnimationGraph>)>,
        weight: f32,
    ) -> Result<(), AnimationEvaluationError>;
}
```
3. The conversion process from a `Curve` to an `AnimationCurve` involves
using wrappers which communicate the intent to animate a particular
property. For example, here is `TranslationCurve`, which wraps a
`Curve<Vec3>` and uses it to animate `Transform::translation`:
```rust
/// This type allows a curve valued in `Vec3` to become an [`AnimationCurve`] that animates
/// the translation component of a transform.
pub struct TranslationCurve<C>(pub C);
```

### Animatable Properties

The `AnimatableProperty` trait survives in the transition, and it can be
used to allow curves to animate arbitrary component properties. The
updated documentation for `AnimatableProperty` explains this process:
<details>
  <summary>Expand AnimatableProperty example</summary

An `AnimatableProperty` is a value on a component that Bevy can animate.

You can implement this trait on a unit struct in order to support
animating
custom components other than transforms and morph weights. Use that type
in
conjunction with `AnimatableCurve` (and perhaps
`AnimatableKeyframeCurve`
to define the animation itself). For example, in order to animate font
size of a
text section from 24 pt. to 80 pt., you might use:

```rust
#[derive(Reflect)]
struct FontSizeProperty;

impl AnimatableProperty for FontSizeProperty {
    type Component = Text;
    type Property = f32;
    fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> {
        Some(&mut component.sections.get_mut(0)?.style.font_size)
    }
}
```

You can then create an `AnimationClip` to animate this property like so:

```rust
let mut animation_clip = AnimationClip::default();
animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableKeyframeCurve::new(
        [
            (0.0, 24.0),
            (1.0, 80.0),
        ]
    )
    .map(AnimatableCurve::<FontSizeProperty, _>::from_curve)
    .expect("Failed to create font size curve")
);
```

Here, the use of `AnimatableKeyframeCurve` creates a curve out of the
given keyframe time-value
pairs, using the `Animatable` implementation of `f32` to interpolate
between them. The
invocation of `AnimatableCurve::from_curve` with `FontSizeProperty`
indicates that the `f32`
output from that curve is to be used to animate the font size of a
`Text` component (as
configured above).


</details>

### glTF Loading

glTF animations are now loaded into `Curve` types of various kinds,
depending on what is being animated and what interpolation mode is being
used. Those types get wrapped into and converted into `Box<dyn
AnimationCurve>` and shoved inside of a `VariableCurve` just like
everybody else.

### Morph Weights

There is an `IterableCurve` abstraction which allows sampling these from
a contiguous buffer without allocating. Its only reason for existing is
that Rust disallows you from naming function types, otherwise we would
just use `Curve` with an iterator output type. (The iterator involves
`Map`, and the name of the function type would have to be able to be
named, but it is not.)

A `WeightsCurve` adaptor turns an `IterableCurve` into an
`AnimationCurve`, so it behaves like everything else in that regard.

## Testing

Tested by running existing animation examples. Interpolation logic has
had additional tests added within the `Curve` API to replace the tests
in `bevy_animation`. Some kinds of out-of-bounds errors have become
impossible.

Performance testing on `many_foxes` (`animate_targets`) suggests that
performance is very similar to the existing implementation. Here are a
couple trace histograms across different runs (yellow is this branch,
red is main).
<img width="669" alt="Screenshot 2024-09-27 at 9 41 50 AM"
src="https://github.com/user-attachments/assets/5ba4e9ac-3aea-452e-aaf8-1492acc2d7fc">
<img width="673" alt="Screenshot 2024-09-27 at 9 45 18 AM"
src="https://github.com/user-attachments/assets/8982538b-04cf-46b5-97b2-164c6bc8162e">

---

## Migration Guide

Most user code that does not directly deal with `AnimationClip` and
`VariableCurve` will not need to be changed. On the other hand,
`VariableCurve` has been completely overhauled. If you were previously
defining animation curves in code using keyframes, you will need to
migrate that code to use curve constructors instead. For example, a
rotation animation defined using keyframes and added to an animation
clip like this:
```rust
animation_clip.add_curve_to_target(
    animation_target_id,
    VariableCurve {
        keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
        keyframes: Keyframes::Rotation(vec![
            Quat::IDENTITY,
            Quat::from_axis_angle(Vec3::Y, PI / 2.),
            Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
            Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
            Quat::IDENTITY,
        ]),
        interpolation: Interpolation::Linear,
    },
);
```

would now be added like this:
```rust
animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableKeyframeCurve::new([0.0, 1.0, 2.0, 3.0, 4.0].into_iter().zip([
        Quat::IDENTITY,
        Quat::from_axis_angle(Vec3::Y, PI / 2.),
        Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
        Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
        Quat::IDENTITY,
    ]))
    .map(RotationCurve)
    .expect("Failed to build rotation curve"),
);
```

Note that the interface of `AnimationClip::add_curve_to_target` has also
changed (as this example shows, if subtly), and now takes its curve
input as an `impl AnimationCurve`. If you need to add a `VariableCurve`
directly, a new method `add_variable_curve_to_target` accommodates that
(and serves as a one-to-one migration in this regard).

### For reviewers

The diff is pretty big, and the structure of some of the changes might
not be super-obvious:
- `keyframes.rs` became `animation_curves.rs`, and `AnimationCurve` is
based heavily on `Keyframes`, with the adaptors also largely following
suite.
- The Curve API adaptor structs were moved from `bevy_math::curve::mod`
into their own module `adaptors`. There are no functional changes to how
these adaptors work; this is just to make room for the specialized
reflection implementations since `mod.rs` was getting kind of cramped.
- The new module `gltf_curves` holds the additional curve constructions
that are needed by the glTF loader. Note that the loader uses a mix of
these and off-the-shelf `bevy_math` curve stuff.
- `animatable.rs` no longer holds logic related to keyframe
interpolation, which is now delegated to the existing abstractions in
`bevy_math::curve::cores`.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com>
@alice-i-cecile
Copy link
Member

Thank you to everyone involved with the authoring or reviewing of this PR! This work is relatively important and needs release notes! Head over to bevyengine/bevy-website#1692 if you'd like to help out.

@UkoeHB UkoeHB mentioned this pull request Nov 4, 2024
11 tasks
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 A-Reflection Runtime information about types C-Feature A new feature, making something new possible M-Needs-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants