[CSS Grid Layout] overflow-position keyword for align and justify properties.

Source/core/rendering/RenderGrid.cpp

Index: Source/core/rendering/RenderGrid.cpp
diff --git a/Source/core/rendering/RenderGrid.cpp b/Source/core/rendering/RenderGrid.cpp
index 40f2d1e2ac51956f535803cef84a2da8f2d36154..9c3c87519724a5b708132d068cb4614962b7f8b2 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -1135,7 +1135,7 @@ LayoutUnit RenderGrid::startOfColumnForChild(const RenderBox& child) const
     return startOfColumn + marginStartForChild(&child);
 }
 
-LayoutUnit RenderGrid::endOfColumnForChild(const RenderBox& child) const
+LayoutUnit RenderGrid::endOfColumnForChild(const RenderBox& child, OverflowAlignment overflow) const
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     LayoutUnit startOfColumn = m_columnPositions[coordinate.columns.resolvedInitialPosition.toInt()];
@@ -1143,105 +1143,123 @@ LayoutUnit RenderGrid::endOfColumnForChild(const RenderBox& child) const
     LayoutUnit columnPosition = startOfColumn + marginStartForChild(&child);
 
     LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.resolvedFinalPosition.next().toInt()];
-    // FIXME: This should account for the grid item's <overflow-position>.
-    return columnPosition + std::max<LayoutUnit>(0, endOfColumn - m_columnPositions[coordinate.columns.resolvedInitialPosition.toInt()] - child.logicalWidth());
+    LayoutUnit childLogicalWidth = child.logicalWidth();
+    LayoutUnit columnWidth = endOfColumn - startOfColumn;
+    if ((childLogicalWidth > columnWidth) && (overflow == OverflowAlignmentTrue || overflow == OverflowAlignmentDefault))
+        return columnPosition + columnWidth - childLogicalWidth;
+    return columnPosition + std::max<LayoutUnit>(0, columnWidth - childLogicalWidth);
 }
 
-LayoutUnit RenderGrid::columnPositionAlignedWithGridContainerStart(const RenderBox& child) const
+LayoutUnit RenderGrid::columnPositionAlignedWithGridContainerStart(const RenderBox& child, OverflowAlignment overflow) const
 {
     if (style()->isLeftToRightDirection())
         return startOfColumnForChild(child);
 
-    return endOfColumnForChild(child);
+    return endOfColumnForChild(child, overflow);
 }
 
-LayoutUnit RenderGrid::columnPositionAlignedWithGridContainerEnd(const RenderBox& child) const
+LayoutUnit RenderGrid::columnPositionAlignedWithGridContainerEnd(const RenderBox& child, OverflowAlignment overflow) const
 {
     if (!style()->isLeftToRightDirection())
         return startOfColumnForChild(child);
 
-    return endOfColumnForChild(child);
+    return endOfColumnForChild(child, overflow);
 }
 
-LayoutUnit RenderGrid::centeredColumnPositionForChild(const RenderBox& child) const
+LayoutUnit RenderGrid::centeredColumnPositionForChild(const RenderBox& child, OverflowAlignment overflow) const
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     LayoutUnit startOfColumn = m_columnPositions[coordinate.columns.resolvedInitialPosition.toInt()];
     LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.resolvedFinalPosition.next().toInt()];
     LayoutUnit columnPosition = startOfColumn + marginStartForChild(&child);
-    // FIXME: This should account for the grid item's <overflow-position>.
-    return columnPosition + std::max<LayoutUnit>(0, endOfColumn - startOfColumn - child.logicalWidth()) / 2;
+    LayoutUnit childLogicalWidth = child.logicalWidth();
+    LayoutUnit columnWidth = endOfColumn - startOfColumn;
+    if ((childLogicalWidth > columnWidth) && (overflow == OverflowAlignmentTrue || overflow == OverflowAlignmentDefault))
+        return columnPosition + (columnWidth - childLogicalWidth) / 2;
+    return columnPosition + std::max<LayoutUnit>(0, columnWidth - childLogicalWidth) / 2;
 }
 
-static ItemPosition resolveJustification(const RenderStyle* parentStyle, const RenderStyle* childStyle)
+static void resolveJustification(const RenderStyle* parentStyle, const RenderStyle* childStyle, ItemPosition& justify, OverflowAlignment &overflow)

[Julien - ping for review, 2014/10/20 16:59:11]

Style: The & should be next to the enum => OverflowAlignment& overflow

[jfernandez, 2014/10/22 21:38:10]

