Skip to content

Commit

Permalink
Remove NOTIFICATION_MOVED_IN_PARENT
Browse files Browse the repository at this point in the history
* 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 godotengine#75627 to be entirely effective.
  • Loading branch information
reduz committed Apr 6, 2023
1 parent 44d5394 commit 104392e
Show file tree
Hide file tree
Showing 12 changed files with 109 additions and 51 deletions.
12 changes: 10 additions & 2 deletions doc/classes/Node.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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].
</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. Comes after [method _ready] callback and follows the same rules.
Expand Down Expand Up @@ -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].
</constant>
<constant name="NOTIFICATION_MOVED_IN_PARENT" value="12">
Notification received when the node is moved in the parent.
<constant name="NOTIFICATION_MOVED_IN_PARENT" value="12" is_deprecated="true">
This notification is deprecated and 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].
Expand Down Expand Up @@ -907,6 +912,9 @@
<constant name="NOTIFICATION_PATH_RENAMED" value="23">
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.
</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>
Expand Down
9 changes: 7 additions & 2 deletions scene/2d/node_2d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 2 additions & 10 deletions scene/2d/skeleton_2d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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++) {
Expand All @@ -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);
Expand Down
13 changes: 4 additions & 9 deletions scene/gui/control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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: {
Expand Down
37 changes: 23 additions & 14 deletions scene/main/canvas_item.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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: {
Expand All @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions scene/main/canvas_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ class CanvasItem : public Node {
virtual Transform2D _edit_get_transform() const;
#endif

void update_draw_order();

/* VISIBILITY */

void set_visible(bool p_visible);
Expand Down
17 changes: 11 additions & 6 deletions scene/main/canvas_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Expand Down
2 changes: 2 additions & 0 deletions scene/main/canvas_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ class CanvasLayer : public Node {
void _validate_property(PropertyInfo &p_property) const;

public:
void update_draw_order();

void set_layer(int p_xform);
int get_layer() const;

Expand Down
16 changes: 9 additions & 7 deletions scene/main/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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--;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion scene/main/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ class Node : public Object {
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,
Expand Down
33 changes: 33 additions & 0 deletions scene/main/viewport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Node *>(obj);
for (int i = 0; i < n->get_child_count(); i++) {
Node *c = n->get_child(i);
CanvasItem *ci = Object::cast_to<CanvasItem>(c);
if (ci) {
ci->update_draw_order();
continue;
}
CanvasLayer *cl = Object::cast_to<CanvasLayer>(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;
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions scene/main/viewport.h
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ class Viewport : public Node {
double tooltip_delay = 0.0;
bool roots_order_dirty = false;
List<Control *> roots;
HashSet<ObjectID> canvas_parents_with_dirty_order;
int canvas_sort_index = 0; //for sorting items with canvas as root
bool dragging = false;
bool drag_successful = false;
Expand Down Expand Up @@ -468,6 +469,8 @@ class Viewport : public Node {
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);

Expand All @@ -480,6 +483,8 @@ class Viewport : public Node {
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;
Expand Down

0 comments on commit 104392e

Please sign in to comment.