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

Timestamp:

Aug 28, 2013, 3:26:10 AM (12 years ago)

Author:

sergio@webkit.org

Message:

[CSS Grid Layout] Fix grid position resolution
​https://bugs.webkit.org/show_bug.cgi?id=119801

Reviewed by Andreas Kling.

From Blink r148833, r148878, r150403 by <​jchaffraix@chromium.org>

Source/WebCore:

Both grid-{column|row}-end and negative positions were not
properly handled in our grid position resolution code. We were
using the same code to resolve all the grid positions without
considering the edges of the grid.

Also refactored the grid size estimation in
resolveGridPositionsFromStyle() so we can use it for the grid size
estimation. The code no longer requires the grid to be filled at
that moment as the specs changed to use the "explicit grid" which
is independent of grid items (only depends on style).

Test: fast/css-grid-layout/grid-item-negative-position-resolution.html

• rendering/RenderGrid.cpp:

(WebCore::RenderGrid::maximumIndexInDirection):
(WebCore::RenderGrid::resolveGridPositionsFromStyle):
(WebCore::adjustGridPositionForSide):
(WebCore::RenderGrid::resolveGridPositionFromStyle):

• rendering/RenderGrid.h:

LayoutTests:

Added a new test to check negative position resolution. Also added
several new test cases to check that we properly resolve grid
positions in the grid edges.

• fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt: Added.
• fast/css-grid-layout/grid-item-negative-position-resolution.html: Added.
• fast/css-grid-layout/grid-item-spanning-resolution-expected.txt:
• fast/css-grid-layout/grid-item-spanning-resolution.html:

File:

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

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -328 +328 @@
 }
 
-static size_t estimatedGridSizeForPosition(const GridPosition& position)
-{
-    if (position.isAuto())
-        return 1;
-
-    // Negative explicit values never grow the grid as they are clamped against
-    // the explicit grid's size. Thus we don't special case them here.
-    return std::max(position.integerPosition(), 1);
-}
-
 size_t RenderGrid::explicitGridColumnCount() const
 {
@@ -353 +343 @@
 
     for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
-        // This function bypasses the cache (cachedGridCoordinate()) as it is used to build it.
-        // Also we can't call resolveGridPositionsFromStyle here as it assumes that the grid is build and we are in
-        // the middle of building it. However we should be able to share more code with the previous logic (FIXME).
-        const GridPosition& initialPosition = (direction == ForColumns) ? child->style()->gridItemColumnStart() : child->style()->gridItemRowStart();
-        const GridPosition& finalPosition = (direction == ForColumns) ? child->style()->gridItemColumnEnd() : child->style()->gridItemRowEnd();
-
-        size_t estimatedSizeForInitialPosition = estimatedGridSizeForPosition(initialPosition);
-        size_t estimatedSizeForFinalPosition = estimatedGridSizeForPosition(finalPosition);
-        ASSERT(estimatedSizeForInitialPosition);
-        ASSERT(estimatedSizeForFinalPosition);
-
-        maximumIndex = std::max(maximumIndex, estimatedSizeForInitialPosition);
-        maximumIndex = std::max(maximumIndex, estimatedSizeForFinalPosition);
+        OwnPtr<GridSpan> positions = resolveGridPositionsFromStyle(child, direction);
+
+        // |positions| is null if we need to run the auto-placement algorithm. Our estimation ignores
+        // this case as the auto-placement algorithm will grow the grid as needed.
+        if (!positions)
+            continue;
+
+        maximumIndex = std::max(maximumIndex, positions->finalPositionIndex + 1);
     }
 
@@ -731 +716 @@
 PassOwnPtr<RenderGrid::GridSpan> RenderGrid::resolveGridPositionsFromStyle(const RenderBox* gridItem, TrackSizingDirection direction) const
 {
-    ASSERT(gridWasPopulated());
-
     const GridPosition& initialPosition = (direction == ForColumns) ? gridItem->style()->gridItemColumnStart() : gridItem->style()->gridItemRowStart();
     const GridPositionSide initialPositionSide = (direction == ForColumns) ? ColumnStartSide : RowStartSide;
@@ -768 +751 @@
 }
 
+static size_t adjustGridPositionForSide(size_t resolvedPosition, RenderGrid::GridPositionSide side)
+{
+    // An item finishing on the N-th line belongs to the N-1-th cell.
+    if (side == RenderGrid::ColumnEndSide || side == RenderGrid::RowEndSide)
+        return resolvedPosition ? resolvedPosition - 1 : 0;
+
+    return resolvedPosition;
+}
+
 size_t RenderGrid::resolveGridPositionFromStyle(const GridPosition& position, GridPositionSide side) const
 {
-    ASSERT(gridWasPopulated());
-
     // FIXME: Handle other values for grid-{row,column} like ranges or line names.
     switch (position.type()) {
     case IntegerPosition: {
+        ASSERT(position.integerPosition());
         if (position.isPositive())
-            return position.integerPosition() - 1;
-
-        size_t resolvedPosition = abs(position.integerPosition());
-        // FIXME: This returns one less than the expected result for side == ColumnStartSide or RowStartSide as we don't properly convert
-        // the grid line to its grid track. However this avoids the issue of growing the grid when inserting the item (e.g. -1 / auto).
+            return adjustGridPositionForSide(position.integerPosition() - 1, side);
+
+        size_t resolvedPosition = abs(position.integerPosition()) - 1;
         const size_t endOfTrack = (side == ColumnStartSide || side == ColumnEndSide) ? explicitGridColumnCount() : explicitGridRowCount();
 
@@ -787 +776 @@
             return 0;
 
-        return endOfTrack - resolvedPosition;
+        return adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
     }
     case AutoPosition: