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

Curve-based animation #15434

Merged
merged 27 commits into from
Sep 30, 2024
Merged

Curve-based animation #15434

merged 27 commits into from
Sep 30, 2024

Conversation

mweatherley
Copy link
Contributor

@mweatherley mweatherley commented Sep 25, 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:
/// 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>;
}
  1. 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:
/// 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:

Expand AnimatableProperty exampleAn 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:

#[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:

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).

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).
Screenshot 2024-09-27 at 9 41 50 AM
Screenshot 2024-09-27 at 9 45 18 AM


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:

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:

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.

@mweatherley mweatherley added C-Feature A new feature, making something new possible A-Animation Make things move and change over time A-Math Fundamental domain-agnostic mathematical operations M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Contentious There are nontrivial implications that should be thought through S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 25, 2024
@IQuick143 IQuick143 self-requested a review September 26, 2024 06:43
Copy link
Contributor

@NthTensor NthTensor left a comment

Choose a reason for hiding this comment

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

Skimmed this over and it looks fantastic! We probably do need to profile this a bit before merge just because animation perf tends to be so sensitive to small changes, so consider this a conditional approval. I'm also getting the tracy mismatch error, I'll post if I figure out out.

@mweatherley
Copy link
Contributor Author

mweatherley commented Sep 27, 2024

Added some tracing results.

(There is also an easy but small perf win available and untaken, which is special-casing sample_clamped for keyframe curves, avoiding some redundant operations. As I recall from an old version of this glTF transition, the impact is small but measurable.)

@alice-i-cecile alice-i-cecile added the M-Needs-Release-Note Work that should be called out in the blog due to impact label Sep 29, 2024
Copy link
Contributor

@aecsocket aecsocket left a comment

Choose a reason for hiding this comment

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

LGTM, no problems with the logic that I can see, just some of the docs and text in general

