[css-grid] Inline size is never indefinite during layout |
|||||
Issue descriptionThis bug is related with the following text on the spec [1]: "If the inline or block size of the grid container is indefinite, <percentage> values relative to that size are treated as auto." The thing is that the inline size is only indefinite while we're computing the intrinsic sizes of the grid container. However during layout the inline size is always definite, so we can resolve the percentage tracks against it. I'm attaching a simple example and the current and expected result. See a more detailed explanation on the CSS WG mailing list: https://lists.w3.org/Archives/Public/www-style/2016Jun/0007.html And also on the Firefox bugzilla discussion: https://bugzilla.mozilla.org/show_bug.cgi?id=1264607 [1] https://drafts.csswg.org/css-grid/#valdef-grid-template-columns-percentage
,
Jun 2 2016
Issue 603854 has been merged into this issue.
,
Jun 2 2016
,
Jun 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0582562513e9373d43f4ed2bbf9d644bb69347f5 commit 0582562513e9373d43f4ed2bbf9d644bb69347f5 Author: rego <rego@igalia.com> Date: Thu Jun 02 16:05:26 2016 [css-grid] Get rid of AvailableSpaceType enum The AvailableSpaceType enum was somehow duplicated with the GridTrackSizing::SizingOperation one. AvailableSpaceType was always AvailableSpaceIndefinite when the SizingOperation is IntrinsicSizeComputation; and the other way around, always AvailableSpaceDefinite when the SizingOperation is TrackSizing. So, we can get rid of AvailableSpaceType enum, and only use SizingOperation to know if we're computing or not the intrinsic sizes of the grid container. No new tests, no change of behavior. BUG= 616716 Review-Url: https://codereview.chromium.org/2030803003 Cr-Commit-Position: refs/heads/master@{#397431} [modify] https://crrev.com/0582562513e9373d43f4ed2bbf9d644bb69347f5/third_party/WebKit/Source/core/layout/LayoutGrid.cpp [modify] https://crrev.com/0582562513e9373d43f4ed2bbf9d644bb69347f5/third_party/WebKit/Source/core/layout/LayoutGrid.h
,
Jun 7 2016
,
Jun 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1 commit 3043a48f261b5d1b4f175a2a8f2373ac22ae41f1 Author: rego <rego@igalia.com> Date: Wed Jun 08 13:33:40 2016 [css-grid] Percentage columns can always be resolved during layout The issue is that the inline size of the grid container is only indefinite while we're computing the intrinsic sizes. During layout we should be able to resolve the percentage tracks against that size. This makes Grid Layout compatible with regular blocks regarding how inline percentages are resolved. The patch passes the SizingOperation enum to LayoutGrid::gridTrackSize(). That way we can know if we're computing the intrinsic sizes or not. It also gets rid of LayoutBox::hasDefiniteLogicalWidth() as it was wrong and not needed actually. Created a new test verifying the expected behavior. Updated the results in a few tests too. BUG= 616716 TEST=fast/css-grid-layout/grid-container-percentage-columns.html Review-Url: https://codereview.chromium.org/2033033002 Cr-Commit-Position: refs/heads/master@{#398535} [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-expected.txt [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-multiple-expected.txt [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-multiple.html [add] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-percentage-columns.html [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/named-grid-line-get-set.html [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-track-breadths-regarding-container-size.html [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/resources/grid-columns-rows-get-set-multiple.js [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/LayoutTests/fast/css-grid-layout/resources/grid-columns-rows-get-set.js [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/Source/core/layout/LayoutBox.h [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp [modify] https://crrev.com/3043a48f261b5d1b4f175a2a8f2373ac22ae41f1/third_party/WebKit/Source/core/layout/LayoutGrid.h
,
Jun 8 2016
,
Jun 9 2016
Tested the issue on Windows 7, Mac 10.11.5, Ubuntu 14.04 using 53.0.2763.0.Observed the percentage.html file output as in expected screenshot. Please find attached screenshot. Marking it as TE-Verified.
,
Jun 9 2016
@ssamanoori you're missing Ahem font: https://www.w3.org/Style/CSS/Test/Fonts/Ahem/ Also I'm not sure if you're enabling the experimental web platform flag in order to properly test grid layout.
,
Jun 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06fdd11aadb98dd6e25c6aaace831998b23eae86 commit 06fdd11aadb98dd6e25c6aaace831998b23eae86 Author: rego <rego@igalia.com> Date: Fri Jun 10 07:23:53 2016 [css-grid] Avoid unneeded calls to percentageLogicalHeightIsResolvable() r398535 was introducing a performance regression on grid layout tests. The problem is that it was calling LayoutGrid::hasDefiniteLogicalSize() which was very slow. r398680 was getting rid of hasDefiniteLogicalSize() and replacing it by calls to LayoutBox::percentageLogicalHeightIsResolvable(). Despite being faster it's still an expensive operation and we don't need to do it always. So in this patch we're avoid calling percentageLogicalHeightIsResolvable() when the min and max track breadths are not percentages. BUG= 618664 , 616716 Review-Url: https://codereview.chromium.org/2051953002 Cr-Commit-Position: refs/heads/master@{#399130} [modify] https://crrev.com/06fdd11aadb98dd6e25c6aaace831998b23eae86/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06fdd11aadb98dd6e25c6aaace831998b23eae86 commit 06fdd11aadb98dd6e25c6aaace831998b23eae86 Author: rego <rego@igalia.com> Date: Fri Jun 10 07:23:53 2016 [css-grid] Avoid unneeded calls to percentageLogicalHeightIsResolvable() r398535 was introducing a performance regression on grid layout tests. The problem is that it was calling LayoutGrid::hasDefiniteLogicalSize() which was very slow. r398680 was getting rid of hasDefiniteLogicalSize() and replacing it by calls to LayoutBox::percentageLogicalHeightIsResolvable(). Despite being faster it's still an expensive operation and we don't need to do it always. So in this patch we're avoid calling percentageLogicalHeightIsResolvable() when the min and max track breadths are not percentages. BUG= 618664 , 616716 Review-Url: https://codereview.chromium.org/2051953002 Cr-Commit-Position: refs/heads/master@{#399130} [modify] https://crrev.com/06fdd11aadb98dd6e25c6aaace831998b23eae86/third_party/WebKit/Source/core/layout/LayoutGrid.cpp |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by r...@igalia.com
, Jun 2 2016