[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 8ad2d787df8d70497a998b80100fb615d471e3d6..482d2c4f5b12f809342340667d3cb912cf3a4b8f 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -1171,20 +1171,26 @@ LayoutUnit RenderGrid::startOfColumnForChild(const RenderBox& child) const
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     LayoutUnit startOfColumn = m_columnPositions[coordinate.columns.resolvedInitialPosition.toInt()];
     // The grid items should be inside the grid container's border box, that's why they need to be shifted.
-    // FIXME: This should account for the grid item's <overflow-position>.
     return startOfColumn + marginStartForChild(&child);
 }
 
 LayoutUnit RenderGrid::endOfColumnForChild(const RenderBox& child) const
 {
+    OverflowAlignment overflow = child.style()->justifySelfOverflowAlignment();
+    ASSERT(overflow == OverflowAlignmentDefault);

[Julien - ping for review, 2014/10/23 00:57:25]

I don't think this is the ASSERT you were thinking of :)

[jfernandez, 2014/10/23 18:09:02]

Done.

+
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     LayoutUnit startOfColumn = m_columnPositions[coordinate.columns.resolvedInitialPosition.toInt()];
     // The grid items should be inside the grid container's border box, that's why they need to be shifted.
     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;
+    // It overflows through the alignment container's 'start' edge (may cause data loss).

[Julien - ping for review, 2014/10/23 00:57:25]

I think we could improve this comment as I had a hard time understanding what it
meant. Here is some suggestion:

// If we overflow our alignment container and overflow is 'true', we ignore the
overflow and just return the value regardless (which may cause data loss as we
overflow the start edge).

// If we overflow and overflow is 'safe', this will correctly clamp us and
return the 'start' edge.

[jfernandez, 2014/10/23 18:09:03]

Acknowledged.

+    if (childLogicalWidth > columnWidth && overflow == OverflowAlignmentTrue)
+        return columnPosition + columnWidth - childLogicalWidth;
+    return columnPosition + std::max<LayoutUnit>(0, columnWidth - childLogicalWidth);

[Julien - ping for review, 2014/10/23 00:57:25]

I think it would be a lot more readable with the following logic:

LayoutUnit offsetFromColumnPosition = columnWidth - childLogicalWidth;
// If overflow is 'safe', we have to make sure we don't overflow the 'start'
edge (potentially cause some data loss as the overflow is unreachable).
if (overflow == OverflowAlignmentSafe)
    offsetFromColumnPosition = clampTo<LayoutUnit>(offsetFromColumnPosition, 0);
// Note you can keep the std::max, I just have a fancy with clampTo these days
return columnPosition + offsetFromColumnPosition;

[jfernandez, 2014/10/23 18:09:03]

Done.

 }
 
 LayoutUnit RenderGrid::columnPositionAlignedWithGridContainerStart(const RenderBox& child) const
