Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 2333583002: [css-grid] Update intrinsic size for the extra sizing alg iteration. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 2 weeks ago by jfernandez
Modified:
4 months ago
CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, svillar, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, Manuel Rego (OOO til July 3rd), leviw+renderwatch, jchaffraix+rendering, blink-reviews, eae+blinkwatch
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[css-grid] Update intrinsic size for the extra sizing alg iteration. The spec states that in case any grid item has changed its 'min content contribution' we must repeat the track sizing algorithm. We were doing so without updating the grid contaner's intrinsic size. Hence, when the grid container was sized under min-content/max-content constraints, we were using an incorrect available space for sizing the column and row tracks. This patchs ensures that any orthogonal flows item is laid out before evaluating its 'min size contribution'. Also, if any item has a new size contribution, we assume the grid container's intrinsic size has changed, so it's recomputed. BUG=628565

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -38 lines) Patch
M third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-spanning-and-orthogonal-flows.html View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html View 3 chunks +10 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-percentages-and-orthogonal-flows.html View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutGrid.h View 2 chunks +3 lines, -2 lines 1 comment Download
M third_party/WebKit/Source/core/layout/LayoutGrid.cpp View 5 chunks +41 lines, -23 lines 4 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 22 (4 generated)
jfernandez
9 months, 2 weeks ago (2016-09-12 07:49:45 UTC) #3
svillar
https://codereview.chromium.org/2333583002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right): https://codereview.chromium.org/2333583002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp#newcode438 third_party/WebKit/Source/core/layout/LayoutGrid.cpp:438: if (!minContentContributionChanged) { I don't get this. For example ...
9 months, 2 weeks ago (2016-09-13 13:44:18 UTC) #5
jfernandez
@cbiesigner, what do you think ? https://codereview.chromium.org/2333583002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right): https://codereview.chromium.org/2333583002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp#newcode438 third_party/WebKit/Source/core/layout/LayoutGrid.cpp:438: if (!minContentContributionChanged) { ...
9 months, 2 weeks ago (2016-09-13 14:57:00 UTC) #6
jfernandez
@mstensho any idea of how to force a relayout of the grid container, so we ...
9 months, 2 weeks ago (2016-09-16 12:28:09 UTC) #8
mstensho
On 2016/09/16 12:28:09, jfernandez wrote: > @mstensho any idea of how to force a relayout ...
9 months, 1 week ago (2016-09-20 07:37:51 UTC) #9
jfernandez
On 2016/09/20 07:37:51, mstensho wrote: > On 2016/09/16 12:28:09, jfernandez wrote: > > @mstensho any ...
9 months, 1 week ago (2016-09-20 08:10:04 UTC) #10
mstensho
On 2016/09/20 08:10:04, jfernandez wrote: > On 2016/09/20 07:37:51, mstensho wrote: > > On 2016/09/16 ...
9 months, 1 week ago (2016-09-20 11:38:41 UTC) #11
jfernandez
On 2016/09/20 11:38:41, mstensho wrote: > On 2016/09/20 08:10:04, jfernandez wrote: > > On 2016/09/20 ...
9 months, 1 week ago (2016-09-20 13:21:56 UTC) #12
cbiesinger
This code may be correct-ish due to https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp?q=preferredLogicalWidthsBecameDirty&sq=package:chromium&l=486 However, I want to advise you against ...
9 months, 1 week ago (2016-09-20 21:07:37 UTC) #13
mstensho
I think I need a (simple) test case. I'm very unfamiliar with grid layout. The ...
9 months, 1 week ago (2016-09-20 21:30:29 UTC) #14
jfernandez
On 2016/09/20 21:07:37, cbiesinger wrote: > This code may be correct-ish due to > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp?q=preferredLogicalWidthsBecameDirty&sq=package:chromium&l=486 ...
9 months, 1 week ago (2016-09-21 10:32:55 UTC) #15
mstensho
On 2016/09/21 10:32:55, jfernandez wrote: > The idea solution, as we internally have discussed, would ...
9 months, 1 week ago (2016-09-21 10:47:49 UTC) #16
jfernandez
On 2016/09/20 21:30:29, mstensho wrote: > I think I need a (simple) test case. I'm ...
9 months, 1 week ago (2016-09-21 11:04:11 UTC) #17
jfernandez
On 2016/09/21 10:47:49, mstensho wrote: > On 2016/09/21 10:32:55, jfernandez wrote: > > The idea ...
9 months, 1 week ago (2016-09-21 11:05:37 UTC) #18
mstensho
On 2016/09/21 11:04:11, jfernandez wrote: > On 2016/09/20 21:30:29, mstensho wrote: > > I think ...
9 months, 1 week ago (2016-09-21 12:13:40 UTC) #19
jfernandez
On 2016/09/21 12:13:40, mstensho wrote: > On 2016/09/21 11:04:11, jfernandez wrote: > > On 2016/09/20 ...
9 months, 1 week ago (2016-09-21 12:30:12 UTC) #20
cbiesinger
Javier and I discussed this a bit in person yesterday. We've already crossed the bridge ...
9 months ago (2016-09-27 10:57:15 UTC) #21
mstensho
4 months ago (2017-02-22 13:12:38 UTC) #22
Please close this if it has been abandoned.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318