From d56d1ff4d2ad81c5291ee5a5e9c2cd3b953a9f0d Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Fri, 19 Apr 2024 09:53:06 +0100 Subject: [PATCH] Deprecate NOTIFICATION_MOVED_IN_PARENT * NOTIFICATION_MOVED_IN_PARENT makes node children management very inefficient. * Replaced by a NOTIFICATION_CHILD_ORDER_CHANGED (and children_changed signal). * Most of the previous tasks carried out by NOTIFICATION_MOVED_IN_PARENT are now done not more than a single time per frame. This PR breaks compatibility (although this notification was very rarely used, even within the engine), but provides an alternate way to do the same. --- doc/classes/Node.xml | 10 ++- main/main.cpp | 6 ++ scene/2d/canvas_item.cpp | 28 ++++---- scene/2d/canvas_item.h | 2 + scene/2d/skeleton_2d.cpp | 23 +++++-- scene/2d/skeleton_2d.h | 1 + scene/gui/control.cpp | 25 +++---- scene/gui/control.h | 2 + scene/main/canvas_layer.cpp | 13 ++-- scene/main/canvas_layer.h | 2 + scene/main/node.cpp | 11 ++-- scene/main/node.h | 10 ++- scene/main/viewport.cpp | 126 ++++++++++++++++++++++++++++++++++++ scene/main/viewport.h | 12 ++++ 14 files changed, 224 insertions(+), 47 deletions(-) diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml index 0307e902ea5..9013d2a4747 100644 --- a/doc/classes/Node.xml +++ b/doc/classes/Node.xml @@ -797,6 +797,11 @@ When this signal is received, the child [code]node[/code] 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. @@ -835,7 +840,7 @@ This notification is emitted [i]after[/i] the related [signal tree_exiting]. - Notification received when the node is moved in the parent. + [i]Deprecated.[/i] This notification is no longer emitted. Use [constant NOTIFICATION_CHILD_ORDER_CHANGED] instead. Notification received when the node is ready. See [method _ready]. @@ -874,6 +879,9 @@ Notification received when the node's [NodePath] changed. + + 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/main/main.cpp b/main/main.cpp index 62726113fc7..92b3f4c9c1f 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -2399,6 +2399,12 @@ bool Main::iteration() { exit = true; } visual_server_callbacks->flush(); + + // Ensure that VisualServer is kept up to date at least once with any ordering changes + // of canvas items before a render. + // This ensures this will be done at least once in apps that create their own MainLoop. + Viewport::flush_canvas_parents_dirty_order(); + message_queue->flush(); VisualServer::get_singleton()->sync(); //sync if still drawing from previous frames. diff --git a/scene/2d/canvas_item.cpp b/scene/2d/canvas_item.cpp index 0b1d0bc865d..0d964491836 100644 --- a/scene/2d/canvas_item.cpp +++ b/scene/2d/canvas_item.cpp @@ -611,20 +611,6 @@ void CanvasItem::_notification(int p_what) { notification(NOTIFICATION_RESET_PHYSICS_INTERPOLATION); } } break; - case NOTIFICATION_MOVED_IN_PARENT: { - if (!is_inside_tree()) { - break; - } - - if (canvas_group != "") { - get_tree()->call_group_flags(SceneTree::GROUP_CALL_UNIQUE, canvas_group, "_toplevel_raise_self"); - } else { - CanvasItem *p = get_parent_item(); - ERR_FAIL_COND(!p); - VisualServer::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); @@ -662,6 +648,20 @@ void CanvasItem::_name_changed_notify() { } #endif +void CanvasItem::update_draw_order() { + if (!is_inside_tree()) { + return; + } + + if (canvas_group != "") { + get_tree()->call_group_flags(SceneTree::GROUP_CALL_UNIQUE, canvas_group, "_toplevel_raise_self"); + } else { + CanvasItem *p = get_parent_item(); + ERR_FAIL_NULL(p); + VisualServer::get_singleton()->canvas_item_set_draw_index(canvas_item, get_index()); + } +} + void CanvasItem::update() { if (!is_inside_tree()) { return; diff --git a/scene/2d/canvas_item.h b/scene/2d/canvas_item.h index c8acd5652f1..8e50a412411 100644 --- a/scene/2d/canvas_item.h +++ b/scene/2d/canvas_item.h @@ -296,6 +296,8 @@ public: virtual Transform2D _edit_get_transform() const; #endif + void update_draw_order(); + /* VISIBILITY */ void set_visible(bool p_visible); diff --git a/scene/2d/skeleton_2d.cpp b/scene/2d/skeleton_2d.cpp index b109007da00..ebbe0775dd3 100644 --- a/scene/2d/skeleton_2d.cpp +++ b/scene/2d/skeleton_2d.cpp @@ -30,9 +30,20 @@ #include "skeleton_2d.h" +void Bone2D::_order_changed_in_parent() { + if (skeleton) { + skeleton->_make_bone_setup_dirty(); + } +} + void Bone2D::_notification(int p_what) { if (p_what == NOTIFICATION_ENTER_TREE) { Node *parent = get_parent(); + + if (parent) { + parent->connect("child_order_changed", this, "_order_changed_in_parent"); + } + parent_bone = Object::cast_to(parent); skeleton = nullptr; while (parent) { @@ -59,13 +70,13 @@ void Bone2D::_notification(int p_what) { skeleton->_make_transform_dirty(); } } - if (p_what == NOTIFICATION_MOVED_IN_PARENT) { - if (skeleton) { - skeleton->_make_bone_setup_dirty(); - } - } if (p_what == NOTIFICATION_EXIT_TREE) { + Node *parent = get_parent(); + if (parent) { + parent->disconnect("child_order_changed", this, "_order_changed_in_parent"); + } + if (skeleton) { for (int i = 0; i < skeleton->bones.size(); i++) { if (skeleton->bones[i].bone == this) { @@ -89,6 +100,8 @@ void Bone2D::_bind_methods() { ClassDB::bind_method(D_METHOD("set_default_length", "default_length"), &Bone2D::set_default_length); ClassDB::bind_method(D_METHOD("get_default_length"), &Bone2D::get_default_length); + ClassDB::bind_method(D_METHOD("_order_changed_in_parent"), &Bone2D::_order_changed_in_parent); + ADD_PROPERTY(PropertyInfo(Variant::TRANSFORM2D, "rest"), "set_rest", "get_rest"); ADD_PROPERTY(PropertyInfo(Variant::REAL, "default_length", PROPERTY_HINT_RANGE, "1,1024,1"), "set_default_length", "get_default_length"); } diff --git a/scene/2d/skeleton_2d.h b/scene/2d/skeleton_2d.h index 30a8d3092f1..22773b8e4cb 100644 --- a/scene/2d/skeleton_2d.h +++ b/scene/2d/skeleton_2d.h @@ -53,6 +53,7 @@ class Bone2D : public Node2D { protected: void _notification(int p_what); static void _bind_methods(); + void _order_changed_in_parent(); public: void set_rest(const Transform2D &p_rest); diff --git a/scene/gui/control.cpp b/scene/gui/control.cpp index 087234bc28c..5785115c424 100644 --- a/scene/gui/control.cpp +++ b/scene/gui/control.cpp @@ -592,22 +592,6 @@ void Control::_notification(int p_notification) { } */ - } break; - case NOTIFICATION_MOVED_IN_PARENT: { - // some parents need to know the order of the children to draw (like TabContainer) - // update if necessary - if (data.parent) { - data.parent->update(); - } - update(); - - if (data.SI) { - get_viewport()->_gui_set_subwindow_order_dirty(); - } - if (data.RI) { - get_viewport()->_gui_set_root_order_dirty(); - } - } break; case NOTIFICATION_RESIZED: { emit_signal(SceneStringNames::get_singleton()->resized); @@ -2665,6 +2649,15 @@ Control::GrowDirection Control::get_v_grow_direction() const { return data.v_grow; } +void Control::_query_order_update(bool &r_subwindow_order_dirty, bool &r_root_order_dirty) const { + if (data.SI) { + r_subwindow_order_dirty = true; + } + if (data.RI) { + r_root_order_dirty = true; + } +} + void Control::_bind_methods() { //ClassDB::bind_method(D_METHOD("_window_resize_event"),&Control::_window_resize_event); ClassDB::bind_method(D_METHOD("_size_changed"), &Control::_size_changed); diff --git a/scene/gui/control.h b/scene/gui/control.h index 8853edeedca..35515690086 100644 --- a/scene/gui/control.h +++ b/scene/gui/control.h @@ -508,6 +508,8 @@ public: virtual void get_argument_options(const StringName &p_function, int p_idx, List *r_options) const; virtual String get_configuration_warning() const; + void _query_order_update(bool &r_subwindow_order_dirty, bool &r_root_order_dirty) const; + Control(); ~Control(); }; diff --git a/scene/main/canvas_layer.cpp b/scene/main/canvas_layer.cpp index 78c004ea023..aa19078c272 100644 --- a/scene/main/canvas_layer.cpp +++ b/scene/main/canvas_layer.cpp @@ -228,15 +228,16 @@ void CanvasLayer::_notification(int p_what) { viewport = RID(); _update_follow_viewport(false); } break; - case NOTIFICATION_MOVED_IN_PARENT: { - // Note: As this step requires traversing the entire scene tree, it is thus expensive - // to move the canvas layer multiple times. Take special care when deleting / moving - // multiple nodes to prevent multiple NOTIFICATION_MOVED_IN_PARENT occurring. - _update_layer_orders(); - } break; } } +void CanvasLayer::update_draw_order() { + // Note: As this step requires traversing the entire scene tree, it is thus expensive + // to move the canvas layer multiple times. Take special care when deleting / moving + // multiple nodes to prevent this happening multiple times. + _update_layer_orders(); +} + Size2 CanvasLayer::get_viewport_size() const { if (!is_inside_tree()) { return Size2(1, 1); diff --git a/scene/main/canvas_layer.h b/scene/main/canvas_layer.h index 47ecfebdc91..03246a541f7 100644 --- a/scene/main/canvas_layer.h +++ b/scene/main/canvas_layer.h @@ -70,6 +70,8 @@ protected: static void _bind_methods(); 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 1ece2879a11..8623cd3b888 100644 --- a/scene/main/node.cpp +++ b/scene/main/node.cpp @@ -426,9 +426,8 @@ void Node::move_child(Node *p_child, int p_pos) { } // notification second move_child_notify(p_child); - for (int i = motion_from; i <= motion_to; i++) { - data.children[i]->notification(NOTIFICATION_MOVED_IN_PARENT); - } + Viewport::notify_canvas_parent_children_moved(*this, motion_from, motion_to + 1); + p_child->_propagate_groups_dirty(); data.blocked--; @@ -1364,9 +1363,11 @@ void Node::remove_child(Node *p_child) { for (int i = idx; i < child_count; i++) { children[i]->data.pos = i; - children[i]->notification(NOTIFICATION_MOVED_IN_PARENT); } + Viewport::notify_canvas_parent_children_moved(*this, idx, child_count); + Viewport::notify_canvas_parent_child_count_reduced(*this); + p_child->data.parent = nullptr; p_child->data.pos = -1; @@ -3193,6 +3194,7 @@ void Node::_bind_methods() { BIND_CONSTANT(NOTIFICATION_DRAG_BEGIN); BIND_CONSTANT(NOTIFICATION_DRAG_END); BIND_CONSTANT(NOTIFICATION_PATH_CHANGED); + BIND_CONSTANT(NOTIFICATION_CHILD_ORDER_CHANGED); BIND_CONSTANT(NOTIFICATION_INTERNAL_PROCESS); BIND_CONSTANT(NOTIFICATION_INTERNAL_PHYSICS_PROCESS); BIND_CONSTANT(NOTIFICATION_POST_ENTER_TREE); @@ -3233,6 +3235,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::INT, "pause_mode", PROPERTY_HINT_ENUM, "Inherit,Stop,Process"), "set_pause_mode", "get_pause_mode"); diff --git a/scene/main/node.h b/scene/main/node.h index 7b7ae20b28b..f688ddc262c 100644 --- a/scene/main/node.h +++ b/scene/main/node.h @@ -134,6 +134,11 @@ private: int process_priority; + // If canvas item children of a node change child order, + // we store this information in the scenetree in a temporary structure + // allocated on demand per node. + uint32_t canvas_parent_id = UINT32_MAX; + // Keep bitpacked values together to get better packing PauseMode pause_mode : 2; PhysicsInterpolationMode physics_interpolation_mode : 2; @@ -279,7 +284,7 @@ public: NOTIFICATION_DRAG_BEGIN = 21, NOTIFICATION_DRAG_END = 22, NOTIFICATION_PATH_CHANGED = 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, @@ -455,6 +460,9 @@ public: _FORCE_INLINE_ bool is_physics_interpolated_and_enabled() const { return is_inside_tree() && get_tree()->is_physics_interpolation_enabled() && is_physics_interpolated(); } void reset_physics_interpolation(); + uint32_t get_canvas_parent_id() const { return data.canvas_parent_id; } + void set_canvas_parent_id(uint32_t p_id) { data.canvas_parent_id = p_id; } + bool is_node_ready() const; void request_ready(); diff --git a/scene/main/viewport.cpp b/scene/main/viewport.cpp index 83694541ae2..ef92a88fadd 100644 --- a/scene/main/viewport.cpp +++ b/scene/main/viewport.cpp @@ -54,6 +54,8 @@ #include "scene/scene_string_names.h" #include "servers/physics_2d_server.h" +LocalVector Viewport::GUI::canvas_parents_dirty_order; + void ViewportTexture::setup_local_to_scene() { Node *local_scene = get_local_scene(); if (!local_scene) { @@ -3214,6 +3216,130 @@ bool Viewport::is_handling_input_locally() const { return handle_input_locally; } +void Viewport::notify_canvas_parent_child_count_reduced(const Node &p_parent) { + uint32_t id = p_parent.get_canvas_parent_id(); + + ERR_FAIL_COND(id == UINT32_MAX); + ERR_FAIL_UNSIGNED_INDEX(id, GUI::canvas_parents_dirty_order.size()); + + GUI::CanvasParent &d = GUI::canvas_parents_dirty_order[id]; + + // No pending children moved. + if (!d.last_child_moved_plus_one) { + return; + } + + uint32_t num_children = p_parent.get_child_count(); + d.last_child_moved_plus_one = MIN(d.last_child_moved_plus_one, num_children); +} + +void Viewport::notify_canvas_parent_children_moved(Node &r_parent, uint32_t p_first_child, uint32_t p_last_child_plus_one) { + // First ensure the parent has a CanvasParent allocated. + uint32_t id = r_parent.get_canvas_parent_id(); + + if (id == UINT32_MAX) { + id = GUI::canvas_parents_dirty_order.size(); + r_parent.set_canvas_parent_id(id); + GUI::canvas_parents_dirty_order.resize(id + 1); + + GUI::CanvasParent &d = GUI::canvas_parents_dirty_order[id]; + d.id = r_parent.get_instance_id(); + d.first_child_moved = p_first_child; + d.last_child_moved_plus_one = p_last_child_plus_one; + return; + } + + GUI::CanvasParent &d = GUI::canvas_parents_dirty_order[id]; + DEV_CHECK_ONCE(d.id == r_parent.get_instance_id()); + d.first_child_moved = MIN(d.first_child_moved, p_first_child); + d.last_child_moved_plus_one = MAX(d.last_child_moved_plus_one, p_last_child_plus_one); +} + +void Viewport::flush_canvas_parents_dirty_order() { + bool canvas_layers_moved = false; + + for (uint32_t n = 0; n < GUI::canvas_parents_dirty_order.size(); n++) { + GUI::CanvasParent &d = GUI::canvas_parents_dirty_order[n]; + + Object *obj = ObjectDB::get_instance(d.id); + if (!obj) { + // May have been deleted. + continue; + } + + Node *node = static_cast(obj); + Control *parent_control = Object::cast_to(node); + + // Allow anything subscribing to this signal (skeletons etc) + // to make any changes necessary. + node->emit_signal("child_order_changed"); + + // Reset the id stored in the node, as this data is cleared after every flush. + node->set_canvas_parent_id(UINT32_MAX); + + // This should be very rare, as the last_child_moved_plus_one is usually kept up + // to date when within the scene tree. But it may become out of date outside the scene tree. + // This will cause no problems, just a very small possibility of more notifications being sent than + // necessary in that very rare situation. + if (d.last_child_moved_plus_one > (uint32_t)node->get_child_count()) { + d.last_child_moved_plus_one = node->get_child_count(); + } + + bool subwindow_order_dirty = false; + bool root_order_dirty = false; + bool control_found = false; + + for (uint32_t c = d.first_child_moved; c < d.last_child_moved_plus_one; c++) { + Node *child = node->get_child(c); + + CanvasItem *ci = Object::cast_to(child); + if (ci) { + ci->update_draw_order(); + + Control *control = Object::cast_to(ci); + if (control) { + control->_query_order_update(subwindow_order_dirty, root_order_dirty); + + if (parent_control && !control_found) { + control_found = true; + + // In case of regressions, this could be moved to AFTER + // the children have been updated. + parent_control->update(); + } + control->update(); + } + + continue; + } + + CanvasLayer *cl = Object::cast_to(child); + if (cl) { + // If any canvas layers moved, we need to do an expensive update, + // so we ensure doing this only once. + if (!canvas_layers_moved) { + canvas_layers_moved = true; + cl->update_draw_order(); + } + } + } + + Viewport *viewport = node->get_viewport(); + if (viewport) { + if (subwindow_order_dirty) { + viewport->_gui_set_subwindow_order_dirty(); + } + if (root_order_dirty) { + viewport->_gui_set_root_order_dirty(); + } + } + + node->notification(Node::NOTIFICATION_CHILD_ORDER_CHANGED); + } + + GUI::canvas_parents_dirty_order.clear(); +} + void Viewport::_validate_property(PropertyInfo &property) const { if (VisualServer::get_singleton()->is_low_end() && (property.name == "hdr" || property.name == "use_32_bpc_depth" || property.name == "debanding" || property.name == "sharpen_intensity" || property.name == "debug_draw")) { // Only available in GLES3. diff --git a/scene/main/viewport.h b/scene/main/viewport.h index 5a751e3b62c..ae70c30aef0 100644 --- a/scene/main/viewport.h +++ b/scene/main/viewport.h @@ -326,6 +326,14 @@ private: bool dragging; bool drag_successful; + struct CanvasParent { + ObjectID id; + uint32_t first_child_moved = UINT32_MAX; + uint32_t last_child_moved_plus_one = 0; + }; + + static LocalVector canvas_parents_dirty_order; + GUI(); } gui; @@ -588,6 +596,10 @@ public: bool gui_is_dragging() const; bool gui_is_drag_successful() const; + static void notify_canvas_parent_children_moved(Node &r_parent, uint32_t p_first_child, uint32_t p_last_child_plus_one); + static void notify_canvas_parent_child_count_reduced(const Node &p_parent); + static void flush_canvas_parents_dirty_order(); + Viewport(); ~Viewport(); };