Changeset 159684 in webkit

Timestamp:

Nov 22, 2013 1:25:01 AM (10 years ago)

Author:

svillar@igalia.com

Message:

[CSS Grid Layout] Run the content-sized tracks sizing algorithm only when required
​https://bugs.webkit.org/show_bug.cgi?id=124039

Reviewed by Dean Jackson.

PerformanceTests:

From Blink r156028 and r156168 by <​jchaffraix@chromium.org>.

New performance tests for layouts in grids with fixed size tracks.

• Layout/fixed-grid-lots-of-data.html: Added.

Source/WebCore:

The current code runs the content sized track sizing algorithm all
the time, which forces a layout even when the track is not
content-sized. This change improves the situation by applying two
optimizations. In the first one, we bail out the algorithm if we
detect that we don't need to run it. And by the second one we
reduce the amount of recomputations by only iterating over the
content sized tracks instead of all of them. Both changes follow
the ideas introduced in Blink r156028 and r156168 by
<​jchaffraix@chromium.org>.

As we changed the way we iterate over children (we use the
GridIterator now) the way they're stored in the RenderGrid changes
too. If a item spans through several "cells" inside the grid, we
will have a reference to it on each of them.

These two changes account for a ~3200% improvement on a i7 M620 in
the test that accompanies this change (15.5 vs 520 run/s).

New perf test: PerformanceTests/Layout/fixed-grid-lots-of-data.html

• rendering/RenderGrid.cpp:

(WebCore::RenderGrid::computedUsedBreadthOfGridTracks): Keep track
of content sized tracks and only iterate over them.
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctionsForItems):
Early return if there are no tracks to pass to the algorithm.

• rendering/RenderGrid.h:
• rendering/style/GridLength.h:

(WebCore::GridLength::isContentSized):

• rendering/style/GridTrackSize.h:

(WebCore::GridTrackSize::isContentSized):

LayoutTests:

From Blink r156028 and r156168 by <​jchaffraix@chromium.org>.

Subtle baseline change due to grids triggering less layouts, which
changes the frame rects between 2 subsequent layouts thus changing
the repaint rectangles.

• fast/css-grid-layout/grid-item-change-column-repaint-expected.txt:
• fast/css-grid-layout/grid-item-change-row-repaint-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/grid-item-change-column-repaint-expected.txt (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/grid-item-change-row-repaint-expected.txt (modified) (1 diff)
• PerformanceTests/ChangeLog (modified) (1 diff)
• PerformanceTests/Layout/fixed-grid-lots-of-data.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (5 diffs)
• Source/WebCore/rendering/RenderGrid.h (modified) (1 diff)
• Source/WebCore/rendering/style/GridLength.h (modified) (1 diff)
• Source/WebCore/rendering/style/GridTrackSize.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-11-08  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Run the content-sized tracks sizing algorithm only when required
+        https://bugs.webkit.org/show_bug.cgi?id=124039
+
+        Reviewed by Dean Jackson.
+
+        From Blink r156028 and r156168 by <jchaffraix@chromium.org>.
+
+        Subtle baseline change due to grids triggering less layouts, which
+        changes the frame rects between 2 subsequent layouts thus changing
+        the repaint rectangles.
+
+        * fast/css-grid-layout/grid-item-change-column-repaint-expected.txt:
+        * fast/css-grid-layout/grid-item-change-row-repaint-expected.txt:
+
 2013-11-21  Frédéric Wang  <fred.wang@free.fr>
 

trunk/LayoutTests/fast/css-grid-layout/grid-item-change-column-repaint-expected.txt

@@ -3 +3 @@
   (rect 8 38 100 50)
   (rect 8 38 50 50)
+  (rect 8 38 100 50)
   (rect 208 38 50 50)
 )

trunk/LayoutTests/fast/css-grid-layout/grid-item-change-row-repaint-expected.txt

@@ -3 +3 @@
   (rect 8 38 100 50)
   (rect 8 38 100 100)
+  (rect 8 38 100 50)
   (rect 8 88 100 100)
 )

trunk/PerformanceTests/ChangeLog