Done.

 {
-    ItemPosition justify = childStyle->justifySelf();
-    if (justify == ItemPositionAuto)
-        justify = (parentStyle->justifyItems() == ItemPositionAuto) ? ItemPositionStretch : parentStyle->justifyItems();
-
-    return justify;
+    justify = childStyle->justifySelf();
+    overflow = childStyle->justifySelfOverflowAlignment();
+    // The auto keyword computes to the parent's justify-items computed value, or to "stretch", if not set or "auto".
+    if (justify == ItemPositionAuto) {
+        if (parentStyle->justifyItems() == ItemPositionAuto) {
+            justify = ItemPositionStretch;
+        } else {
+            justify = parentStyle->justifyItems();
+            overflow = parentStyle->justifyItemsOverflowAlignment();
+        }
+    }
+    // The default overflow alignment is 'true' for Grid Items.
+    if (overflow == OverflowAlignmentDefault)

[Julien - ping for review, 2014/10/20 16:59:10]

It's super weird to have to do this conversion here. It's a cleaner design to
have StyleAdjuster take care of it.

It would also remove the need for the out-parameters.

[jfernandez, 2014/10/22 21:38:10]

Yes, I agree. I fact, the same happens with the alignSelf/justifySelf properties
but unfortunately, it wont work for every case. The reason why this
resolveAlignment methods are still needed is because of the wrapper renders and
other elements manipulating the Render Tree. The StyleResolver logic happens
before having a RenderTree, so even that most of the cases are covered, I've
found several tests rasing assertions in debug mode when using only the
StyleAdjuster for those properties.

However, I'll implement the overflow resolution in the StyleAdjuster and give it
a try.

+        overflow = OverflowAlignmentTrue;
 }
 
 LayoutUnit RenderGrid::columnPositionForChild(const RenderBox& child) const
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
+    ItemPosition justifySelf;
+    OverflowAlignment overflow;
+    resolveJustification(style(), child.style(), justifySelf, overflow);
 
