Add sample_clamped method to animation curves, returning Boxed values

[mbrea-c]

Objective

Currently the evaluation of AnimationCurves is coupled to the Bevy animation system. This PR provides an integration point for plugins that would like to reuse the animation loading (e.g. from GLTF) and sampling code but implement a custom animation composition system (the use case I have in mind is bevy_animation_graph).

Solution

The AnimationCurve trait provides a sample_clamped method, which returns a boxed version of the result of sampling the underlying curve:

fn sample_clamped(&self, t: f32) -> Box<dyn Any>;

Each of the provided implementations of AnimationCurve return a custom sampled type, e.g.:

#[derive(Reflect)]
pub struct TranslationCurveSample(Vec3);

impl<C> AnimationCurve for TranslationCurve<C>
where
    C: AnimationCompatibleCurve<Vec3>,
{
    // ...

    fn sample_clamped(&self, t: f32) -> Box<dyn Any> {
        let value = self.0.sample_clamped(t);

        Box::new(TranslationCurveSample(value))
    }
}

This fits with the "open polymorphism" approach that the rest of the rest of the animation curves code is using. Users can then check the TypeId of the returned type to determine what to do with the sampled value.

Alternative solutions

Closed polymorphism

Provide an enum value as the result of AnimationCurve::sample_clamped:

pub enum CurveSample {
    Translation(Vec3),
    Rotation(Quat),
    Scale(Vec3),
    AnimatableCurve(AnimatableCurveSample),
}

However this would be at odds with the current implementation of AnimationCurve, where users can implement their own AnimationCurve types.

Testing

• Added a new doctest, run all unit tests.
• Ran animation examples to ensure no regressions.
• No benchmarking done, since the existing animation flow is untouched.
• Pending testing tasks:
  • AnimatableCurve sampling.
  • WeightsCurve sampling.
  • Test using this API in bevy_animation_graph.

---

Showcase

bevy_animation_graph example

bevy_animation_graph evaluates animation curves in "Clip" nodes, and stores the
sampled values for all targets in a Pose struct which is made available to
other nodes as an output.

Currently evaluating animation curves in bevy_animation_graph involves abusing
the Reflect API to access private fields in the curve evaluators:

fn sample_animation_curve(curve: &VariableCurve, time: f32) -> CurveValue {
    let mut evaluator = curve.0.create_evaluator();
    let evaluator_type_id = curve.0.evaluator_type();
    let evaluator_type_name = evaluator.reflect_type_info().type_path();
    let node_index = AnimationNodeIndex::default();

    curve
        .0
        .apply(evaluator.as_mut(), time, 1., node_index)
        .unwrap();

    if evaluator_type_id == TypeId::of::<TranslationCurveEvaluator>() {
        let t = evaluator
            .as_reflect()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("evaluator")
            .unwrap()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("stack")
            .unwrap()
            .reflect_ref()
            .as_list()
            .unwrap()
            .get(0)
            .unwrap()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("value")
            .unwrap()
            .try_downcast_ref::<Vec3>()
            .unwrap();
        CurveValue::Translation(*t)
    } else if evaluator_type_id == TypeId::of::<RotationCurveEvaluator>() {
        let r = evaluator
            .as_reflect()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("evaluator")
            .unwrap()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("stack")
            .unwrap()
            .reflect_ref()
            .as_list()
            .unwrap()
            .get(0)
            .unwrap()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("value")
            .unwrap()
            .try_downcast_ref::<Quat>()
            .unwrap();
        CurveValue::Rotation(*r)
    } else if evaluator_type_id == TypeId::of::<ScaleCurveEvaluator>() {
        let s = evaluator
            .as_reflect()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("evaluator")
            .unwrap()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("stack")
            .unwrap()
            .reflect_ref()
            .as_list()
            .unwrap()
            .get(0)
            .unwrap()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("value")
            .unwrap()
            .try_downcast_ref::<Vec3>()
            .unwrap();
        CurveValue::Scale(*s)
    } else if evaluator_type_name
        // Need to resort to this because WeightsCurveEvaluator is private
        .starts_with("bevy_animation::animation_curves::WeightsCurveEvaluator")
    {
        let w = evaluator
            .as_reflect()
            .reflect_ref()
            .as_struct()
            .unwrap()
            .field("stack_morph_target_weights")
            .unwrap()
            .try_downcast_ref::<Vec<f32>>()
            .unwrap()
            .clone();
        CurveValue::BoneWeights(w)
    } else {
        panic!("Animatable curve type not yet supported!");
    }
}

With this PR, we can instead do the following:

fn sample_animation_curve(curve: &VariableCurve, time: f32) -> CurveValue {
    let curve_sample = curve.0.sample_clampled(time);
    let sample_type_id = curve_sample.as_ref().type_id();

    if TypeId::of::<TranslationCurveSample>() == sample_type_id {
        let cast_sample = curve_sample.downcast::<TranslationCurveSample>().unwrap();
        CurveValue::Translation(cast_sample.0)
    } else if TypeId::of::<RotationCurveSample>() == sample_type_id {
        let cast_sample = curve_sample.downcast::<RotationCurveSample>().unwrap();
        CurveValue::Rotation(cast_sample.0)
    } else if TypeId::of::<ScaleCurveSample>() == sample_type_id {
        let cast_sample = curve_sample.downcast::<ScaleCurveSample>().unwrap();
        CurveValue::Scale(cast_sample.0)
    } else if TypeId::of::<WeightsCurveSample>() == sample_type_id {
        let cast_sample = curve_sample.downcast::<WeightsCurveSample>().unwrap();
        CurveValue::Weights(cast_sample.0)
    } else {
        panic!("Animatable curve type not yet supported!");
    }
}