@@ +1 @@
+2013-11-08  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Run the content-sized tracks sizing algorithm only when required
+        https://bugs.webkit.org/show_bug.cgi?id=124039
+
+        Reviewed by Dean Jackson.
+
+        From Blink r156028 and r156168 by <jchaffraix@chromium.org>.
+
+        New performance tests for layouts in grids with fixed size tracks.
+
+        * Layout/fixed-grid-lots-of-data.html: Added.
+
 2013-11-19  Manuel Rego Casasnovas  <rego@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-11-08  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Run the content-sized tracks sizing algorithm only when required
+        https://bugs.webkit.org/show_bug.cgi?id=124039
+
+        Reviewed by Dean Jackson.
+
+        The current code runs the content sized track sizing algorithm all
+        the time, which forces a layout even when the track is not
+        content-sized. This change improves the situation by applying two
+        optimizations. In the first one, we bail out the algorithm if we
+        detect that we don't need to run it. And by the second one we
+        reduce the amount of recomputations by only iterating over the
+        content sized tracks instead of all of them. Both changes follow
+        the ideas introduced in Blink r156028 and r156168 by
+        <jchaffraix@chromium.org>.
+
+        As we changed the way we iterate over children (we use the
+        GridIterator now) the way they're stored in the RenderGrid changes
+        too. If a item spans through several "cells" inside the grid, we
+        will have a reference to it on each of them.
+
+        These two changes account for a ~3200% improvement on a i7 M620 in
+        the test that accompanies this change (15.5 vs 520 run/s).
+
+        New perf test: PerformanceTests/Layout/fixed-grid-lots-of-data.html
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::computedUsedBreadthOfGridTracks): Keep track
+        of content sized tracks and only iterate over them.
+        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
+        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctionsForItems):
+        Early return if there are no tracks to pass to the algorithm.
+        * rendering/RenderGrid.h:
+        * rendering/style/GridLength.h:
+        (WebCore::GridLength::isContentSized):
+        * rendering/style/GridTrackSize.h:
+        (WebCore::GridTrackSize::isContentSized):
+
 2013-11-22  Manuel Rego Casasnovas  <rego@igalia.com>
 

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -281 +281 @@
     LayoutUnit availableLogicalSpace = (direction == ForColumns) ? availableLogicalWidth() : availableLogicalHeight(IncludeMarginBorderPadding);
     Vector<GridTrack>& tracks = (direction == ForColumns) ? columnTracks : rowTracks;
+    Vector<size_t> contentSizedTracks;
     for (size_t i = 0; i < tracks.size(); ++i) {
         GridTrack& track = tracks[i];
@@ -291 +292 @@
 
         track.m_maxBreadth = std::max(track.m_maxBreadth, track.m_usedBreadth);
-    }
-
-    // FIXME: We shouldn't call resolveContentBasedTrackSizingFunctions if we have no min-content / max-content tracks.
-    resolveContentBasedTrackSizingFunctions(direction, columnTracks, rowTracks, availableLogicalSpace);
+
+        if (trackSize.isContentSized())
+            contentSizedTracks.append(i);
+    }
+
+    if (!contentSizedTracks.isEmpty())
+        resolveContentBasedTrackSizingFunctions(direction, columnTracks, rowTracks, contentSizedTracks);
+
+    for (size_t i = 0; i < tracks.size(); ++i) {
+        ASSERT(tracks[i].m_maxBreadth != infinity);
+        availableLogicalSpace -= tracks[i].m_usedBreadth;
+    }
 
     if (availableLogicalSpace <= 0)
@@ -478 +487 @@
 }
 
