AnimationTree: Add2 in BlendTree does not seem to be additive

[jedStevens]

Godot version:
3.2 df876601c8

OS/device including version:
Manjaro Linux

Issue description:
I was building a blendtree expecting to see the pose of my root bone to be added together. This node seems to blend between two poses rather than produce a sum of poses.

Steps to reproduce:
I have a jumping animation and a running animation both with the root bone controlling the motion. As in the attached video, you can see that the forward motion is reduced as the jump is increased.

Is this the expected result? Am I doing something wrong?

I read the documentation and expected this node to produce additive results.

Example:
https://www.youtube.com/watch?v=bkDDoQMfCoc

[jedStevens]

After doing some peeking into the C++ it looks like the Add nodes use the same _blend_node internal function for blending. I'm not sure sure how simply changing the filter mode makes the function become additive, but maybe there should be an internal _add_node function for additive logic, but I understand that transforms might not work so easily in this case.

I might do some experiments but I have a feeling this is way above my head to do correctly for all cases. Either way it would be nice to have a node for the AnimationTree that adds rotation and position to another, at least, that's what I expected Add2 to do. Am I missing something here? I feel like this might be me not understanding some core aspect of these nodes; and if so I think the explanation should be in the docs.

[jedStevens]

I think I may have found the issue.

It looks like at line 406 in scene/animation/animation_blend_tree.cpp there is the function for processing the Add2 node. It looks like it computes a second value that isn't used. I'm not sure why this might be, but after playing around I noticed scons would complain if I didn't use a variable SO I'm taking a wild guess and saying that I think this function was either modified or not finished. I haven't checked the git history yet but I have a feeling the following code is wrong:

float AnimationNodeAdd2::process(float p_time, bool p_seek) {
	float amount = get_parameter(add_amount);
	float rem0 = blend_input(0, p_time, p_seek, 1.0, FILTER_IGNORE, !sync);
	blend_input(1, p_time, p_seek, amount, FILTER_PASS, !sync);
	return rem0;
}

What I mean is the second blend_input call isn't used, it also doesn't seem to be writing to any parameter that is used in the return line. I wonder if maybe the last line should be something like return rem0 + rem1 instead, but I've tried this without luck so far.

Anyone have any ideas on this?

[jedStevens]

The original file Juan pushed hasn't been changed so I supposed this is intended. After reading a bit online, I've come to understand that the animations are being mixed rather than added. I wonder if there is a better name for the node to cause less confusion.

It would also be nice to have a node that does combine two poses in an additive way; this way two root motion animations can be summed rather than averaged. It's just odd to find a node named add and not having the joint transforms added together.

I see this as useful for applying crouching poses, for example creating an animation or static pose of only a crouch applied to a rest pose, could then be applied across many animations thus reducing animator workload and allowing for animation driven characters.

I guess I'll get cracking on the node I actually want and make a PR.

[jaja00100]

I had the same problem. I thought Add2 was going to add the transformations as it happens in blender. It would be very good if it worked that way. Is there any other way to achieve this behavior in godot?

[jedStevens]

@jaja00100 I am working on a node for the animation tree called Join2 to preform this operation, there are just some functions missing in order to do so that I have to write, but first I need to go through some of the blend code and understand it a bit better. I am also busy working on a project in Godot so I will probably push a PR whenever I end up finally needing that feature.

[jaja00100]

@jedStevens It would be great. I really need this. Thanks for helping.

[jedStevens]

I am hoping that Juan sees this at some point though, because he wrote that file and hasn't changed it much as well as it lacks enough comments for me to understand it quickly. I imagine he might be able to whip this node up pretty quick.

[jaja00100]

I also took a look at the code and couldn't quite understand it. But I ended up managing to make it work as I wanted.

[jedStevens]

Yeah there are ways around this but I find without this node that my animation workload increases significantly without it. I'm mostly struggling with the activity map and this COW array for the blend data. There has to be a way to combine two tracks in an additive method. I'm busy for today but might have some time tonight to keep cracking at it.

I'm also not a core dev so I hope I can slap something together that's half decent tonight, but no guarantees.

[jaja00100]

I modified the source code of the existing nodes Add2 and Add3 and managed to make them work as in blender. But I don't think it should be a good thing to do that. But, for my purposes, I think it will be enough. I can show it later so you can see if it has anything useful.

[jedStevens]

