[css-grid] Stretch alignment doesn't work for orthogonal flows
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Aug 2016 20:11:16 +0000 (20:11 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Aug 2016 20:11:16 +0000 (20:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160076

Reviewed by Darin Adler.

Source/WebCore:

After implementing orthogonal flow support for track sizing and basic
alignment logic, we can now implement stretching logic in orthogonal
scenarios, which was not allowed so far.

Thanks to the recent changes which made the grid layout code more
independent to the grid container's and its children's flow, the
implementation of the stretching logic can be done in a clearer way.

This patch implements the missing logic and performs some refactoring
so it became flow direction independent.

Test: fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows.html

* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):

LayoutTests:

Additional layout tests to verify the stretching logic works as
expected in orthogonal flow scenarios.

* fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows-expected.txt: Added.
* fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204734 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGrid.cpp
Source/WebCore/rendering/RenderGrid.h

index cae80d2..64c849d 100644 (file)
@@ -1,3 +1,16 @@
+2016-08-22  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] Stretch alignment doesn't work for orthogonal flows
+        https://bugs.webkit.org/show_bug.cgi?id=160076
+
+        Reviewed by Darin Adler.
+
+        Additional layout tests to verify the stretching logic works as
+        expected in orthogonal flow scenarios.
+
+        * fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows-expected.txt: Added.
+        * fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows.html: Added.
+
 2016-08-22  Ryosuke Niwa  <rniwa@webkit.org>
 
         Rename CustomElementsRegistry to CustomElementRegistry