-void RenderGrid::resolveContentBasedTrackSizingFunctions(TrackSizingDirection direction, Vector<GridTrack>& columnTracks, Vector<GridTrack>& rowTracks, LayoutUnit& availableLogicalSpace)
+void RenderGrid::resolveContentBasedTrackSizingFunctions(TrackSizingDirection direction, Vector<GridTrack>& columnTracks, Vector<GridTrack>& rowTracks, Vector<size_t>& contentSizedTracks)
 {
     // FIXME: Split the grid tracks into groups that doesn't overlap a <flex> grid track.
 
-    Vector<GridTrack>& tracks = (direction == ForColumns) ? columnTracks : rowTracks;
-
-    // FIXME: Per step 2 of the specification, we should order the grid items by increasing span.
-    for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
-        resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, child, &GridTrackSize::hasMinOrMaxContentMinTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
-        resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, child, &GridTrackSize::hasMaxContentMinTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
-        resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, child, &GridTrackSize::hasMinOrMaxContentMaxTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
-        resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, child, &GridTrackSize::hasMaxContentMaxTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
-    }
-
-    for (size_t i = 0; i < tracks.size(); ++i) {
-        GridTrack& track = tracks[i];
+    for (size_t i = 0; i < contentSizedTracks.size(); ++i) {
+        GridIterator iterator(m_grid, direction, contentSizedTracks[i]);
+        while (RenderBox* gridItem = iterator.nextGridItem()) {
+            resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, gridItem, &GridTrackSize::hasMinOrMaxContentMinTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
+            resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, gridItem, &GridTrackSize::hasMaxContentMinTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
+            resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, gridItem, &GridTrackSize::hasMinOrMaxContentMaxTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
+            resolveContentBasedTrackSizingFunctionsForItems(direction, columnTracks, rowTracks, gridItem, &GridTrackSize::hasMaxContentMaxTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
+        }
+
+        GridTrack& track = (direction == ForColumns) ? columnTracks[i] : rowTracks[i];
         if (track.m_maxBreadth == infinity)
             track.m_maxBreadth = track.m_usedBreadth;
-
-        availableLogicalSpace -= track.m_usedBreadth;
     }
 }
@@ -516 +521 @@
         tracks.append(&track);
     }
+
+    if (tracks.isEmpty())
+        return;
 
     LayoutUnit additionalBreadthSpace = (this->*sizingFunction)(gridItem, direction, columnTracks);
@@ -593 +601 @@
 void RenderGrid::insertItemIntoGrid(RenderBox* child, const GridCoordinate& coordinate)
 {
-    m_grid[coordinate.rows.initialPositionIndex][coordinate.columns.initialPositionIndex].append(child);
+    for (size_t row = coordinate.rows.initialPositionIndex; row <= coordinate.rows.finalPositionIndex; ++row) {
+        for (size_t column = coordinate.columns.initialPositionIndex; column <= coordinate.columns.finalPositionIndex; ++column)
+            m_grid[row][column].append(child);
+    }
     m_gridItemCoordinate.set(child, coordinate);
 }

trunk/Source/WebCore/rendering/RenderGrid.h

@@ -70 +70 @@
     LayoutUnit computeUsedBreadthOfMaxLength(TrackSizingDirection, const GridLength&, LayoutUnit usedBreadth) const;
     LayoutUnit computeUsedBreadthOfSpecifiedLength(TrackSizingDirection, const Length&) const;
-    void resolveContentBasedTrackSizingFunctions(TrackSizingDirection, Vector<GridTrack>& columnTracks, Vector<GridTrack>& rowTracks, LayoutUnit& availableLogicalSpace);
+    void resolveContentBasedTrackSizingFunctions(TrackSizingDirection, Vector<GridTrack>& columnTracks, Vector<GridTrack>& rowTracks, Vector<size_t>& contentSizedTracks);
 
     void growGrid(TrackSizingDirection);

trunk/Source/WebCore/rendering/style/GridLength.h

@@ -74 +74 @@
     }
 
+    bool isContentSized() const { return m_type == LengthType && (m_length.isAuto() || m_length.isMinContent() || m_length.isMaxContent()); }
+
 private:
     // Ideally we would put the 2 following fields in a union, but Length has a constructor,

trunk/Source/WebCore/rendering/style/GridTrackSize.h

@@ -95 +95 @@
     GridTrackSizeType type() const { return m_type; }
 
+    bool isContentSized() const { return m_minTrackBreadth.isContentSized() || m_maxTrackBreadth.isContentSized(); }
+
     bool operator==(const GridTrackSize& other) const
     {