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

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