Skip to content

Conversation

@emanvidmaker
Copy link
Contributor

due to logic oversight, update_shape can get queued twice to run on the first frame, making scenes csg shapes laggy to open/load in, with this redundant call.

This change makes sure that it cant queue twice by marking it as dirty as soon as possible, so it cant queue again.


Found out about the double queue behavior when i was making a benching tool for csg and logging functions calls.
Where i saw that "update_shape" gets called twice on scene load.

...
NOTIFICATION_READY --- 0 Microseconds
NOTIFICATION_TRANSFORM_CHANGED --- 7 Microseconds
CSGShape3D::_update_collision_faces --- 68733 Microseconds
CSGShape3D::update_shape --- 1672920 Microseconds
CSGShape3D::_update_collision_faces --- 67424 Microseconds
CSGShape3D::update_shape --- 127781 Microseconds
===-===FRAME 1 ===-===
===-===FRAME 2 ===-===
===-===FRAME 3 ===-===
===-===FRAME 4 ===-===
===-===FRAME 5 ===-===
...

A bit of code-base reading later, found out that "_make_dirty" 's logic can double queue it on specific but common scenario. Where its a root shape and not dirty it will call_deferred twice.

Old code: (macros removed for clarity)

void CSGShape3D::_make_dirty(bool p_parent_removing) {
    if ((p_parent_removing || is_root_shape()) && !dirty) {
        callable_mp(this, &CSGShape3D::update_shape).call_deferred(); // ...
    }

    if (!is_root_shape()) {
        parent_shape->_make_dirty();
    }
    else if (!dirty) {
        callable_mp(this, &CSGShape3D::update_shape).call_deferred();
    }
    dirty = true;
  
}

by setting dirty = true; earlier in the logic it prevents this double queue from happening and calls it once. (since the second queue up looks if dirty is false)

new code: (macros removed for clarity)

void CSGShape3D::_make_dirty(bool p_parent_removing) {
    if ((p_parent_removing || is_root_shape()) && !dirty) {
        callable_mp(this, &CSGShape3D::update_shape).call_deferred(); // ...
        dirty = true; // +++ new code, now the else if will fail if this ran and avoid a requeue
    }

    if (!is_root_shape()) {
        parent_shape->_make_dirty();
    }
    else if (!dirty) {
        callable_mp(this, &CSGShape3D::update_shape).call_deferred();
    }
    dirty = true;
 }

With this fix, it now logs like this:

...
NOTIFICATION_READY --- 0 Microseconds
NOTIFICATION_TRANSFORM_CHANGED --- 6 Microseconds
CSGShape3D::_update_collision_faces --- 67143 Microseconds
CSGShape3D::update_shape --- 1569224 Microseconds
===-===FRAME 1 ===-===
===-===FRAME 2 ===-===
===-===FRAME 3 ===-===
===-===FRAME 4 ===-===
===-===FRAME 5 ===-===
...

And noticeably reduces scene loading time a second or so, since update_shape can get laggy and now only calls once.

due to some logic oversight, update_shape gets queued twice to run on the first frame, making scenes csg shapes laggy to open/load in.
@emanvidmaker emanvidmaker requested a review from a team as a code owner December 26, 2025 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant