Changeset 173620 in webkit for trunk/Source/WebCore/rendering/RenderGrid.cpp

Timestamp:

Sep 15, 2014, 7:20:46 AM (11 years ago)

Author:

svillar@igalia.com

Message:

[CSS Grid Layout] Sort items by span when resolving content-based track sizing functions
​https://bugs.webkit.org/show_bug.cgi?id=135701

Reviewed by Darin Adler.

Source/WebCore:

Section 10.4 of the specs mentions that we should first treat non
spanning items and then incrementally proceed with items with
greater spans when resolving the track breaths in the Track Sizing
Algorithm.

As a nice side effect we're removing the multiple processing of
spanning grid items caused by GridIterator (it returns the same
item as many times as the number of cells it spans). This adds a
~4% performance penalty in auto-grid-lots-of-data.html mainly due
to the use of a hash to remove duplicates.

Test: fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution.html

• rendering/RenderGrid.cpp:

(WebCore::RenderGrid::gridItemSpan):
(WebCore::gridItemWithSpanSorter):
(WebCore::uniquePointerInPair):
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):

• rendering/RenderGrid.h:

LayoutTests:

Tests that check that items are sorted by span to resolve content
based track sizing functions instead of directly using DOM order.

• fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution-expected.txt: Added.
• fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution.html: Added.

File:

• trunk/Source/WebCore/rendering/RenderGrid.cpp (modified) (4 diffs)

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -532 +532 @@
 }
 
+class GridItemWithSpan {
+public:
+    GridItemWithSpan(RenderBox& gridItem, GridCoordinate coordinate, GridTrackSizingDirection direction)
+        : m_gridItem(gridItem)
+        , m_coordinate(coordinate)
+    {
+        const GridSpan& span = (direction == ForRows) ? coordinate.rows : coordinate.columns;
+        m_span = span.resolvedFinalPosition.toInt() - span.resolvedInitialPosition.toInt() + 1;
+    }
+
+    RenderBox& gridItem() const { return m_gridItem; }
+    GridCoordinate coordinate() const { return m_coordinate; }
+
+    bool operator<(const GridItemWithSpan other) const
+    {
+        return m_span < other.m_span;
+    }
+
+private:
+    std::reference_wrapper<RenderBox> m_gridItem;
+    GridCoordinate m_coordinate;
+    size_t m_span;
+};
+
 void RenderGrid::resolveContentBasedTrackSizingFunctions(GridTrackSizingDirection direction, GridSizingData& sizingData)
 {
@@ -538 +562 @@
     for (size_t i = 0; i < sizingData.contentSizedTracksIndex.size(); ++i) {
         GridIterator iterator(m_grid, direction, sizingData.contentSizedTracksIndex[i]);
+        HashSet<RenderBox*> itemsSet;
+        Vector<GridItemWithSpan> itemsSortedByIncreasingSpan;
+
         while (RenderBox* gridItem = iterator.nextGridItem()) {
-            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, *gridItem, &GridTrackSize::hasMinOrMaxContentMinTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
-            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, *gridItem, &GridTrackSize::hasMaxContentMinTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
-            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, *gridItem, &GridTrackSize::hasMinOrMaxContentMaxTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
-            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, *gridItem, &GridTrackSize::hasMaxContentMaxTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
+            if (itemsSet.add(gridItem).isNewEntry)
+                itemsSortedByIncreasingSpan.append(GridItemWithSpan(*gridItem, cachedGridCoordinate(*gridItem), direction));
+        }
+        std::stable_sort(itemsSortedByIncreasingSpan.begin(), itemsSortedByIncreasingSpan.end());
+
+        for (auto& itemWithSpan : itemsSortedByIncreasingSpan) {
+            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMinOrMaxContentMinTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
+            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMaxContentMinTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
+            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMinOrMaxContentMaxTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
+            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMaxContentMaxTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
         }
 
@@ -551 +584 @@
 }
 
-void RenderGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection direction, GridSizingData& sizingData, RenderBox& gridItem, FilterFunction filterFunction, SizingFunction sizingFunction, AccumulatorGetter trackGetter, AccumulatorGrowFunction trackGrowthFunction)
-{
-    const GridCoordinate coordinate = cachedGridCoordinate(gridItem);
+void RenderGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection direction, GridSizingData& sizingData, GridItemWithSpan& gridItemWithSpan, FilterFunction filterFunction, SizingFunction sizingFunction, AccumulatorGetter trackGetter, AccumulatorGrowFunction trackGrowthFunction)
+{
+    const GridCoordinate& coordinate = gridItemWithSpan.coordinate();
     const GridResolvedPosition initialTrackPosition = (direction == ForColumns) ? coordinate.columns.resolvedInitialPosition : coordinate.rows.resolvedInitialPosition;
     const GridResolvedPosition finalTrackPosition = (direction == ForColumns) ? coordinate.columns.resolvedFinalPosition : coordinate.rows.resolvedFinalPosition;
@@ -570 +603 @@
         return;
 
-    LayoutUnit additionalBreadthSpace = (this->*sizingFunction)(gridItem, direction, sizingData.columnTracks);
+    LayoutUnit additionalBreadthSpace = (this->*sizingFunction)(gridItemWithSpan.gridItem(), direction, sizingData.columnTracks);
     for (GridResolvedPosition trackPositionForSpace = initialTrackPosition; trackPositionForSpace <= finalTrackPosition; ++trackPositionForSpace) {
         GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackPositionForSpace.toInt()] : sizingData.rowTracks[trackPositionForSpace.toInt()];