diff --git a/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows-expected.txt b/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows-expected.txt
new file mode 100644 (file)
index 0000000..b0eec20
--- /dev/null
@@ -0,0 +1,66 @@
+This test checks that stretching alignment works as expected with orthogonal flows.
+
+HORIZONTAL vs VERTICAL-RL
+
+row1/col1: fixed width and height - row1/col2: fixed width and auto height - row2/col1: auto width and fixed height - row2/col2: auto width and height, but start.
+
+XXX
+XXX
+XXX
+XXX
+PASS
+row1/co1l: bottom auto margin - row1/col2: left auto margin - row2/col1: top auto margin - row2/col2: right auto margin.
+
+XXX
+XXX
+XXX
+XXX
+PASS
+HORIZONTAL vs VERTICAL-LR
+
+row1/col1: fixed width and height - row1/col2: fixed width and auto height - row2/col1: auto width and fixed height - row2/col2: auto width and height, but start.
+
+XXX
+XXX
+XXX
+XXX
+PASS
+row1/co1l: bottom auto margin - row1/col2: left auto margin - row2/col1: top auto margin - row2/col2: right auto margin.
+
+XXX
+XXX
+XXX
+XXX
+PASS
+VERTICAL-RL vs HORIZONTAL
+
+row1/col1: fixed width and height - row1/col2: fixed width and auto height - row2/col1: auto width and fixed height - row2/col2: auto width and height, but start.
+
+XXX
+XXX
+XXX
+XXX
+PASS
+row1/co1l: bottom auto margin - row1/col2: left auto margin - row2/col1: top auto margin - row2/col2: right auto margin.
+
+XXX
+XXX
+XXX
+XXX
+PASS
+VERTICAL-LR vs HORIZONTAL
+
+row1/col1: fixed width and height - row1/col2: fixed width and auto height - row2/col1: auto width and fixed height - row2/col2: auto width and height, but start.
+
+XXX
+XXX
+XXX
+XXX
+PASS
+row1/co1l: bottom auto margin - row1/col2: left auto margin - row2/col1: top auto margin - row2/col2: right auto margin.
+
+XXX
+XXX
+XXX
+XXX
+PASS
diff --git a/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows.html b/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows.html
new file mode 100644 (file)
index 0000000..d69aa7e
--- /dev/null
@@ -0,0 +1,117 @@
+<!DOCTYPE html>
+<link href="resources/grid.css" rel="stylesheet">
+<link href="resources/grid-alignment.css" rel="stylesheet">
+<link href="../css-intrinsic-dimensions/resources/width-keyword-classes.css" rel="stylesheet">
+<script src="../../resources/check-layout.js"></script>
+<style>
+body {
+    margin: 0;
+}
+.container {
+    position: relative;
+}
+.grid {
+    grid-template-columns: 100px 100px;
+    grid-template-rows: 150px 150px;
+    font: 10px/1 ahem;
+}
+.widthAndHeightSet {
+    width: 20px;
+    height: 40px;
+}
+.onlyWidthSet { width: 20px; }
+.onlyHeightSet { height: 40px; }
+.maxHeight { max-height: 160px; }
+.maxWidth { max-width: 90px; }
+.minWidth { min-width: 120px; }
+.minHeight { min-height: 220px; }
+.topAutoMargin { margin-top: auto; }
+.bottomAutoMargin { margin-bottom: auto; }
+.leftAutoMargin { margin-left: auto; }
+.rightAutoMargin { margin-right: auto; }
+</style>
+<body onload="checkLayout('.grid')">
+<div id="log"></div>
+<p>This test checks that stretching alignment works as expected with orthogonal flows.</p>
+
+<p>HORIZONTAL vs VERTICAL-RL</p>
+<p><b>row1/col1:</b> fixed width and height - <b>row1/col2:</b> fixed width and auto height - <b>row2/col1:</b> auto width and fixed height - <b>row2/col2:</b> auto width and height, but start.</p>
+<div class="container">
+    <div class="grid fit-content" data-expected-width="200" data-expected-height="300">
+        <div class="verticalRL firstRowFirstColumn   selfStretch   widthAndHeightSet " data-expected-width="20"  data-expected-height="40">XXX</div>
+        <div class="verticalRL firstRowSecondColumn  sefStretch    onlyWidthSet      " data-expected-width="20"  data-expected-height="150">XXX</div>
+        <div class="verticalRL secondRowFirstColumn  selfStretch    onlyHeightSet     " data-expected-width="100" data-expected-height="40">XXX</div>
+        <div class="verticalRL secondRowSecondColumn selfSelfStart                   " data-expected-width="10"  data-expected-height="30">XXX</div>
+    </div>
+</div>
+<p><b>row1/co1l:</b> bottom auto margin - <b>row1/col2:</b> left auto margin - <b>row2/col1:</b> top auto margin - <b>row2/col2:</b> right auto margin.</p>
+<div class="container">
+    <div class="grid fit-content" data-expected-width="200" data-expected-height="300">
+        <div class="verticalRL firstRowFirstColumn   selfStretch bottomAutoMargin " data-expected-width="100" data-expected-height="30">XXX</div>
+        <div class="verticalRL firstRowSecondColumn  seffStretch leftAutoMargin   " data-expected-width="10"  data-expected-height="150">XXX</div>
+        <div class="verticalRL secondRowFirstColumn  selffStretch topAutoMargin    " data-expected-width="100" data-expected-height="30">XXX</div>
+        <div class="verticalRL secondRowSecondColumn selffStretch rightAutoMargin  " data-expected-width="10"  data-expected-height="150">XXX</div>
+    </div>
+</div>
+
+<p>HORIZONTAL vs VERTICAL-LR</p>
+<p><b>row1/col1:</b> fixed width and height - <b>row1/col2:</b> fixed width and auto height - <b>row2/col1:</b> auto width and fixed height - <b>row2/col2:</b> auto width and height, but start.</p>
+<div class="container">
+    <div class="grid fit-content" data-expected-width="200" data-expected-height="300">
+        <div class="verticalLR firstRowFirstColumn   selfStretch   widthAndHeightSet " data-expected-width="20"  data-expected-height="40">XXX</div>
+        <div class="verticalLR firstRowSecondColumn  sefStretch    onlyWidthSet      " data-expected-width="20"  data-expected-height="150">XXX</div>
+        <div class="verticalLR secondRowFirstColumn  selfStretch    onlyHeightSet     " data-expected-width="100" data-expected-height="40">XXX</div>
+        <div class="verticalLR secondRowSecondColumn selfSelfStart                   " data-expected-width="10"  data-expected-height="30">XXX</div>
+    </div>
+</div>
+<p><b>row1/co1l:</b> bottom auto margin - <b>row1/col2:</b> left auto margin - <b>row2/col1:</b> top auto margin - <b>row2/col2:</b> right auto margin.</p>
+<div class="container">
+    <div class="grid fit-content" data-expected-width="200" data-expected-height="300">
+        <div class="verticalLR firstRowFirstColumn   selfStretch bottomAutoMargin " data-expected-width="100" data-expected-height="30">XXX</div>
+        <div class="verticalLR firstRowSecondColumn  seffStretch leftAutoMargin   " data-expected-width="10"  data-expected-height="150">XXX</div>
+        <div class="verticalLR secondRowFirstColumn  selffStretch topAutoMargin    " data-expected-width="100" data-expected-height="30">XXX</div>
+        <div class="verticalLR secondRowSecondColumn selffStretch rightAutoMargin  " data-expected-width="10"  data-expected-height="150">XXX</div>
+    </div>
+</div>
+
+<p>VERTICAL-RL vs HORIZONTAL</p>
+<p><b>row1/col1:</b> fixed width and height - <b>row1/col2:</b> fixed width and auto height - <b>row2/col1:</b> auto width and fixed height - <b>row2/col2:</b> auto width and height, but start.</p>
+<div class="container">
+    <div class="grid fit-content verticalRL" data-expected-width="300" data-expected-height="200">
+        <div class="horizonalTB firstRowFirstColumn   selfStretch   widthAndHeightSet " data-expected-width="20"  data-expected-height="40">XXX</div>
+        <div class="horizonalTB firstRowSecondColumn  sefStretch    onlyWidthSet      " data-expected-width="20"  data-expected-height="100">XXX</div>
+        <div class="horizonalTB secondRowFirstColumn  selfStretch    onlyHeightSet     " data-expected-width="150" data-expected-height="40">XXX</div>
+        <div class="horizonalTB secondRowSecondColumn selfSelfStart                   " data-expected-width="10"  data-expected-height="30">XXX</div>
+    </div>
+</div>
+<p><b>row1/co1l:</b> bottom auto margin - <b>row1/col2:</b> left auto margin - <b>row2/col1:</b> top auto margin - <b>row2/col2:</b> right auto margin.</p>
+<div class="container">
+    <div class="grid fit-content verticalRL" data-expected-width="300" data-expected-height="200">
+        <div class="horizonalTB firstRowFirstColumn   selfStretch bottomAutoMargin " data-expected-width="150" data-expected-height="30">XXX</div>
+        <div class="horizonalTB firstRowSecondColumn  seffStretch leftAutoMargin   " data-expected-width="10"  data-expected-height="100">XXX</div>
+        <div class="horizonalTB secondRowFirstColumn  selffStretch topAutoMargin    " data-expected-width="150" data-expected-height="30">XXX</div>
+        <div class="horizonalTB secondRowSecondColumn selffStretch rightAutoMargin  " data-expected-width="10"  data-expected-height="100">XXX</div>
+    </div>
+</div>
+
+<p>VERTICAL-LR vs HORIZONTAL</p>
+<p><b>row1/col1:</b> fixed width and height - <b>row1/col2:</b> fixed width and auto height - <b>row2/col1:</b> auto width and fixed height - <b>row2/col2:</b> auto width and height, but start.</p>
+<div class="container">
+    <div class="grid fit-content verticalLR" data-expected-width="300" data-expected-height="200">
+        <div class="horizonalTB firstRowFirstColumn   selfStretch   widthAndHeightSet " data-expected-width="20"  data-expected-height="40">XXX</div>
+        <div class="horizonalTB firstRowSecondColumn  sefStretch    onlyWidthSet      " data-expected-width="20"  data-expected-height="100">XXX</div>
+        <div class="horizonalTB secondRowFirstColumn  selfStretch    onlyHeightSet     " data-expected-width="150" data-expected-height="40">XXX</div>
+        <div class="horizonalTB secondRowSecondColumn selfSelfStart                   " data-expected-width="10"  data-expected-height="30">XXX</div>
+    </div>
+</div>
+<p><b>row1/co1l:</b> bottom auto margin - <b>row1/col2:</b> left auto margin - <b>row2/col1:</b> top auto margin - <b>row2/col2:</b> right auto margin.</p>
+<div class="container">
+    <div class="grid fit-content verticalLR" data-expected-width="300" data-expected-height="200">
+        <div class="horizonalTB firstRowFirstColumn   selfStretch bottomAutoMargin " data-expected-width="150" data-expected-height="30">XXX</div>
+        <div class="horizonalTB firstRowSecondColumn  seffStretch leftAutoMargin   " data-expected-width="10"  data-expected-height="100">XXX</div>
+        <div class="horizonalTB secondRowFirstColumn  selffStretch topAutoMargin    " data-expected-width="150" data-expected-height="30">XXX</div>
+        <div class="horizonalTB secondRowSecondColumn selffStretch rightAutoMargin  " data-expected-width="10"  data-expected-height="100">XXX</div>
+    </div>
+</div>
+
+</body>
index 66353fe..5ef3f2d 100644 (file)
@@ -1,3 +1,27 @@
+2016-08-22  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] Stretch alignment doesn't work for orthogonal flows
+        https://bugs.webkit.org/show_bug.cgi?id=160076
+
+        Reviewed by Darin Adler.
+
+        After implementing orthogonal flow support for track sizing and basic
+        alignment logic, we can now implement stretching logic in orthogonal
+        scenarios, which was not allowed so far.
+
+        Thanks to the recent changes which made the grid layout code more
+        independent to the grid container's and its children's flow, the
+        implementation of the stretching logic can be done in a clearer way.
+
+        This patch implements the missing logic and performs some refactoring
+        so it became flow direction independent.
+
+        Test: fast/css-grid-layout/grid-align-justify-stretch-with-orthogonal-flows.html
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
+
+
 2016-08-19  Brent Fulgham  <bfulgham@apple.com>
 
         Invalid resource load statistics iterator when redirecting