-    switch (resolveJustification(style(), child.style())) {
+    switch (justifySelf) {
     case ItemPositionSelfStart:
         // For orthogonal writing-modes, this computes to 'start'
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
         if (hasOrthogonalWritingMode)
-            return columnPositionAlignedWithGridContainerStart(child);
+            return columnPositionAlignedWithGridContainerStart(child, overflow);
 
         // self-start is based on the child's direction. That's why we need to check against the grid container's direction.
         if (child.style()->direction() != style()->direction())
-            return columnPositionAlignedWithGridContainerEnd(child);
+            return columnPositionAlignedWithGridContainerEnd(child, overflow);
 
-        return columnPositionAlignedWithGridContainerStart(child);
+        return columnPositionAlignedWithGridContainerStart(child, overflow);
     case ItemPositionSelfEnd:
         // For orthogonal writing-modes, this computes to 'start'
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
         if (hasOrthogonalWritingMode)
-            return columnPositionAlignedWithGridContainerEnd(child);
+            return columnPositionAlignedWithGridContainerEnd(child, overflow);
 
         // self-end is based on the child's direction. That's why we need to check against the grid container's direction.
         if (child.style()->direction() != style()->direction())
-            return columnPositionAlignedWithGridContainerStart(child);
+            return columnPositionAlignedWithGridContainerStart(child, overflow);
 
-        return columnPositionAlignedWithGridContainerEnd(child);
+        return columnPositionAlignedWithGridContainerEnd(child, overflow);
 
     case ItemPositionFlexStart:
         // Only used in flex layout, for other layout, it's equivalent to 'start'.
-        return columnPositionAlignedWithGridContainerStart(child);
+        return columnPositionAlignedWithGridContainerStart(child, overflow);
     case ItemPositionFlexEnd:
         // Only used in flex layout, for other layout, it's equivalent to 'start'.
-        return columnPositionAlignedWithGridContainerEnd(child);
+        return columnPositionAlignedWithGridContainerEnd(child, overflow);
 
     case ItemPositionLeft:
         // If the property's axis is not parallel with the inline axis, this is equivalent to ‘start’.
         if (!isHorizontalWritingMode())
-            return columnPositionAlignedWithGridContainerStart(child);
+            return columnPositionAlignedWithGridContainerStart(child, overflow);
 
         if (style()->isLeftToRightDirection())
-            return columnPositionAlignedWithGridContainerStart(child);
+            return columnPositionAlignedWithGridContainerStart(child, overflow);
 
-        return columnPositionAlignedWithGridContainerEnd(child);
+        return columnPositionAlignedWithGridContainerEnd(child, overflow);
     case ItemPositionRight:
         // If the property's axis is not parallel with the inline axis, this is equivalent to ‘start’.
         if (!isHorizontalWritingMode())
-            return columnPositionAlignedWithGridContainerStart(child);
+            return columnPositionAlignedWithGridContainerStart(child, overflow);
 
         if (style()->isLeftToRightDirection())
-            return columnPositionAlignedWithGridContainerEnd(child);
+            return columnPositionAlignedWithGridContainerEnd(child, overflow);
 
-        return columnPositionAlignedWithGridContainerStart(child);
+        return columnPositionAlignedWithGridContainerStart(child, overflow);
 
     case ItemPositionCenter:
-        return centeredColumnPositionForChild(child);
+        return centeredColumnPositionForChild(child, overflow);
     case ItemPositionStart:
-        return columnPositionAlignedWithGridContainerStart(child);
+        return columnPositionAlignedWithGridContainerStart(child, overflow);
     case ItemPositionEnd:
-        return columnPositionAlignedWithGridContainerEnd(child);
+        return columnPositionAlignedWithGridContainerEnd(child, overflow);
 
     case ItemPositionAuto:
         break;
@@ -1256,7 +1274,7 @@ LayoutUnit RenderGrid::columnPositionForChild(const RenderBox& child) const
     return 0;
 }
 
-LayoutUnit RenderGrid::endOfRowForChild(const RenderBox& child) const
+LayoutUnit RenderGrid::endOfRowForChild(const RenderBox& child, OverflowAlignment overflow) const
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
 
@@ -1265,8 +1283,11 @@ LayoutUnit RenderGrid::endOfRowForChild(const RenderBox& child) const
     LayoutUnit rowPosition = startOfRow + marginBeforeForChild(&child);
 
     LayoutUnit endOfRow = m_rowPositions[coordinate.rows.resolvedFinalPosition.next().toInt()];
-    // FIXME: This should account for the grid item's <overflow-position>.
-    return rowPosition + std::max<LayoutUnit>(0, endOfRow - startOfRow - child.logicalHeight());
+    LayoutUnit childLogicalHeight = child.logicalHeight();
+    LayoutUnit rowHeight = endOfRow - startOfRow;
+    if ((childLogicalHeight > rowHeight) && (overflow == OverflowAlignmentTrue || overflow == OverflowAlignmentDefault))

[Julien - ping for review, 2014/10/20 16:59:11]

Can we really get overflow == OverflowAlignmentDefault as we eliminate it when
resolving the justification / alignment?

[jfernandez, 2014/10/22 21:38:10]

I was thinking on other calls to this method which might happen before the
justification /alignment resolution. I agree it might be overprotected logic;
anyway, such code will be changed in the next patch and no need for this
anymore.

+        return rowPosition + rowHeight - childLogicalHeight;
+    return rowPosition + std::max<LayoutUnit>(0, rowHeight - childLogicalHeight);

[Julien - ping for review, 2014/10/20 16:59:11]

I think this needs a comment why it's correct (it took me a while to assess that
it was, I was going to say we should use return startOfRowForChild).

[jfernandez, 2014/10/22 21:38:10]

Done.

 }
 
 LayoutUnit RenderGrid::startOfRowForChild(const RenderBox& child) const
@@ -1281,32 +1302,46 @@ LayoutUnit RenderGrid::startOfRowForChild(const RenderBox& child) const
     return rowPosition;
 }
 
-LayoutUnit RenderGrid::centeredRowPositionForChild(const RenderBox& child) const
+LayoutUnit RenderGrid::centeredRowPositionForChild(const RenderBox& child, OverflowAlignment overflow) const
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
 
     // The grid items should be inside the grid container's border box, that's why they need to be shifted.
     LayoutUnit startOfRow = m_rowPositions[coordinate.rows.resolvedInitialPosition.toInt()] + marginBeforeForChild(&child);
     LayoutUnit endOfRow = m_rowPositions[coordinate.rows.resolvedFinalPosition.next().toInt()];
-
-    // FIXME: This should account for the grid item's <overflow-position>.
-    return startOfRow + std::max<LayoutUnit>(0, endOfRow - startOfRow - child.logicalHeight()) / 2;
+    LayoutUnit rowPosition = startOfRow + marginBeforeForChild(&child);
+    LayoutUnit childLogicalHeight = child.logicalHeight();
+    LayoutUnit rowHeight = endOfRow - startOfRow;
+    if ((childLogicalHeight > rowHeight) && (overflow == OverflowAlignmentTrue || overflow == OverflowAlignmentDefault))

