Changeset 210211 in webkit

Timestamp:

Dec 30, 2016 2:53:58 AM (7 years ago)

Author:

svillar@igalia.com

Message:

[css-grid] Isolate instrinsic size computation from layout
​https://bugs.webkit.org/show_bug.cgi?id=166530

Reviewed by Darin Adler.

Source/WebCore:

This is the last patch of the items placement data refactoring. By using a different Grid
instance in computeIntrinsicLogicalWidths we effectively isolate the intrinsic size
computation from the layout. They are now using different data structures so they don't
interfere each other.

This also means that we no longer reuse the placement of items done in the intrinsic size
computation. That shouldn't be a big issue once we make m_grid persistent on RenderGrid.

Last but not least, this patch finally removes the ugly const_cast's we had in
computeIntrinsicLogicalWidths() as we no longer modify the internal state of RenderGrid.

Tests: fast/css-grid-layout/grid-auto-repeat-intrinsic.html

fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks.html

• rendering/RenderGrid.cpp:

(WebCore::RenderGrid::computeIntrinsicLogicalWidths): Use a new instance of Grid.
(WebCore::RenderGrid::computeEmptyTracksForAutoRepeat): Fixed a missing m_grid -> grid
rename.
(WebCore::RenderGrid::placeItemsOnGrid): Constify it. It nows repositions the items if the
number of auto repeat tracks has changed.

• rendering/RenderGrid.h:

LayoutTests:

• fast/css-grid-layout/grid-auto-repeat-intrinsic-expected.txt: Added.
• fast/css-grid-layout/grid-auto-repeat-intrinsic.html: Added.
• fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks-expected.txt: Added.
• fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/grid-auto-repeat-intrinsic-expected.txt (added)
• LayoutTests/fast/css-grid-layout/grid-auto-repeat-intrinsic.html (added)
• LayoutTests/fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks-expected.txt (added)
• LayoutTests/fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (5 diffs)
• Source/WebCore/rendering/RenderGrid.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-12-28  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-grid] Isolate instrinsic size computation from layout
+        https://bugs.webkit.org/show_bug.cgi?id=166530
+
+        Reviewed by Darin Adler.
+
+        * fast/css-grid-layout/grid-auto-repeat-intrinsic-expected.txt: Added.
+        * fast/css-grid-layout/grid-auto-repeat-intrinsic.html: Added.
+        * fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks-expected.txt: Added.
+        * fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks.html: Added.
+
 2016-12-28  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-12-28  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-grid] Isolate instrinsic size computation from layout
+        https://bugs.webkit.org/show_bug.cgi?id=166530
+
+        Reviewed by Darin Adler.
+
+        This is the last patch of the items placement data refactoring. By using a different Grid
+        instance in computeIntrinsicLogicalWidths we effectively isolate the intrinsic size
+        computation from the layout. They are now using different data structures so they don't
+        interfere each other.
+
+        This also means that we no longer reuse the placement of items done in the intrinsic size
+        computation. That shouldn't be a big issue once we make m_grid persistent on RenderGrid.
+
+        Last but not least, this patch finally removes the ugly const_cast's we had in
+        computeIntrinsicLogicalWidths() as we no longer modify the internal state of RenderGrid.
+
+        Tests: fast/css-grid-layout/grid-auto-repeat-intrinsic.html
+               fast/css-grid-layout/grid-change-intrinsic-size-with-auto-repeat-tracks.html
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::computeIntrinsicLogicalWidths): Use a new instance of Grid.
+        (WebCore::RenderGrid::computeEmptyTracksForAutoRepeat): Fixed a missing m_grid -> grid
+        rename.
+        (WebCore::RenderGrid::placeItemsOnGrid): Constify it. It nows repositions the items if the
+        number of auto repeat tracks has changed.
+        * rendering/RenderGrid.h:
+
 2016-12-30  Michael Catanzaro  <mcatanzaro@igalia.com>
 

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -736 +736 @@
 void RenderGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
 {
-    bool wasPopulated = !m_grid.needsItemsPlacement();
-    if (!wasPopulated)
-        const_cast<RenderGrid*>(this)->placeItemsOnGrid(const_cast<Grid&>(m_grid), IntrinsicSizeComputation);
-
-    GridSizingData sizingData(numTracks(ForColumns, m_grid), numTracks(ForRows, m_grid), const_cast<Grid&>(m_grid));
+    Grid grid(const_cast<RenderGrid&>(*this));
+    placeItemsOnGrid(grid, IntrinsicSizeComputation);
+
+    GridSizingData sizingData(numTracks(ForColumns, grid), numTracks(ForRows, grid), grid);
     sizingData.setAvailableSpace(std::nullopt);
     sizingData.setFreeSpace(ForColumns, std::nullopt);
@@ -753 +752 @@
     minLogicalWidth += scrollbarWidth;
     maxLogicalWidth += scrollbarWidth;
-
-    if (!wasPopulated)
-        const_cast<RenderGrid*>(this)->clearGrid();
 }
 
@@ -1709 +1705 @@
     unsigned lastAutoRepeatTrack = firstAutoRepeatTrack + grid.autoRepeatTracks(direction);
 
-    if (!m_grid.hasGridItems()) {
+    if (!grid.hasGridItems()) {
         emptyTrackIndexes = std::make_unique<OrderedTrackIndexSet>();
         for (unsigned trackIndex = firstAutoRepeatTrack; trackIndex < lastAutoRepeatTrack; ++trackIndex)
@@ -1726 +1722 @@
 }
 
-void RenderGrid::placeItemsOnGrid(Grid& grid, SizingOperation sizingOperation)
-{
-    ASSERT(grid.needsItemsPlacement());
-    ASSERT(!grid.hasGridItems());
-
+void RenderGrid::placeItemsOnGrid(Grid& grid, SizingOperation sizingOperation) const
+{
     unsigned autoRepeatColumns = computeAutoRepeatTracksCount(ForColumns, sizingOperation);
     unsigned autoRepeatRows = computeAutoRepeatTracksCount(ForRows, sizingOperation);
-    grid.setAutoRepeatTracks(autoRepeatRows, autoRepeatColumns);
-
+    if (autoRepeatColumns != grid.autoRepeatTracks(ForColumns) || autoRepeatRows != grid.autoRepeatTracks(ForRows)) {
+        grid.setNeedsItemsPlacement(true);
+        grid.setAutoRepeatTracks(autoRepeatRows, autoRepeatColumns);
+    }
+
+    if (!grid.needsItemsPlacement())
+        return;
+
+    ASSERT(!grid.hasGridItems());
     populateExplicitGridAndOrderIterator(grid);
 
@@ -2794 +2794 @@
     // because not having rows implies that there are no "normal" children (out-of-flow children are
     // not stored in m_grid).
-    ASSERT(!m_grid.needsItemsPlacement());
+    ASSERT(!grid.needsItemsPlacement());
     if (direction == ForRows)
         return grid.numTracks(ForRows);

trunk/Source/WebCore/rendering/RenderGrid.h

@@ -95 +95 @@
     std::unique_ptr<OrderedTrackIndexSet> computeEmptyTracksForAutoRepeat(Grid&, GridTrackSizingDirection) const;
 
-    void placeItemsOnGrid(Grid&, SizingOperation);
+    void placeItemsOnGrid(Grid&, SizingOperation) const;
     void populateExplicitGridAndOrderIterator(Grid&) const;
     std::unique_ptr<GridArea> createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(Grid&, const RenderBox&, GridTrackSizingDirection, const GridSpan&) const;