Implement justify-self handling in grid

Source/core/rendering/RenderGrid.cpp

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1071 matching lines @@
     for (size_t i = 0; i < m_rowPositions.size() - 1; ++i)
         m_rowPositions[i + 1] = m_rowPositions[i] + sizingData.rowTracks[i].m_usedBreadth;
 }
 
 LayoutPoint RenderGrid::findChildLogicalPosition(RenderBox* child, const GridSizingData& sizingData)
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     ASSERT(coordinate.columns.initialPositionIndex < sizingData.columnTracks.size());
     ASSERT(coordinate.rows.initialPositionIndex < sizingData.rowTracks.size());
 
+    LayoutUnit startOfColumn = m_columnPositions[coordinate.columns.initialPositionIndex];
+    LayoutUnit startOfRow = m_rowPositions[coordinate.rows.initialPositionIndex];
     // The grid items should be inside the grid container's border box, that's why they need to be shifted.
-    return LayoutPoint(m_columnPositions[coordinate.columns.initialPositionIndex] + marginStartForChild(child), m_rowPositions[coordinate.rows.initialPositionIndex] + marginBeforeForChild(child));
+    LayoutUnit columnPosition = startOfColumn + marginStartForChild(child);
+    LayoutUnit rowPosition = startOfRow + marginBeforeForChild(child);
+
+    ItemPosition childJustifySelf = child->style()->justifySelf();

[ojan, 2014/01/22 23:43:14]

Maybe move this and the switch statement below into a helper function? Then you
don't need the comment explaining what this switch does since the function name
will make that clear.

+    // This first switch pre-processes the justify-self value following the rules in css-align. It also resolves
+    // all the position into the grid container's logical coordinate for easier computation (and code sharing).
+    switch (childJustifySelf) {
+    case ItemPositionStretch:
+    case ItemPositionBaseline:
+    case ItemPositionCenter:
+    case ItemPositionStart:
+    case ItemPositionEnd:
+        // Nothing to do.
+        break;
+
+    case ItemPositionAuto:
+        // FIXME: We should use 'justify-items' value and convert any auto value here. For now, we ignore it.
+        break;
+    case ItemPositionSelfStart:
+        if (child->style()->direction() != style()->direction())
+            childJustifySelf = ItemPositionEnd;
+        else
+            childJustifySelf = ItemPositionStart;
+        break;
+    case ItemPositionSelfEnd:
+        if (child->style()->direction() != style()->direction())
+            childJustifySelf = ItemPositionStart;
+        else
+            childJustifySelf = ItemPositionEnd;
+        break;
+    case ItemPositionFlexStart:
+    case ItemPositionFlexEnd:

[ojan, 2014/01/22 23:43:14]

It's weird the flex-end is equivalent to start. I'd expect it to be equivalent
to end.

+        // Only used in flex layout, for other layout, it's equivalent to 'start'.
+        childJustifySelf = ItemPositionStart;
+        break;
+

[ojan, 2014/01/22 23:43:14]

Are these line-breaks intentional?

+    case ItemPositionLeft:
+        // If the property's axis is not parallel with the inline axis, this is equivalent to ‘start’.
+        if (!isHorizontalWritingMode())
+            childJustifySelf = ItemPositionStart;
+        else
+            childJustifySelf = style()->isLeftToRightDirection() ? ItemPositionStart : ItemPositionEnd;
+        break;
+    case ItemPositionRight:
+        // If the property's axis is not parallel with the inline axis, this is equivalent to ‘start’.
+        if (!isHorizontalWritingMode())
+            childJustifySelf = ItemPositionStart;
+        else
+            childJustifySelf = style()->isLeftToRightDirection() ? ItemPositionEnd : ItemPositionStart;
+        break;
+    }

[Julien - ping for review, 2014/01/22 19:13:42]

I am not happy with this 2-pass code. I will come up with a better way but in
the meantime, suggestions are very welcome.

[ojan, 2014/01/22 23:43:14]

Seems fine to me if you move the first pass into a helper function.

+
+    switch (childJustifySelf) {
+    case ItemPositionCenter: {
+        LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.finalPositionIndex + 1];
+        columnPosition += std::max<LayoutUnit>(0, endOfColumn - startOfColumn - child->logicalWidth()) / 2;
+        break;
+    }
+    case ItemPositionLeft:
+    case ItemPositionRight:
+    case ItemPositionFlexStart:
+    case ItemPositionFlexEnd:
+    case ItemPositionSelfStart:
+    case ItemPositionSelfEnd:
+        ASSERT_NOT_REACHED();
+        break;
+    case ItemPositionStart:
+        // Move the item to account for the column's direction.

[ojan, 2014/01/22 23:43:14]

Meh. this comment and the one below don't really tell me anything useful.

+        if (!style()->isLeftToRightDirection()) {
+            LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.finalPositionIndex + 1];
+            columnPosition += std::max<LayoutUnit>(0, endOfColumn - startOfColumn - child->logicalWidth());
+        }
+        break;
+    case ItemPositionEnd:
+        // Move the item to account for the column's direction.
+        if (style()->isLeftToRightDirection()) {
+            LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.finalPositionIndex + 1];
+            columnPosition += std::max<LayoutUnit>(0, endOfColumn - startOfColumn - child->logicalWidth());
+        }
+        break;
+    case ItemPositionStretch:
+        // FIXME: Handle this value.
+    case ItemPositionBaseline:
+        // FIXME: This is blocked on implementing the grid container's baseline.
+    case ItemPositionAuto:
+        // FIXME: We should never see this value (per the comment above).

[ojan, 2014/01/22 23:43:14]

Per which comment?

+        break;
+    }
+
+    return LayoutPoint(columnPosition, rowPosition);
 }
 
 static GridSpan dirtiedGridAreas(const Vector<LayoutUnit>& coordinates, LayoutUnit start, LayoutUnit end)
 {
     // This function does a binary search over the coordinates.
     // FIXME: This doesn't work with grid items overflowing their grid areas and should be tested & fixed.
 
     size_t startGridAreaIndex = std::upper_bound(coordinates.begin(), coordinates.end() - 1, start) - coordinates.begin();
     if (startGridAreaIndex > 0)
         --startGridAreaIndex;
@@ skipping 49 matching lines @@
     if (isOutOfFlowPositioned())
         return "RenderGrid (positioned)";
     if (isAnonymous())
         return "RenderGrid (generated)";
     if (isRelPositioned())
         return "RenderGrid (relative positioned)";
     return "RenderGrid";
 }
 
 } // namespace WebCore