Skip to content

Optimize animation performance #110474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Ryan-000 wants to merge 1 commit into from
Closed

Optimize animation performance #110474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/string/node_path.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ bool NodePath::operator==(const NodePath &p_path) const {
return false;
}

if (data->hash_cache_valid && p_path.data->hash_cache_valid) {
if (data->hash_cache != p_path.data->hash_cache) {
return false;
}
}

if (data->absolute != p_path.data->absolute) {
return false;
}
Expand Down
29 changes: 18 additions & 11 deletions core/templates/a_hash_map.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,22 @@ class AHashMap {
return nullptr;
}

TValue &get_value_ref_or_add_default(const TKey &p_key, bool &r_was_added) {

@Ivorforce Ivorforce Sep 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the function you're looking for already exists (operator[]). Unless you really do need r_was_added? But I don't see you use this function at all. Did you add it by mistake?

@Ryan-000 Ryan-000 Sep 13, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need r_was_added here

https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_tree.cpp#L136

https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_mixer.cpp#L139

@Ivorforce Ivorforce Sep 13, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't resolve comments prematurely.
Thanks for the links, for some reason GitHub didn't show me those on search. Still, i don't like duplicating methods. Figuring this out can wait until after this comes out of draft again though.

@Nazarwadim Nazarwadim Sep 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use find + insert_new instead

For example
https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_tree.cpp#L136

Here you can:

	AHashMap<StringName, Pair<Variant, bool>> &property_map = process_state->tree->property_map;
	auto iter = property_cache.find(p_name);
	if(!iter) {
		iter = property_cache.insert_new(p_name, 0);
	} else {
		return property_map.get_by_index(iter->value).value.first;
	}

@Ryan-000 Ryan-000 Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use find + insert_new instead

For example

https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_tree.cpp#L136

Here you can:

	AHashMap<StringName, Pair<Variant, bool>> &property_map = process_state->tree->property_map;
	auto iter = property_cache.find(p_name);
	if(!iter) {
		iter = property_cache.insert_new(p_name, 0);
	} else {
		return property_map.get_by_index(iter->value).value.first;
	}

Problem is using find then insert_new hashes the key twice, while get_value_ref_or_add_default hashes it once.

uint32_t element_idx = 0;
uint32_t meta_idx = 0;
uint32_t hash = _hash(p_key);
bool exists = _lookup_idx_with_hash(p_key, element_idx, meta_idx, hash);

if (exists) {
r_was_added = false;
return _elements[element_idx].value;
} else {
r_was_added = true;
element_idx = _insert_element(p_key, TValue(), hash);
return _elements[element_idx].value;
}
}

