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

Add sample_clamped method to animation curves, returning Boxed values #16395

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mbrea-c
Copy link

@mbrea-c mbrea-c commented Nov 15, 2024

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

Copy link
Contributor

Welcome, new contributor!

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

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Animation Make things move and change over time C-Feature A new feature, making something new possible and removed C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Nov 15, 2024
@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Nov 15, 2024
@alice-i-cecile alice-i-cecile modified the milestones: 0.15, 0.16 Nov 15, 2024
@alice-i-cecile alice-i-cecile added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Nov 15, 2024
@mweatherley
Copy link
Contributor

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

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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants