Title: [237884] trunk
Revision
237884
Author
jfernan...@igalia.com
Date
2018-11-06 13:45:40 -0800 (Tue, 06 Nov 2018)

Log Message

CSS grid elements with justify-content: space-around have extra whitespace, sometimes a lot
https://bugs.webkit.org/show_bug.cgi?id=191308

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Imported WPT to cover the behavior changes added in this patch.

* resources/import-expectations.json:
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001-expected.txt: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002-expected.txt: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003-expected.txt: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004-expected.txt: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html: Added.
* web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log:

Source/WebCore:

The CSS WG resolved [1] that Content Alignment should account to the
track sizing algorithm.

The sizing algorithm has been modified so that two new steps (1.5
and 2.5) were added to compute the Content Alignment offsets after
resolving the columns' and rows' sizes respectively.

This change decouples the Content Alignment logic from the tracks
position, so that we can use it as part of the track sizing algorithm.

I also had to store the whole ContentAlignmentData structure in two
class attributes. We need both, position and distribution offsets, to
be used in different parts of the layout logic.

[1] https://github.com/w3c/csswg-drafts/issues/2557

Tests: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-cols-filled-shrinkwrap-001.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-cols-spanned-shrinkwrap-001.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-rows-filled-shrinkwrap-001.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-rows-spanned-shrinkwrap-001.html

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::gridAreaBreadthForChild const):
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::repeatTracksSizingIfNeeded):
(WebCore::RenderGrid::layoutBlock):
(WebCore::RenderGrid::gridItemOffset const):
(WebCore::RenderGrid::trackSizesForComputedStyle const):
(WebCore::RenderGrid::populateGridPositionsForDirection):
(WebCore::RenderGrid::gridAreaBreadthForOutOfFlowChild):
(WebCore::contentDistributionOffset):
(WebCore::RenderGrid::computeContentPositionAndDistributionOffset):
(WebCore::RenderGrid::nonCollapsedTracks const):
* rendering/RenderGrid.h:
(WebCore::ContentAlignmentData::isValid):
(WebCore::ContentAlignmentData::defaultOffsets):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (237883 => 237884)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2018-11-06 21:34:27 UTC (rev 237883)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2018-11-06 21:45:40 UTC (rev 237884)
@@ -1,3 +1,23 @@
+2018-11-06  Javier Fernandez  <jfernan...@igalia.com>
+
+        CSS grid elements with justify-content: space-around have extra whitespace, sometimes a lot
+        https://bugs.webkit.org/show_bug.cgi?id=191308
+
+        Reviewed by Dean Jackson.
+
+        Imported WPT to cover the behavior changes added in this patch.
+
+        * resources/import-expectations.json:
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001-expected.txt: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002-expected.txt: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003-expected.txt: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004-expected.txt: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log:
+
 2018-11-06  Youenn Fablet  <you...@apple.com>
 
         Layout Test imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html is flaky

Modified: trunk/LayoutTests/imported/w3c/resources/import-expectations.json (237883 => 237884)


--- trunk/LayoutTests/imported/w3c/resources/import-expectations.json	2018-11-06 21:34:27 UTC (rev 237883)
+++ trunk/LayoutTests/imported/w3c/resources/import-expectations.json	2018-11-06 21:45:40 UTC (rev 237884)
@@ -68,6 +68,7 @@
     "web-platform-tests/css/css-display": "import", 
     "web-platform-tests/css/css-grid": "import", 
     "web-platform-tests/css/css-grid/grid-definition/": "import", 
+    "web-platform-tests/css/css-grid/layout-algorithm/": "import", 
     "web-platform-tests/css/css-logical": "import", 
     "web-platform-tests/css/css-multicol": "import", 
     "web-platform-tests/css/css-properties-values-api": "import", 
@@ -353,4 +354,4 @@
     "web-platform-tests/worklets": "skip", 
     "web-platform-tests/x-frame-options": "skip", 
     "web-platform-tests/xhr": "import"
-}
\ No newline at end of file
+}

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001-expected.txt (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001-expected.txt	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,4 @@
+XXX XX X XX X XXX
+
+PASS .grid 1 
+

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Content Distribution and the track sizing algorithm</title>
+<link rel="author" title="Javier Fernandez Garcia-Boente" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="stylesheet" href=""
+<meta name="flags" content="ahem">
+<meta name="assert" content="Content Distribution on the inline-axis may affect to the row track's size.">
+<style>
+.grid {
+  display: inline-grid;
+  background: grey;
+  grid-template-columns: 50px 50px;
+  font: 20px/1 Ahem;
+  width: 200px;
+}
+
+.item {
+  grid-column: span 2;
+  background: green;
+}
+</style>
+<script src=""
+<script src=""
+<script src=""
+
+<body _onload_="checkLayout('.grid')">
+    <div class="grid justifyContentSpaceBetween" data-expected-width="200" data-expected-height="40">
+        <div class="item" data-expected-width="200" data-expected-height="40">XXX XX X XX X XXX</div>
+    </div>
+</body>

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002-expected.txt (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002-expected.txt	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,4 @@
+XXX XX X XX X XXX
+
+PASS .grid 1 
+

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Content Distribution and the track sizing algorithm</title>
+<link rel="author" title="Javier Fernandez Garcia-Boente" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="stylesheet" href=""
+<meta name="flags" content="ahem">
+<meta name="assert" content="Content Distribution on the block-axis may affect to the column track's size.">
+<style>
+.grid {
+  display: inline-grid;
+  background: grey;
+  grid-template-rows: 50px 50px;
+  font: 20px/1 Ahem;
+  height: 200px;
+}
+
+.item {
+  grid-row: span 2;
+  background: green;
+  writing-mode: vertical-lr;
+}
+</style>
+<script src=""
+<script src=""
+<script src=""
+
+<!--  Heuristic for estimating row-size for orthogonal items should
+also consider Content Alignment, so that grid container width is 40px.
+https://github.com/w3c/csswg-drafts/issues/2697  -->
+<body _onload_="checkLayout('.grid')">
+    <div class="grid justifyContentStart alignContentSpaceBetween" data-expected-width="80" data-expected-height="200">
+        <div class="item" data-expected-width="40" data-expected-height="200">XXX XX X XX X XXX</div>
+    </div>
+</body>

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003-expected.txt (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003-expected.txt	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,5 @@
+XXXX XXX
+XXX
+
+PASS .grid 1 
+

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Content Distribution and the track sizing algorithm</title>
+<link rel="author" title="Javier Fernandez Garcia-Boente" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="stylesheet" href=""
+<meta name="flags" content="ahem">
+<meta name="assert" content="Content Distribution offset doesn't account for tracks with non-spanning items.">
+<style>
+.grid {
+  display: inline-grid;
+  background: grey;
+  grid-template-columns: 100px;
+  font: 20px/1 Ahem;
+  width: 200px;
+}
+.item1 {
+  background: green;
+  grid-column: 1;
+  grid-row: 1;
+}
+.item2 {
+  background: blue;
+  grid-column: 1;
+  grid-row: 2;
+}
+</style>
+<script src=""
+<script src=""
+<script src=""
+
+<body _onload_="checkLayout('.grid')">
+    <div class="grid justifyContentSpaceAround" data-expected-width="200" data-expected-height="60">
+        <div class="item1" data-expected-width="100" data-expected-height="40">XXXX XXX</div>
+        <div class="item2" data-expected-width="100" data-expected-height="20">XXX</div>
+    </div>
+</body>

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004-expected.txt (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004-expected.txt	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,5 @@
+XXXX XXX
+XXX
+
+PASS .grid 1 
+

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html (0 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html	2018-11-06 21:45:40 UTC (rev 237884)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Content Distribution and the track sizing algorithm</title>
+<link rel="author" title="Javier Fernandez Garcia-Boente" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="help" href=""
+<link rel="stylesheet" href=""
+<meta name="flags" content="ahem">
+<meta name="assert" content="Content Distribution offset doesn't account for relative sized tracks with non-spanning items.">
+<style>
+.grid {
+  display: inline-grid;
+  background: grey;
+  grid-template-columns: 50%;
+  font: 20px/1 Ahem;
+}
+
+.item1 {
+  background: green;
+  grid-column: 1;
+  grid-row: 1;
+}
+.item2 {
+  background: blue;
+  grid-column: 2;
+  grid-row: 1;
+}
+</style>
+<script src=""
+<script src=""
+<script src=""
+
+<body _onload_="checkLayout('.grid')">
+    <div class="grid justifyContentSpaceBetween" data-expected-width="220" data-expected-height="40">
+        <div class="item1" data-expected-width="110" data-expected-height="40">XXXX XXX</div>
+        <div class="item2" data-expected-width="60" data-expected-height="40">XXX</div>
+    </div>
+</body>

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log (237883 => 237884)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log	2018-11-06 21:34:27 UTC (rev 237883)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log	2018-11-06 21:45:40 UTC (rev 237884)
@@ -1,7 +1,7 @@
 The tests in this directory were imported from the W3C repository.
 Do NOT modify these tests directly in WebKit.
 Instead, create a pull request on the WPT github:
-	https://github.com/w3c/web-platform-tests
+	https://github.com/web-platform-tests/wpt
 
 Then run the Tools/Scripts/import-w3c-tests in WebKit to reimport
 
@@ -14,6 +14,10 @@
 None
 ------------------------------------------------------------------------
 List of files:
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-intrinsic-size-with-orthogonal-items.html

Modified: trunk/Source/WebCore/ChangeLog (237883 => 237884)


--- trunk/Source/WebCore/ChangeLog	2018-11-06 21:34:27 UTC (rev 237883)
+++ trunk/Source/WebCore/ChangeLog	2018-11-06 21:45:40 UTC (rev 237884)
@@ -1,3 +1,51 @@
+2018-11-06  Javier Fernandez  <jfernan...@igalia.com>
+
+        CSS grid elements with justify-content: space-around have extra whitespace, sometimes a lot
+        https://bugs.webkit.org/show_bug.cgi?id=191308
+
+        Reviewed by Dean Jackson.
+
+        The CSS WG resolved [1] that Content Alignment should account to the
+        track sizing algorithm.
+
+        The sizing algorithm has been modified so that two new steps (1.5
+        and 2.5) were added to compute the Content Alignment offsets after
+        resolving the columns' and rows' sizes respectively.
+
+        This change decouples the Content Alignment logic from the tracks
+        position, so that we can use it as part of the track sizing algorithm.
+
+        I also had to store the whole ContentAlignmentData structure in two
+        class attributes. We need both, position and distribution offsets, to
+        be used in different parts of the layout logic.
+
+        [1] https://github.com/w3c/csswg-drafts/issues/2557
+
+        Tests: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-cols-filled-shrinkwrap-001.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-cols-spanned-shrinkwrap-001.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-rows-filled-shrinkwrap-001.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-rows-spanned-shrinkwrap-001.html
+
+        * rendering/GridTrackSizingAlgorithm.cpp:
+        (WebCore::GridTrackSizingAlgorithm::gridAreaBreadthForChild const):
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::repeatTracksSizingIfNeeded):
+        (WebCore::RenderGrid::layoutBlock):
+        (WebCore::RenderGrid::gridItemOffset const):
+        (WebCore::RenderGrid::trackSizesForComputedStyle const):
+        (WebCore::RenderGrid::populateGridPositionsForDirection):
+        (WebCore::RenderGrid::gridAreaBreadthForOutOfFlowChild):
+        (WebCore::contentDistributionOffset):
+        (WebCore::RenderGrid::computeContentPositionAndDistributionOffset):
+        (WebCore::RenderGrid::nonCollapsedTracks const):
+        * rendering/RenderGrid.h:
+        (WebCore::ContentAlignmentData::isValid):
+        (WebCore::ContentAlignmentData::defaultOffsets):
+
 2018-11-06  Sihui Liu  <sihui_...@apple.com>
 
         IndexedDB: WAL file keeps growing

Modified: trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp (237883 => 237884)


--- trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp	2018-11-06 21:34:27 UTC (rev 237883)
+++ trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp	2018-11-06 21:45:40 UTC (rev 237884)
@@ -555,11 +555,19 @@
 
 LayoutUnit GridTrackSizingAlgorithm::gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection direction) const
 {
+    bool addContentAlignmentOffset =
+        direction == ForColumns && m_sizingState == RowSizingFirstIteration;
     // To determine the column track's size based on an orthogonal grid item we need it's logical
     // height, which may depend on the row track's size. It's possible that the row tracks sizing
     // logic has not been performed yet, so we will need to do an estimation.
-    if (direction == ForRows && m_sizingState == ColumnSizingFirstIteration)
-        return assumedRowsSizeForOrthogonalChild(child);
+    if (direction == ForRows && (m_sizingState == ColumnSizingFirstIteration || m_sizingState == ColumnSizingSecondIteration)) {
+        ASSERT(GridLayoutFunctions::isOrthogonalChild(*m_renderGrid, child));
+        // FIXME (jfernandez) Content Alignment should account for this heuristic.
+        // https://github.com/w3c/csswg-drafts/issues/2697
+        if (m_sizingState == ColumnSizingFirstIteration)
+            return assumedRowsSizeForOrthogonalChild(child);
+        addContentAlignmentOffset = true;
+    }
 
     const Vector<GridTrack>& allTracks = tracks(direction);
     const GridSpan& span = m_grid.gridItemSpan(child, direction);
@@ -567,6 +575,9 @@
     for (auto trackPosition : span)
         gridAreaBreadth += allTracks[trackPosition].baseSize();
 
+    if (addContentAlignmentOffset)
+        gridAreaBreadth += (span.integerSpan() - 1) * m_renderGrid->gridItemOffset(direction);
+
     gridAreaBreadth += m_renderGrid->guttersSize(m_grid, direction, span.startLine(), span.integerSpan(), availableSpace(direction));
 
     return gridAreaBreadth;

Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (237883 => 237884)


--- trunk/Source/WebCore/rendering/RenderGrid.cpp	2018-11-06 21:34:27 UTC (rev 237883)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp	2018-11-06 21:45:40 UTC (rev 237884)
@@ -49,16 +49,6 @@
     ForbidInfinity,
 };
 
-struct ContentAlignmentData {
-    WTF_MAKE_FAST_ALLOCATED;
-public:
-    bool isValid() { return positionOffset >= 0 && distributionOffset >= 0; }
-    static ContentAlignmentData defaultOffsets() { return {-1, -1}; }
-
-    LayoutUnit positionOffset;
-    LayoutUnit distributionOffset;
-};
-
 RenderGrid::RenderGrid(Element& element, RenderStyle&& style)
     : RenderBlock(element, WTFMove(style), 0)
     , m_grid(*this)
@@ -170,7 +160,9 @@
     // condition to detect whether child's min-content contribution has changed or not.
     if (m_grid.hasAnyOrthogonalGridItem() || m_trackSizingAlgorithm.hasAnyPercentSizedRowsIndefiniteHeight()) {
         computeTrackSizesForDefiniteSize(ForColumns, availableSpaceForColumns);
+        computeContentPositionAndDistributionOffset(ForColumns, m_trackSizingAlgorithm.freeSpace(ForColumns).value(), nonCollapsedTracks(ForColumns));
         computeTrackSizesForDefiniteSize(ForRows, availableSpaceForRows);
+        computeContentPositionAndDistributionOffset(ForRows, m_trackSizingAlgorithm.freeSpace(ForRows).value(), nonCollapsedTracks(ForRows));
     }
 }
 
@@ -226,12 +218,20 @@
         LayoutUnit availableSpaceForColumns = availableLogicalWidth();
         placeItemsOnGrid(m_grid, availableSpaceForColumns);
 
-        // At this point the logical width is always definite as the above call to updateLogicalWidth()
-        // properly resolves intrinsic sizes. We cannot do the same for heights though because many code
-        // paths inside updateLogicalHeight() require a previous call to setLogicalHeight() to resolve
-        // heights properly (like for positioned items for example).
+        // 1- First, the track sizing algorithm is used to resolve the sizes of the
+        // grid columns.
+        // At this point the logical width is always definite as the above call to
+        // updateLogicalWidth() properly resolves intrinsic sizes. We cannot do the
+        // same for heights though because many code paths inside
+        // updateLogicalHeight() require a previous call to setLogicalHeight() to
+        // resolve heights properly (like for positioned items for example).
         computeTrackSizesForDefiniteSize(ForColumns, availableSpaceForColumns);
 
+        // 1.5- Compute Content Distribution offsets for column tracks
+        computeContentPositionAndDistributionOffset(ForColumns, m_trackSizingAlgorithm.freeSpace(ForColumns).value(), nonCollapsedTracks(ForColumns));
+
+        // 2- Next, the track sizing algorithm resolves the sizes of the grid rows,
+        // using the grid column sizes calculated in the previous step.
         if (!hasDefiniteLogicalHeight) {
             m_minContentHeight = LayoutUnit();
             m_maxContentHeight = LayoutUnit();
@@ -256,6 +256,9 @@
         if (!hasDefiniteLogicalHeight)
             m_trackSizingAlgorithm.setFreeSpace(ForRows, logicalHeight() - trackBasedLogicalHeight);
 
+        // 2.5- Compute Content Distribution offsets for rows tracks
+        computeContentPositionAndDistributionOffset(ForRows, m_trackSizingAlgorithm.freeSpace(ForRows).value(), nonCollapsedTracks(ForRows));
+
         // 3- If the min-content contribution of any grid items have changed based on the row
         // sizes calculated in step 2, steps 1 and 2 are repeated with the new min-content
         // contribution (once only).
@@ -310,7 +313,7 @@
 
 LayoutUnit RenderGrid::gridItemOffset(GridTrackSizingDirection direction) const
 {
-    return direction == ForRows ? m_offsetBetweenRows : m_offsetBetweenColumns;
+    return direction == ForRows ? m_offsetBetweenRows.distributionOffset : m_offsetBetweenColumns.distributionOffset;
 }
 
 LayoutUnit RenderGrid::guttersSize(const Grid& grid, GridTrackSizingDirection direction, unsigned startLine, unsigned span, std::optional<LayoutUnit> availableSize) const
@@ -805,7 +808,7 @@
     bool isRowAxis = direction == ForColumns;
     auto& positions = isRowAxis ? m_columnPositions : m_rowPositions;
     size_t numPositions = positions.size();
-    LayoutUnit offsetBetweenTracks = isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;
+    LayoutUnit offsetBetweenTracks = isRowAxis ? m_offsetBetweenColumns.distributionOffset : m_offsetBetweenRows.distributionOffset;
 
     Vector<LayoutUnit> tracks;
     if (numPositions < 2)
@@ -962,7 +965,7 @@
     unsigned lastLine = numberOfLines - 1;
     bool hasCollapsedTracks = m_grid.hasAutoRepeatEmptyTracks(direction);
     size_t numberOfCollapsedTracks = hasCollapsedTracks ? m_grid.autoRepeatEmptyTracks(direction)->size() : 0;
-    ContentAlignmentData offset = computeContentPositionAndDistributionOffset(direction, m_trackSizingAlgorithm.freeSpace(direction).value(), numberOfTracks - numberOfCollapsedTracks);
+    const auto& offset = direction == ForColumns ? m_offsetBetweenColumns : m_offsetBetweenRows;
     auto& positions = isRowAxis ? m_columnPositions : m_rowPositions;
     positions.resize(numberOfLines);
     auto borderAndPadding = isRowAxis ? borderAndPaddingLogicalLeft() : borderAndPaddingBefore();
@@ -999,8 +1002,6 @@
             positions[lastLine] += gapAccumulator - offsetAccumulator;
         }
     }
-    auto& offsetBetweenTracks = isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;
-    offsetBetweenTracks = offset.distributionOffset;
 }
 
 static LayoutUnit computeOverflowAlignmentOffset(OverflowAlignment overflow, LayoutUnit trackSize, LayoutUnit childSize)
@@ -1462,7 +1463,7 @@
         if (endLine > 0 && endLine < lastLine) {
             ASSERT(!m_grid.needsItemsPlacement());
             end -= guttersSize(m_grid, direction, endLine - 1, 2, availableSizeForGutters);
-            end -= isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;
+            end -= isRowAxis ? m_offsetBetweenColumns.distributionOffset : m_offsetBetweenRows.distributionOffset;
         }
     }
     return std::max(end - start, LayoutUnit());
@@ -1550,35 +1551,46 @@
     return ContentPosition::Normal;
 }
 
-static ContentAlignmentData contentDistributionOffset(const LayoutUnit& availableFreeSpace, ContentPosition& fallbackPosition, ContentDistribution distribution, unsigned numberOfGridTracks)
+static void contentDistributionOffset(ContentAlignmentData& offset, const LayoutUnit& availableFreeSpace, ContentPosition& fallbackPosition, ContentDistribution distribution, unsigned numberOfGridTracks)
 {
     if (distribution != ContentDistribution::Default && fallbackPosition == ContentPosition::Normal)
         fallbackPosition = resolveContentDistributionFallback(distribution);
 
+    // Initialize to an invalid offset.
+    offset.positionOffset = LayoutUnit(-1);
+    offset.distributionOffset = LayoutUnit(-1);
     if (availableFreeSpace <= 0)
-        return ContentAlignmentData::defaultOffsets();
+        return;
 
+    LayoutUnit positionOffset;
     LayoutUnit distributionOffset;
     switch (distribution) {
     case ContentDistribution::SpaceBetween:
         if (numberOfGridTracks < 2)
-            return ContentAlignmentData::defaultOffsets();
-        return {0, availableFreeSpace / (numberOfGridTracks - 1)};
+            return;
+        distributionOffset = availableFreeSpace / (numberOfGridTracks - 1);
+        positionOffset = LayoutUnit();
+        break;
     case ContentDistribution::SpaceAround:
         if (numberOfGridTracks < 1)
-            return ContentAlignmentData::defaultOffsets();
+            return;
         distributionOffset = availableFreeSpace / numberOfGridTracks;
-        return {distributionOffset / 2, distributionOffset};
+        positionOffset = distributionOffset / 2;
+        break;
     case ContentDistribution::SpaceEvenly:
         distributionOffset = availableFreeSpace / (numberOfGridTracks + 1);
-        return {distributionOffset, distributionOffset};
+        positionOffset = distributionOffset;
+        break;
     case ContentDistribution::Stretch:
     case ContentDistribution::Default:
-        return ContentAlignmentData::defaultOffsets();
+        return;
+    default:
+        ASSERT_NOT_REACHED();
+        return;
     }
 
-    ASSERT_NOT_REACHED();
-    return ContentAlignmentData::defaultOffsets();
+    offset.positionOffset = positionOffset;
+    offset.distributionOffset = distributionOffset;
 }
 
 StyleContentAlignmentData RenderGrid::contentAlignment(GridTrackSizingDirection direction) const
