Skip to content

Commit

Permalink
Fix additive blending of quaternions (#15662)
Browse files Browse the repository at this point in the history
# Objective

Fixes #13832

## Solution

Additively blend quaternions like this:
```rust
rotation = Quat::slerp(Quat::IDENTITY, incoming_rotation, weight) * rotation;
```

## Testing

Ran `animation_masks`, which behaves the same as before. (In the context
of an animation being blended only onto the base pose, there is no
difference.)

We should create some examples that actually exercise more of the
capabilities of the `AnimationGraph` so that issues like this can become
more visible in general. (On the other hand, I'm quite certain this was
wrong before.)

## Migration Guide

This PR changes the implementation of `Quat: Animatable`, which was not
used internally by Bevy prior to this release version. If you relied on
the old behavior of additive quaternion blending in manual applications,
that code will have to be updated, as the old behavior was incorrect.
  • Loading branch information
mweatherley authored Oct 5, 2024
1 parent 25bfa80 commit 42e0771
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 17 deletions.
16 changes: 13 additions & 3 deletions crates/bevy_animation/src/animatable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ impl Animatable for Transform {
if input.additive {
translation += input.weight * Vec3A::from(input.value.translation);
scale += input.weight * Vec3A::from(input.value.scale);
rotation = rotation.slerp(input.value.rotation, input.weight);
rotation =
Quat::slerp(Quat::IDENTITY, input.value.rotation, input.weight) * rotation;
} else {
translation = Vec3A::interpolate(
&translation,
Expand Down Expand Up @@ -181,8 +182,17 @@ impl Animatable for Quat {
#[inline]
fn blend(inputs: impl Iterator<Item = BlendInput<Self>>) -> Self {
let mut value = Self::IDENTITY;
for input in inputs {
value = Self::interpolate(&value, &input.value, input.weight);
for BlendInput {
weight,
value: incoming_value,
additive,
} in inputs
{
if additive {
value = Self::slerp(Self::IDENTITY, incoming_value, weight) * value;
} else {
value = Self::interpolate(&value, &incoming_value, weight);
}
}
value
}
Expand Down
21 changes: 7 additions & 14 deletions crates/bevy_animation/src/animation_curves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,7 @@ pub struct AnimatableCurve<P, C> {
///
/// You shouldn't ordinarily need to instantiate one of these manually. Bevy
/// will automatically do so when you use an [`AnimatableCurve`] instance.
#[derive(Reflect, FromReflect)]
#[reflect(from_reflect = false)]
#[derive(Reflect)]
pub struct AnimatableCurveEvaluator<P>
where
P: AnimatableProperty,
Expand Down Expand Up @@ -346,8 +345,7 @@ pub struct TranslationCurve<C>(pub C);
///
/// You shouldn't need to instantiate this manually; Bevy will automatically do
/// so.
#[derive(Reflect, FromReflect)]
#[reflect(from_reflect = false)]
#[derive(Reflect)]
pub struct TranslationCurveEvaluator {
evaluator: BasicAnimationCurveEvaluator<Vec3>,
}
Expand Down Expand Up @@ -444,8 +442,7 @@ pub struct RotationCurve<C>(pub C);
///
/// You shouldn't need to instantiate this manually; Bevy will automatically do
/// so.
#[derive(Reflect, FromReflect)]
#[reflect(from_reflect = false)]
#[derive(Reflect)]
pub struct RotationCurveEvaluator {
evaluator: BasicAnimationCurveEvaluator<Quat>,
}
Expand Down Expand Up @@ -542,8 +539,7 @@ pub struct ScaleCurve<C>(pub C);
///
/// You shouldn't need to instantiate this manually; Bevy will automatically do
/// so.
#[derive(Reflect, FromReflect)]
#[reflect(from_reflect = false)]
#[derive(Reflect)]
pub struct ScaleCurveEvaluator {
evaluator: BasicAnimationCurveEvaluator<Vec3>,
}
Expand Down Expand Up @@ -636,8 +632,7 @@ impl AnimationCurveEvaluator for ScaleCurveEvaluator {
#[reflect(from_reflect = false)]
pub struct WeightsCurve<C>(pub C);

#[derive(Reflect, FromReflect)]
#[reflect(from_reflect = false)]
#[derive(Reflect)]
struct WeightsCurveEvaluator {
/// The values of the stack, in which each element is a list of morph target
/// weights.
Expand Down Expand Up @@ -825,8 +820,7 @@ impl AnimationCurveEvaluator for WeightsCurveEvaluator {
}
}

#[derive(Reflect, FromReflect)]
#[reflect(from_reflect = false)]
#[derive(Reflect)]
struct BasicAnimationCurveEvaluator<A>
where
A: Animatable,
Expand All @@ -835,8 +829,7 @@ where
blend_register: Option<(A, f32)>,
}

#[derive(Reflect, FromReflect)]
#[reflect(from_reflect = false)]
#[derive(Reflect)]
struct BasicAnimationCurveEvaluatorStackElement<A>
where
A: Animatable,
Expand Down

0 comments on commit 42e0771

Please sign in to comment.