Title: [286148] trunk
Revision
286148
Author
[email protected]
Date
2021-11-24 07:04:31 -0800 (Wed, 24 Nov 2021)

Log Message

[css-grid] Track sizing algorithm not repeated even if used flex fraction would change
https://bugs.webkit.org/show_bug.cgi?id=232617

Reviewed by Javier Fernandez.

Source/WebCore:

According to https://drafts.csswg.org/css-grid/#algo-flex-tracks, when row height is
indefinite, for each grid item that crosses a flexible track, we run the track sizing
algorithm under a max-content constraint to find the flex fraction. Then we work out
the grid container height as definite, which may cause the flex fraction change. At this
point, we need to repeat the track sizing algorithm for row and layout the grid for real.
The current implementation doesn't repeat the track sizing algorithm for row.

The complication with calling RenderGrid::repeatTracksSizingIfNeeded() for flex max-sizing
is that it might change a grid item's status of participating in Baseline Alignment for
a cyclic sizing dependncy case, which should be definitively excluded. See
https://github.com/w3c/csswg-drafts/issues/3046 for more details. This issue should be handled
in a seperate bug. This CL only handle test cases that don't have baseline alignment specified.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::initializeTrackSizes):
(WebCore::GridTrackSizingAlgorithm::setup):
(WebCore::GridTrackSizingAlgorithm::reset):
* rendering/GridTrackSizingAlgorithm.h:
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::repeatTracksSizingIfNeeded):
(WebCore::RenderGrid::layoutBlock):
* rendering/RenderGrid.h:

LayoutTests:

Unskip two tests that are passing.

* TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (286147 => 286148)


--- trunk/LayoutTests/ChangeLog	2021-11-24 14:14:28 UTC (rev 286147)
+++ trunk/LayoutTests/ChangeLog	2021-11-24 15:04:31 UTC (rev 286148)
@@ -1,3 +1,14 @@
+2021-11-24  Ziran Sun  <[email protected]>
+
+        [css-grid] Track sizing algorithm not repeated even if used flex fraction would change
+        https://bugs.webkit.org/show_bug.cgi?id=232617
+
+        Reviewed by Javier Fernandez.
+
+        Unskip two tests that are passing.
+ 
+        * TestExpectations:
+
 2021-11-24  Manuel Rego Casasnovas  <[email protected]>
 
         [selectors] Import 2 new :focus-visible tests from WPT

Modified: trunk/LayoutTests/TestExpectations (286147 => 286148)


--- trunk/LayoutTests/TestExpectations	2021-11-24 14:14:28 UTC (rev 286147)
+++ trunk/LayoutTests/TestExpectations	2021-11-24 15:04:31 UTC (rev 286148)
@@ -1396,8 +1396,6 @@
 webkit.org/b/231021 imported/w3c/web-platform-tests/css/css-grid/grid-items/replaced-element-015.html [ ImageOnlyFailure ]
 
 imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-areas-overflowing-grid-container-009.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/flex-sizing-rows-indefinite-height.html [ ImageOnlyFailure ]
-webkit.org/b/231021 imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-template-flexible-rerun-track-sizing.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-align-content-001.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-align-content-002.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-align-content-003.html [ ImageOnlyFailure ]

Modified: trunk/Source/WebCore/ChangeLog (286147 => 286148)


--- trunk/Source/WebCore/ChangeLog	2021-11-24 14:14:28 UTC (rev 286147)
+++ trunk/Source/WebCore/ChangeLog	2021-11-24 15:04:31 UTC (rev 286148)
@@ -1,3 +1,33 @@
+2021-11-24  Ziran Sun  <[email protected]>
+
+        [css-grid] Track sizing algorithm not repeated even if used flex fraction would change
+        https://bugs.webkit.org/show_bug.cgi?id=232617
+
+        Reviewed by Javier Fernandez.
+
+        According to https://drafts.csswg.org/css-grid/#algo-flex-tracks, when row height is
+        indefinite, for each grid item that crosses a flexible track, we run the track sizing
+        algorithm under a max-content constraint to find the flex fraction. Then we work out
+        the grid container height as definite, which may cause the flex fraction change. At this
+        point, we need to repeat the track sizing algorithm for row and layout the grid for real.
+        The current implementation doesn't repeat the track sizing algorithm for row.
+
+        The complication with calling RenderGrid::repeatTracksSizingIfNeeded() for flex max-sizing
+        is that it might change a grid item's status of participating in Baseline Alignment for
+        a cyclic sizing dependncy case, which should be definitively excluded. See
+        https://github.com/w3c/csswg-drafts/issues/3046 for more details. This issue should be handled
+        in a seperate bug. This CL only handle test cases that don't have baseline alignment specified. 
+        
+        * rendering/GridTrackSizingAlgorithm.cpp:
+        (WebCore::GridTrackSizingAlgorithm::initializeTrackSizes):
+        (WebCore::GridTrackSizingAlgorithm::setup):
+        (WebCore::GridTrackSizingAlgorithm::reset):
+        * rendering/GridTrackSizingAlgorithm.h:
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::repeatTracksSizingIfNeeded):
+        (WebCore::RenderGrid::layoutBlock):
+        * rendering/RenderGrid.h:
+
 2021-11-24  Alan Bujtas  <[email protected]>
 
         [IFC][Integration] RenderQuote is not a RenderText

Modified: trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp (286147 => 286148)


--- trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp	2021-11-24 14:14:28 UTC (rev 286147)
+++ trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp	2021-11-24 15:04:31 UTC (rev 286148)
@@ -1165,6 +1165,7 @@
     ASSERT(m_flexibleSizedTracksIndex.isEmpty());
     ASSERT(m_autoSizedTracksForStretchIndex.isEmpty());
     ASSERT(!m_hasPercentSizedRowsIndefiniteHeight);
+    ASSERT(!m_hasFlexibleMaxTrackBreadth);
 
     Vector<GridTrack>& allTracks = tracks(m_direction);
     const bool indefiniteHeight = m_direction == ForRows && !m_renderGrid->hasDefiniteLogicalHeight();
@@ -1187,9 +1188,14 @@
         if (trackSize.hasAutoMaxTrackBreadth() && !trackSize.isFitContent())
             m_autoSizedTracksForStretchIndex.append(i);
 
-        if (!m_hasPercentSizedRowsIndefiniteHeight && indefiniteHeight) {
+        if (indefiniteHeight) {
             auto& rawTrackSize = rawGridTrackSize(m_direction, i);
-            if (rawTrackSize.minTrackBreadth().isPercentage() || rawTrackSize.maxTrackBreadth().isPercentage())
+            // Set the flag for repeating the track sizing algorithm. For flexible tracks, as per spec https://drafts.csswg.org/css-grid/#algo-flex-tracks,
+            // in clause "if the free space is an indefinite length:", it states that "If using this flex fraction would cause the grid to be smaller than
+            // the grid container’s min-width/height (or larger than the grid container’s max-width/height), then redo this step".
+            if (!m_hasFlexibleMaxTrackBreadth && rawTrackSize.maxTrackBreadth().isFlex())
+                m_hasFlexibleMaxTrackBreadth = true;
+            if (!m_hasPercentSizedRowsIndefiniteHeight && (rawTrackSize.minTrackBreadth().isPercentage() || rawTrackSize.maxTrackBreadth().isPercentage()))
                 m_hasPercentSizedRowsIndefiniteHeight = true;
         }
     }
@@ -1361,6 +1367,7 @@
 
     m_needsSetup = false;
     m_hasPercentSizedRowsIndefiniteHeight = false;
+    m_hasFlexibleMaxTrackBreadth = false;
 
     computeBaselineAlignmentContext();
 }
@@ -1432,6 +1439,7 @@
     setAvailableSpace(ForRows, std::nullopt);
     setAvailableSpace(ForColumns, std::nullopt);
     m_hasPercentSizedRowsIndefiniteHeight = false;
+    m_hasFlexibleMaxTrackBreadth = false;
 }
 
 #if ASSERT_ENABLED

Modified: trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h (286147 => 286148)


--- trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h	2021-11-24 14:14:28 UTC (rev 286147)
+++ trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h	2021-11-24 15:04:31 UTC (rev 286148)
@@ -143,6 +143,8 @@
     LayoutUnit computeTrackBasedSize() const;
 
     bool hasAnyPercentSizedRowsIndefiniteHeight() const { return m_hasPercentSizedRowsIndefiniteHeight; }
+    bool hasAnyFlexibleMaxTrackBreadth() const { return m_hasFlexibleMaxTrackBreadth; }
+    bool hasAnyBaselineAlignmentItem() const { return !m_columnBaselineItemsMap.isEmpty() || !m_rowBaselineItemsMap.isEmpty(); }
 
 #if ASSERT_ENABLED
     bool tracksAreWiderThanMinTrackBreadth() const;
@@ -199,6 +201,7 @@
     bool wasSetup() const { return !!m_strategy; }
     bool m_needsSetup { true };
     bool m_hasPercentSizedRowsIndefiniteHeight { false };
+    bool m_hasFlexibleMaxTrackBreadth { false };
     std::optional<LayoutUnit> m_availableSpaceRows;
     std::optional<LayoutUnit> m_availableSpaceColumns;
 

Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (286147 => 286148)


--- trunk/Source/WebCore/rendering/RenderGrid.cpp	2021-11-24 14:14:28 UTC (rev 286147)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp	2021-11-24 15:04:31 UTC (rev 286148)
@@ -161,7 +161,14 @@
     // a new cycle of the sizing algorithm; there may be more. In addition, not all the
     // cases with orthogonal flows require this extra cycle; we need a more specific
     // condition to detect whether child's min-content contribution has changed or not.
-    if (m_hasAnyOrthogonalItem || m_trackSizingAlgorithm.hasAnyPercentSizedRowsIndefiniteHeight() || m_hasAspectRatioBlockSizeDependentItem) {
+    // The complication with repeating the track sizing algorithm for flex max-sizing is that
+    // it might change a grid item's status of participating in Baseline Alignment for
+    // a cyclic sizing dependncy case, which should be definitively excluded. See
+    // https://github.com/w3c/csswg-drafts/issues/3046 for details.
+    // FIXME: we are avoiding repeating the track sizing algorithm for grid item with baseline alignment
+    // here in the case of using flex max-sizing functions. We probably also need to investigate whether
+    // it is applicable for the case of percent-sized rows with indefinite height as well.
+    if (m_hasAnyOrthogonalItem || m_trackSizingAlgorithm.hasAnyPercentSizedRowsIndefiniteHeight() || (m_trackSizingAlgorithm.hasAnyFlexibleMaxTrackBreadth() && !m_trackSizingAlgorithm.hasAnyBaselineAlignmentItem()) || m_hasAspectRatioBlockSizeDependentItem) {
         computeTrackSizesForDefiniteSize(ForColumns, availableSpaceForColumns);
         computeContentPositionAndDistributionOffset(ForColumns, m_trackSizingAlgorithm.freeSpace(ForColumns).value(), nonCollapsedTracks(ForColumns));
         computeTrackSizesForDefiniteSize(ForRows, availableSpaceForRows);

Modified: trunk/Source/WebCore/rendering/RenderGrid.h (286147 => 286148)


--- trunk/Source/WebCore/rendering/RenderGrid.h	2021-11-24 14:14:28 UTC (rev 286147)
+++ trunk/Source/WebCore/rendering/RenderGrid.h	2021-11-24 15:04:31 UTC (rev 286148)
@@ -207,6 +207,7 @@
     bool m_hasAnyOrthogonalItem {false};
     bool m_hasAspectRatioBlockSizeDependentItem { false };
     bool m_baselineItemsCached {false};
+    bool m_hasAnyBaselineAlignmentItem { false };
 };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to