Yeah if you want to post the snippet of changed code, that might help me. The only thing with the blend trees is I'm also not sure how to deal with non-transform tracks. All I mean is to combine two transforms to get the results we want, a matrix multiplication needs to be preformed, but with a float it would be an addition. I just don't want to make a PR of code that only works under specific conditions, I'd rather it apply to all animation track types. I'll just have to do some test I suppose, I'm not exactly sure where this should occur.

[jaja00100]

godot/scene/animation/animation_tree.h

Lines 70 to 73 in 5058244

	const Vector<float> *track_blends;
	float blend;
	bool seeked;
	};

...
                const Vector<float> *track_blends; 
		float blend;
		bool seeked;
		bool add_directly;
...

godot/scene/animation/animation_tree.h

Lines 98 to 99 in 5058244

	HashMap<NodePath, bool> filter;
	bool filter_enabled;

        HashMap<NodePath, bool> filter;
        bool filter_enabled;
        bool add_directly;

godot/scene/animation/animation_tree.cpp

Lines 116 to 119 in 5058244

	anim_state.animation = animation;
	anim_state.seeked = p_seeked;
	state->animation_states.push_back(anim_state);

        anim_state.animation = animation;
	anim_state.seeked = p_seeked;
	anim_state.add_directly = add_directly;

	if (add_directly) add_directly = !add_directly;

	state->animation_states.push_back(anim_state);

godot/scene/animation/animation_tree.cpp

Lines 958 to 970 in 5058244

	if (err != OK)
	continue;
	t->loc = t->loc.linear_interpolate(loc, blend);
	if (t->rot_blend_accum == 0) {
	t->rot = rot;
	t->rot_blend_accum = blend;
	} else {
	float rot_total = t->rot_blend_accum + blend;
	t->rot = rot.slerp(t->rot, t->rot_blend_accum / rot_total).normalized();
	t->rot_blend_accum = rot_total;
	}
	t->scale = t->scale.linear_interpolate(scale, blend);

        if (err != OK)
	        continue;

	if (!as.add_directly) {
	        t->loc = t->loc.linear_interpolate(loc, blend);
	        if (t->rot_blend_accum == 0) {
		        t->rot = rot;
		        t->rot_blend_accum = blend;
	        } else {
		       float rot_total = t->rot_blend_accum + blend;
		        t->rot = rot.slerp(t->rot, t->rot_blend_accum / rot_total).normalized();
		        t->rot_blend_accum = rot_total;
	       }
		t->scale = t->scale.linear_interpolate(scale, blend);
	} else {
		t->loc += (loc)*blend;
		t->scale = t->scale.linear_interpolate(scale, blend);
		Quat q = Quat().slerp(rot.normalized(), blend).normalized();
		t->rot = (t->rot*q).normalized();
	}

I added a new blend_input_add function in "animation_tree.cpp"

float AnimationNode::blend_input_add(int p_input, float p_time, bool p_seek, float p_blend, FilterAction p_filter, bool p_optimize) {
	float ret = blend_input(p_input, p_time, p_seek,p_blend, p_filter,p_optimize);
	if (ret != 0) {
		if (parent) {
			AnimationNodeBlendTree *blend_tree = Object::cast_to<AnimationNodeBlendTree>(parent);
			ERR_FAIL_COND_V(!blend_tree, 0);

			StringName node_name = connections[1];

			if (blend_tree->has_node(node_name)) {
				Ref<AnimationNode> node = blend_tree->get_node(node_name);
				node->add_directly = true;
			}
		}
	}
	
	return ret;
}

godot/scene/animation/animation_blend_tree.cpp

Lines 406 to 413 in 5058244

	float AnimationNodeAdd2::process(float p_time, bool p_seek) {
	float amount = get_parameter(add_amount);
	float rem0 = blend_input(0, p_time, p_seek, 1.0, FILTER_IGNORE, !sync);
	blend_input(1, p_time, p_seek, amount, FILTER_PASS, !sync);
	return rem0;
	}

float AnimationNodeAdd2::process(float p_time, bool p_seek) {

	float amount = get_parameter(add_amount);
	float rem0 = blend_input(0, p_time, p_seek, 1.0, FILTER_IGNORE, !sync);
	blend_input_add(1, p_time, p_seek, amount, FILTER_PASS, !sync);

	return rem0;
}

[jaja00100]

Of course it is far from good, but it can help with something.

[jedStevens]

This is useful thanks for this.

I think the final solution for this will look a bit different but this is a good boost in the right direction.

[Calinou]

Is this related to #34694?

[jedStevens]

@Calinou I would think so.

