Additive blending of Quats looks suspect

[mweatherley]

I believe this was previously noticed by @djeedai in the review of the Animatable trait rollout.

I am not an expert in this subject, but here is my understanding.

Context

In additive animation blending, a sample from an animation clip represents a relative displacement rather than an absolute quantity; this is applied on top of other animations by combining the displacement with the absolute quantity "underneath". Like non-additive animation blending (which interpolates between two absolute quantities instead), this can be used in concert with a weight which indicates the strength of the sample's influence in the blend. For additive blending, this means that, as the weight tends to zero, the displacement also attenuates; with a weight of zero, the blend should have no effect on the existing quantity.

For example, additive blending of translational components works something like this:

let (displacement, weight): (Vec3, f32) = incoming_sample;
translation += displacement * weight

The right-hand side of this is perhaps best conceptualized as:

Vec3::lerp(Vec3::ZERO, displacement, weight)

In other words, the additively blended sample is interpolated with zero and then composed with the existing value.

The mismatch

Now, currently additive blending for Quat works like this:

let (rot_displacement, weight): (Quat, f32) = incoming_sample;
rotation = rotation.slerp(rot_displacement, weight);

What I would expect based on my understanding of additive blending is instead something like this:

let (rot_displacement, weight): (Quat, f32) = incoming_sample;
rotation = Quat::slerp(Quat::IDENTITY, rot_displacement, weight) * rotation;

Note that these are definitely not equivalent — when the incoming sample is Quat::IDENTITY, the latter never has any effect, while the former (with positive weight) will slerp the current rotation state towards the default orientation.

Of course, it's possible this was an intentional decision and I'm missing something; this is just my understanding of things.

[IQuick143]

I also wonder how this works with adding multiple additive animations. (That is an intended use-case right?)
In the case of translations, this operation is associative, in the current quaternion case it is not. (but would be in the suggested one).

To me it looks like we're trying to exponentiate group elements here.

We have some group G (of translations, rotations, etc...) we are animating and we have a sample s.
We then have another element a which we want to additively blend, that is, compose it: s <- a * s
(question: is the composition supposed to be a left or right composition? it doesn't matter for translations as they commute, but it does matter for rotations)

We then want to introduce a real-number weight parameter w such that w = 0 does nothing (s <- s) and w = 1 does the full effect of a (s <- a * s), intuitively I'd think that w = 2 should do the effect twice s <- a * a * s and w = 0.5 should do it "half" in the sense that it would have to be applied twice to obtain the effect of w = 1.

This points me to the idea that we're looking for an operation of group exponentiation a^w that follows the standard expected behaviour:

a^0 = identity
a^1 = a
a^(x+y) = a^x * a^y
notably:
a^2 = a * a
and
a = a^0.5 * a^0.5

As far as I know, the sound way to mathematically define this is to:
A) map the group element into its lie algebra (which is a vector space)
B) scale the vector
C) map back to a group element
a^w := exp(w * ln(a))

In the case of translations, the group of vectors is already the lie algebra, hence no visible mapping happens and we just do the vector scaling.
In the case of rotations, unless I'm mistaken, this procedure yields
Quat::slerp(Quat::IDENTITY, a, w)

TL;DR: From a math standpoint I believe that the code is wrong and the new proposal is sound.
I don't know if there's an animation reason to do it mathematically unsoundly like this (why is this behaviour desired?)
Also I wonder if we should left-compose or right-compose the additive rotation?

[IQuick143]

I think left-composing might be the correct option, because it applies the additive operation "after" the base one, which sounds more like the expected behaviour of "adding on top". But I can't speak for animation.

[djeedai]

Yes I'm convinced the current code is wrong as commented in the original PR #4482 and I had linked an animation article coming to the same conclusions as above that you need to slerp with identity. See also #4482 (comment) which I don't necessarily agree (I'm not sure) with but would have been worth an answer too.