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

Issue 2856163002: [css-grid] Avoid the relayout forced on orthogonal grid items (Closed)

Created:
3 years, 7 months ago by jfernandez
Modified:
3 years, 7 months ago
CC:
chromium-reviews, jfernandez, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, Manuel Rego, leviw+renderwatch, jchaffraix+rendering, blink-reviews, eae+blinkwatch
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[css-grid] Avoid the relayout forced on orthogonal grid items Blink performs a pre-layout of any orthogonal box present in the layout tree. This operation helps to figure out the orthogonal box's height used during the container's intrinsic or preferred width computation. However, we decided to remove grid items from the candidates to perform such prelayout operation. Check out https://crrev.com/2698663003 for more details, but basically we have done that because grid items use the Grid Area abstraction as container, which is not defined at the time the prelayout operation is performed. In order to make the Grid's layout logic repeatable, we were clearing the items' overrideSize so it doesn't interfere in the subsequent layout executions (eg, scrolling forces an additional call to the layout algorithm). This logic still applies. However, due to the prelayout operation, we needed to force a layout of the orthogonal item after clearing the override sizes. Since we don't execute the prelayout on orthogonal grid items anymore, we don't need to force a new layout. Review-Url: https://codereview.chromium.org/2856163002 Cr-Commit-Position: refs/heads/master@{#469964} Committed: https://chromium.googlesource.com/chromium/src/+/b55d5e08cb57256f0c80eca93c5beaf9d4c821aa

Patch Set 1 #

Patch Set 2 : Do no clear overrideContainingBlock #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -12 lines) Patch
M third_party/WebKit/Source/core/layout/LayoutGrid.cpp View 1 1 chunk +1 line, -12 lines 0 comments Download

Messages

Total messages: 20 (12 generated)
jfernandez
PTAL
3 years, 7 months ago (2017-05-03 14:50:32 UTC) #4
svillar
This patch unconditionally clears the containing block override sizes forcing all the items to be ...
3 years, 7 months ago (2017-05-03 17:16:14 UTC) #5
cbiesinger
On 2017/05/03 17:16:14, svillar wrote: > This patch unconditionally clears the containing block override sizes ...
3 years, 7 months ago (2017-05-04 20:48:51 UTC) #6
svillar
On 2017/05/04 20:48:51, cbiesinger wrote: > On 2017/05/03 17:16:14, svillar wrote: > > This patch ...
3 years, 7 months ago (2017-05-05 08:12:38 UTC) #7
jfernandez
PTAL
3 years, 7 months ago (2017-05-05 09:32:36 UTC) #8
svillar
lgtm. Please fix the bug description. The 3rd paragraph is not accurate any more.
3 years, 7 months ago (2017-05-08 07:25:24 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2856163002/20001
3 years, 7 months ago (2017-05-08 09:11:34 UTC) #17
commit-bot: I haz the power
3 years, 7 months ago (2017-05-08 11:28:48 UTC) #20
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b55d5e08cb57256f0c80eca93c5b...

Powered by Google App Engine
This is Rietveld 408576698