-
Notifications
You must be signed in to change notification settings - Fork 32
optimize the update loop #695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ef0c89a to
fd12df2
Compare
| let updateParent = false; | ||
| // reset update type | ||
| this.updateType = 0; | ||
| this.childUpdateType = 0; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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
| let childClippingRect = this.clippingRect; | ||
|
|
||
| if (this.rtt === true) { | ||
| childClippingRect = NO_CLIPPING_RECT; | ||
| } |
There was a problem hiding this comment.
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
| if (childUpdateType !== 0) { | ||
| child.setUpdateType(childUpdateType); | ||
| } |
There was a problem hiding this comment.
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.
| if (this.updateType === UpdateType.None) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
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.
wouterlucas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good- thanks!


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