Index: third_party/WebKit/Source/core/layout/LayoutGrid.cpp |
diff --git a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp |
index 05fd3d244912532c2fec9eb817b7fac0e7a959a7..3d125377b9952fdb1d04949f2bfbcd050b847416 100644 |
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp |
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp |
@@ -249,7 +249,45 @@ public: |
enum SizingOperation { TrackSizing, IntrinsicSizeComputation }; |
SizingOperation sizingOperation { TrackSizing }; |
- |
+ enum SizingState { ColumnSizingFirstIteration, RowSizingFirstIteration, ColumnSizingSecondIteration, RowSizingSecondIteration}; |
+ SizingState sizingState { ColumnSizingFirstIteration }; |
+ void nextState() |
+ { |
+ switch (sizingState) { |
+ case ColumnSizingFirstIteration: |
+ sizingState = RowSizingFirstIteration; |
+ break; |
+ case RowSizingFirstIteration: |
+ sizingState = ColumnSizingSecondIteration; |
+ break; |
+ case ColumnSizingSecondIteration: |
+ sizingState = RowSizingSecondIteration; |
+ break; |
+ case RowSizingSecondIteration: |
+ sizingState = ColumnSizingFirstIteration; |
+ break; |
+ default: |
+ NOTREACHED(); |
+ sizingState = ColumnSizingFirstIteration; |
+ } |
+ } |
svillar
2016/04/14 08:58:42
I think it's better to merge SizingOperation and S
jfernandez
2016/05/18 10:31:40
I'm not sure about this, for several reasons. Firs
|
+ bool sanityCheck(GridTrackSizingDirection direction) |
+ { |
+ switch (sizingState) { |
+ case ColumnSizingFirstIteration: |
+ return direction == ForColumns ? true : false; |
+ case RowSizingFirstIteration: |
+ return direction == ForRows ? true : false; |
+ case ColumnSizingSecondIteration: |
+ if (direction == ForRows) |
+ sizingState = RowSizingFirstIteration; |
+ return true; |
+ case RowSizingSecondIteration: |
+ return direction == ForRows ? true : false; |
+ } |
+ NOTREACHED(); |
+ return false; |
+ } |
svillar
2016/04/14 08:58:41
Let's enclose this in debugging guards as it's mea
jfernandez
2016/05/18 10:31:40
Done.
|
private: |
LayoutUnit freeSpaceForColumns { }; |
LayoutUnit freeSpaceForRows { }; |
@@ -332,12 +370,32 @@ LayoutUnit LayoutGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizi |
void LayoutGrid::computeTrackSizesForDirection(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit freeSpace) |
{ |
ASSERT(freeSpace >= 0); |
+ DCHECK(sizingData.sanityCheck(direction)); |
sizingData.freeSpaceForDirection(direction) = freeSpace - guttersSize(direction, direction == ForRows ? gridRowCount() : gridColumnCount()); |
sizingData.sizingOperation = GridSizingData::TrackSizing; |
LayoutUnit baseSizes, growthLimits; |
computeUsedBreadthOfGridTracks(direction, sizingData, baseSizes, growthLimits, AvailableSpaceDefinite); |
ASSERT(tracksAreWiderThanMinTrackBreadth(direction, sizingData)); |
+ sizingData.nextState(); |
+} |
+ |
+void LayoutGrid::repeatTracksSizingIfNeeded(GridSizingData& sizingData, LayoutUnit availableSpaceForColumns, LayoutUnit availableSpaceForRows) |
+{ |
+ DCHECK(sizingData.sizingState > GridSizingData::RowSizingFirstIteration); |
+ |
+ // In orthogonal flow cases column track's size is determined by using the computed |
+ // row track's size, which it was estimated during the first cycle of the sizing |
+ // algorithm. Hence we need to repeat computeUsedBreadthOfGridTracks for both, |
+ // columns and rows, to determine the final values. |
+ // TODO (lajava): orthogonal flows is just one of the cases which may require |
svillar
2016/04/14 08:58:41
which other cases?
jfernandez
2016/05/18 10:31:40
Well, the spec states the following on this regard
|
+ // a new cycle of the sizing algorithm; there may be more. In addition, not all the |
+ // cases with orthogonal flows require this extra cycle; we need a more specific |
+ // condition to detect whether child's min-content contribution has changed or not. |
+ if (m_hasAnyOrthogonalChild) { |
+ computeTrackSizesForDirection(ForColumns, sizingData, availableSpaceForColumns); |
+ computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows); |
+ } |
} |
void LayoutGrid::layoutBlock(bool relayoutChildren) |
@@ -368,14 +426,16 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) |
// properly resolves intrinsic sizes. We cannot do the same for heights though because many code |
// paths inside updateLogicalHeight() require a previous call to setLogicalHeight() to resolve |
// heights properly (like for positioned items for example). |
- computeTrackSizesForDirection(ForColumns, sizingData, availableLogicalWidth()); |
+ LayoutUnit availableSpaceForColumns = availableLogicalWidth(); |
+ computeTrackSizesForDirection(ForColumns, sizingData, availableSpaceForColumns); |
// 2- Next, the track sizing algorithm resolves the sizes of the grid rows, using the |
// grid column sizes calculated in the previous step. |
+ LayoutUnit availableSpaceForRows = availableLogicalHeight(ExcludeMarginBorderPadding); |
svillar
2016/04/14 08:58:41
Better not do this. Calling availableXXX only make
jfernandez
2016/05/18 10:31:40
Done.
|
if (logicalHeightWasIndefinite) |
computeIntrinsicLogicalHeight(sizingData); |
else |
- computeTrackSizesForDirection(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding)); |
+ computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows); |
setLogicalHeight(computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight()); |
LayoutUnit oldClientAfterEdge = clientLogicalBottom(); |
@@ -383,8 +443,14 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) |
// The above call might have changed the grid's logical height depending on min|max height restrictions. |
// Update the sizes of the rows whose size depends on the logical height (also on definite|indefinite sizes). |
+ availableSpaceForRows = contentLogicalHeight(); |
svillar
2016/04/14 08:58:41
Instead you can define this variable right here.
jfernandez
2016/05/18 10:31:40
Done.
|
if (logicalHeightWasIndefinite) |
- computeTrackSizesForDirection(ForRows, sizingData, contentLogicalHeight()); |
+ computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows); |
+ |
+ // 3- If the min-content contribution of any grid items have changed based on the row |
+ // sizes calculated in step 2, steps 1 and 2 are repeated with the new min-content |
+ // contribution (once only). |
+ repeatTracksSizingIfNeeded(sizingData, availableSpaceForColumns, availableSpaceForRows); |
// Grid container should have the minimum height of a line if it's editable. That doesn't affect track sizing though. |
if (hasLineIfEmpty()) |
@@ -667,7 +733,7 @@ bool LayoutGrid::hasDefiniteLogicalSize(GridTrackSizingDirection direction) cons |
static bool hasOverrideContainingBlockContentSizeForChild(const LayoutBox& child, GridTrackSizingDirection direction) |
{ |
- return direction == ForColumns ? child.hasOverrideContainingBlockLogicalWidth() : child.overrideContainingBlockContentLogicalHeight(); |
+ return direction == ForColumns ? child.hasOverrideContainingBlockLogicalWidth() : child.hasOverrideContainingBlockLogicalHeight(); |
svillar
2016/04/14 08:58:41
Ups, this is a bug per se. Let's do it separately
|
} |
static LayoutUnit overrideContainingBlockContentSizeForChild(const LayoutBox& child, GridTrackSizingDirection direction) |
@@ -745,9 +811,6 @@ GridTrackSizingDirection LayoutGrid::flowAwareDirectionForChild(const LayoutBox& |
LayoutUnit LayoutGrid::minSizeForChild(LayoutBox& child, GridTrackSizingDirection direction, GridSizingData& sizingData) |
{ |
- // TODO(svillar): Properly support orthogonal writing mode. |
- if (isOrthogonalChild(child)) |
- return LayoutUnit(); |
GridTrackSizingDirection childInlineDirection = flowAwareDirectionForChild(child, ForColumns); |
bool isRowAxis = direction == childInlineDirection; |
const Length& childSize = isRowAxis ? child.styleRef().logicalWidth() : child.styleRef().logicalHeight(); |
@@ -779,9 +842,6 @@ bool LayoutGrid::updateOverrideContainingBlockContentSizeForChild(LayoutBox& chi |
LayoutUnit LayoutGrid::minContentForChild(LayoutBox& child, GridTrackSizingDirection direction, GridSizingData& sizingData) |
{ |
- // FIXME: Properly support orthogonal writing mode. |
- if (isOrthogonalChild(child)) |
- return LayoutUnit(); |
GridTrackSizingDirection childInlineDirection = flowAwareDirectionForChild(child, ForColumns); |
if (direction == childInlineDirection) { |
// If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is |
@@ -802,8 +862,6 @@ LayoutUnit LayoutGrid::minContentForChild(LayoutBox& child, GridTrackSizingDirec |
LayoutUnit LayoutGrid::maxContentForChild(LayoutBox& child, GridTrackSizingDirection direction, GridSizingData& sizingData) |
{ |
- if (isOrthogonalChild(child)) |
- return LayoutUnit(); |
GridTrackSizingDirection childInlineDirection = flowAwareDirectionForChild(child, ForColumns); |
if (direction == childInlineDirection) { |
// If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is |
@@ -1190,10 +1248,13 @@ void LayoutGrid::placeItemsOnGrid() |
Vector<LayoutBox*> autoMajorAxisAutoGridItems; |
Vector<LayoutBox*> specifiedMajorAxisAutoGridItems; |
+ m_hasAnyOrthogonalChild = false; |
for (LayoutBox* child = m_orderIterator.first(); child; child = m_orderIterator.next()) { |
if (child->isOutOfFlowPositioned()) |
continue; |
+ m_hasAnyOrthogonalChild = m_hasAnyOrthogonalChild || isOrthogonalChild(*child); |
+ |
GridArea area = cachedGridArea(*child); |
if (!area.rows.isIndefinite()) |
area.rows.translate(abs(m_smallestRowStart)); |
@@ -1476,7 +1537,7 @@ void LayoutGrid::layoutGridItems(GridSizingData& sizingData) |
LayoutUnit overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChildIncludingAlignmentOffsets(*child, ForRows, sizingData); |
SubtreeLayoutScope layoutScope(*child); |
- if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight())) |
+ if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && (child->hasRelativeLogicalHeight() || isOrthogonalChild(*child)))) |
layoutScope.setNeedsLayout(child, LayoutInvalidationReason::GridChanged); |
child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth); |
@@ -1638,9 +1699,39 @@ GridSpan LayoutGrid::cachedGridSpan(const LayoutBox& gridItem, GridTrackSizingDi |
return direction == ForColumns ? area.columns : area.rows; |
} |
+bool LayoutGrid::gridLengthIsIndefinite(const GridLength& length, GridTrackSizingDirection direction) const |
svillar
2016/04/14 08:58:41
The name suggests that you're checking the length
jfernandez
2016/05/18 10:31:40
Acknowledged.
|
+{ |
+ return length.isContentSized() || length.isFlex() || !hasDefiniteLogicalSize(direction); |
svillar
2016/04/14 08:58:42
This usage of hasDefiniteLogicalSize() is not corr
jfernandez
2016/05/18 10:31:40
I don't think there is anything wrong with current
svillar
2016/05/26 08:49:02
BTW you are considering percentages as definite si
jfernandez
2016/05/27 09:41:22
Note that the GridLength is extracted suing the gr
|
+} |
+ |
+LayoutUnit LayoutGrid::assumedRowsBreadthForOrthogonalChild(const LayoutBox& child) const |
svillar
2016/04/14 08:58:41
Breadth -> Size
jfernandez
2016/05/18 10:31:40
Done.
|
+{ |
+ ASSERT(isOrthogonalChild(child)); |
svillar
2016/04/14 08:58:41
DCHECK
jfernandez
2016/05/18 10:31:40
Done.
|
+ const GridSpan& span = cachedGridSpan(child, ForRows); |
+ LayoutUnit gridAreaBreadth; |
+ for (auto trackPosition : span) { |
+ const GridLength& maxTrackBreadth = gridTrackSize(ForRows, trackPosition).maxTrackBreadth(); |
+ if (gridLengthIsIndefinite(maxTrackBreadth, ForRows)) { |
+ gridAreaBreadth = child.maxPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(child); |
+ break; |
svillar
2016/04/14 08:58:41
This looks wrong, because the final value of gridA
jfernandez
2016/05/18 10:31:40
I understand your concerns, however, I think that
svillar
2016/05/26 08:49:02
If this is the case then we should have some ASSER
jfernandez
2016/05/27 09:41:22
I don't think ASSERT that makes sense. One should
|
+ } |
+ gridAreaBreadth += valueForLength(maxTrackBreadth.length(), LayoutUnit()); |
svillar
2016/04/14 08:58:41
If I am not wrong the second value is the maximum
jfernandez
2016/05/18 10:31:40
Your assumption on the usage of such second argume
svillar
2016/05/26 08:49:02
Yes I expect it to work for all the potential case
jfernandez
2016/05/27 09:41:22
Ill fix the case of percentage sizes for tracks an
|
+ } |
+ |
+ gridAreaBreadth += guttersSize(ForRows, span.integerSpan()); |
+ |
+ return gridAreaBreadth; |
+} |
+ |
LayoutUnit LayoutGrid::gridAreaBreadthForChild(const LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData) const |
{ |
- const Vector<GridTrack>& tracks = direction == ForColumns ? sizingData.columnTracks : sizingData.rowTracks; |
+ // To determine the column track's size based on an orthogonal grid item we need it's logical height, which |
+ // may depend on the row track's size. It's possible that the row tracks sizing logic has not been performed yet, |
+ // so we will need to do an estimation. |
+ if (direction == ForRows && sizingData.sizingState == GridSizingData::ColumnSizingFirstIteration) |
+ return assumedRowsBreadthForOrthogonalChild(child); |
+ |
+ const Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks; |
svillar
2016/04/14 08:58:41
No need for parentheses.
jfernandez
2016/05/18 10:31:40
Done.
|
const GridSpan& span = cachedGridSpan(child, direction); |
LayoutUnit gridAreaBreadth; |
for (const auto& trackPosition : span) |
@@ -2108,6 +2199,7 @@ ContentAlignmentData LayoutGrid::computeContentPositionAndDistributionOffset(Gri |
LayoutPoint LayoutGrid::findChildLogicalPosition(const LayoutBox& child, GridSizingData& sizingData) const |
{ |
+ LayoutUnit columnAxisOffset = columnAxisOffsetForChild(child, sizingData); |
LayoutUnit rowAxisOffset = rowAxisOffsetForChild(child, sizingData); |
// We stored m_columnPosition's data ignoring the direction, hence we might need now |
// to translate positions from RTL to LTR, as it's more convenient for painting. |
@@ -2117,7 +2209,12 @@ LayoutPoint LayoutGrid::findChildLogicalPosition(const LayoutBox& child, GridSiz |
rowAxisOffset = rightGridEdgePosition - (rowAxisOffset + child.logicalWidth()); |
} |
- return LayoutPoint(rowAxisOffset, columnAxisOffsetForChild(child, sizingData)); |
+ // "In the positioning phase [...] calculations are performed according to the writing mode |
+ // of the containing block of the box establishing the orthogonal flow." However, the |
+ // resulting LayoutPoint will be used in 'setLogicalPosition' in order to set the child's |
+ // logical position, which will only take into account the child's writing-mode. |
+ LayoutPoint childLocation(rowAxisOffset, columnAxisOffset); |
+ return isOrthogonalChild(child) ? childLocation.transposedPoint() : childLocation; |
} |
void LayoutGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset) const |