Title: [192153] trunk
Revision
192153
Author
svil...@igalia.com
Date
2015-11-09 04:24:38 -0800 (Mon, 09 Nov 2015)

Log Message

[css-grid] Grid placement conflict handling
https://bugs.webkit.org/show_bug.cgi?id=150891

Reviewed by Darin Adler.

Source/WebCore:

If the placement for a grid item contains two lines, and the
start line is further end-ward than the end line, swap the two
lines. If the start line is equal to the end line, remove the
end line.

Test: fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html

* rendering/style/GridResolvedPosition.cpp:
(WebCore::resolveNamedGridLinePositionFromStyle):
(WebCore::resolveGridPositionFromStyle):
(WebCore::GridResolvedPosition::GridResolvedPosition):
(WebCore::GridResolvedPosition::resolveGridPositionsFromStyle):
(WebCore::adjustGridPositionForSide): Deleted.
* rendering/style/GridResolvedPosition.h:
(WebCore::GridResolvedPosition::prev):

LayoutTests:

Updated the expectations for
named-grid-lines-with-named-grid-areas-dynamic-get-set.html
because two of the tests where positioning an item with a
start-line > end-line (it was removing the end line instead of
swapping them).

* fast/css-grid-layout/named-grid-lines-with-named-grid-areas-dynamic-get-set.html:
* fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line-expected.html: Added.
* fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (192152 => 192153)


--- trunk/LayoutTests/ChangeLog	2015-11-09 12:13:06 UTC (rev 192152)
+++ trunk/LayoutTests/ChangeLog	2015-11-09 12:24:38 UTC (rev 192153)
@@ -1,3 +1,20 @@
+2015-11-05  Sergio Villar Senin  <svil...@igalia.com>
+
+        [css-grid] Grid placement conflict handling
+        https://bugs.webkit.org/show_bug.cgi?id=150891
+
+        Reviewed by Darin Adler.
+
+        Updated the expectations for
+        named-grid-lines-with-named-grid-areas-dynamic-get-set.html
+        because two of the tests where positioning an item with a
+        start-line > end-line (it was removing the end line instead of
+        swapping them).
+
+        * fast/css-grid-layout/named-grid-lines-with-named-grid-areas-dynamic-get-set.html:
+        * fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line-expected.html: Added.
+        * fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html: Added.
+
 2015-11-09  Xabier Rodriguez Calvar  <calva...@igalia.com>
 
         Unreviewed.

Modified: trunk/LayoutTests/fast/css-grid-layout/named-grid-lines-with-named-grid-areas-dynamic-get-set.html (192152 => 192153)


--- trunk/LayoutTests/fast/css-grid-layout/named-grid-lines-with-named-grid-areas-dynamic-get-set.html	2015-11-09 12:13:06 UTC (rev 192152)
+++ trunk/LayoutTests/fast/css-grid-layout/named-grid-lines-with-named-grid-areas-dynamic-get-set.html	2015-11-09 12:24:38 UTC (rev 192153)
@@ -130,8 +130,8 @@
     testLayout({ 'areas': templateAreaOne, 'columns': columnRepeatedNames, 'rows': rowRepeatedNames }, itemsRepeat);
     testLayout({ 'columns': columnRepeatedNames, 'areas': templateAreaOne, 'rows': rowRepeatedNames }, itemsRepeat);
 
