[css-grid] Handle min-content/max-content with orthogonal flows

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 632 matching lines @@
     // If |child| has a percentage logical height, we shouldn't let it override its intrinsic height, which is
     // what we are interested in here. Thus we need to set the override logical height to -1 (no possible resolution).
     child.setOverrideContainingBlockContentLogicalHeight(-1);
     child.layoutIfNeeded();
     return child.logicalHeight() + child.marginLogicalHeight();
 }
 
 LayoutUnit RenderGrid::minContentForChild(RenderBox& child, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // FIXME: Properly support orthogonal writing mode.
     if (hasOrthogonalWritingMode)
-        return 0;
+        direction = direction == ForColumns ? ForRows : ForColumns;

[svillar, 2015/01/12 08:27:51]

As a side note, we have a lot of direction==ForColumns checks inside RenderGrid.
I remember having an inline isForColumns() at some point. I think it makes sense
to re-add it.

 
     if (direction == ForColumns) {
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
         return child.minPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(child);
     }
 
     return logicalHeightForChild(child, columnTracks);
 }
 
 LayoutUnit RenderGrid::maxContentForChild(RenderBox& child, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // FIXME: Properly support orthogonal writing mode.
     if (hasOrthogonalWritingMode)
-        return LayoutUnit();
+        direction = direction == ForColumns ? ForRows : ForColumns;
 
     if (direction == ForColumns) {
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
         return child.maxPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(child);
     }
 
     return logicalHeightForChild(child, columnTracks);
 }
 
@@ skipping 441 matching lines @@
         }
 
         // Because the grid area cannot be styled, we don't need to adjust
         // the grid breadth to account for 'box-sizing'.
         LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
         LayoutUnit oldOverrideContainingBlockContentLogicalHeight = child->hasOverrideContainingBlockLogicalHeight() ? child->overrideContainingBlockContentLogicalHeight() : LayoutUnit();
 
         LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(*child, ForColumns, sizingData.columnTracks);
         LayoutUnit overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChild(*child, ForRows, sizingData.rowTracks);
 
+        bool hasOrthogonalWritingMode = child->isHorizontalWritingMode() != isHorizontalWritingMode();

[Julien - ping for review, 2015/01/21 09:32:10]

Why do we need to _always_ force a layout in the orthogonal writing mode case?

+
         SubtreeLayoutScope layoutScope(*child);
-        if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
+        if (hasOrthogonalWritingMode || oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
             layoutScope.setNeedsLayout(child);
 
         child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
         child->setOverrideContainingBlockContentLogicalHeight(overrideContainingBlockContentLogicalHeight);
 
         applyStretchAlignmentToChildIfNeeded(*child, overrideContainingBlockContentLogicalHeight);
 
         child->layoutIfNeeded();
 
 #if ENABLE(ASSERT)
         const GridCoordinate& coordinate = cachedGridCoordinate(*child);
         ASSERT(coordinate.columns.resolvedInitialPosition.toInt() < sizingData.columnTracks.size());
         ASSERT(coordinate.rows.resolvedInitialPosition.toInt() < sizingData.rowTracks.size());
 #endif
         child->setLogicalLocation(findChildLogicalPosition(*child, contentPositionOffset));
 
         // Keep track of children overflowing their grid area as we might need to paint them even if the grid-area is
         // not visible
-        if (child->logicalHeight() > overrideContainingBlockContentLogicalHeight
-            || child->logicalWidth() > overrideContainingBlockContentLogicalWidth)
+        bool overflowingContainingBlockHeight = hasOrthogonalWritingMode ? child->logicalWidth() > overrideContainingBlockContentLogicalHeight : child->logicalHeight() > overrideContainingBlockContentLogicalHeight;
+        bool overflowingContainingBlockWidth = hasOrthogonalWritingMode ? child->logicalHeight() > overrideContainingBlockContentLogicalWidth : child->logicalHeight() > overrideContainingBlockContentLogicalWidth;
+        if (overflowingContainingBlockHeight || overflowingContainingBlockWidth)
             m_gridItemsOverflowingGridArea.append(child);
     }
 
     for (const auto& row : sizingData.rowTracks)
         setLogicalHeight(logicalHeight() + row.m_usedBreadth);
 
     // Min / max logical height is handled by the call to updateLogicalHeight in layoutBlock.
 
     setLogicalHeight(logicalHeight() + borderAndPaddingLogicalHeight());
 }
@@ skipping 481 matching lines @@
     case ContentPositionAuto:
         break;
     }
 
     ASSERT_NOT_REACHED();
     return 0;
 }
 
 LayoutPoint RenderGrid::findChildLogicalPosition(const RenderBox& child, LayoutSize contentAlignmentOffset) const
 {
+    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
+    LayoutUnit rowPosition = rowPositionForChild(child);
     LayoutUnit columnPosition = columnPositionForChild(child);
     // We stored m_columnPositions's data ignoring the direction, hence we might need now
     // to translate positions from RTL to LTR, as it's more convenient for painting.
     if (!style()->isLeftToRightDirection())
         columnPosition = (m_columnPositions[m_columnPositions.size() - 1] + borderAndPaddingLogicalLeft()) - columnPosition - child.logicalWidth();
 
+    // We stored column's and row's positions without considering orthogonal flows, so now we
+    // need to flip the final LayoutPoint coordinates if that's the case.
+    LayoutPoint childLocation = hasOrthogonalWritingMode ? LayoutPoint(rowPosition, columnPosition) : LayoutPoint(columnPosition, rowPosition);

[Julien - ping for review, 2015/01/21 09:32:10]

I really don't understand this line. All the offset computation should be done
in the grid container's coordinate system as per CSS writing-mode:

"In the positioning phase [...] calculations are performed according to the
writing mode of the containing block of the box establishing the orthogonal
flow."

+
     // The Content Alignment offset accounts for the RTL to LTR flip.
-    LayoutPoint childLocation(columnPosition, rowPositionForChild(child));
     childLocation.move(contentAlignmentOffset);

[Julien - ping for review, 2015/01/21 09:32:10]

This is also incoherent with flipping coordinates above, which means we probably
don't support alignment with writing mode correctly.

 
     return childLocation;
 }
 
 void RenderGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 const char* RenderGrid::renderName() const
 {
     if (isFloating())
         return "RenderGrid (floating)";
     if (isOutOfFlowPositioned())
         return "RenderGrid (positioned)";
     if (isAnonymous())
         return "RenderGrid (generated)";
     if (isRelPositioned())
         return "RenderGrid (relative positioned)";
     return "RenderGrid";
 }
 
 } // namespace blink