Skip to content

Commit

Permalink
Deferred NOTIFICATION_MOVED_IN_PARENT and register interest
Browse files Browse the repository at this point in the history
Adds the ability to defer sending NOTIFICATION_MOVED_IN_PARENT to the next flush, instead of sending notifications immediately. This system allows the prevention of duplicate notifications on the same frame / tick, which can result in large numbers of notifications and slowdown.

For efficiency, stores the range of children moved on the parent node, rather than storing each child individually.

Additionally 2D nodes register an interest in observing NOTIFICATION_MOVED_IN_PARENT, this is tested before sending the final notification.
  • Loading branch information
lawnjelly committed Mar 11, 2023
1 parent 8e7bb46 commit ac5f91a
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 5 deletions.
1 change: 1 addition & 0 deletions scene/2d/canvas_item.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,7 @@ CanvasItem::CanvasItem() :
light_mask = 1;

C = nullptr;
_set_observe_notification_moved_in_parent(true);
}

CanvasItem::~CanvasItem() {
Expand Down
1 change: 1 addition & 0 deletions scene/main/canvas_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ CanvasLayer::CanvasLayer() {
visible = true;
follow_viewport = false;
follow_viewport_scale = 1.0;
_set_observe_notification_moved_in_parent(true);
}

CanvasLayer::~CanvasLayer() {
Expand Down
34 changes: 29 additions & 5 deletions scene/main/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,16 @@ 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);
if (is_inside_tree()) {
SceneTree *tree = get_tree();
tree->notify_children_moved(*this, motion_from, motion_to + 1);
} else {
for (int i = motion_from; i <= motion_to; i++) {
Node *child = data.children[i];
if (child->data.observe_notification_moved_in_parent) {
child->notification(NOTIFICATION_MOVED_IN_PARENT);
}
}
}
p_child->_propagate_groups_dirty();

Expand Down Expand Up @@ -1352,9 +1360,21 @@ 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.pos = i;
children[i]->notification(NOTIFICATION_MOVED_IN_PARENT);
if (is_inside_tree()) {
SceneTree *tree = get_tree();
tree->notify_children_moved(*this, idx, child_count);
for (int i = idx; i < child_count; i++) {
children[i]->data.pos = i;
}
tree->notify_child_count_reduced(*this);
} else {
for (int i = idx; i < child_count; i++) {
Node *child = children[i];
child->data.pos = i;
if (child->data.observe_notification_moved_in_parent) {
child->notification(NOTIFICATION_MOVED_IN_PARENT);
}
}
}

p_child->data.parent = nullptr;
Expand Down Expand Up @@ -3251,6 +3271,7 @@ Node::Node() {
data.physics_interpolation_reset_requested = false;
data.physics_interpolated_client_side = false;
data.use_identity_transform = false;
data.observe_notification_moved_in_parent = false;

data.owner = nullptr;
data.OW = nullptr;
Expand All @@ -3270,6 +3291,9 @@ Node::Node() {
data.ready_first = true;
data.editable_instance = false;

data.first_child_moved = UINT32_MAX;
data.last_child_moved_plus_one = 0;

orphan_node_count++;
}

Expand Down
10 changes: 10 additions & 0 deletions scene/main/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ class Node : public Object {

int process_priority;

// In order to prevent sending NOTIFICATION_MOVED_IN_PARENT multiple
// times per tick / frame, we defer this to once per tick, and maintain a range of moved
// children to send the notification.
// If "last_child_moved_plus_one" is set to zero, no children were moved this tick.
uint32_t first_child_moved;
uint32_t last_child_moved_plus_one;

// Keep bitpacked values together to get better packing
PauseMode pause_mode : 2;
PhysicsInterpolationMode physics_interpolation_mode : 2;
Expand Down Expand Up @@ -181,6 +188,8 @@ class Node : public Object {
bool ready_notified : 1; //this is a small hack, so if a node is added during _ready() to the tree, it correctly gets the _ready() notification
bool ready_first : 1;

bool observe_notification_moved_in_parent : 1;

mutable NodePath *path_cache;

} data;
Expand Down Expand Up @@ -257,6 +266,7 @@ class Node : public Object {
bool _is_physics_interpolation_reset_requested() const { return data.physics_interpolation_reset_requested; }
void _set_use_identity_transform(bool p_enable);
bool _is_using_identity_transform() const { return data.use_identity_transform; }
void _set_observe_notification_moved_in_parent(bool p_enable) { data.observe_notification_moved_in_parent = p_enable; }

public:
enum {
Expand Down
75 changes: 75 additions & 0 deletions scene/main/scene_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ bool SceneTree::idle(float p_time) {

process_tweens(p_time, false);

flush_children_moved(); // flush again any notifications issued in the idle before the draw occurs
flush_transform_notifications(); //additional transforms after timers update

_call_idle_callbacks();
Expand Down Expand Up @@ -2236,6 +2237,80 @@ void SceneTree::get_argument_options(const StringName &p_function, int p_idx, Li
}
}

void SceneTree::notify_child_count_reduced(Node &p_parent) {
// Uncomment the line below to use a passthrough rather than deferred.
// #define GODOT_SCENE_TREE_MOVED_IN_PARENT_PASSTHROUGH
#ifndef GODOT_SCENE_TREE_MOVED_IN_PARENT_PASSTHROUGH
Node::Data &d = p_parent.data;

// no pending children moved
if (!d.last_child_moved_plus_one) {
return;
}

uint32_t num_children = p_parent.get_child_count();
if (num_children < d.last_child_moved_plus_one) {
d.last_child_moved_plus_one = num_children;
}
#endif
}

void SceneTree::notify_children_moved(Node &p_parent, uint32_t p_first_child, uint32_t p_last_child_plus_one) {
#ifdef GODOT_SCENE_TREE_MOVED_IN_PARENT_PASSTHROUGH
for (uint32_t n = p_first_child; n < p_last_child_plus_one; n++) {
Node *child = p_parent->get_child(n);
if (child->data.observe_notification_moved_in_parent) {
child->notification(Node::NOTIFICATION_MOVED_IN_PARENT);
}
}
#else
Node::Data &d = p_parent.data;

// Should we add? (i.e. first occurrence this tick / frame)
if (!d.last_child_moved_plus_one) {
_pending_children_moved_parents.push_back(p_parent.get_instance_id());
}

d.first_child_moved = MIN(p_first_child, d.first_child_moved);
d.last_child_moved_plus_one = MAX(p_last_child_plus_one, d.last_child_moved_plus_one);
#endif
}

void SceneTree::flush_children_moved() {
for (uint32_t n = 0; n < _pending_children_moved_parents.size(); n++) {
ObjectID id = _pending_children_moved_parents[n];
Object *obj = ObjectDB::get_instance(id);
if (obj) {
// Should always be a node by definition
// (only nodes get sent deferred notifications currently).
// Check this in DEV_ENABLED?
Node *node = (Node *)obj;

Node::Data &d = node->data;

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

for (uint32_t c = d.first_child_moved; c < d.last_child_moved_plus_one; c++) {
Node *child = node->get_child(c);
if (child->data.observe_notification_moved_in_parent) {
node->get_child(c)->notification(Node::NOTIFICATION_MOVED_IN_PARENT);
}
}

d.first_child_moved = UINT32_MAX;
d.last_child_moved_plus_one = 0;
}
}

_pending_children_moved_parents.clear();
}

SceneTree::SceneTree() {
if (singleton == nullptr) {
singleton = this;
Expand Down
8 changes: 8 additions & 0 deletions scene/main/scene_tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,15 @@ class SceneTree : public MainLoop {
static int idle_callback_count;
void _call_idle_callbacks();

// List of parent nodes that have children moved within,
// that are in need of a deferred NOTIFICATION_MOVED_IN_PARENT.
LocalVector<ObjectID> _pending_children_moved_parents;
void notify_children_moved(Node &p_parent, uint32_t p_first_child, uint32_t p_last_child_plus_one);
void notify_child_count_reduced(Node &p_parent);

protected:
void flush_children_moved();

void _notification(int p_notification);
static void _bind_methods();

Expand Down

0 comments on commit ac5f91a

Please sign in to comment.