Furthermore, the API in this PR opens the door to adding support for
AnimatableCurves, whereas before this was not possible (or at least I could
not find a way).

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[mweatherley]

This is fairly close to what I had imagined in this direction (see also #15664 ); another idea would be to yield a dyn Curve<T> in some way from a dyn AnimationCurve when you have extrinsic knowledge of the return type; e.g.

fn downcast_curve<T>(self) -> Option<Box<dyn Curve<T>>> {
    // yield a boxed version of the underlying curve if `T` matches the curve type, otherwise None
}

I think this is a bit more reusable, but the use-cases might be slightly different, since the dependency on the curve type is a bit different. The main upside with something like this would be that you can do other stuff with the curve API instead of only having the output value. (The main downside in my view is that implementing this requires unsafe code, since this is effectively a "custom Any".)

[mbrea-c]

This is fairly close to what I had imagined in this direction (see also #15664 ); another idea would be to yield a dyn Curve<T> in some way from a dyn AnimationCurve when you have extrinsic knowledge of the return type; e.g.

fn downcast_curve<T>(self) -> Option<Box<dyn Curve<T>>> {
    // yield a boxed version of the underlying curve if `T` matches the curve type, otherwise None
}

I think this is a bit more reusable, but the use-cases might be slightly different, since the dependency on the curve type is a bit different. The main upside with something like this would be that you can do other stuff with the curve API instead of only having the output value. (The main downside in my view is that implementing this requires unsafe code, since this is effectively a "custom Any".)

I like that, but maybe there's value in having both. I think only exposing a Box<dyn Curve<T>> wouldn't cover the use case I'm thinking of:

1. My main issue is that we may not have have extrinsic knowledge of the return type; e.g. you have an AnimationClip and want to evaluate all curves in it; all you know is that there's a bunch of VariableCurves, which could have as output type a Vec3, Quat, Vec<f32> or any other arbitrary property (AnimatableProperty::Property). The API I proposed returns a boxed value whose type you can check, so it works in this case. Also, you have a single boxed type for all AnimatableCurve types (regardless of AnimatableProperty), which currently are hard to deal with without knowing the type in advance as the AnimatableCurveEvaluator's type depends on the AnimatableCurve type parameter.
2. Even if return types are equal, we would want to differentiate between different kinds of AnimationCurve in order to apply the result correctly (e.g. translation and scale are both Vec3). This one we could workaround by checking the type of the evaluator, but that feels more hacky (maybe that's just me).
3. WeightsCurve uses an IterableCurve<f32> instead of a Curve<Vec<f32>>. I haven't looked through all of the new Curve code yet so maybe there's a way to adapt the former to the latter? Otherwise this seems like a problem for this API.

What do you think?

[mweatherley]

My main issue is that we may not have have extrinsic knowledge of the return type; e.g. you have an AnimationClip and want to evaluate all curves in it; all you know is that there's a bunch of VariableCurves, which could have as output type a Vec3, Quat, Vec<f32> or any other arbitrary property (AnimatableProperty::Property). The API I proposed returns a boxed value whose type you can check, so it works in this case. Also, you have a single boxed type for all AnimatableCurve types (regardless of AnimatableProperty), which currently are hard to deal with without knowing the type in advance as the AnimatableCurveEvaluator's type depends on the AnimatableCurve type parameter.

That makes sense. I have been kind of wondering if the right direction for the animation system long-term would be to tie every animation curve to a specific AnimatableProperty type (instead of making Transform parts behave specially, for example); that way, the relationship between an AnimationCurve and its curve output would at least be reified in a sort of natural way (as the Property type of the AnimatableProperty).

I don't think having both is unreasonable :)

Even if return types are equal, we would want to differentiate between different kinds of AnimationCurve in order to apply the result correctly (e.g. translation and scale are both Vec3). This one we could workaround by checking the type of the evaluator, but that feels more hacky (maybe that's just me).

Yeah, this is another reason to want to tie things to AnimatableProperty.

WeightsCurve uses an IterableCurve<f32> instead of a Curve<Vec<f32>>. I haven't looked through all of the new Curve code yet so maybe there's a way to adapt the former to the latter? Otherwise this seems like a problem for this API.

Every IterableCurve<f32> can also be a Curve<Vec<f32>> just by calling collect, which is probably what I would do for this API, even if it involves special-casing. There isn't really a way around this: glTF morph weights curves don't even have a 'static return type (since the returned iterator has a lifetime dependence on the curve itself), so they are like Curve<T> where T: !Any.

(For some background, the only reason that IterableCurve exists is that it would be preferable for these curves to just be Curve<T> where T is an iterator, but the type of that iterator is made impossible to name by the Rust compiler. Hopefully that will change somewhat in the future.)

[mbrea-c]

That makes sense. I have been kind of wondering if the right direction for the animation system long-term would be to tie every animation curve to a specific AnimatableProperty type (instead of making Transform parts behave specially, for example); that way, the relationship between an AnimationCurve and its curve output would at least be reified in a sort of natural way (as the Property type of the AnimatableProperty).

I don't know what the reasons were for keeping the Transform and weight curves separate tbh (performance maybe?), but I agree that having a more "natural" relationship between the AnimationCurve and its output type would be a good thing. Still worried about the dynamic use case though—unless I'm missing something, having a P: AnimatableProperty type parameter on all curves would make type id comparisons complicated when there's no extrinsic type information about the curve—, what would be the API you're thinking of?

(sorry for the long time between replies btw, hard to find time after day job with end of year deadlines approaching)