From 104392ef4ea7b983b27c44de72adfc627500e814 Mon Sep 17 00:00:00 2001 From: Juan Linietsky Date: Wed, 5 Apr 2023 18:53:32 +0200 Subject: [PATCH] Remove NOTIFICATION_MOVED_IN_PARENT * This notification makes node children management very inefficient. * Replaced by a NOTIFICATION_CHILDREN_CHANGED (and children_changed signal). * Changed Canvas code (and similar) to use the above signal, to perform more efficiently. This PR breaks compatibility (although this notification was very rarely used, even within the engine), but provides an alternate way to do the same. It is required for the changes in #75627 to be entirely effective. --- doc/classes/Node.xml | 12 ++++++++++-- scene/2d/node_2d.cpp | 9 +++++++-- scene/2d/skeleton_2d.cpp | 12 ++---------- scene/gui/control.cpp | 13 ++++--------- scene/main/canvas_item.cpp | 37 +++++++++++++++++++++++-------------- scene/main/canvas_item.h | 2 ++ scene/main/canvas_layer.cpp | 17 +++++++++++------ scene/main/canvas_layer.h | 2 ++ scene/main/node.cpp | 16 +++++++++------- scene/main/node.h | 2 +- scene/main/viewport.cpp | 33 +++++++++++++++++++++++++++++++++ scene/main/viewport.h | 5 +++++ 12 files changed, 109 insertions(+), 51 deletions(-) diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml index cb829ae1836..42418b3e8dd 100644 --- a/doc/classes/Node.xml +++ b/doc/classes/Node.xml @@ -830,6 +830,11 @@ When this signal is received, the child [param node] is still in the tree and valid. This signal is emitted [i]after[/i] the child node's own [signal tree_exiting] and [constant NOTIFICATION_EXIT_TREE]. + + + Emitted when the list of children is changed. This happens when child nodes are added, moved or removed. + + Emitted when the node is ready. Comes after [method _ready] callback and follows the same rules. @@ -867,8 +872,8 @@ Notification received when the node is about to exit a [SceneTree]. This notification is emitted [i]after[/i] the related [signal tree_exiting]. - - Notification received when the node is moved in the parent. + + This notification is deprecated and is no longer emitted. Use [constant NOTIFICATION_CHILD_ORDER_CHANGED] instead. Notification received when the node is ready. See [method _ready]. @@ -907,6 +912,9 @@ Notification received when the node's name or one of its parents' name is changed. This notification is [i]not[/i] received when the node is removed from the scene tree to be added to another parent later on. + + Notification received when the list of children is changed. This happens when child nodes are added, moved or removed. + Notification received every frame when the internal process flag is set (see [method set_process_internal]). diff --git a/scene/2d/node_2d.cpp b/scene/2d/node_2d.cpp index 1c0efe773fd..43a2f62163b 100644 --- a/scene/2d/node_2d.cpp +++ b/scene/2d/node_2d.cpp @@ -385,9 +385,14 @@ Point2 Node2D::to_global(Point2 p_local) const { void Node2D::_notification(int p_notification) { switch (p_notification) { - case NOTIFICATION_MOVED_IN_PARENT: { + case NOTIFICATION_ENTER_TREE: { if (get_viewport()) { - get_viewport()->gui_set_root_order_dirty(); + get_parent()->connect(SNAME("child_order_changed"), callable_mp(get_viewport(), &Viewport::gui_set_root_order_dirty), CONNECT_REFERENCE_COUNTED); + } + } break; + case NOTIFICATION_EXIT_TREE: { + if (get_viewport()) { + get_parent()->disconnect(SNAME("child_order_changed"), callable_mp(get_viewport(), &Viewport::gui_set_root_order_dirty)); } } break; } diff --git a/scene/2d/skeleton_2d.cpp b/scene/2d/skeleton_2d.cpp index 4fdc7b35840..f2270d2b18a 100644 --- a/scene/2d/skeleton_2d.cpp +++ b/scene/2d/skeleton_2d.cpp @@ -115,6 +115,7 @@ void Bone2D::_notification(int p_what) { bone.bone = this; skeleton->bones.push_back(bone); skeleton->_make_bone_setup_dirty(); + get_parent()->connect(SNAME("child_order_changed"), callable_mp(skeleton, &Skeleton2D::_make_bone_setup_dirty), CONNECT_REFERENCE_COUNTED); } cache_transform = get_transform(); @@ -154,15 +155,6 @@ void Bone2D::_notification(int p_what) { #endif // TOOLS_ENABLED } break; - case NOTIFICATION_MOVED_IN_PARENT: { - if (skeleton) { - skeleton->_make_bone_setup_dirty(); - } - if (copy_transform_to_cache) { - cache_transform = get_transform(); - } - } break; - case NOTIFICATION_EXIT_TREE: { if (skeleton) { for (int i = 0; i < skeleton->bones.size(); i++) { @@ -172,7 +164,7 @@ void Bone2D::_notification(int p_what) { } } skeleton->_make_bone_setup_dirty(); - skeleton = nullptr; + get_parent()->disconnect(SNAME("child_order_changed"), callable_mp(skeleton, &Skeleton2D::_make_bone_setup_dirty)); } parent_bone = nullptr; set_transform(cache_transform); diff --git a/scene/gui/control.cpp b/scene/gui/control.cpp index 569b89f6885..8957a9492e3 100644 --- a/scene/gui/control.cpp +++ b/scene/gui/control.cpp @@ -3021,6 +3021,8 @@ void Control::_notification(int p_notification) { Viewport *viewport = get_viewport(); ERR_FAIL_COND(!viewport); data.RI = viewport->_gui_add_root_control(this); + + get_parent()->connect(SNAME("child_order_changed"), callable_mp(get_viewport(), &Viewport::gui_set_root_order_dirty), CONNECT_REFERENCE_COUNTED); } data.parent_canvas_item = get_parent_item(); @@ -3049,24 +3051,17 @@ void Control::_notification(int p_notification) { if (data.RI) { get_viewport()->_gui_remove_root_control(data.RI); data.RI = nullptr; + get_parent()->disconnect(SNAME("child_order_changed"), callable_mp(get_viewport(), &Viewport::gui_set_root_order_dirty)); } data.parent_canvas_item = nullptr; data.is_rtl_dirty = true; } break; - case NOTIFICATION_MOVED_IN_PARENT: { + case NOTIFICATION_CHILD_ORDER_CHANGED: { // Some parents need to know the order of the children to draw (like TabContainer), // so we update them just in case. - Control *parent_control = get_parent_control(); - if (parent_control) { - parent_control->queue_redraw(); - } queue_redraw(); - - if (data.RI) { - get_viewport()->gui_set_root_order_dirty(); - } } break; case NOTIFICATION_RESIZED: { diff --git a/scene/main/canvas_item.cpp b/scene/main/canvas_item.cpp index 72fb8387320..4eabc4916cb 100644 --- a/scene/main/canvas_item.cpp +++ b/scene/main/canvas_item.cpp @@ -188,10 +188,13 @@ void CanvasItem::_enter_canvas() { // Resolves to nullptr if the node is top_level. CanvasItem *parent_item = get_parent_item(); + if (get_parent()) { + get_viewport()->canvas_parent_mark_dirty(get_parent()); + } + if (parent_item) { canvas_layer = parent_item->canvas_layer; RenderingServer::get_singleton()->canvas_item_set_parent(canvas_item, parent_item->get_canvas_item()); - RenderingServer::get_singleton()->canvas_item_set_draw_index(canvas_item, get_index()); RenderingServer::get_singleton()->canvas_item_set_visibility_layer(canvas_item, visibility_layer); } else { Node *n = this; @@ -227,8 +230,6 @@ void CanvasItem::_enter_canvas() { } else { get_viewport()->gui_reset_canvas_sort_index(); } - - get_tree()->call_group_flags(SceneTree::GROUP_CALL_UNIQUE | SceneTree::GROUP_CALL_DEFERRED, canvas_group, SNAME("_top_level_raise_self")); } pending_update = false; @@ -302,21 +303,12 @@ void CanvasItem::_notification(int p_what) { if (!block_transform_notify && !xform_change.in_list()) { get_tree()->xform_change_list.add(&xform_change); } - } break; - case NOTIFICATION_MOVED_IN_PARENT: { - if (!is_inside_tree()) { - break; + if (get_viewport()) { + get_parent()->connect(SNAME("child_order_changed"), callable_mp(get_viewport(), &Viewport::canvas_parent_mark_dirty).bind(get_parent()), CONNECT_REFERENCE_COUNTED); } - if (canvas_group != StringName()) { - get_tree()->call_group_flags(SceneTree::GROUP_CALL_UNIQUE | SceneTree::GROUP_CALL_DEFERRED, canvas_group, "_top_level_raise_self"); - } else { - ERR_FAIL_COND_MSG(!get_parent_item(), "Moved child is in incorrect state (no canvas group, no canvas item parent)."); - RenderingServer::get_singleton()->canvas_item_set_draw_index(canvas_item, get_index()); - } } break; - case NOTIFICATION_EXIT_TREE: { if (xform_change.in_list()) { get_tree()->xform_change_list.remove(&xform_change); @@ -332,6 +324,10 @@ void CanvasItem::_notification(int p_what) { } global_invalid = true; parent_visible_in_tree = false; + + if (get_viewport()) { + get_parent()->disconnect(SNAME("child_order_changed"), callable_mp(get_viewport(), &Viewport::canvas_parent_mark_dirty).bind(get_parent())); + } } break; case NOTIFICATION_VISIBILITY_CHANGED: { @@ -340,6 +336,19 @@ void CanvasItem::_notification(int p_what) { } } +void CanvasItem::update_draw_order() { + if (!is_inside_tree()) { + return; + } + + if (canvas_group != StringName()) { + get_tree()->call_group_flags(SceneTree::GROUP_CALL_UNIQUE | SceneTree::GROUP_CALL_DEFERRED, canvas_group, "_top_level_raise_self"); + } else { + ERR_FAIL_COND_MSG(!get_parent_item(), "Moved child is in incorrect state (no canvas group, no canvas item parent)."); + RenderingServer::get_singleton()->canvas_item_set_draw_index(canvas_item, get_index()); + } +} + void CanvasItem::_window_visibility_changed() { if (visible) { _propagate_visibility_changed(window->is_visible()); diff --git a/scene/main/canvas_item.h b/scene/main/canvas_item.h index 5fbf0431598..ea1e666b08d 100644 --- a/scene/main/canvas_item.h +++ b/scene/main/canvas_item.h @@ -212,6 +212,8 @@ public: virtual Transform2D _edit_get_transform() const; #endif + void update_draw_order(); + /* VISIBILITY */ void set_visible(bool p_visible); diff --git a/scene/main/canvas_layer.cpp b/scene/main/canvas_layer.cpp index 76cc17922ad..274bd1ad681 100644 --- a/scene/main/canvas_layer.cpp +++ b/scene/main/canvas_layer.cpp @@ -180,25 +180,30 @@ void CanvasLayer::_notification(int p_what) { viewport = vp->get_viewport_rid(); RenderingServer::get_singleton()->viewport_attach_canvas(viewport, canvas); - RenderingServer::get_singleton()->viewport_set_canvas_stacking(viewport, canvas, layer, get_index()); RenderingServer::get_singleton()->viewport_set_canvas_transform(viewport, canvas, transform); _update_follow_viewport(); + + if (vp) { + get_parent()->connect(SNAME("child_order_changed"), callable_mp(vp, &Viewport::canvas_parent_mark_dirty).bind(get_parent()), CONNECT_REFERENCE_COUNTED); + vp->canvas_parent_mark_dirty(get_parent()); + } } break; case NOTIFICATION_EXIT_TREE: { ERR_FAIL_NULL_MSG(vp, "Viewport is not initialized."); + get_parent()->disconnect(SNAME("child_order_changed"), callable_mp(vp, &Viewport::canvas_parent_mark_dirty).bind(get_parent())); vp->_canvas_layer_remove(this); RenderingServer::get_singleton()->viewport_remove_canvas(viewport, canvas); viewport = RID(); _update_follow_viewport(false); } break; + } +} - case NOTIFICATION_MOVED_IN_PARENT: { - if (is_inside_tree()) { - RenderingServer::get_singleton()->viewport_set_canvas_stacking(viewport, canvas, layer, get_index()); - } - } break; +void CanvasLayer::update_draw_order() { + if (is_inside_tree()) { + RenderingServer::get_singleton()->viewport_set_canvas_stacking(viewport, canvas, layer, get_index()); } } diff --git a/scene/main/canvas_layer.h b/scene/main/canvas_layer.h index 47077dc7fdf..be1a377a142 100644 --- a/scene/main/canvas_layer.h +++ b/scene/main/canvas_layer.h @@ -67,6 +67,8 @@ protected: void _validate_property(PropertyInfo &p_property) const; public: + void update_draw_order(); + void set_layer(int p_xform); int get_layer() const; diff --git a/scene/main/node.cpp b/scene/main/node.cpp index 22bcfc947bd..d8375dbc9f1 100644 --- a/scene/main/node.cpp +++ b/scene/main/node.cpp @@ -393,9 +393,9 @@ void Node::_move_child(Node *p_child, int p_index, bool p_ignore_end) { } // notification second move_child_notify(p_child); - for (int i = motion_from; i <= motion_to; i++) { - data.children[i]->notification(NOTIFICATION_MOVED_IN_PARENT); - } + notification(NOTIFICATION_CHILD_ORDER_CHANGED); + emit_signal(SNAME("child_order_changed")); + p_child->_propagate_groups_dirty(); data.blocked--; @@ -1124,6 +1124,8 @@ void Node::_add_child_nocheck(Node *p_child, const StringName &p_name) { //recognize children created in this node constructor p_child->data.parent_owned = data.in_constructor; add_child_notify(p_child); + notification(NOTIFICATION_CHILD_ORDER_CHANGED); + emit_signal(SNAME("child_order_changed")); } void Node::add_child(Node *p_child, bool p_force_readable_name, InternalMode p_internal) { @@ -1213,10 +1215,8 @@ void Node::remove_child(Node *p_child) { child_count = data.children.size(); children = data.children.ptrw(); - for (int i = idx; i < child_count; i++) { - children[i]->data.index = i; - children[i]->notification(NOTIFICATION_MOVED_IN_PARENT); - } + notification(NOTIFICATION_CHILD_ORDER_CHANGED); + emit_signal(SNAME("child_order_changed")); p_child->data.parent = nullptr; p_child->data.index = -1; @@ -2974,6 +2974,7 @@ void Node::_bind_methods() { BIND_CONSTANT(NOTIFICATION_DRAG_BEGIN); BIND_CONSTANT(NOTIFICATION_DRAG_END); BIND_CONSTANT(NOTIFICATION_PATH_RENAMED); + BIND_CONSTANT(NOTIFICATION_CHILD_ORDER_CHANGED); BIND_CONSTANT(NOTIFICATION_INTERNAL_PROCESS); BIND_CONSTANT(NOTIFICATION_INTERNAL_PHYSICS_PROCESS); BIND_CONSTANT(NOTIFICATION_POST_ENTER_TREE); @@ -3027,6 +3028,7 @@ void Node::_bind_methods() { ADD_SIGNAL(MethodInfo("tree_exited")); ADD_SIGNAL(MethodInfo("child_entered_tree", PropertyInfo(Variant::OBJECT, "node", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_DEFAULT, "Node"))); ADD_SIGNAL(MethodInfo("child_exiting_tree", PropertyInfo(Variant::OBJECT, "node", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_DEFAULT, "Node"))); + ADD_SIGNAL(MethodInfo("child_order_changed")); ADD_PROPERTY(PropertyInfo(Variant::STRING_NAME, "name", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NONE), "set_name", "get_name"); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "unique_name_in_owner", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NO_EDITOR), "set_unique_name_in_owner", "is_unique_name_in_owner"); diff --git a/scene/main/node.h b/scene/main/node.h index 8ce42d04bdb..939632770e0 100644 --- a/scene/main/node.h +++ b/scene/main/node.h @@ -264,7 +264,7 @@ public: NOTIFICATION_DRAG_BEGIN = 21, NOTIFICATION_DRAG_END = 22, NOTIFICATION_PATH_RENAMED = 23, - //NOTIFICATION_TRANSLATION_CHANGED = 24, moved below + NOTIFICATION_CHILD_ORDER_CHANGED = 24, NOTIFICATION_INTERNAL_PROCESS = 25, NOTIFICATION_INTERNAL_PHYSICS_PROCESS = 26, NOTIFICATION_POST_ENTER_TREE = 27, diff --git a/scene/main/viewport.cpp b/scene/main/viewport.cpp index feddb1e779c..9f9a2eeb18d 100644 --- a/scene/main/viewport.cpp +++ b/scene/main/viewport.cpp @@ -165,6 +165,31 @@ ViewportTexture::~ViewportTexture() { } } +void Viewport::_process_dirty_canvas_parent_orders() { + for (const ObjectID &id : gui.canvas_parents_with_dirty_order) { + Object *obj = ObjectDB::get_instance(id); + if (!obj) { + continue; // May have been deleted. + } + + Node *n = static_cast(obj); + for (int i = 0; i < n->get_child_count(); i++) { + Node *c = n->get_child(i); + CanvasItem *ci = Object::cast_to(c); + if (ci) { + ci->update_draw_order(); + continue; + } + CanvasLayer *cl = Object::cast_to(c); + if (cl) { + cl->update_draw_order(); + } + } + } + + gui.canvas_parents_with_dirty_order.clear(); +} + void Viewport::_sub_window_update_order() { if (gui.sub_windows.size() < 2) { return; @@ -929,6 +954,14 @@ Rect2 Viewport::get_visible_rect() const { return r; } +void Viewport::canvas_parent_mark_dirty(Node *p_node) { + bool request_update = gui.canvas_parents_with_dirty_order.is_empty(); + gui.canvas_parents_with_dirty_order.insert(p_node->get_instance_id()); + if (request_update) { + MessageQueue::get_singleton()->push_callable(callable_mp(this, &Viewport::_process_dirty_canvas_parent_orders)); + } +} + void Viewport::_update_audio_listener_2d() { if (AudioServer::get_singleton()) { AudioServer::get_singleton()->notify_listener_changed(); diff --git a/scene/main/viewport.h b/scene/main/viewport.h index 5213c0db018..f12e7921a3d 100644 --- a/scene/main/viewport.h +++ b/scene/main/viewport.h @@ -376,6 +376,7 @@ private: double tooltip_delay = 0.0; bool roots_order_dirty = false; List roots; + HashSet canvas_parents_with_dirty_order; int canvas_sort_index = 0; //for sorting items with canvas as root bool dragging = false; bool drag_successful = false; @@ -468,6 +469,8 @@ private: virtual bool _can_consume_input_events() const { return true; } uint64_t event_count = 0; + void _process_dirty_canvas_parent_orders(); + protected: void _set_size(const Size2i &p_size, const Size2i &p_size_2d_override, bool p_allocated); @@ -480,6 +483,8 @@ protected: static void _bind_methods(); public: + void canvas_parent_mark_dirty(Node *p_node); + uint64_t get_processed_events_count() const { return event_count; } AudioListener2D *get_audio_listener_2d() const;