crates/bevy_animation/src/animation_curves.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/animation_curves.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/curve/adaptors.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/gltf_curves.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/gltf_curves.rs Outdated Show resolved Hide resolved
examples/animation/animated_transform.rs Outdated Show resolved Hide resolved
examples/animation/animated_ui.rs Outdated Show resolved Hide resolved
examples/animation/animated_ui.rs Outdated Show resolved Hide resolved
examples/animation/animated_transform.rs Outdated Show resolved Hide resolved
examples/animation/animated_transform.rs Outdated Show resolved Hide resolved
github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2024
(Note: #15434 implements something very similar to this for functional
curve adaptors, which is why they aren't present in this PR.)

# Objective

Previously, there was basically no chance that the
explicitly-interpolating sample curve structs from the `Curve` API would
actually be `Reflect`. The reason for this is functional programming:
the structs contain an explicit interpolation `I: Fn(&T, &T, f32) -> T`
which, under typical circumstances, will never be `Reflect`, which
prevents the derive from realistically succeeding. In fact, they won't
be a lot of other things either, notably including both`Debug` and
`TypePath`, which are also required for reflection to succeed.

The goal of this PR is to weaken the implementations of reflection
traits for these structs so that they can implement `Reflect` under
reasonable circumstances. (Notably, they will still not be
`FromReflect`, which is unavoidable.)

## Solution

The function fields are marked as `#[reflect(ignore)]`, and the derive
macro for `Reflect` has `FromReflect` disabled. (This is not fully
optimal, but we don't presently have any kind of "read-only" attribute
for these fields.) Additionally, these structs receive custom `Debug`
and `TypePath` implementations that display the function's (unstable!)
type name instead of its value or type path (respectively). In the case
of `TypePath`, this is a bit janky, but the instability of `type_name`
won't generally present an issue for generics, which would have to be
registered manually in the type registry anyway, which is impossible
because the function type parameters cannot be named.

(And in general, the "blessed" route for such cases would generally
involve manually monomorphizing the function parameter away, which also
allows access to `FromReflect` etc. through very ordinary use of the
derive macro.)

## Testing

Tests in the new `bevy_math::curve::sample_curves` module guarantee that
these are actually `Reflect` under reasonable circumstances.

---

## Future changes

If and when function item types become `Default`, these types will need
to receive custom `FromReflect` implementations that exploit it. Such a
custom implementation would also be desirable if users start doing
things like wrapping function items in `Default`/`FromReflect` wrappers
that still implement a `Fn` trait.

Additionally, if function types become nameable in user-space, the
stance on `Debug`/`TypePath` may bear reexamination, since partial
monomorphization through wrappers would make implementing reflect traits
for function types potentially more viable.

---------

Co-authored-by: Gino Valente <[email protected]>
@alice-i-cecile alice-i-cecile 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 30, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Delightful. I really like the API, the general direction, and especially the detailed, conversational docs. Merging.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 30, 2024
Merged via the queue into bevyengine:main with commit 429987e Sep 30, 2024
27 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Oct 2, 2024
# Objective

It is somewhat unlikely we will actually be able to support
`TransformCurve` (introduced in #15434) after the `AnimationGraph`
evaluation order changes in the immediate future. This is because
correctly blending overlapping animation properties is nontrivial, and
`Transform` overlaps with all of its own fields. We could still
potentially create something like this in the future, but it's likely to
require significant design and implementation work. By way of contrast,
the single-property wrappers `TranslationCurve`, `ScaleCurve`, and
`RotationCurve` should work perfectly fine, since they are
non-overlapping.

In this version release, creating `TransformCurve` in userspace is also
quite easy if desired (see the deletions from this PR).

## Solution

Delete `TransformCurve`. 

## Migration Guide

There is no released version that contains this, but we should make sure
that `TransformCurve` is excluded from the release notes for #15434 if
we merge this pull request.
@mweatherley mweatherley deleted the curve-anim branch October 2, 2024 21:55
robtfm pushed a commit to robtfm/bevy that referenced this pull request Oct 4, 2024
…ine#15493)

(Note: bevyengine#15434 implements something very similar to this for functional
curve adaptors, which is why they aren't present in this PR.)

# Objective

Previously, there was basically no chance that the
explicitly-interpolating sample curve structs from the `Curve` API would
actually be `Reflect`. The reason for this is functional programming:
the structs contain an explicit interpolation `I: Fn(&T, &T, f32) -> T`
which, under typical circumstances, will never be `Reflect`, which
prevents the derive from realistically succeeding. In fact, they won't
be a lot of other things either, notably including both`Debug` and
`TypePath`, which are also required for reflection to succeed.

The goal of this PR is to weaken the implementations of reflection
traits for these structs so that they can implement `Reflect` under
reasonable circumstances. (Notably, they will still not be
`FromReflect`, which is unavoidable.)

## Solution

The function fields are marked as `#[reflect(ignore)]`, and the derive
macro for `Reflect` has `FromReflect` disabled. (This is not fully
optimal, but we don't presently have any kind of "read-only" attribute
for these fields.) Additionally, these structs receive custom `Debug`
and `TypePath` implementations that display the function's (unstable!)
type name instead of its value or type path (respectively). In the case
of `TypePath`, this is a bit janky, but the instability of `type_name`
won't generally present an issue for generics, which would have to be
registered manually in the type registry anyway, which is impossible
because the function type parameters cannot be named.

(And in general, the "blessed" route for such cases would generally
involve manually monomorphizing the function parameter away, which also
allows access to `FromReflect` etc. through very ordinary use of the
derive macro.)

## Testing

Tests in the new `bevy_math::curve::sample_curves` module guarantee that
these are actually `Reflect` under reasonable circumstances.

---

## Future changes

If and when function item types become `Default`, these types will need
to receive custom `FromReflect` implementations that exploit it. Such a
custom implementation would also be desirable if users start doing
things like wrapping function items in `Default`/`FromReflect` wrappers
that still implement a `Fn` trait.

Additionally, if function types become nameable in user-space, the
stance on `Debug`/`TypePath` may bear reexamination, since partial
monomorphization through wrappers would make implementing reflect traits
for function types potentially more viable.

---------

Co-authored-by: Gino Valente <[email protected]>
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 <[email protected]>
Co-authored-by: aecsocket <[email protected]>
robtfm pushed a commit to robtfm/bevy that referenced this pull request Oct 4, 2024
# Objective

It is somewhat unlikely we will actually be able to support
`TransformCurve` (introduced in bevyengine#15434) after the `AnimationGraph`
evaluation order changes in the immediate future. This is because
correctly blending overlapping animation properties is nontrivial, and
`Transform` overlaps with all of its own fields. We could still
potentially create something like this in the future, but it's likely to
require significant design and implementation work. By way of contrast,
the single-property wrappers `TranslationCurve`, `ScaleCurve`, and
`RotationCurve` should work perfectly fine, since they are
non-overlapping.

In this version release, creating `TransformCurve` in userspace is also
quite easy if desired (see the deletions from this PR).

## Solution

Delete `TransformCurve`. 

## Migration Guide

There is no released version that contains this, but we should make sure
that `TransformCurve` is excluded from the release notes for bevyengine#15434 if
we merge this pull request.
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-Math Fundamental domain-agnostic mathematical operations C-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants