Changeset 286148 in webkit

Timestamp:

Nov 24, 2021, 7:04:31 AM (5 years ago)

Author:

Ziran Sun

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp (modified) (4 diffs)
• Source/WebCore/rendering/GridTrackSizingAlgorithm.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-11-24  Ziran Sun  <zsun@igalia.com>
+
+        [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  <rego@igalia.com>
 

trunk/LayoutTests/TestExpectations

@@ -1397 +1397 @@
 
 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 ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-24  Ziran Sun  <zsun@igalia.com>
+
+        [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  <zalan@apple.com>
 

trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp

@@ -1166 +1166 @@
     ASSERT(m_autoSizedTracksForStretchIndex.isEmpty());
     ASSERT(!m_hasPercentSizedRowsIndefiniteHeight);
+    ASSERT(!m_hasFlexibleMaxTrackBreadth);
 
     Vector<GridTrack>& allTracks = tracks(m_direction);
@@ -1188 +1189 @@
             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;
         }
@@ -1362 +1368 @@
     m_needsSetup = false;
     m_hasPercentSizedRowsIndefiniteHeight = false;
+    m_hasFlexibleMaxTrackBreadth = false;
 
     computeBaselineAlignmentContext();
@@ -1433 +1440 @@
     setAvailableSpace(ForColumns, std::nullopt);
     m_hasPercentSizedRowsIndefiniteHeight = false;
+    m_hasFlexibleMaxTrackBreadth = false;
 }
 

trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h

@@ -144 +144 @@
 
     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
@@ -200 +202 @@
     bool m_needsSetup { true };
     bool m_hasPercentSizedRowsIndefiniteHeight { false };
+    bool m_hasFlexibleMaxTrackBreadth { false };
     std::optional<LayoutUnit> m_availableSpaceRows;
     std::optional<LayoutUnit> m_availableSpaceColumns;

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -162 +162 @@
     // 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));

trunk/Source/WebCore/rendering/RenderGrid.h

@@ -208 +208 @@
     bool m_hasAspectRatioBlockSizeDependentItem { false };
     bool m_baselineItemsCached {false};
+    bool m_hasAnyBaselineAlignmentItem { false };
 };