The source file I'm referencing above is essentially Juan's original code for this and it is light for comments so it isn't super easy to go in and modify; especially since I'm not familiar with the pattern he is using for this.

I'm under the impression that we need to modify something based on track type. I think this is because naturally when we want an additive animation we want to see a transform track applied on top of another using matrix multiplication, however when we want to add rotation to a sprite that is animated, we could just use the current Add2 node and filter the properties to get a desired motion with rotation.

I think with another node, we can get the additive skeletal animations we want and not break Add2 for others. I think by taking some of @jaja00100 's code and shoving it into a new node could work, but I don't want a new variable for add_directly, I think this would work better as a new node entirely. However if Juan can do this easily that would be awesome, but I know he's busy with a ton of stuff for 4.0 so I didn't want to bug him to do it.

[jaja00100]

I see no problem in creating a new node for this function, but the behavior of the current nodes (Add2 and Add3) seems very strange to me. With values -1 or 1 they seem to act like the blend node, but with values between -1 and 1 they behave close to what I expected. I don't know, maybe someone has found some use for them as they are.

[Calinou]

cc @reduz, as he implemented the animation state machine.

[Dancovich]

I believe it's also important to improve the documentation on the Add2 and Add3 nodes, as right now it's difficult to predict what they'll do.

I have a model of a spaceship that has a radar on top. The root bone is weighted to most of the ship except the radar and the radar bone (which is a child of the root bone) is weighted to the radar. No vertices are weighted to more than one bone, either they are 100% weighted to the root bone or the radar bone.

The radar bone just rotates over and over while the root bone has a "hover" animation which slightly changes rotation and location of the ship over time.

I intended to add them together to create a ship that hovers while it's radar scans. If I just use an AnimationPlayer and play these animations by themselves they work as intended, but if I try to use Add2 the radar turns 180º, resets and turn the same 180º again. At first I thought there was some gimbal lock going on but I'm correctly using quaternions in Blender and, as I said, the animations work by themselves.

Here's the video of this effect happening. If you believe any source codes could add to the discussion I can post a small project containing only the model with the AnimationTree.

https://www.youtube.com/watch?v=zoff63Ns8X4

[jitspoe]

Yeah, I noticed this as well, and there doesn't seem to be any documentation on what add2 is supposed to do. I'm assuming this is a bug, because the behavior of add2 at 100% seems to be the same as just blending 2 animations at 50%.

[thimenesup]

Heh, I actually also noticed that something was off about animation adding in the tree too, but thought that maybe I was a matter of the animations chosen not being the best... But today I saw this video about BlendTree adding in Unity and I found the same animations, so I replicated it in Godot, and the added animation result its different/not expected/worse as you may see in the project attached. (Based in the footage in the video at 0:35)

I think that the problem is that when adding the animation the implementation must have its skeleton transforms substracted from the default/rest/T pose, I brought that conclusion when I also changed the add pose to the skeleton rest one and the animation result was getting affected even though the expected result would be that the animation was exactly the same. (As in the video at 0:50 , ignoring the root rotation change)

This is the minimal reproduction project

[Dancovich]

I ended up solving my issue by making each animation ONLY AFFECT the bones needed. It doesn't matter that on Blender animation A only affects the root bone and animation B only affects the radar bone, when Godot imports this it will create a track for the radar bone in animation A (with only one keyframe) and a track for the root bone in animation B (also only one keyframe).

I manually removed those tracks and the issue disappeared. There's a feature on the Godot importer to filter out bones you don't want to include in each animation and I created a script to exclude those bones when importing.

[andersoncsmat]

@Dancovich I don't know if you want to use Add2 specifically for some reason, but you could solve your problem just using Blend2 and edit the filters.

[Jorhlok]

I bodged a local copyof godot 3.2.2 with the code by @jaja00100 and with the addition of the function signature of "float AnimationNode::blend_input_add()" in "animation_tree.h" and also for reasons I don't understand changing "float AnimationNodeAdd2::process()" to

float AnimationNodeAdd2::process(float p_time, bool p_seek) {

	float amount = get_parameter(add_amount);
	float rem0 = blend_input_add(0, p_time, p_seek, 1.0, FILTER_IGNORE, !sync); //why do I need to change this to ..._add()?
	blend_input_add(1, p_time, p_seek, amount, FILTER_PASS, !sync);

	return rem0;
}

This gives the behavior I'd expect of adding one animation over the top of another. If that's not the intended behavior, there should be a node for it. The behavior I wanted could not be gotten with either blend2 or add2 and filtering.

Anyhow, I couldn't get Add3 to work like that no matter what but at least I can work around that.