@@ -1586,54 +1598,64 @@
     return direction == ForColumns ? style().resolvedJustifyContent(contentAlignmentNormalBehaviorGrid()) : style().resolvedAlignContent(contentAlignmentNormalBehaviorGrid());
 }
 
-ContentAlignmentData RenderGrid::computeContentPositionAndDistributionOffset(GridTrackSizingDirection direction, const LayoutUnit& availableFreeSpace, unsigned numberOfGridTracks) const
+void RenderGrid::computeContentPositionAndDistributionOffset(GridTrackSizingDirection direction, const LayoutUnit& availableFreeSpace, unsigned numberOfGridTracks)
 {
     bool isRowAxis = direction == ForColumns;
+    auto& offset =
+        isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;
     auto contentAlignmentData = contentAlignment(direction);
     auto position = contentAlignmentData.position();
     // If <content-distribution> value can't be applied, 'position' will become the associated
     // <content-position> fallback value.
-    auto contentAlignment = contentDistributionOffset(availableFreeSpace, position, contentAlignmentData.distribution(), numberOfGridTracks);
-    if (contentAlignment.isValid())
-        return contentAlignment;
+    contentDistributionOffset(offset, availableFreeSpace, position, contentAlignmentData.distribution(), numberOfGridTracks);
+    if (offset.isValid())
+        return;
 
-    if (availableFreeSpace <= 0 && contentAlignmentData.overflow() == OverflowAlignment::Safe)
-        return {0, 0};
+    if (availableFreeSpace <= 0 && contentAlignmentData.overflow() == OverflowAlignment::Safe) {
+        offset.positionOffset = LayoutUnit();
+        offset.distributionOffset = LayoutUnit();
+        return;
+    }
 
+    LayoutUnit positionOffset;
     switch (position) {
     case ContentPosition::Left:
-        // The align-content's axis is always orthogonal to the inline-axis.
-        return {0, 0};
+        ASSERT(isRowAxis);
+        break;
     case ContentPosition::Right:
-        if (isRowAxis)
-            return {availableFreeSpace, 0};
-        // The align-content's axis is always orthogonal to the inline-axis.
-        return {0, 0};
+        ASSERT(isRowAxis);
+        positionOffset = availableFreeSpace;
+        break;
     case ContentPosition::Center:
-        return {availableFreeSpace / 2, 0};
+        positionOffset = availableFreeSpace / 2;
+        break;
     case ContentPosition::FlexEnd: // Only used in flex layout, for other layout, it's equivalent to 'end'.
     case ContentPosition::End:
         if (isRowAxis)
-            return {style().isLeftToRightDirection() ? availableFreeSpace : LayoutUnit(), LayoutUnit()};
-        return {availableFreeSpace, 0};
+            positionOffset = style().isLeftToRightDirection() ? availableFreeSpace : LayoutUnit();
+        else
+            positionOffset = availableFreeSpace;
+        break;
     case ContentPosition::FlexStart: // Only used in flex layout, for other layout, it's equivalent to 'start'.
     case ContentPosition::Start:
         if (isRowAxis)
-            return {style().isLeftToRightDirection() ? LayoutUnit() : availableFreeSpace, LayoutUnit()};
-        return {0, 0};
+            positionOffset = style().isLeftToRightDirection() ? LayoutUnit() : availableFreeSpace;
+        break;
     case ContentPosition::Baseline:
     case ContentPosition::LastBaseline:
         // FIXME: Implement the previous values. For now, we always 'start' align.
         // http://webkit.org/b/145566
         if (isRowAxis)
-            return {style().isLeftToRightDirection() ? LayoutUnit() : availableFreeSpace, LayoutUnit()};
-        return {0, 0};
+            positionOffset = style().isLeftToRightDirection() ? LayoutUnit() : availableFreeSpace;
+        break;
     case ContentPosition::Normal:
-        break;
+    default:
+        ASSERT_NOT_REACHED();
+        return;
     }
 
-    ASSERT_NOT_REACHED();
-    return {0, 0};
+    offset.positionOffset = positionOffset;
+    offset.distributionOffset = LayoutUnit();
 }
 
 LayoutUnit RenderGrid::translateOutOfFlowRTLCoordinate(const RenderBox& child, LayoutUnit coordinate) const
