Merge pull request #82248 from lawnjelly/child_order_changed

[3.x] Deprecate `NOTIFICATION_MOVED_IN_PARENT`

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].
 			</description>
 		</signal>
+		<signal name="child_order_changed">
+			<description>
+				Emitted when the list of children is changed. This happens when child nodes are added, moved, or removed.
+			</description>
+		</signal>
 		<signal name="ready">
 			<description>
 				Emitted when the node is ready.
@@ -835,7 +840,7 @@
 			This notification is emitted [i]after[/i] the related [signal tree_exiting].
 		</constant>
 		<constant name="NOTIFICATION_MOVED_IN_PARENT" value="12">
-			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.
 		</constant>
 		<constant name="NOTIFICATION_READY" value="13">
 			Notification received when the node is ready. See [method _ready].
@@ -874,6 +879,9 @@
 		<constant name="NOTIFICATION_PATH_CHANGED" value="23">
 			Notification received when the node's [NodePath] changed.
 		</constant>
+		<constant name="NOTIFICATION_CHILD_ORDER_CHANGED" value="24">
+			Notification received when the list of children is changed. This happens when child nodes are added, moved, or removed.
+		</constant>
 		<constant name="NOTIFICATION_INTERNAL_PROCESS" value="25">
 			Notification received every frame when the internal process flag is set (see [method set_process_internal]).
 		</constant>

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.

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;

scene/2d/canvas_item.h

@@ -296,6 +296,8 @@ class CanvasItem : public Node {
 	virtual Transform2D _edit_get_transform() const;
 #endif
 
+	void update_draw_order();
+
 	/* VISIBILITY */
 
 	void set_visible(bool p_visible);

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<Bone2D>(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");
 }

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);

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);

scene/gui/control.h

@@ -508,6 +508,8 @@ class Control : public CanvasItem {
 	virtual void get_argument_options(const StringName &p_function, int p_idx, List<String> *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();
 };

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);

scene/main/canvas_layer.h

@@ -70,6 +70,8 @@ class CanvasLayer : public Node {
 	static void _bind_methods();
 
 public:
+	void update_draw_order();
+
 	void set_layer(int p_xform);
 	int get_layer() const;
 

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");
 

scene/main/node.h

@@ -134,6 +134,11 @@ class Node : public Object {
 
 		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 @@ class Node : public Object {
 		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 @@ class Node : public Object {
 	_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();