From 6f7525c396cb28f4eb89203b476fd4a787cfd68d Mon Sep 17 00:00:00 2001 From: HP van Braam Date: Tue, 26 Nov 2024 00:04:25 +0100 Subject: [PATCH] Improve Scene Tree editor performance We now cache the Node*<>TreeItem* mapping in the SceneTreeEditor. This allows us to make targeted updates to the Tree used to display the scene tree in the editor. Previously on almost all changes to the scene tree the editor would rebuild the entire widget, causing a large number of deallocations an allocations. We now carefully manipulate the Tree widget in-situ saving a large number of these allocations. In order to know what Nodes need to be updated we add a editor_state_changed signal to Node, this is a TOOLS_ENABLED, editor-only signal fired when changes to Node happen that are relevant to editor state. We also now make sure that when nodes are moved/renamed we don't check expensive properties that cannot contain NodePaths. This saves a lot of time when SceneTreeDock renames a node in a scene with a lot of MeshInstances. This makes renaming nodes go from ~27 seconds to ~2 seconds on large scenes. SceneTreeEditor instances will now also not do all of the potentially expensive update work if they are invisible. This behavior is turned off by default so it won't affect existing users. This change allows the editor to only update SceneTreeEditors that actually in view. In practice this means that for most changes instead of updating 6 SceneTreeEditors we only update 1 instantly, and the others only when they become visible. There is definitely more that could be done, but this is already a massive improvement. In complex scenes we see an improvement of 10x, things that used to take ~30 seconds now only take 2. This fixes #83460 I want to thank KoBeWi, TokisanGames, a-johnston, aniel080400 for their tireless testing. And AeioMuch for their testing and providing a fix for the hover issue. --- core/object/object.h | 1 + doc/classes/@GlobalScope.xml | 2 +- doc/classes/Node.xml | 5 + doc/classes/TreeItem.xml | 6 + editor/connections_dialog.cpp | 1 + editor/gui/scene_tree_editor.cpp | 718 +++++++++++++++++++++++++------ editor/gui/scene_tree_editor.h | 68 ++- editor/reparent_dialog.cpp | 1 + editor/scene_tree_dock.cpp | 109 +++-- editor/scene_tree_dock.h | 1 + scene/gui/tree.cpp | 15 +- scene/gui/tree.h | 1 + scene/main/node.cpp | 56 ++- scene/main/node.h | 7 + scene/resources/mesh.cpp | 8 +- 15 files changed, 818 insertions(+), 181 deletions(-) diff --git a/core/object/object.h b/core/object/object.h index da8e8d1b86e..7467a86732c 100644 --- a/core/object/object.h +++ b/core/object/object.h @@ -89,6 +89,7 @@ enum PropertyHint { PROPERTY_HINT_DICTIONARY_TYPE, PROPERTY_HINT_TOOL_BUTTON, PROPERTY_HINT_ONESHOT, ///< the property will be changed by self after setting, such as AudioStreamPlayer.playing, Particles.emitting. + PROPERTY_HINT_NO_NODEPATH, /// < this property will not contain a NodePath, regardless of type (Array, Dictionary, List, etc.). Needed for SceneTreeDock. PROPERTY_HINT_MAX, }; diff --git a/doc/classes/@GlobalScope.xml b/doc/classes/@GlobalScope.xml index cc5c1e1e682..e7f043fff8b 100644 --- a/doc/classes/@GlobalScope.xml +++ b/doc/classes/@GlobalScope.xml @@ -2943,7 +2943,7 @@ Hints that a property will be changed on its own after setting, such as [member AudioStreamPlayer.playing] or [member GPUParticles3D.emitting]. - + Represents the size of the [enum PropertyHint] enum. diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml index 1e12d619e22..cbf6d048ebb 100644 --- a/doc/classes/Node.xml +++ b/doc/classes/Node.xml @@ -1068,6 +1068,11 @@ Emitted when the node's editor description field changed. + + + Emitted when an attribute of the node that is relevant to the editor is changed. Only emitted in the editor. + + Emitted when the node is considered ready, after [method _ready] is called. diff --git a/doc/classes/TreeItem.xml b/doc/classes/TreeItem.xml index d04a6f7316d..cce31b4bb1d 100644 --- a/doc/classes/TreeItem.xml +++ b/doc/classes/TreeItem.xml @@ -36,6 +36,12 @@ Calls the [param method] on the actual TreeItem and its children recursively. Pass parameters as a comma separated list. + + + + Removes all buttons from all columns of this item. + + diff --git a/editor/connections_dialog.cpp b/editor/connections_dialog.cpp index 53b1f2a759a..f06765e982a 100644 --- a/editor/connections_dialog.cpp +++ b/editor/connections_dialog.cpp @@ -735,6 +735,7 @@ ConnectDialog::ConnectDialog() { from_signal->set_editable(false); tree = memnew(SceneTreeEditor(false)); + tree->set_update_when_invisible(false); tree->set_connecting_signal(true); tree->set_show_enabled_subscene(true); tree->set_v_size_flags(Control::SIZE_FILL | Control::SIZE_EXPAND); diff --git a/editor/gui/scene_tree_editor.cpp b/editor/gui/scene_tree_editor.cpp index 2d51716c88d..154168f7716 100644 --- a/editor/gui/scene_tree_editor.cpp +++ b/editor/gui/scene_tree_editor.cpp @@ -61,7 +61,7 @@ void SceneTreeEditor::_cell_button_pressed(Object *p_item, int p_column, int p_i } if (connect_to_script_mode) { - return; //don't do anything in this mode + return; // Don't do anything in this mode. } TreeItem *item = Object::cast_to(p_item); @@ -152,7 +152,8 @@ void SceneTreeEditor::_cell_button_pressed(Object *p_item, int p_column, int p_i const String line = all_warnings.substr(start, end - start); lines.append(line); } - all_warnings = String("\n").join(lines).indent(" ").replace(U" •", U"\n•").substr(2); // We don't want the first two newlines. + // We don't want the first two newlines. + all_warnings = String("\n").join(lines).indent(" ").replace(U" •", U"\n•").substr(2); warning->set_text(all_warnings); warning->popup_centered(); @@ -217,12 +218,35 @@ void SceneTreeEditor::_toggle_visible(Node *p_node) { } } -void SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent) { +void SceneTreeEditor::_update_node_path(Node *p_node, bool p_recursive) { if (!p_node) { return; } - // only owned nodes are editable, since nodes can create their own (manually owned) child nodes, + HashMap::Iterator I = node_cache.get(p_node); + if (!I) { + return; + } + + I->value.item->set_metadata(0, p_node->get_path()); + + if (!p_recursive) { + return; + } + + int cc = p_node->get_child_count(false); + for (int i = 0; i < cc; i++) { + Node *c = p_node->get_child(i, false); + _update_node_path(c, p_recursive); + } +} + +void SceneTreeEditor::_update_node_subtree(Node *p_node, TreeItem *p_parent, bool p_force) { + if (!p_node) { + return; + } + + // Only owned nodes are editable, since nodes can create their own (manually owned) child nodes, // which the editor needs not to know about. bool part_of_subscene = false; @@ -230,42 +254,152 @@ void SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent) { if (!display_foreign && p_node->get_owner() != get_scene_node() && p_node != get_scene_node()) { if ((show_enabled_subscene || can_open_instance) && p_node->get_owner() && (get_scene_node()->is_editable_instance(p_node->get_owner()))) { part_of_subscene = true; - //allow + // Allow. } else { + // Stale node, remove recursively. + node_cache.remove(p_node, true); return; } } else { part_of_subscene = p_node != get_scene_node() && get_scene_node()->get_scene_inherited_state().is_valid() && get_scene_node()->get_scene_inherited_state()->find_node_by_path(get_scene_node()->get_path_to(p_node)) >= 0; } - TreeItem *item = tree->create_item(p_parent); + HashMap::Iterator I = node_cache.get(p_node); + TreeItem *item = nullptr; - item->set_text(0, p_node->get_name()); - item->set_text_overrun_behavior(0, TextServer::OVERRUN_NO_TRIMMING); - if (can_rename && !part_of_subscene) { - item->set_editable(0, true); + bool is_new = false; + + if (I) { + item = I->value.item; + TreeItem *current_parent = item->get_parent(); + + // Our parent might be re-created because of a changed type. + if (p_parent && p_parent != current_parent) { + if (current_parent) { + current_parent->remove_child(item); + } + p_parent->add_child(item); + I->value.removed = false; + _move_node_item(p_parent, I); + } + + if (I->value.has_moved_children) { + _move_node_children(I); + } + } else { + int index = -1; + // Check to see if there is a root node for us to reuse. + if (!p_parent) { + item = tree->get_root(); + if (!item) { + item = tree->create_item(nullptr); + index = 0; + } + } else { + index = p_node->get_index(false); + item = tree->create_item(p_parent, index); + } + + I = node_cache.add(p_node, item); + I->value.index = index; + is_new = true; + } + + if (!(p_force || I->value.dirty)) { + // Nothing to do. + return; + } + + _update_node(p_node, item, part_of_subscene); + I->value.dirty = false; + I->value.can_process = p_node->can_process(); + + // Force update all our children if we are new or if we were forced to update. + bool force_update_children = p_force || is_new; + // Update all our children. + for (int i = 0; i < p_node->get_child_count(false); i++) { + _update_node_subtree(p_node->get_child(i, false), item, force_update_children); + } + + if (valid_types.size()) { + bool valid = false; + for (const StringName &E : valid_types) { + if (p_node->is_class(E) || + EditorNode::get_singleton()->is_object_of_custom_type(p_node, E)) { + valid = true; + break; + } else { + Ref