[Julien - ping for review, 2014/10/20 16:59:11]

Unneeded parenthesis around the first clause (it applies to all the new if's in
the *rowpositionforchild).

[jfernandez, 2014/10/22 21:38:10]

Done.

+        return rowPosition + (rowHeight - childLogicalHeight) / 2;
+    return rowPosition + std::max<LayoutUnit>(0, rowHeight - childLogicalHeight) / 2;
 }
 
 // FIXME: We should move this logic to the StyleAdjuster or the StyleBuilder.
-static ItemPosition resolveAlignment(const RenderStyle* parentStyle, const RenderStyle* childStyle)
+static void resolveAlignment(const RenderStyle* parentStyle, const RenderStyle* childStyle, ItemPosition& align, OverflowAlignment &overflow)
 {
-    ItemPosition align = childStyle->alignSelf();
+    align = childStyle->alignSelf();
+    overflow = childStyle->alignSelfOverflowAlignment();
     // The auto keyword computes to the parent's align-items computed value, or to "stretch", if not set or "auto".
-    if (align == ItemPositionAuto)
-        align = (parentStyle->alignItems() == ItemPositionAuto) ? ItemPositionStretch : parentStyle->alignItems();
-    return align;
+    if (align == ItemPositionAuto) {
+        if (parentStyle->alignItems() == ItemPositionAuto) {
+            align = ItemPositionStretch;
+        } else {
+            align = parentStyle->alignItems();
+            overflow = parentStyle->alignItemsOverflowAlignment();
+        }
+    }
+    // The default overflow alignment is 'true' for Grid Items.
+    if (overflow == OverflowAlignmentDefault)
+        overflow = OverflowAlignmentTrue;
 }
 
 LayoutUnit RenderGrid::rowPositionForChild(const RenderBox& child) const
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    ItemPosition alignSelf = resolveAlignment(style(), child.style());
+    ItemPosition alignSelf;
+    OverflowAlignment overflow;
+    resolveAlignment(style(), child.style(), alignSelf, overflow);
 
     switch (alignSelf) {
     case ItemPositionSelfStart:
@@ -1317,20 +1352,20 @@ LayoutUnit RenderGrid::rowPositionForChild(const RenderBox& child) const
 
         // self-start is based on the child's block axis direction. That's why we need to check against the grid container's block flow.
         if (child.style()->writingMode() != style()->writingMode())
-            return endOfRowForChild(child);
+            return endOfRowForChild(child, overflow);
 
         return startOfRowForChild(child);
     case ItemPositionSelfEnd:
         // If orthogonal writing-modes, this computes to 'End'.
         // FIXME: grid track sizing and positioning does not support orthogonal modes yet.
         if (hasOrthogonalWritingMode)
-            return endOfRowForChild(child);
+            return endOfRowForChild(child, overflow);
 
         // self-end is based on the child's block axis direction. That's why we need to check against the grid container's block flow.
         if (child.style()->writingMode() != style()->writingMode())
             return startOfRowForChild(child);
 
-        return endOfRowForChild(child);
+        return endOfRowForChild(child, overflow);
 
     case ItemPositionLeft:
         // orthogonal modes make property and inline axes to be parallel, but in any case
@@ -1344,14 +1379,14 @@ LayoutUnit RenderGrid::rowPositionForChild(const RenderBox& child) const
         // orthogonal modes make property and inline axes to be parallel.
         // FIXME: grid track sizing and positioning does not support orthogonal modes yet.
         if (hasOrthogonalWritingMode)
-            return endOfRowForChild(child);
+            return endOfRowForChild(child, overflow);
 
         // self-align's axis is never parallel to the inline axis, except in orthogonal
         // writing-mode, so this is equivalent to 'Start'.
         return startOfRowForChild(child);
 
     case ItemPositionCenter:
-        return centeredRowPositionForChild(child);
+        return centeredRowPositionForChild(child, overflow);
         // Only used in flex layout, for other layout, it's equivalent to 'Start'.
     case ItemPositionFlexStart:
     case ItemPositionStart:
@@ -1359,7 +1394,7 @@ LayoutUnit RenderGrid::rowPositionForChild(const RenderBox& child) const
         // Only used in flex layout, for other layout, it's equivalent to 'End'.
     case ItemPositionFlexEnd:
     case ItemPositionEnd:
-        return endOfRowForChild(child);
+        return endOfRowForChild(child, overflow);
     case ItemPositionStretch:
         // FIXME: Implement the Stretch value. For now, we always start align the child.
         return startOfRowForChild(child);