[css-grid] Allow to place positioned grid items on the padding

third_party/WebKit/Source/core/layout/LayoutGrid.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 1475 matching lines @@
 void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, GridTrackSizingDirection direction, LayoutUnit& offset, LayoutUnit& breadth)
 {
     ASSERT(child.isHorizontalWritingMode() == isHorizontalWritingMode());
 
     GridSpan positions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), child, direction);
     if (positions.isIndefinite()) {
         offset = LayoutUnit();
         breadth = (direction == ForColumns) ? clientLogicalWidth() : clientLogicalHeight();
         return;
     }
-    positions.translate(direction == ForColumns ? m_smallestColumnStart : m_smallestRowStart);
+    int smallestStart = abs(direction == ForColumns ? m_smallestColumnStart : m_smallestRowStart);

[svillar, 2016/01/19 12:04:51]

I'm starting to do

bool isRowAxis = direction == ForColumns

and then use it all along the code. I think we can do it here as well, as you're
anyway touching lines with that comparison.

[Manuel Rego, 2016/01/19 15:52:25]

Done.

+    int resolvedInitialPosition = positions.untranslatedResolvedInitialPosition() + smallestStart;
+    int resolvedFinalPosition = positions.untranslatedResolvedFinalPosition() + smallestStart;

[svillar, 2016/01/19 12:04:51]

This manual translation requires some explanation in a comment.

Also, as we're translating perhaps we should use size_t for those variables.

[Manuel Rego, 2016/01/19 15:52:25]

Added comment.
But that's precisely the issue, that for abspos items even after translated
we could have negative values, so we need to use ints.

 
     GridPosition startPosition = (direction == ForColumns) ? child.style()->gridColumnStart() : child.style()->gridRowStart();
     GridPosition endPosition = (direction == ForColumns) ? child.style()->gridColumnEnd() : child.style()->gridRowEnd();
-    size_t lastTrackIndex = (direction == ForColumns ? gridColumnCount() : gridRowCount()) - 1;
+    int lastTrackIndex = (direction == ForColumns ? gridColumnCount() : gridRowCount());

[svillar, 2016/01/19 12:04:51]

Perhaps we should use this change to rename this to lastExplicitTrackIndex in
order to make it clear that positioned items ignore implicit position

[Manuel Rego, 2016/01/19 15:52:25]

Done.

 
     bool startIsAuto = startPosition.isAuto()
         || (startPosition.isNamedGridArea() && !GridResolvedPosition::isValidNamedLineOrArea(startPosition.namedGridLine(), styleRef(), GridResolvedPosition::initialPositionSide(direction)))
-        || (positions.resolvedInitialPosition() > lastTrackIndex);
+        || (resolvedInitialPosition < 0)
+        || (resolvedInitialPosition > lastTrackIndex);
     bool endIsAuto = endPosition.isAuto()
         || (endPosition.isNamedGridArea() && !GridResolvedPosition::isValidNamedLineOrArea(endPosition.namedGridLine(), styleRef(), GridResolvedPosition::finalPositionSide(direction)))
-        || (positions.resolvedFinalPosition() - 1 > lastTrackIndex);
+        || (resolvedFinalPosition < 0)
+        || (resolvedFinalPosition > lastTrackIndex);
 
-    size_t firstPosition = 0;
-    size_t initialPosition = startIsAuto ? firstPosition : positions.resolvedInitialPosition();
-    size_t lastPosition = lastTrackIndex;
-    size_t finalPosition = endIsAuto ? lastPosition : positions.resolvedFinalPosition() - 1;
-
-    // Positioned children do not grow the grid, so we need to clamp the positions to avoid ending up outside of it.
-    initialPosition = std::min(initialPosition, lastPosition);
-    finalPosition = std::min(finalPosition, lastPosition);
+    size_t initialPosition = startIsAuto ? 0 : resolvedInitialPosition;
+    size_t finalPosition = endIsAuto ? lastTrackIndex : resolvedFinalPosition;
 
     LayoutUnit start = startIsAuto ? LayoutUnit() : (direction == ForColumns) ?  m_columnPositions[initialPosition] : m_rowPositions[initialPosition];
-    LayoutUnit end = endIsAuto ? (direction == ForColumns) ? logicalWidth() : logicalHeight() : (direction == ForColumns) ?  m_columnPositions[finalPosition + 1] : m_rowPositions[finalPosition + 1];
+    LayoutUnit end = endIsAuto ? (direction == ForColumns) ? logicalWidth() : logicalHeight() : (direction == ForColumns) ?  m_columnPositions[finalPosition] : m_rowPositions[finalPosition];

[jfernandez, 2016/01/18 17:57:42]

I'd simplify this line, if possible.

[Manuel Rego, 2016/01/19 15:52:25]

Now that we use isForColumns I think it's not that bad.
We could use if/else lines if needed.

 
     breadth = end - start;
 
     if (startIsAuto)
         breadth -= (direction == ForColumns) ? borderStart() : borderBefore();
     else
         start -= ((direction == ForColumns) ? borderStart() : borderBefore());
 
     if (endIsAuto) {
         breadth -= (direction == ForColumns) ? borderEnd() : borderAfter();
@@ skipping 540 matching lines @@
 
     return LayoutPoint(rowAxisOffset, columnAxisOffsetForChild(child));
 }
 
 void LayoutGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset) const
 {
     GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 } // namespace blink