[TokageItLab]

I was faced the trouble with this issue when I was using AnimationTree recently. The current NodeAdd2 calculates differently for a root bone and all other bones. This difference makes slippage between the root motion and bone animation, so I think it should fix.

[TokageItLab]

It's unnatural not to name the @jaja00100 algorithm as NodeAdd2, assuming we keep the currently implemented Add algorithm behavior for compatibility. So how about implementing switching with check box?

And for NodeAdd3, what do they expect to see in the negative side of the behavior? If they're expecting to subtract two animations, I think we'll need to implement NodeSub as well...

If they simply want to add in both directions, they can rewrite it as follows. I changed some of the code in @jaja00100 by passing an argument to blend_input() with switching flag in NodeAdd3.

animation_tree.cpp

float AnimationNode::blend_input(int p_input, float p_time, bool p_seek, float p_blend, FilterAction p_filter, bool p_optimize, bool p_add_directly) {
	ERR_FAIL_INDEX_V(p_input, inputs.size(), 0);
	ERR_FAIL_COND_V(!state, 0);

	AnimationNodeBlendTree *blend_tree = Object::cast_to<AnimationNodeBlendTree>(parent);
	ERR_FAIL_COND_V(!blend_tree, 0);

	StringName node_name = connections[p_input];

	if (!blend_tree->has_node(node_name)) {
		String name = blend_tree->get_node_name(Ref<AnimationNode>(this));
		make_invalid(vformat(RTR("Nothing connected to input '%s' of node '%s'."), get_input_name(p_input), name));
		return 0;
	} else if (p_add_directly) {
		Ref<AnimationNode> node = blend_tree->get_node(node_name);
		node->add_directly = p_add_directly;
	}

	Ref<AnimationNode> node = blend_tree->get_node(node_name);

	//inputs.write[p_input].last_pass = state->last_pass;
	float activity = 0;
	float ret = _blend_node(node_name, blend_tree->get_node_connection_array(node_name), NULL, node, p_time, p_seek, p_blend, p_filter, p_optimize, &activity);

	Vector<AnimationTree::Activity> *activity_ptr = state->tree->input_activity_map.getptr(base_path);

	if (activity_ptr && p_input < activity_ptr->size()) {
		activity_ptr->write[p_input].last_pass = state->last_pass;
		activity_ptr->write[p_input].activity = activity;
	}
	return ret;
}

animation_blend_tree.cpp

float AnimationNodeAdd3::process(float p_time, bool p_seek) {

	float amount = get_parameter(add_amount);
	float rem0 = blend_input(1, p_time, p_seek, 1.0, FILTER_IGNORE, !sync);
	if (amount < 0) {
		blend_input(0, p_time, p_seek, -amount, FILTER_PASS, !sync, add_directly);
		blend_input(2, p_time, p_seek, 0, FILTER_PASS, !sync);
	} else {
		blend_input(2, p_time, p_seek, amount, FILTER_PASS, !sync, add_directly);
		blend_input(0, p_time, p_seek, 0, FILTER_PASS, !sync);
	}

	return rem0;
}

https://github.com/TokageItLab/godot/tree/fix-node-add2

I'm also fixing some other AnimationTree bugs. If you're okay with this implementation, I will create PR.

[Vicmas21]

very, very, very thanks

[sabudum]

Is this getting fixed anytime soon?

[Calinou]

Is this getting fixed anytime soon?

We don't know for certain, but I doubt it will be fixed for 3.2.4. See #34134.

[TokageItLab]

Closed by #57675.

[Calinou]

Closed by #57675.

Is a fix for 3.x planned? I feel this is important enough to warrant a 3.x backport, considering how long this issue was open for.

[TokageItLab]

I suppose we could do a backport if needed, but I worked with 4.0 in resolving this and found a major incompatibility there, see #59536 and #59961.

A minimal implementation would be to salvage #42302, but it is not perfect and probably does not add correctly other than rotation.

[Riordan-DC]

I suppose we could do a backport if needed, but I worked with 4.0 in resolving this and found a major incompatibility there, see #59536 and #59961.

A minimal implementation would be to salvage #42302, but it is not perfect and probably does not add correctly other than rotation.

@TokageItLab
I'd like to echo @Calinou and voice my support for fixing the broken additive blending in 3.x.
Blends and Adds are about everything you need for AAA complex animation and without a backport 3.x will never have such an important feature.
(correct me if I am wrong but I dont think this issue has been fixed elsewhere in 3.x?)