[css-grid] Grid track sizing algorithm logical height computation unn…

…ecessarily dirties grid items. https://bugs.webkit.org/show_bug.cgi?id=271083 rdar://124713418 Reviewed by Matt Woodrow. In certain situations when trying to compute the logical height for a grid item, the grid track sizing algorithm will update the grid item's overriding containing block size and mark it dirty for layout. This dirtying would occur even it we end up setting the override size to the same value and could result in bad performance with particular types of content. For example, nested grid content would run grid layout multiple times which is currently expensive. In this patch we avoid this extra call to layout by checking to see if the override size is already set to the value we are attempting to set it to. If it is then we will avoid dirtying the renderer. This also ended up exposing an invalidation bug in which we were not properly invalidating the grid items when there was a style change on the grid related to its column or row sizes. This was demonstrated with new failures in css-grid/layout-algorithm/grid-intrinsic-track-sizes-001.html which was performing this behavior. Now when the grid style changes we will check to see if any of the sizes for the columns or the rows are different. If this occurs we should mark the grid items as dirty. This is likely to be more than necessary since we could probably try to identify the exact set of grid items that need to be invalidated, but this approach is the least risky for now. Future patches should attempt to reign this invalidation in a bit more. Without this patch I was getting about 2 runs/s and afterwards I was able to get about ~690 runs/s. * PerformanceTests/Layout/nested-grid-subgrid-free-space-columns.html: Added. * Source/WebCore/Headers.cmake: * Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp: (WebCore::GridTrackSizingAlgorithmStrategy::logicalHeightForChild const): * Source/WebCore/rendering/RenderGrid.cpp: (WebCore::RenderGrid::styleDidChange): * Source/WebCore/rendering/style/RenderStyle.h: * Source/WebCore/rendering/style/RenderStyleInlines.h: (WebCore::RenderStyle::gridTrackSizes const): Canonical link: https://commits.webkit.org/276633@main
WebKit · Mar 25, 2024 · ba849a6 · ba849a6
1 parent 6c80904
commit ba849a6
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 1 deletion.
diff --git a/PerformanceTests/Layout/nested-grid-subgrid-free-space-columns.html b/PerformanceTests/Layout/nested-grid-subgrid-free-space-columns.html
@@ -0,0 +1,62 @@
+<html>
+<head>
+<style>
+.grid {
+    display: grid;
+    grid-template-columns: [start] 1fr 1fr [end];
+    border: 1px solid blue;
+}
+
+.subgrid {
+    display: grid;
+    grid-template-columns: subgrid;
+    grid-column: start / end;
+    padding-inline: 10px;
+    padding-block: 10px;
+    border: 1px solid black;
+}
+</style>
+<script src="../resources/runner.js"></script>
+<script>
+function startTest() {
+    document.body.offsetHeight;
+
+    var index = 0;
+    PerfTestRunner.measureRunsPerSecond({run: function() {
+        document.body.style.width = ++index % 2 ? "99%" : "98%";
+        document.body.offsetHeight;
+    }});
+}
+</script>
+</head>
+<body onload="startTest()">
+    <div class="grid">
+        <div class="subgrid">
+            <div class="grid">
+                <div class="subgrid">
+                    <div class="subgrid">
+                        <div class="grid">
+                            <div class="subgrid">
+                                <div class="subgrid">
+                                    <div class="grid">
+                                        <div class="subgrid">
+                                            <div class="grid">
+                                                <div class="subgrid">
+                                                    <div class="subgrid">
+                                                        <div class="grid">
+                                                        </div>
+                                                    </div>
+                                                </div>
+                                            </div>
+                                        </div>
+                                    </div>
+                                </div>
+                            </div>
+                        </div>
+                    </div>
+                </div>
+            </div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/Source/WebCore/Headers.cmake b/Source/WebCore/Headers.cmake
@@ -2409,6 +2409,7 @@ set(WebCore_PRIVATE_FRAMEWORK_HEADERS
     rendering/style/GridArea.h
     rendering/style/GridLength.h
     rendering/style/GridPosition.h
+    rendering/style/GridPositionsResolver.h
     rendering/style/GridTrackSize.h
     rendering/style/LineClampValue.h
     rendering/style/ListStyleType.h

diff --git a/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp b/Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp
@@ -829,7 +829,7 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::logicalHeightForChild(RenderBox& ch
     GridTrackSizingDirection childBlockDirection = GridLayoutFunctions::flowAwareDirectionForChild(*renderGrid(), child, GridTrackSizingDirection::ForRows);
     // If |child| has a relative logical height, we shouldn't let it override its intrinsic height, which is
     // what we are interested in here. Thus we need to set the block-axis override size to nullopt (no possible resolution).
-    if (shouldClearOverridingContainingBlockContentSizeForChild(child, GridTrackSizingDirection::ForRows)) {
+    if (GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, GridTrackSizingDirection::ForRows) && shouldClearOverridingContainingBlockContentSizeForChild(child, GridTrackSizingDirection::ForRows)) {
         setOverridingContainingBlockContentSizeForChild(*renderGrid(), child, childBlockDirection, std::nullopt);
         child.setNeedsLayout(MarkOnlyThis);
     }

diff --git a/Source/WebCore/rendering/RenderGrid.cpp b/Source/WebCore/rendering/RenderGrid.cpp
@@ -87,6 +87,16 @@ void RenderGrid::styleDidChange(StyleDifference diff, const RenderStyle* oldStyl
         return;
 
     const RenderStyle& newStyle = this->style();
+
+    auto hasDifferentTrackSizes = [&newStyle, &oldStyle](GridTrackSizingDirection direction) {
+        return newStyle.gridTrackSizes(direction) != oldStyle->gridTrackSizes(direction);
+    };
+
+    if (hasDifferentTrackSizes(GridTrackSizingDirection::ForColumns) || hasDifferentTrackSizes(GridTrackSizingDirection::ForRows)) {
+        for (auto& child : childrenOfType<RenderBox>(*this))
+            child.setChildNeedsLayout();
+    }
+
     if (oldStyle->resolvedAlignItems(selfAlignmentNormalBehavior(this)).position() == ItemPosition::Stretch) {
         // Style changes on the grid container implying stretching (to-stretch) or
         // shrinking (from-stretch) require the affected items to be laid out again.

diff --git a/Source/WebCore/rendering/style/RenderStyle.h b/Source/WebCore/rendering/style/RenderStyle.h
@@ -147,6 +147,7 @@ enum class FlexDirection : uint8_t;
 enum class FlexWrap : uint8_t;
 enum class Float : uint8_t;
 enum class FontOrientation : bool;
+enum class GridTrackSizingDirection : uint8_t;
 enum class HangingPunctuation : uint8_t;
 enum class Hyphens : uint8_t;
 enum class ImageRendering : uint8_t;
@@ -775,6 +776,7 @@ class RenderStyle {
 
     inline const Vector<GridTrackSize>& gridColumnTrackSizes() const;
     inline const Vector<GridTrackSize>& gridRowTrackSizes() const;
+    inline const Vector<GridTrackSize>& gridTrackSizes(GridTrackSizingDirection) const;
     inline const GridTrackList& gridColumnList() const;
     inline const GridTrackList& gridRowList() const;
     inline const Vector<GridTrackSize>& gridAutoRepeatColumns() const;

diff --git a/Source/WebCore/rendering/style/RenderStyleInlines.h b/Source/WebCore/rendering/style/RenderStyleInlines.h
@@ -29,6 +29,7 @@
 #include "Element.h"
 #include "FontCascadeDescription.h"
 #include "GraphicsTypes.h"
+#include "GridPositionsResolver.h"
 #include "ImageOrientation.h"
 #include "RenderStyle.h"
 #include "ScrollTypes.h"
@@ -242,6 +243,7 @@ inline bool RenderStyle::gridMasonryColumns() const { return m_nonInheritedData-
 inline bool RenderStyle::gridMasonryRows() const { return m_nonInheritedData->rareData->grid->masonryRows(); }
 inline const GridTrackList& RenderStyle::gridRowList() const { return m_nonInheritedData->rareData->grid->rows(); }
 inline const Vector<GridTrackSize>& RenderStyle::gridRowTrackSizes() const { return m_nonInheritedData->rareData->grid->gridRowTrackSizes(); }
+inline const Vector<GridTrackSize>& RenderStyle::gridTrackSizes(GridTrackSizingDirection direction) const { return direction == GridTrackSizingDirection::ForRows ? m_nonInheritedData->rareData->grid->gridRowTrackSizes() : m_nonInheritedData->rareData->grid->gridColumnTrackSizes(); }
 inline bool RenderStyle::gridSubgridColumns() const { return m_nonInheritedData->rareData->grid->subgridColumns(); }
 inline bool RenderStyle::gridSubgridRows() const { return m_nonInheritedData->rareData->grid->subgridRows(); }
 inline OptionSet<HangingPunctuation> RenderStyle::hangingPunctuation() const { return OptionSet<HangingPunctuation>::fromRaw(m_rareInheritedData->hangingPunctuation); }