index 5f8a699..40d71c1 100644 (file)
@@ -2166,6 +2166,16 @@ LayoutUnit RenderGrid::availableAlignmentSpaceForChildBeforeStretching(LayoutUni
     return gridAreaBreadthForChild - (child.needsLayout() ? computeMarginLogicalSizeForChild(ForRows, child) : marginLogicalHeightForChild(child));
 }
 
+StyleSelfAlignmentData RenderGrid::alignSelfForChild(const RenderBox& child) const
+{
+    return child.style().resolvedAlignSelf(style(), selfAlignmentNormalBehavior);
+}
+
+StyleSelfAlignmentData RenderGrid::justifySelfForChild(const RenderBox& child) const
+{
+    return child.style().resolvedJustifySelf(style(), selfAlignmentNormalBehavior);
+}
+
 // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
 void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child)
 {
@@ -2175,23 +2185,17 @@ void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child)
     // not, evaluating the conditions which might have changed since the old values were set.
     child.clearOverrideLogicalContentHeight();
 
-    auto& gridStyle = style();
-    auto& childStyle = child.style();
-    bool isHorizontalMode = isHorizontalWritingMode();
-    bool hasAutoSizeInColumnAxis = isHorizontalMode ? childStyle.height().isAuto() : childStyle.width().isAuto();
-    bool allowedToStretchChildAlongColumnAxis = hasAutoSizeInColumnAxis && !childStyle.marginBeforeUsing(&gridStyle).isAuto() && !childStyle.marginAfterUsing(&gridStyle).isAuto();
-    if (allowedToStretchChildAlongColumnAxis && childStyle.resolvedAlignSelf(style(), selfAlignmentNormalBehavior).position() == ItemPositionStretch) {
-        // TODO (lajava): If the child has orthogonal flow, then it already has an override height set, so use it.
-        // TODO (lajava): grid track sizing and positioning do not support orthogonal modes yet.
-        if (child.isHorizontalWritingMode() == isHorizontalMode) {
-            LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(child.overrideContainingBlockContentLogicalHeight().value(), child);
-            LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight, Nullopt);
-            child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight());
-            if (desiredLogicalHeight != child.logicalHeight()) {
-                // TODO (lajava): Can avoid laying out here in some cases. See https://webkit.org/b/87905.
-                child.setLogicalHeight(0);
-                child.setNeedsLayout();
-            }
+    GridTrackSizingDirection childBlockDirection = flowAwareDirectionForChild(child, ForRows);
+    bool blockFlowIsColumnAxis = childBlockDirection == ForRows;
+    bool allowedToStretchChildBlockSize = blockFlowIsColumnAxis ? allowedToStretchChildAlongColumnAxis(child) : allowedToStretchChildAlongRowAxis(child);
+    if (allowedToStretchChildBlockSize) {
+        LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(overrideContainingBlockContentSizeForChild(child, childBlockDirection).value(), child);
+        LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight, LayoutUnit(-1));
+        child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight());
+        if (desiredLogicalHeight != child.logicalHeight()) {
+            // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
+            child.setLogicalHeight(LayoutUnit());
+            child.setNeedsLayout();
         }
     }
 }