bool has(const TKey &p_key) const {
uint32_t _idx = 0;
uint32_t meta_idx = 0;
Expand Down Expand Up @@ -584,17 +600,8 @@ class AHashMap {
}

TValue &operator[](const TKey &p_key) {
uint32_t element_idx = 0;
uint32_t meta_idx = 0;
uint32_t hash = _hash(p_key);
bool exists = _lookup_idx_with_hash(p_key, element_idx, meta_idx, hash);

if (exists) {
return _elements[element_idx].value;
} else {
element_idx = _insert_element(p_key, TValue(), hash);
return _elements[element_idx].value;
}
bool dummy;
return get_value_ref_or_add_default(p_key, dummy);
}

/* Insert */
Expand Down
4 changes: 2 additions & 2 deletions editor/animation/animation_blend_space_1d_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ void AnimationNodeBlendSpace1DEditor::_blend_space_gui_input(const Ref<InputEven
animations_to_add.clear();

LocalVector<StringName> classes;
ClassDB::get_inheriters_from_class("AnimationRootNode", classes);
ClassDB::get_inheriters_from_class(SNAME("AnimationRootNode"), classes);
classes.sort_custom<StringName::AlphCompare>();

menu->add_submenu_node_item(TTR("Add Animation"), animations_menu);

List<StringName> names;
LocalVector<StringName> names;
tree->get_animation_list(&names);

for (const StringName &E : names) {
Expand Down
2 changes: 1 addition & 1 deletion editor/animation/animation_blend_space_1d_editor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class AnimationNodeBlendSpace1DEditor : public AnimationTreeNodeEditorPlugin {

PopupMenu *menu = nullptr;
PopupMenu *animations_menu = nullptr;
Vector<String> animations_to_add;
Vector<StringName> animations_to_add;
float add_point_pos = 0.0f;
Vector<real_t> points;

Expand Down
2 changes: 1 addition & 1 deletion editor/animation/animation_blend_space_2d_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void AnimationNodeBlendSpace2DEditor::_blend_space_gui_input(const Ref<InputEven

menu->add_submenu_node_item(TTR("Add Animation"), animations_menu);

List<StringName> names;
LocalVector<StringName> names;
tree->get_animation_list(&names);
for (const StringName &E : names) {
animations_menu->add_icon_item(get_editor_theme_icon(SNAME("Animation")), E);
Expand Down
2 changes: 1 addition & 1 deletion editor/animation/animation_blend_space_2d_editor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class AnimationNodeBlendSpace2DEditor : public AnimationTreeNodeEditorPlugin {

PopupMenu *menu = nullptr;
PopupMenu *animations_menu = nullptr;
Vector<String> animations_to_add;
Vector<StringName> animations_to_add;
Vector2 add_point_pos;
Vector<Vector2> points;

Expand Down
55 changes: 34 additions & 21 deletions editor/animation/animation_blend_tree_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,18 @@ void AnimationNodeBlendTreeEditor::update_graph() {
if (updating || blend_tree.is_null()) {
return;
}
if (graph_update_queued) {
return;
}
graph_update_queued = true;
// Defer to idle time, so multiple requests can be merged.
callable_mp(this, &AnimationNodeBlendTreeEditor::update_graph_immediately).call_deferred();
}

void AnimationNodeBlendTreeEditor::update_graph_immediately() {
if (updating || blend_tree.is_null()) {
return;
}

AnimationTree *tree = AnimationTreeEditor::get_singleton()->get_animation_tree();
if (!tree) {
Expand Down Expand Up @@ -189,7 +201,7 @@ void AnimationNodeBlendTreeEditor::update_graph() {
node->set_slot(base + i, true, read_only ? -1 : 0, get_theme_color(SceneStringName(font_color), SNAME("Label")), false, 0, Color());
}

List<PropertyInfo> pinfo;
LocalVector<PropertyInfo> pinfo;
agnode->get_parameter_list(&pinfo);
for (const PropertyInfo &F : pinfo) {
if (!(F.usage & PROPERTY_USAGE_EDITOR)) {
Expand All @@ -209,7 +221,7 @@ void AnimationNodeBlendTreeEditor::update_graph() {
}
prop->set_name_split_ratio(ratio);
prop->update_property();
prop->connect("property_changed", callable_mp(this, &AnimationNodeBlendTreeEditor::_property_changed));
prop->connect(SNAME("property_changed"), callable_mp(this, &AnimationNodeBlendTreeEditor::_property_changed));

if (F.hint == PROPERTY_HINT_RESOURCE_TYPE) {
// Give the resource editor some more space to make the inside readable.
Expand All @@ -223,7 +235,7 @@ void AnimationNodeBlendTreeEditor::update_graph() {
}
}

node->connect("dragged", callable_mp(this, &AnimationNodeBlendTreeEditor::_node_dragged).bind(E));
node->connect(SNAME("dragged"), callable_mp(this, &AnimationNodeBlendTreeEditor::_node_dragged).bind(E));

if (AnimationTreeEditor::get_singleton()->can_edit(agnode)) {
node->add_child(memnew(HSeparator));
Expand Down Expand Up @@ -263,7 +275,7 @@ void AnimationNodeBlendTreeEditor::update_graph() {

ProgressBar *pb = memnew(ProgressBar);

List<StringName> anims;
LocalVector<StringName> anims;
tree->get_animation_list(&anims);

for (const StringName &F : anims) {
Expand All @@ -276,25 +288,25 @@ void AnimationNodeBlendTreeEditor::update_graph() {
animations[E] = pb;
node->add_child(pb);

mb->get_popup()->connect("index_pressed", callable_mp(this, &AnimationNodeBlendTreeEditor::_anim_selected).bind(options, E), CONNECT_DEFERRED);
mb->get_popup()->connect(SNAME("index_pressed"), callable_mp(this, &AnimationNodeBlendTreeEditor::_anim_selected).bind(options, E), CONNECT_DEFERRED);
}

Ref<StyleBox> sb_panel = node->get_theme_stylebox(SceneStringName(panel), "GraphNode")->duplicate();
Ref<StyleBox> sb_panel = node->get_theme_stylebox(SceneStringName(panel), SNAME("GraphNode"))->duplicate();
if (sb_panel.is_valid()) {
sb_panel->set_content_margin(SIDE_TOP, 12 * EDSCALE);
sb_panel->set_content_margin(SIDE_BOTTOM, 12 * EDSCALE);
node->add_theme_style_override(SceneStringName(panel), sb_panel);
}

node->add_theme_constant_override("separation", 4 * EDSCALE);
node->add_theme_constant_override(SNAME("separation"), 4 * EDSCALE);
}

List<AnimationNodeBlendTree::NodeConnection> node_connections;
blend_tree->get_node_connections(&node_connections);

for (const AnimationNodeBlendTree::NodeConnection &E : node_connections) {
StringName from = E.output_node;
StringName to = E.input_node;
const StringName &from = E.output_node;
const StringName &to = E.input_node;
int to_idx = E.input_index;

graph->connect_node(from, 0, to, to_idx);
Expand All @@ -304,6 +316,7 @@ void AnimationNodeBlendTreeEditor::update_graph() {
graph->set_minimap_opacity(graph_minimap_opacity);
float graph_lines_curvature = EDITOR_GET("editors/visual_editors/lines_curvature");
graph->set_connection_lines_curvature(graph_lines_curvature);
graph_update_queued = false;
}

void AnimationNodeBlendTreeEditor::_file_opened(const String &p_file) {
Expand Down Expand Up @@ -756,16 +769,16 @@ bool AnimationNodeBlendTreeEditor::_update_filters(const Ref<AnimationNode> &ano

updating = true;

HashSet<String> paths;
HashMap<String, RBSet<String>> types;
HashSet<NodePath> paths;
HashMap<NodePath, RBSet<String>> types;
{
List<StringName> animation_list;
LocalVector<StringName> animation_list;
tree->get_animation_list(&animation_list);

for (const StringName &E : animation_list) {
Ref<Animation> anim = tree->get_animation(E);
for (int i = 0; i < anim->get_track_count(); i++) {
String track_path = String(anim->track_get_path(i));
NodePath track_path = anim->track_get_path(i);
paths.insert(track_path);

String track_type_name;
Expand Down Expand Up @@ -796,8 +809,7 @@ bool AnimationNodeBlendTreeEditor::_update_filters(const Ref<AnimationNode> &ano

HashMap<String, TreeItem *> parenthood;

for (const String &E : paths) {
NodePath path = E;
for (const NodePath &path : paths) {
TreeItem *ti = nullptr;
String accum;
for (int i = 0; i < path.get_name_count(); i++) {
Expand All @@ -817,8 +829,8 @@ bool AnimationNodeBlendTreeEditor::_update_filters(const Ref<AnimationNode> &ano
ti->set_selectable(0, false);
ti->set_editable(0, false);

if (base->has_node(accum)) {
Node *node = base->get_node(accum);
Node *node = base->get_node_or_null(accum);
if (node) {
ti->set_icon(0, EditorNode::get_singleton()->get_object_icon(node, "Node"));
}

Expand All @@ -827,10 +839,7 @@ bool AnimationNodeBlendTreeEditor::_update_filters(const Ref<AnimationNode> &ano
}
}

Node *node = nullptr;
if (base->has_node(accum)) {
node = base->get_node(accum);
}
Node *node = base->get_node_or_null(accum);
if (!node) {
continue; //no node, can't edit
}
Expand Down Expand Up @@ -968,6 +977,10 @@ void AnimationNodeBlendTreeEditor::_notification(int p_what) {
return; // Node has been changed.
}

if (graph_update_queued) {
return;
}

String error;

error = tree->get_editor_error_message();
Expand Down
2 changes: 2 additions & 0 deletions editor/animation/animation_blend_tree_editor_plugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class AnimationNodeBlendTreeEditor : public AnimationTreeNodeEditorPlugin {
void _node_changed(const StringName &p_node_name);

String current_node_rename_text;
bool graph_update_queued;
bool updating;

void _connection_request(const String &p_from, int p_from_index, const String &p_to, int p_to_index);
Expand Down Expand Up @@ -166,6 +167,7 @@ class AnimationNodeBlendTreeEditor : public AnimationTreeNodeEditorPlugin {
virtual void edit(const Ref<AnimationNode> &p_node) override;

void update_graph();
void update_graph_immediately();

AnimationNodeBlendTreeEditor();
};
Expand Down
6 changes: 3 additions & 3 deletions editor/animation/animation_library_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ void AnimationLibraryEditor::_load_files(const PackedStringArray &p_paths) {
continue;
}

List<StringName> libs;
LocalVector<StringName> libs;
mixer->get_animation_library_list(&libs);
bool is_already_added = false;
for (const StringName &K : libs) {
Expand Down Expand Up @@ -684,7 +684,7 @@ void AnimationLibraryEditor::update_tree() {
Color ss_color = get_theme_color(SNAME("prop_subsection"), EditorStringName(Editor));

TreeItem *root = tree->create_item();
List<StringName> libs;
LocalVector<StringName> libs;
Vector<uint64_t> collapsed_lib_ids = _load_mixer_libs_folding();

mixer->get_animation_library_list(&libs);
Expand Down Expand Up @@ -951,7 +951,7 @@ String AnimationLibraryEditor::_get_mixer_signature() const {
String signature = String();

// Get all libraries sorted for consistency
List<StringName> libs;
LocalVector<StringName> libs;
mixer->get_animation_library_list(&libs);
libs.sort_custom<StringName::AlphCompare>();

Expand Down
10 changes: 5 additions & 5 deletions editor/animation/animation_player_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,9 +811,9 @@ void AnimationPlayerEditor::_update_animation_blend() {

blend_editor.tree->clear();

String current = animation->get_item_text(animation->get_selected());
StringName current = animation->get_item_text(animation->get_selected());

List<StringName> anims;
LocalVector<StringName> anims;
player->get_animation_list(&anims);
TreeItem *root = blend_editor.tree->create_item();
updating_blends = true;
Expand Down Expand Up @@ -1020,7 +1020,7 @@ void AnimationPlayerEditor::_update_player() {
return;
}

List<StringName> libraries;
LocalVector<StringName> libraries;
player->get_animation_library_list(&libraries);

int active_idx = -1;
Expand Down Expand Up @@ -1144,7 +1144,7 @@ void AnimationPlayerEditor::_update_name_dialog_library_dropdown() {
}
}

List<StringName> libraries;
LocalVector<StringName> libraries;
player->get_animation_library_list(&libraries);
library->clear();

Expand Down Expand Up @@ -1478,7 +1478,7 @@ void AnimationPlayerEditor::_current_animation_changed(const String &p_name) {
}

// Determine the read-only status of the animation's library and the libraries as a whole.
List<StringName> libraries;
LocalVector<StringName> libraries;
player->get_animation_library_list(&libraries);

bool current_animation_library_is_readonly = false;
Expand Down
4 changes: 2 additions & 2 deletions editor/animation/animation_state_machine_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ void AnimationNodeStateMachineEditor::_open_menu(const Vector2 &p_position) {
animations_menu->clear();
animations_to_add.clear();

List<StringName> animation_names;
LocalVector<StringName> animation_names;
tree->get_animation_list(&animation_names);
menu->add_submenu_node_item(TTR("Add Animation"), animations_menu);
if (animation_names.is_empty()) {
Expand Down Expand Up @@ -777,7 +777,7 @@ void AnimationNodeStateMachineEditor::_add_animation_type(int p_index) {

anim->set_animation(animations_to_add[p_index]);

String base_name = animations_to_add[p_index].validate_node_name();
String base_name = String(animations_to_add[p_index]).validate_node_name();
int base = 1;
String name = base_name;
while (state_machine->has_node(name)) {
Expand Down
2 changes: 1 addition & 1 deletion editor/animation/animation_state_machine_editor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class AnimationNodeStateMachineEditor : public AnimationTreeNodeEditorPlugin {
PopupMenu *state_machine_menu = nullptr;
PopupMenu *end_menu = nullptr;
PopupMenu *animations_menu = nullptr;
Vector<String> animations_to_add;
Vector<StringName> animations_to_add;
Vector<String> nodes_to_connect;

Vector2 add_node_pos;
Expand Down
Loading
Loading