-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Merge dirty relayout boundaries after RenderObject.invokeLayoutCallback
#105175
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
Merge dirty relayout boundaries after RenderObject.invokeLayoutCallback
#105175
Conversation
RenderObject.invokeLayoutCallback
| } | ||
| } | ||
| } finally { | ||
| _shouldMergeDirtyNodes = false; |
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.
If the node causing _shouldMergeDirtyNodes to be set to true is the last node in dirtyNodes this could cause an unnecessary node merging, no?
Scenario: Line 1000 causes _shouldMergeDirtyNodes to go to true and since there are no more nodes in dirtyNodes, we exit the for loop. Since there are new node in _nodesNeedingLayout we do another run through the body of the while: We sort the nodes, enter the for loop again and since _shouldMergeDirtyNodes is still true, we do an unnecessary node merge.
goderbauer
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.
LGTM
An example where widget 3's subtree will be laid out twice in one frame:
When the
InheritedWidgetchanges, widget 3's render object will be marked as dirty so the initial dirty list will be:[RenderLayoutBuilder, RenderWidget3].After
PipelineOwner.flushLayoutstarts, theLayoutBuilderrebuilds, dirtying Widget 2 and causing it to produce different constraints for widget 3. But sincePipelineOwner.flushLayoutis already running so the list of dirty relayout boundaries is already locked. SoRenderWidget3still gets to do layout beforeRenderWidget2, using outdated constraints.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.