index 52effa8..54ac3a9 100644 (file)
@@ -171,7 +171,13 @@ private:
     LayoutUnit marginLogicalHeightForChild(const RenderBox&) const;
     LayoutUnit computeMarginLogicalSizeForChild(GridTrackSizingDirection, const RenderBox&) const;
     LayoutUnit availableAlignmentSpaceForChildBeforeStretching(LayoutUnit gridAreaBreadthForChild, const RenderBox&) const;
+    StyleSelfAlignmentData justifySelfForChild(const RenderBox&) const;
+    StyleSelfAlignmentData alignSelfForChild(const RenderBox&) const;
     void applyStretchAlignmentToChildIfNeeded(RenderBox&);
+    bool hasAutoSizeInColumnAxis(const RenderBox& child) const { return isHorizontalWritingMode() ? child.style().height().isAuto() : child.style().width().isAuto(); }
+    bool hasAutoSizeInRowAxis(const RenderBox& child) const { return isHorizontalWritingMode() ? child.style().width().isAuto() : child.style().height().isAuto(); }
+    bool allowedToStretchChildAlongColumnAxis(const RenderBox& child) const { return alignSelfForChild(child).position() == ItemPositionStretch && hasAutoSizeInColumnAxis(child) && !hasAutoMarginsInColumnAxis(child); }
+    bool allowedToStretchChildAlongRowAxis(const RenderBox& child) const { return justifySelfForChild(child).position() == ItemPositionStretch && hasAutoSizeInRowAxis(child) && !hasAutoMarginsInRowAxis(child); }
     bool hasAutoMarginsInColumnAxis(const RenderBox&) const;
     bool hasAutoMarginsInRowAxis(const RenderBox&) const;
     void resetAutoMarginsAndLogicalTopInColumnAxis(RenderBox& child);