Incorporate all node weights in additive blending (#16279)

# Objective

In the existing implementation, additive blending effectively treats the
node with least index specially by basically forcing its weight to be
`1.0` regardless of what its computed weight would be (based on the
weights in the `AnimationGraph` and `AnimationPlayer`).

Arguably this makes some amount of sense, because the "base" animation
is often one which was not authored to be used additively, meaning that
its sampled values are interpreted absolutely rather than as deltas.
However, this also leads to strange behavior with respect to animation
masks: if the "base" animation is masked out on some target, then the
next node is treated as the "base" animation, despite the fact that it
would normally be interpreted additively, and the weight of that
animation is thrown away as a result.

This is all kind of weird and revolves around special treatment (if the
behavior is even really intentional in the first place). From a
mathematical standpoint, there is nothing special about how the "base"
animation must be treated other than having a weight of 1.0 under an
`Add` node, which is something that the user can do without relying on
some bizarre corner-case behavior of the animation system — this is the
only present situation under which weights are discarded.

This PR changes this behavior so that the weight of every node is
incorporated. In other words, for an animation graph that looks like
this:
```text
┌───────────────┐                                 
│Base clip      ┼──┐                              
│      0.5      │  │                              
└───────────────┘  │                              
┌───────────────┐  │  ┌───────────────┐     ┌────┐
│Additive clip 1┼──┼─►┤Additive blend ┼────►│Root│
│      0.1      │  │  │      1.0      │     └────┘
└───────────────┘  │  └───────────────┘           
┌───────────────┐  │                              
│Additive clip 2┼──┘                              
│      0.2      │                                 
└───────────────┘                                 
```

Previously, the result would have been
```text
base_clip + 0.1 * additive_clip_1 + 0.2 * additive_clip_2
```

whereas now it would be
```text
0.5 * base_clip + 0.1 * additive_clip_1 + 0.2 * additive_clip_2
```

and in the scenario where `base_clip` is masked out:
```text
additive_clip_1 + 0.2 * additive_clip_2
```
vs.
```text
0.1 * additive_clip_1 + 0.2 * additive_clip_2
```

## Solution

For background, the way that the additive blending procedure works is
something like this:
- During graph traversal, the node values and weights of the children
are pushed onto the evaluator `stack`. The traversal order guarantees
that the item with least node index will be on top.
- Once we reach the `Add` node itself, we start popping off the `stack`
and into the evaluator's `blend_register`, which is an accumulator
holding up to one weight-value pair:
- If the `blend_register` is empty, it is filled using data from the top
of the `stack`.
- Otherwise, the `blend_register` is combined with data popped from the
`stack` and updated.

In the example above, the additive blending steps would look like this
(with the pre-existing implementation):
1. The `blend_register` is empty, so we pop `(base_clip, 0.5)` from the
top of the `stack` and put it in. Now the value of the `blend_register`
is `(base_clip, 0.5)`.
2. The `blend_register` is non-empty: we pop `(additive_clip_1, 0.1)`
from the top of the `stack` and combine it additively with the value in
the `blend_register`, forming `(base_clip + 0.1 * additive_clip_1, 0.6)`
in the `blend_register` (the carried weight value goes unused).
3. The `blend_register` is non-empty: we pop `(additive_clip_2, 0.2)`
from the top of the `stack` and combine it additively with the value in
the `blend_register`, forming `(base_clip + 0.1 * additive_clip_1 + 0.2
* additive_clip_2, 0.8)` in the `blend_register`.

The solution in this PR changes step 1: the `base_clip` is multiplied by
its weight as it is added to the `blend_register` in the first place,
yielding `0.5 * base_clip + 0.1 * additive_clip_1 + 0.2 *
additive_clip_2` as the final result.

### Note for reviewers

It might be tempting to look at the code, which contains a segment that
looks like this:
```rust
if additive {
    current_value = A::blend(
        [
            BlendInput {
                weight: 1.0, // <--
                value: current_value,
                additive: true,
            },
            BlendInput {
                weight: weight_to_blend,
                value: value_to_blend,
                additive: true,
            },
        ]
        .into_iter(),
    );
} 
```
and conclude that the explicit value of `1.0` is responsible for
overwriting the weight of the base animation. This is incorrect.

Rather, this additive blend has to be written this way because it is
multiplying the *existing value in the blend register* by 1 (i.e. not
doing anything) before adding the next value to it. Changing this to
another quantity (e.g. the existing weight) would cause the value in the
blend register to be spuriously multiplied down.

## Testing

Tested on `animation_masks` example. Checked `morph_weights` example as
well.

## Migration Guide

I will write a migration guide later if this change is not included in
0.15.
This commit is contained in:
Matty 2024-11-07 14:12:08 -05:00 committed by GitHub
parent 1e1b6e5b6d
commit 9beb1d96e7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 31 additions and 6 deletions

View file

@ -746,7 +746,15 @@ impl WeightsCurveEvaluator {
None => {
self.blend_register_blend_weight = Some(weight_to_blend);
self.blend_register_morph_target_weights.clear();
self.blend_register_morph_target_weights.extend(stack_iter);
// In the additive case, the values pushed onto the blend register need
// to be scaled by the weight.
if additive {
self.blend_register_morph_target_weights
.extend(stack_iter.map(|m| m * weight_to_blend));
} else {
self.blend_register_morph_target_weights.extend(stack_iter);
}
}
Some(ref mut current_weight) => {
@ -877,7 +885,9 @@ where
} = self.stack.pop().unwrap();
match self.blend_register.take() {
None => self.blend_register = Some((value_to_blend, weight_to_blend)),
None => {
self.initialize_blend_register(value_to_blend, weight_to_blend, additive);
}
Some((mut current_value, mut current_weight)) => {
current_weight += weight_to_blend;
@ -912,6 +922,22 @@ where
Ok(())
}
fn initialize_blend_register(&mut self, value: A, weight: f32, additive: bool) {
if additive {
let scaled_value = A::blend(
[BlendInput {
weight,
value,
additive: true,
}]
.into_iter(),
);
self.blend_register = Some((scaled_value, weight));
} else {
self.blend_register = Some((value, weight));
}
}
fn push_blend_register(
&mut self,
weight: f32,

View file

@ -217,9 +217,8 @@ pub enum AnimationNodeType {
/// additively.
///
/// The weights of all the children of this node are *not* normalized to
/// 1.0. Rather, the first child is used as a base, ignoring its weight,
/// while the others are multiplied by their respective weights and then
/// added in sequence to the base.
/// 1.0. Rather, each child is multiplied by its respective weight and
/// added in sequence.
///
/// Add nodes are primarily useful for superimposing an animation for a
/// portion of a rig on top of the main animation. For example, an add node

View file

@ -361,7 +361,7 @@ fn setup_animation_graph_once_loaded(
.from_asset("models/animated/Fox.glb"),
);
let mask = if animation_index == 0 { 0 } else { 0x3f };
animation_graph.add_clip_with_mask(handle, mask, 0.0, blend_node)
animation_graph.add_clip_with_mask(handle, mask, 1.0, blend_node)
});
// Create each mask group.