-    var itemsRepeatTwo = [ { 'column': 'd', 'row': 'c', 'x': '0', 'y': '150', 'width': '350', 'height': '200' },
-			   { 'column': 'd-start / d-end', 'row': 'c-start / c-end', 'x': '0', 'y': '150', 'width': '350', 'height': '200'},
+    var itemsRepeatTwo = [ { 'column': 'd', 'row': 'c', 'x': '0', 'y': '50', 'width': '350', 'height': '100' },
+			   { 'column': 'd-start / d-end', 'row': 'c-start / c-end', 'x': '0', 'y': '50', 'width': '350', 'height': '100'},
 			   { 'column': 'c', 'row': 'd', 'x': '150', 'y': '0', 'width': '200', 'height': '150' } ];
 
     testLayout({ 'areas': templateAreaTwo, 'areas': templateAreaOne, 'columns': columnRepeatedNames, 'rows': rowRepeatedNames }, itemsRepeat);

Added: trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line-expected.html (0 => 192153)


--- trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line-expected.html	2015-11-09 12:24:38 UTC (rev 192153)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+
+<style>
+.grid {
+    -webkit-grid-template-columns: [a] 50px [b] 50px [c] 50px [d];
+    -webkit-grid-auto-rows: 20px;
+    -webkit-grid-auto-columns: 20px;
+}
+</style>
+
+<div class="grid">
+
+    <div style="-webkit-grid-column: 1 / 3; background-color: gray"></div>
+    <div style="-webkit-grid-column: 1 / -2; background-color: purple"></div>
+    <div style="-webkit-grid-column: -2 / 4; background-color: orange"></div>
+
+    <div style="-webkit-grid-column: 2 / d; background-color: gray"></div>
+    <div style="-webkit-grid-column: b / -1; background-color: purple"></div>
+    <div style="-webkit-grid-column: c / d; background-color: orange"></div>
+
+    <div style="-webkit-grid-column: a -1 / 3; background-color: gray"></div>
+    <div style="-webkit-grid-column: b 1 / -1; background-color: purple"></div>
+    <div style="-webkit-grid-column: c -1 / d; background-color: orange"></div>
+
+    <div style="-webkit-grid-column: 2 / 3; background-color: blue"></div>
+    <div style="-webkit-grid-column: 1 / 3; background-color: lightblue"></div>
+    <div style="-webkit-grid-column: 2 / 4; background-color: maroon"></div>
+
+    <div style="-webkit-grid-column: -3 / -3; background-color: lightgreen"></div>
+    <div style="-webkit-grid-column: 3 / 3; background-color: green"></div>
+
+</div>
Property changes on: trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line-expected.html
___________________________________________________________________

Added: svn:mime-type

Added: svn:eol-style

Added: trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html (0 => 192153)


--- trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html	2015-11-09 12:24:38 UTC (rev 192153)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+
+<style>
+.grid {
+    -webkit-grid-template-columns: [a] 50px [b] 50px [c] 50px [d];
+    -webkit-grid-auto-rows: 20px;
+    -webkit-grid-auto-columns: 20px;
+}
+</style>
+
+<div class="grid">
+
+    <div style="-webkit-grid-column: 3 / 1; background-color: gray"></div>
+    <div style="-webkit-grid-column: -2 / 1; background-color: purple"></div>
+    <div style="-webkit-grid-column: 4 / -2; background-color: orange"></div>
+
+    <div style="-webkit-grid-column: d / 2; background-color: gray"></div>
+    <div style="-webkit-grid-column: -1 / b; background-color: purple"></div>
+    <div style="-webkit-grid-column: d / c; background-color: orange"></div>
+
+    <div style="-webkit-grid-column: 3 / a -1; background-color: gray"></div>
+    <div style="-webkit-grid-column: -1 / b 1; background-color: purple"></div>
+    <div style="-webkit-grid-column: d / c -1; background-color: orange"></div>
+
+    <!-- Check that we do not break already valid use cases. -->
+    <div style="-webkit-grid-column: -3 / -2; background-color: blue"></div>
+    <div style="-webkit-grid-column: -4 / -2; background-color: lightblue"></div>
+    <div style="-webkit-grid-column: -3 / -1; background-color: maroon"></div>
+
+    <div style="-webkit-grid-column: 2 / 2; background-color: lightgreen"></div>
+    <div style="-webkit-grid-column: c / c; background-color: green"></div>
+
+</div>
Property changes on: trunk/LayoutTests/fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html
___________________________________________________________________

Added: svn:mime-type

Added: svn:eol-style

Modified: trunk/Source/WebCore/ChangeLog (192152 => 192153)


--- trunk/Source/WebCore/ChangeLog	2015-11-09 12:13:06 UTC (rev 192152)
+++ trunk/Source/WebCore/ChangeLog	2015-11-09 12:24:38 UTC (rev 192153)
@@ -1,3 +1,26 @@
+2015-11-05  Sergio Villar Senin  <svil...@igalia.com>
+
+        [css-grid] Grid placement conflict handling
+        https://bugs.webkit.org/show_bug.cgi?id=150891
+
+        Reviewed by Darin Adler.
+
+        If the placement for a grid item contains two lines, and the
+        start line is further end-ward than the end line, swap the two
+        lines. If the start line is equal to the end line, remove the
+        end line.
+
+        Test: fast/css-grid-layout/swap-lines-if-start-is-further-endward-than-end-line.html
+
+        * rendering/style/GridResolvedPosition.cpp:
+        (WebCore::resolveNamedGridLinePositionFromStyle):
+        (WebCore::resolveGridPositionFromStyle):
+        (WebCore::GridResolvedPosition::GridResolvedPosition):
+        (WebCore::GridResolvedPosition::resolveGridPositionsFromStyle):
+        (WebCore::adjustGridPositionForSide): Deleted.
+        * rendering/style/GridResolvedPosition.h:
+        (WebCore::GridResolvedPosition::prev):
+
 2015-11-08  Gyuyoung Kim  <gyuyoung....@webkit.org>
 
         [EFL] Add UserAgentEFl.cpp|h

Modified: trunk/Source/WebCore/rendering/style/GridResolvedPosition.cpp (192152 => 192153)


--- trunk/Source/WebCore/rendering/style/GridResolvedPosition.cpp	2015-11-09 12:13:06 UTC (rev 192152)
+++ trunk/Source/WebCore/rendering/style/GridResolvedPosition.cpp	2015-11-09 12:24:38 UTC (rev 192153)
@@ -114,15 +114,6 @@
     return resolvedPosition ? GridResolvedPosition(resolvedPosition - 1) : GridResolvedPosition(0);
 }
 
-static GridResolvedPosition adjustGridPositionForSide(unsigned resolvedPosition, GridPositionSide side)
-{
-    // An item finishing on the N-th line belongs to the N-1-th cell.
-    if (side == ColumnEndSide || side == RowEndSide)
-        return adjustGridPositionForRowEndColumnEndSide(resolvedPosition);
-
-    return GridResolvedPosition(resolvedPosition);
-}
-
 static GridResolvedPosition resolveNamedGridLinePositionFromStyle(const RenderStyle& gridContainerStyle, const GridPosition& position, GridPositionSide side)
 {
     ASSERT(!position.namedGridLine().isNull());
@@ -133,7 +124,7 @@
         if (position.isPositive())
             return 0;
         const unsigned lastLine = explicitGridSizeForSide(gridContainerStyle, side);
-        return adjustGridPositionForSide(lastLine, side);
+        return lastLine;
     }
 
     unsigned namedGridLineIndex;
@@ -141,7 +132,7 @@
         namedGridLineIndex = std::min<unsigned>(position.integerPosition(), it->value.size()) - 1;
     else
         namedGridLineIndex = std::max<int>(0, it->value.size() - abs(position.integerPosition()));
-    return adjustGridPositionForSide(it->value[namedGridLineIndex], side);
+    return it->value[namedGridLineIndex];
 }
 
 static inline unsigned firstNamedGridLineBeforePosition(unsigned position, const Vector<unsigned>& gridLines)
@@ -255,7 +246,7 @@
 
         // Handle <integer> explicit position.
         if (position.isPositive())
-            return adjustGridPositionForSide(position.integerPosition() - 1, side);
+            return position.integerPosition() - 1;
 
         unsigned resolvedPosition = abs(position.integerPosition()) - 1;
         const unsigned endOfTrack = explicitGridSizeForSide(gridContainerStyle, side);
@@ -264,7 +255,7 @@
         if (endOfTrack < resolvedPosition)
             return 0;
 
