Optimize animation performance and improve internals #112308

Ryan-000 · 2025-11-02T07:03:55Z

Supersedes #110474

TokageItLab · 2025-11-02T18:34:13Z

scene/animation/animation_tree.cpp

+thread_local AnimationNode::ProcessState *AnimationNode::tls_process_state = nullptr;
+thread_local AnimationNodeInstance *AnimationNode::current_instance = nullptr;


I think the difference from #110474 is that this was added. Are there any other changes? Is a new benchmark comparison possible?

If #110474 (review) issue was due to multiple same AnimationNode resources being instantiated and placed, then the fix to manage each resource's instance makes sense. I confirmed that this no longer causes #110474 (review) issue.

However, since I recall past issues similar to #110506, I'm a bit concerned about whether using thread_local is appropriate. Testing in a project editing scenes containing a large number of AnimationTree instances might be necessary.

Also, I have some concerns regarding code readability. Passing instance pointers as arguments is indeed safe, but since there should be never occur to access different ProcessState and AnimationNodeInstance within a single AnimationNode like:

double length_a = instance_a.get_parameter(current_length); double length_b = instance_b.get_parameter(current_length);

, so it always p_instance.get_parameter(), it would be ideal if there is a way to omit them, same with old code. This would significantly reduce the number of lines requiring modification. Do you have any ideas @lyuma @SaracenOne?

If there are no other way, I assume it can be accepted depending on the benchmark improvement. In other words, we want a benchmark comparison before and after, focusing solely on changes to the AnimationTree instance, excluding any other changes. This was one reason why I suggested separating the PR.

AThousandShips · 2025-11-03T08:46:59Z

scene/animation/animation_mixer.h


-	void get_animation_list(List<StringName> *p_animations) const;
-	Ref<Animation> get_animation(const StringName &p_name) const;
+	void get_animation_list(LocalVector<StringName> *p_animations, const bool p_sorted = true) const;


Suggested change

void get_animation_list(LocalVector<StringName> *p_animations, const bool p_sorted = true) const;

void get_animation_list(LocalVector<StringName> *p_animations, bool p_sorted = true) const;

No const for simple types

AThousandShips · 2025-11-03T08:47:10Z

scene/animation/animation_blend_tree.h

 	virtual Variant get_parameter_default_value(const StringName &p_parameter) const override;

-	virtual NodeTimeInfo get_node_time_info() const override; // Wrapper of get_parameter().
+	virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const override; // Wrapper of get_parameter().


Suggested change

virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const override; // Wrapper of get_parameter().

virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &p_instance, ProcessState &p_process_state) const override; // Wrapper for get_parameter().

AThousandShips · 2025-11-03T08:47:22Z

editor/animation/animation_blend_tree_editor_plugin.h

+	void _node_renamed(const String &p_text, Ref<AnimationNode> p_node, StringName p_name);
+	void _node_renamed_focus_out(Ref<AnimationNode> p_node, StringName p_name);


Suggested change

void _node_renamed(const String &p_text, Ref<AnimationNode> p_node, StringName p_name);

void _node_renamed_focus_out(Ref<AnimationNode> p_node, StringName p_name);

void _node_renamed(const String &p_text, Ref<AnimationNode> p_node, const StringName &p_name);

void _node_renamed_focus_out(Ref<AnimationNode> p_node, const StringName &p_name);

AThousandShips · 2025-11-03T08:47:34Z

scene/animation/animation_tree.h

+	void set_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state, const NodeTimeInfo &p_node_time_info); // Wrapper of set_parameter().
+	virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const; // Wrapper of get_parameter().


Suggested change

void set_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state, const NodeTimeInfo &p_node_time_info); // Wrapper of set_parameter().

virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const; // Wrapper of get_parameter().

void set_node_time_info(AnimationNodeInstance &p_instance, ProcessState &p_process_state, const NodeTimeInfo &p_node_time_info); // Wrapper for set_parameter().

virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &p_instance, ProcessState &p_process_state) const; // Wrapper for get_parameter().

AThousandShips · 2025-11-03T08:48:19Z

scene/animation/animation_tree.h

+	AnimationNodeInstance *parent = nullptr;
+	// TODO: Maybe ptr to parent AnimationNodeInstance for faster access???
+	AnimationNode *parent_node = nullptr;
+	// Multiple AnimationNodeInstance's can share the same node btw


Suggested change

// Multiple AnimationNodeInstance's can share the same node btw

// Multiple AnimationNodeInstances can share the same node btw.

AThousandShips · 2025-11-03T08:52:02Z

scene/animation/animation_mixer.cpp

 				ad.name = key;
 				ad.last_update = animation_set_update_pass;
-				animation_set.insert(ad.name, ad);
+				//todo: possible issue here, not using key, but instead ad.name?


Suggested change

//todo: possible issue here, not using key, but instead ad.name?

// TODO: possible issue here, not using key, but instead ad.name?

AThousandShips · 2025-11-03T08:51:53Z

scene/animation/animation_mixer.cpp

-	anims.sort();
-	for (const String &E : anims) {
-		p_animations->push_back(E);
+void AnimationMixer::get_animation_list(LocalVector<StringName> *p_animations, const bool p_sorted) const {


Suggested change

void AnimationMixer::get_animation_list(LocalVector<StringName> *p_animations, const bool p_sorted) const {

void AnimationMixer::get_animation_list(LocalVector<StringName> *p_animations, bool p_sorted) const {

AThousandShips · 2025-11-03T08:51:44Z

scene/animation/animation_blend_tree.cpp

 }

-AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info() const {
+AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const {


Suggested change

AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const {

AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info(AnimationNodeInstance &p_instance, ProcessState &p_process_state) const {

AThousandShips · 2025-11-03T08:51:35Z

scene/animation/animation_blend_tree.cpp


-	return _blend_node(output, "output", this, pi, FILTER_IGNORE, true, p_test_only, nullptr);
+	AnimationNodeInstance &output_instance = p_instance.get_child_instance_by_path(SceneStringName(output));
+	return _blend_node(p_process_state, p_instance, output_instance, SNAME("output"), this, pi, FILTER_IGNORE, true, p_test_only, nullptr);


Suggested change

return _blend_node(p_process_state, p_instance, output_instance, SNAME("output"), this, pi, FILTER_IGNORE, true, p_test_only, nullptr);

return _blend_node(p_process_state, p_instance, output_instance, SceneStringName(output), this, pi, FILTER_IGNORE, true, p_test_only, nullptr);

AThousandShips · 2025-11-03T08:51:23Z

scene/animation/animation_tree.cpp

-			property_map[new_name] = property_map[E.name];
-			property_map.erase(E.name);
+	//print_line("Node: " + ObjectDB::get_instance(p_oid)->get_class() + " (" + itos(p_oid) + ") renamed: " + p_old_name + " -> " + p_new_name);
+	for (auto &pp : instance_paths[p_oid]) {


Ryan-000 requested review from a team as code owners November 2, 2025 07:03

Optimize animation performance and improve internals

ba1f999

Ryan-000 force-pushed the Optimize-Animation-Performance branch from a5d9659 to ba1f999 Compare November 2, 2025 07:05

Chaosus added enhancement topic:animation performance labels Nov 2, 2025

Chaosus added this to the 4.x milestone Nov 2, 2025

fire added this to Animation Team Issue Triage Nov 2, 2025

TokageItLab reviewed Nov 2, 2025

View reviewed changes

AThousandShips reviewed Nov 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Optimize animation performance and improve internals #112308

Optimize animation performance and improve internals #112308

Ryan-000 commented Nov 2, 2025

Uh oh!

TokageItLab Nov 2, 2025 •

edited

Loading

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

AThousandShips Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		thread_local AnimationNode::ProcessState *AnimationNode::tls_process_state = nullptr;
		thread_local AnimationNodeInstance *AnimationNode::current_instance = nullptr;

	void get_animation_list(LocalVector<StringName> *p_animations, const bool p_sorted = true) const;
	void get_animation_list(LocalVector<StringName> *p_animations, bool p_sorted = true) const;

	virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const override; // Wrapper of get_parameter().
	virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &p_instance, ProcessState &p_process_state) const override; // Wrapper for get_parameter().

		void _node_renamed(const String &p_text, Ref<AnimationNode> p_node, StringName p_name);
		void _node_renamed_focus_out(Ref<AnimationNode> p_node, StringName p_name);

		void set_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state, const NodeTimeInfo &p_node_time_info); // Wrapper of set_parameter().
		virtual NodeTimeInfo get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const; // Wrapper of get_parameter().

	// Multiple AnimationNodeInstance's can share the same node btw
	// Multiple AnimationNodeInstances can share the same node btw.

	//todo: possible issue here, not using key, but instead ad.name?
	// TODO: possible issue here, not using key, but instead ad.name?

	void AnimationMixer::get_animation_list(LocalVector<StringName> *p_animations, const bool p_sorted) const {
	void AnimationMixer::get_animation_list(LocalVector<StringName> *p_animations, bool p_sorted) const {

	AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info(AnimationNodeInstance &instance, ProcessState &p_process_state) const {
	AnimationNode::NodeTimeInfo AnimationNodeAnimation::get_node_time_info(AnimationNodeInstance &p_instance, ProcessState &p_process_state) const {

	return _blend_node(p_process_state, p_instance, output_instance, SNAME("output"), this, pi, FILTER_IGNORE, true, p_test_only, nullptr);
	return _blend_node(p_process_state, p_instance, output_instance, SceneStringName(output), this, pi, FILTER_IGNORE, true, p_test_only, nullptr);

Uh oh!

Optimize animation performance and improve internals #112308

Are you sure you want to change the base?

Optimize animation performance and improve internals #112308

Conversation

Ryan-000 commented Nov 2, 2025

Uh oh!

TokageItLab Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TokageItLab Nov 2, 2025 •

edited

Loading