@@ -1675,6 +1697,15 @@
     return isOrthogonalChild ? childLocation.transposedPoint() : childLocation;
 }
 
+unsigned RenderGrid::nonCollapsedTracks(GridTrackSizingDirection direction) const
+{
+    auto& tracks = m_trackSizingAlgorithm.tracks(direction);
+    size_t numberOfTracks = tracks.size();
+    bool hasCollapsedTracks = m_grid.hasAutoRepeatEmptyTracks(direction);
+    size_t numberOfCollapsedTracks = hasCollapsedTracks ? m_grid.autoRepeatEmptyTracks(direction)->size() : 0;
+    return numberOfTracks - numberOfCollapsedTracks;
+}
+
 unsigned RenderGrid::numTracks(GridTrackSizingDirection direction, const Grid& grid) const
 {
     // Due to limitations in our internal representation, we cannot know the number of columns from

Modified: trunk/Source/WebCore/rendering/RenderGrid.h (237883 => 237884)


--- trunk/Source/WebCore/rendering/RenderGrid.h	2018-11-06 21:34:27 UTC (rev 237883)
+++ trunk/Source/WebCore/rendering/RenderGrid.h	2018-11-06 21:45:40 UTC (rev 237884)
@@ -35,8 +35,16 @@
 class GridArea;
 class GridSpan;
 
-struct ContentAlignmentData;
+struct ContentAlignmentData {
+    WTF_MAKE_NONCOPYABLE(ContentAlignmentData); WTF_MAKE_FAST_ALLOCATED;
+public:
+    ContentAlignmentData() = default;
+    bool isValid() const { return positionOffset >= 0 && distributionOffset >= 0; }
 
+    LayoutUnit positionOffset;
+    LayoutUnit distributionOffset;
+};
+
 enum GridAxisPosition {GridAxisStart, GridAxisEnd, GridAxisCenter};
 enum GridAxis { GridRowAxis, GridColumnAxis };
 
@@ -64,6 +72,7 @@
 
     // Required by GridTrackSizingAlgorithm. Keep them under control.
     LayoutUnit guttersSize(const Grid&, GridTrackSizingDirection, unsigned startLine, unsigned span, std::optional<LayoutUnit> availableSize) const;
+    LayoutUnit gridItemOffset(GridTrackSizingDirection) const;
 
     StyleContentAlignmentData contentAlignment(GridTrackSizingDirection) const;
 
@@ -133,7 +142,7 @@
     GridAxisPosition rowAxisPositionForChild(const RenderBox&) const;
     LayoutUnit columnAxisOffsetForChild(const RenderBox&) const;
     LayoutUnit rowAxisOffsetForChild(const RenderBox&) const;
-    ContentAlignmentData computeContentPositionAndDistributionOffset(GridTrackSizingDirection, const LayoutUnit& availableFreeSpace, unsigned numberOfGridTracks) const;
+    void computeContentPositionAndDistributionOffset(GridTrackSizingDirection, const LayoutUnit& availableFreeSpace, unsigned numberOfGridTracks);
     LayoutPoint findChildLogicalPosition(const RenderBox&) const;
     GridArea cachedGridArea(const RenderBox&) const;
     GridSpan cachedGridSpan(const RenderBox&, GridTrackSizingDirection) const;
@@ -162,8 +171,8 @@
 
     LayoutUnit gridGap(GridTrackSizingDirection) const;
     LayoutUnit gridGap(GridTrackSizingDirection, std::optional<LayoutUnit> availableSize) const;
-    LayoutUnit gridItemOffset(GridTrackSizingDirection) const;
 
+    unsigned nonCollapsedTracks(GridTrackSizingDirection) const;
     unsigned numTracks(GridTrackSizingDirection, const Grid&) const;
 
     LayoutUnit translateOutOfFlowRTLCoordinate(const RenderBox&, LayoutUnit) const;
@@ -175,8 +184,8 @@
 
     Vector<LayoutUnit> m_columnPositions;
     Vector<LayoutUnit> m_rowPositions;
-    LayoutUnit m_offsetBetweenColumns;
-    LayoutUnit m_offsetBetweenRows;
+    ContentAlignmentData m_offsetBetweenColumns;
+    ContentAlignmentData m_offsetBetweenRows;
 
     typedef HashMap<const RenderBox*, std::optional<size_t>> OutOfFlowPositionsMap;
     OutOfFlowPositionsMap m_outOfFlowItemColumn;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to