-        return adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
+        return endOfTrack - resolvedPosition;
     }
     case NamedGridAreaPosition:
     {
@@ -277,13 +268,13 @@
         const NamedGridLinesMap& gridLineNames = gridLinesForSide(gridContainerStyle, side);
         auto implicitLine = gridLineNames.find(implicitNamedGridLineForSide(namedGridLine, side));
         if (implicitLine != gridLineNames.end())
-            return adjustGridPositionForSide(implicitLine->value[0], side);
+            return implicitLine->value[0];
 
         // Otherwise, if there is a named line with the specified name, contributes the first such line to the grid
         // item's placement.
         auto explicitLine = gridLineNames.find(namedGridLine);
         if (explicitLine != gridLineNames.end())
-            return adjustGridPositionForSide(explicitLine->value[0], side);
+            return explicitLine->value[0];
 
         // If none of the above works specs mandate us to treat it as auto BUT we should have detected it before calling
         // this function in resolveGridPositionsFromStyle(). We should be covered anyway by the ASSERT at the beginning
@@ -306,7 +297,10 @@
     ASSERT(position.integerPosition());
     unsigned integerPosition = position.integerPosition() - 1;
 
-    m_integerPosition = adjustGridPositionForSide(integerPosition, side).m_integerPosition;
+    if (isStartSide(side) && integerPosition)
+        --integerPosition;
+
+    m_integerPosition = integerPosition;
 }
 
 GridUnresolvedSpan GridResolvedPosition::unresolvedSpanFromStyle(const RenderStyle& gridContainerStyle, const RenderBox& gridItem, GridTrackSizingDirection direction)
@@ -326,10 +320,13 @@
 {
     ASSERT(!unresolvedSpan.requiresAutoPlacement());
 
+    // We must create the GridSpan using finalPosition.prev() because GridSpan stores cell indexes and
+    // an item finishing on the N-th line belongs to the N-1-th cell.
+
     if (unresolvedSpan.initialPosition().shouldBeResolvedAgainstOppositePosition()) {
         // Infer the position from the final position ('auto / 1' or 'span 2 / 3' case).
         auto finalResolvedPosition = resolveGridPositionFromStyle(gridContainerStyle, unresolvedSpan.finalPosition(), unresolvedSpan.finalPositionSide());
-        return resolveGridPositionAgainstOppositePosition(gridContainerStyle, finalResolvedPosition, unresolvedSpan.initialPosition(), unresolvedSpan.initialPositionSide());
+        return resolveGridPositionAgainstOppositePosition(gridContainerStyle, finalResolvedPosition.prev(), unresolvedSpan.initialPosition(), unresolvedSpan.initialPositionSide());
     }
 
     if (unresolvedSpan.finalPosition().shouldBeResolvedAgainstOppositePosition()) {
@@ -341,8 +338,10 @@
     GridResolvedPosition resolvedInitialPosition = resolveGridPositionFromStyle(gridContainerStyle, unresolvedSpan.initialPosition(), unresolvedSpan.initialPositionSide());
     GridResolvedPosition resolvedFinalPosition = resolveGridPositionFromStyle(gridContainerStyle, unresolvedSpan.finalPosition(), unresolvedSpan.finalPositionSide());
 
-    // If 'grid-row-end' specifies a line at or before that specified by 'grid-row-start', it computes to 'span 1'.
-    return GridSpan(resolvedInitialPosition, std::max<GridResolvedPosition>(resolvedInitialPosition, resolvedFinalPosition));
+    if (resolvedInitialPosition > resolvedFinalPosition)
+        std::swap(resolvedInitialPosition, resolvedFinalPosition);
+
+    return GridSpan(resolvedInitialPosition, std::max(resolvedInitialPosition, resolvedFinalPosition.prev()));
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/rendering/style/GridResolvedPosition.h (192152 => 192153)


--- trunk/Source/WebCore/rendering/style/GridResolvedPosition.h	2015-11-09 12:13:06 UTC (rev 192152)
+++ trunk/Source/WebCore/rendering/style/GridResolvedPosition.h	2015-11-09 12:24:38 UTC (rev 192153)
@@ -133,6 +133,11 @@
         return GridResolvedPosition(m_integerPosition + 1);
     }
 
+    GridResolvedPosition prev() const
+    {
+        return m_integerPosition ? m_integerPosition - 1 : 0;
+    }
+
     static GridSpan resolveGridPositionsFromAutoPlacementPosition(const RenderStyle&, const RenderBox&, GridTrackSizingDirection, const GridResolvedPosition&);
     static GridSpan resolveGridPositionsFromStyle(const GridUnresolvedSpan&, const RenderStyle&);
     static GridUnresolvedSpan unresolvedSpanFromStyle(const RenderStyle&, const RenderBox&, GridTrackSizingDirection);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to