Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(512)

Issue 1196553004: Short-circuit the relayoutSubtreeRoot when the child couldn't change dimensions anyway because the … (Closed)

Created:
5 years, 6 months ago by Hixie
Modified:
5 years, 6 months ago
Reviewers:
eseidel
CC:
abarth-chromium, gregsimon, jackson_old, mojo-reviews_chromium.org, qsr+mojo_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Short-circuit the relayoutSubtreeRoot when the child couldn't change dimensions anyway because the parent constrained it. The relayout subtree root concept is intended to handle the case where a node, when it lays itself out for a second time, changes its opinion about what dimensions it should be. In such a situation, the parent, if it based its own opinion about what size _it_ should be on the child's dimensions, would also need to lay itself out again. Thus, when this scenario is possible, the child remembers the parent, and when it would be told to relayout, we actually start the layout with the parent. In practice, this chains, and we end up with nodes that point to ancestors ten or more steps up the tree such that when the inner most child re-lays-out, the whole app ends up relaying out. This patch tries to short-circuit this for the case where the constraints being applied to the child are such that actually, the child has no choice about its dimensions. In that case, the parent can't change dimensions when the child re-lays-out. This makes a huge difference on the stocks demo app. Without this, on the third rendered frame, there are 72 relayoutSubtreeRoot links, the deepest chain is 8 deep, and 9 of the chains are only 1 level deep. With it, there are 63 relayoutSubtreeRoot links, the deepest chain is only 4 deep, and 38 of the chains are only 1 level deep. R=eseidel@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/963a55323141d0398230f1f405c00d85019c4767

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -9 lines) Patch
M sky/examples/rendering/sector_layout.dart View 3 chunks +4 lines, -2 lines 0 comments Download
M sky/sdk/lib/rendering/box.dart View 2 chunks +3 lines, -3 lines 0 comments Download
M sky/sdk/lib/rendering/object.dart View 3 chunks +9 lines, -4 lines 0 comments Download

Messages

Total messages: 3 (1 generated)
eseidel
lgtm, but I'm not sure I'm the most informed reviewer here.
5 years, 6 months ago (2015-06-22 23:32:34 UTC) #2
Hixie
5 years, 6 months ago (2015-06-22 23:51:29 UTC) #3
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
963a55323141d0398230f1f405c00d85019c4767 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698