@@ -1205,12 +1211,19 @@ LayoutUnit RenderGrid::columnPositionAlignedWithGridContainerEnd(const RenderBox
 
 LayoutUnit RenderGrid::centeredColumnPositionForChild(const RenderBox& child) const
 {
+    OverflowAlignment overflow = child.style()->justifySelfOverflowAlignment();
+    ASSERT(overflow == OverflowAlignmentDefault);

[Julien - ping for review, 2014/10/23 00:57:25]

Same comment about the ASSERT.

[jfernandez, 2014/10/23 18:09:02]

Done.

+
     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;
+    // It overflows through both alignment container's 'start' and 'end' edges (may cause data loss).
+    if (childLogicalWidth > columnWidth && overflow == OverflowAlignmentTrue)
+        return columnPosition + (columnWidth - childLogicalWidth) / 2;
+    return columnPosition + std::max<LayoutUnit>(0, columnWidth - childLogicalWidth) / 2;

[Julien - ping for review, 2014/10/23 00:57:25]

Same general comments.

[jfernandez, 2014/10/23 18:09:03]

Done.

 }
 
 static ItemPosition resolveJustification(const RenderStyle* parentStyle, const RenderStyle* childStyle)
@@ -1218,7 +1231,6 @@ static ItemPosition resolveJustification(const RenderStyle* parentStyle, const R
     ItemPosition justify = childStyle->justifySelf();
     if (justify == ItemPositionAuto)
         justify = (parentStyle->justifyItems() == ItemPositionAuto) ? ItemPositionStretch : parentStyle->justifyItems();
-
     return justify;
 }
 
@@ -1298,6 +1310,8 @@ LayoutUnit RenderGrid::columnPositionForChild(const RenderBox& child) const
 
 LayoutUnit RenderGrid::endOfRowForChild(const RenderBox& child) const
 {
+    OverflowAlignment overflow = child.style()->alignSelfOverflowAlignment();
+    ASSERT(overflow == OverflowAlignmentDefault);

[Julien - ping for review, 2014/10/23 00:57:25]

Ditto.

[jfernandez, 2014/10/23 18:09:02]

Done.

     const GridCoordinate& coordinate = cachedGridCoordinate(child);
 
     LayoutUnit startOfRow = m_rowPositions[coordinate.rows.resolvedInitialPosition.toInt()];
@@ -1305,8 +1319,12 @@ 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;
+    // It overflows through the alignment container's 'end' edge (may cause data loss).
+    if (childLogicalHeight > rowHeight && overflow == OverflowAlignmentTrue)
+        return rowPosition + rowHeight - childLogicalHeight;
+    return rowPosition + std::max<LayoutUnit>(0, rowHeight - childLogicalHeight);

[Julien - ping for review, 2014/10/23 00:57:25]

Ditto.

[jfernandez, 2014/10/23 18:09:02]

Done.

 }
 
 LayoutUnit RenderGrid::startOfRowForChild(const RenderBox& child) const
@@ -1315,7 +1333,6 @@ LayoutUnit RenderGrid::startOfRowForChild(const RenderBox& child) const
 
     LayoutUnit startOfRow = m_rowPositions[coordinate.rows.resolvedInitialPosition.toInt()];
     // The grid items should be inside the grid container's border box, that's why they need to be shifted.
-    // FIXME: This should account for the grid item's <overflow-position>.
     LayoutUnit rowPosition = startOfRow + marginBeforeForChild(&child);
 
     return rowPosition;
@@ -1323,22 +1340,27 @@ LayoutUnit RenderGrid::startOfRowForChild(const RenderBox& child) const
 
 LayoutUnit RenderGrid::centeredRowPositionForChild(const RenderBox& child) const
 {
+    OverflowAlignment overflow = child.style()->alignSelfOverflowAlignment();
+    ASSERT(overflow == OverflowAlignmentDefault);

[Julien - ping for review, 2014/10/23 00:57:25]

Ditto.

[jfernandez, 2014/10/23 18:09:02]

Done.

     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;
+    // It overflows through both alignment container's 'start' and 'end' edges (may cause data loss).
+    if (childLogicalHeight > rowHeight && overflow == OverflowAlignmentTrue)
+        return rowPosition + (rowHeight - childLogicalHeight) / 2;
+    return rowPosition + std::max<LayoutUnit>(0, rowHeight - childLogicalHeight) / 2;

[Julien - ping for review, 2014/10/23 00:57:24]

Ditto.

[jfernandez, 2014/10/23 18:09:03]

Done.

 }
 
 LayoutUnit RenderGrid::rowPositionForChild(const RenderBox& child) const
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    ItemPosition alignSelf = RenderStyle::resolveAlignment(style(), child.style());
 
-    switch (alignSelf) {
+    switch (RenderStyle::resolveAlignment(style(), child.style())) {
     case ItemPositionSelfStart:
         // If orthogonal writing-modes, this computes to 'Start'.
         // FIXME: grid track sizing and positioning does not support orthogonal modes yet.