Skip to content

Conversation

@chiefcll
Copy link
Contributor

@chiefcll chiefcll commented Jan 7, 2026

See comments in line - #687 can be closed out for this PR

let updateParent = false;
// reset update type
this.updateType = 0;
this.childUpdateType = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reset happens at beginning of loop rather than end


if (this.renderState === CoreNodeRenderState.OutOfBounds) {
updateType &= ~UpdateType.RenderBounds; // remove render bounds update
this.updateType = updateType;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just needed to be set back. Before this updateType &= ~UpdateType.RenderBounds; was doing nothing as it was changing a local variable and then returning

Comment on lines +1240 to +1244
let childClippingRect = this.clippingRect;

if (this.rtt === true) {
childClippingRect = NO_CLIPPING_RECT;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this check outside the for loop as its for the parent and not each child

Comment on lines +1249 to +1251
if (childUpdateType !== 0) {
child.setUpdateType(childUpdateType);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip updating all children nodes and parent nodes if we're just resetting anyhow.

Comment on lines -1041 to -1043
if (this.updateType === UpdateType.None) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed as the child for loop checks this.

@chiefcll
Copy link
Contributor Author

chiefcll commented Jan 7, 2026

Also I believe the test is broken as the code was broken before...

Expected says red rectangle is out of bounds, when it is In bounds.
viewport-events-17-expected

With the update, the red rectangle is in the viewport.
viewport-events-17-actual

Copy link
Contributor

@wouterlucas wouterlucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good- thanks!

@wouterlucas wouterlucas added this pull request to the merge queue Jan 8, 2026
Merged via the queue into lightning-js:main with commit 4a6f52b Jan 8, 2026
2 checks passed
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.

2 participants