Modified: trunk/LayoutTests/ChangeLog (190632 => 190633)
--- trunk/LayoutTests/ChangeLog 2015-10-06 19:13:46 UTC (rev 190632)
+++ trunk/LayoutTests/ChangeLog 2015-10-06 19:23:52 UTC (rev 190633)
@@ -1,3 +1,15 @@
+2015-10-06 Javier Fernandez <jfernan...@igalia.com>
+
+ [CSS Grid Layout] Don't need to reset auto-margins during grid items layout
+ https://bugs.webkit.org/show_bug.cgi?id=149764
+
+ Reviewed by Darin Adler.
+
+ Removed a duplicated layout tests.
+
+ * fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt: Removed.
+ * fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html: Removed.
+
2015-10-02 Jon Honeycutt <jhoneyc...@apple.com>
Import some Blink layout tests.
Deleted: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt (190632 => 190633)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt 2015-10-06 19:13:46 UTC (rev 190632)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt 2015-10-06 19:23:52 UTC (rev 190633)
@@ -1,4 +0,0 @@
-The grids below had initially 'stretched' items, but we have changed 'height' and 'margin' to values which don't allow stretch. This test verifies that the layout algorithm properly detects such changes and clear the override height accordingly.
-
-PASS
-PASS
Deleted: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html (190632 => 190633)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html 2015-10-06 19:13:46 UTC (rev 190632)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html 2015-10-06 19:23:52 UTC (rev 190633)
@@ -1,37 +0,0 @@
-<!DOCTYPE HTML>
-<link href="" rel="stylesheet">
-<script src=""
-<style>
-.grid {
- -webkit-grid-template: 200px 200px / 200px 200px;
- width: -webkit-fit-content;
- position: relative;
-}
-#fromFixedHeight { height: 100px; }
-#fromMarginAuto { margin: auto; }
-</style>
-<p>The grids below had initially 'stretched' items, but we have changed 'height' and 'margin' to values which don't allow stretch. This test verifies that the layout algorithm properly detects such changes and clear the override height accordingly.</p>
-<div class="grid">
- <div id="toFixedHeight" class="firstRowFirstColumn" data-expected-height="100"></div>
- <div class="firstRowSecondColumn" data-expected-height="200"></div>
- <div class="secondRowFirstColumn" data-expected-height="200"></div>
- <div id="fromFixedHeight" class="secondRowSecondColumn" data-expected-height="200"></div>
-</div>
-<div class="grid">
- <div id="toMarginAuto" class="firstRowFirstColumn" data-expected-height="100">
- <div style="height: 100px"></div>
- </div>
- <div class="firstRowSecondColumn" data-expected-height="200"></div>
- <div class="secondRowFirstColumn" data-expected-height="200"></div>
- <div id="fromMarginAuto" class="secondRowSecondColumn" data-expected-height="200">
- <div style="height: 100px"></div>
- </div>
-</div>
-<script>
-document.body.offsetLeft;
-document.getElementById("fromFixedHeight").style.height = "auto";
-document.getElementById("toFixedHeight").style.height = "100px";
-document.getElementById("fromMarginAuto").style.margin = "0";
-document.getElementById("toMarginAuto").style.margin = "auto";
-checkLayout(".grid");
-</script>
Modified: trunk/Source/WebCore/ChangeLog (190632 => 190633)
--- trunk/Source/WebCore/ChangeLog 2015-10-06 19:13:46 UTC (rev 190632)
+++ trunk/Source/WebCore/ChangeLog 2015-10-06 19:23:52 UTC (rev 190633)
@@ -1,3 +1,26 @@
+2015-10-06 Javier Fernandez <jfernan...@igalia.com>
+
+ [CSS Grid Layout] Don't need to reset auto-margins during grid items layout
+ https://bugs.webkit.org/show_bug.cgi?id=149764
+
+ Reviewed by Darin Adler.
+
+ This patch implements a refactoring of the auto-margin alignment code for grid
+ items so it uses start/end and before/after margin logic terms.
+
+ I addition, it avoids resetting the auto-margin values, which requires an extra
+ layout, before applying the alignment logic.
+
+ No new tests because there is no behavior change.
+
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::computeMarginLogicalHeightForChild): Computing margins if child needs layout.
+ (WebCore::RenderGrid::availableAlignmentSpaceForChildBeforeStretching):
+ (WebCore::RenderGrid::updateAutoMarginsInRowAxisIfNeeded): Using start/end logical margins.
+ (WebCore::RenderGrid::updateAutoMarginsInColumnAxisIfNeeded): Using before/after logical margins.
+ (WebCore::RenderGrid::columnAxisOffsetForChild): Just added comment.
+ (WebCore::RenderGrid::rowAxisOffsetForChild): Just added comment.
+
2015-10-06 Tim Horton <timothy_hor...@apple.com>
Tile map shows a green rect when threaded scrolling is disabled
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (190632 => 190633)
--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2015-10-06 19:13:46 UTC (rev 190632)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2015-10-06 19:23:52 UTC (rev 190633)
@@ -1270,8 +1270,6 @@
|| ((!oldOverrideContainingBlockContentLogicalHeight || oldOverrideContainingBlockContentLogicalHeight.value() != overrideContainingBlockContentLogicalHeight)
&& child->hasRelativeLogicalHeight()))
child->setNeedsLayout(MarkOnlyThis);
- else
- resetAutoMarginsAndLogicalTopInColumnAxis(*child);
child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
child->setOverrideContainingBlockContentLogicalHeight(overrideContainingBlockContentLogicalHeight);
@@ -1412,9 +1410,24 @@
return isHorizontalWritingMode() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
}
+LayoutUnit RenderGrid::computeMarginLogicalHeightForChild(const RenderBox& child) const
+{
+ if (!child.style().hasMargin())
+ return 0;
+
+ LayoutUnit marginBefore;
+ LayoutUnit marginAfter;
+ child.computeBlockDirectionMargins(this, marginBefore, marginAfter);
+
+ return marginBefore + marginAfter;
+}
+
LayoutUnit RenderGrid::availableAlignmentSpaceForChildBeforeStretching(LayoutUnit gridAreaBreadthForChild, const RenderBox& child) const
{
- return gridAreaBreadthForChild - marginLogicalHeightForChild(child);
+ // Because we want to avoid multiple layouts, stretching logic might be performed before
+ // children are laid out, so we can't use the child cached values. Hence, we need to
+ // compute margins in order to determine the available height before stretching.
+ return gridAreaBreadthForChild - (child.needsLayout() ? computeMarginLogicalHeightForChild(child) : marginLogicalHeightForChild(child));
}
// FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
@@ -1481,57 +1494,24 @@
}
// FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
-void RenderGrid::resetAutoMarginsAndLogicalTopInColumnAxis(RenderBox& child)
-{
- if (hasAutoMarginsInColumnAxis(child) || child.needsLayout()) {
- child.clearOverrideLogicalContentHeight();
- child.updateLogicalHeight();
- if (isHorizontalWritingMode()) {
- if (child.style().marginTop().isAuto())
- child.setMarginTop(0);
- if (child.style().marginBottom().isAuto())
- child.setMarginBottom(0);
- } else {
- if (child.style().marginLeft().isAuto())
- child.setMarginLeft(0);
- if (child.style().marginRight().isAuto())
- child.setMarginRight(0);
- }
-
- }
-}
-
-// FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
void RenderGrid::updateAutoMarginsInRowAxisIfNeeded(RenderBox& child)
{
ASSERT(!child.isOutOfFlowPositioned());
- ASSERT(child.overrideContainingBlockContentLogicalWidth());
LayoutUnit availableAlignmentSpace = child.overrideContainingBlockContentLogicalWidth().value() - child.logicalWidth();
if (availableAlignmentSpace <= 0)
return;
- bool isHorizontal = isHorizontalWritingMode();
- Length topOrLeft = isHorizontal ? child.style().marginLeft() : child.style().marginTop();
- Length bottomOrRight = isHorizontal ? child.style().marginRight() : child.style().marginBottom();
- if (topOrLeft.isAuto() && bottomOrRight.isAuto()) {
- if (isHorizontal) {
- child.setMarginLeft(availableAlignmentSpace / 2);
- child.setMarginRight(availableAlignmentSpace / 2);
- } else {
- child.setMarginTop(availableAlignmentSpace / 2);
- child.setMarginBottom(availableAlignmentSpace / 2);
- }
- } else if (topOrLeft.isAuto()) {
- if (isHorizontal)
- child.setMarginLeft(availableAlignmentSpace);
- else
- child.setMarginTop(availableAlignmentSpace);
- } else if (bottomOrRight.isAuto()) {
- if (isHorizontal)
- child.setMarginRight(availableAlignmentSpace);
- else
- child.setMarginBottom(availableAlignmentSpace);
+ const RenderStyle& parentStyle = style();
+ Length marginStart = child.style().marginStartUsing(&parentStyle);
+ Length marginEnd = child.style().marginEndUsing(&parentStyle);
+ if (marginStart.isAuto() && marginEnd.isAuto()) {
+ child.setMarginStart(availableAlignmentSpace / 2, &parentStyle);
+ child.setMarginEnd(availableAlignmentSpace / 2, &parentStyle);
+ } else if (marginStart.isAuto()) {
+ child.setMarginStart(availableAlignmentSpace, &parentStyle);
+ } else if (marginEnd.isAuto()) {
+ child.setMarginEnd(availableAlignmentSpace, &parentStyle);
}
}
@@ -1539,33 +1519,21 @@
void RenderGrid::updateAutoMarginsInColumnAxisIfNeeded(RenderBox& child)
{
ASSERT(!child.isOutOfFlowPositioned());
- ASSERT(child.overrideContainingBlockContentLogicalHeight());
LayoutUnit availableAlignmentSpace = child.overrideContainingBlockContentLogicalHeight().value() - child.logicalHeight();
if (availableAlignmentSpace <= 0)
return;
- bool isHorizontal = isHorizontalWritingMode();
- Length topOrLeft = isHorizontal ? child.style().marginTop() : child.style().marginLeft();
- Length bottomOrRight = isHorizontal ? child.style().marginBottom() : child.style().marginRight();
- if (topOrLeft.isAuto() && bottomOrRight.isAuto()) {
- if (isHorizontal) {
- child.setMarginTop(availableAlignmentSpace / 2);
- child.setMarginBottom(availableAlignmentSpace / 2);
- } else {
- child.setMarginLeft(availableAlignmentSpace / 2);
- child.setMarginRight(availableAlignmentSpace / 2);
- }
- } else if (topOrLeft.isAuto()) {
- if (isHorizontal)
- child.setMarginTop(availableAlignmentSpace);
- else
- child.setMarginLeft(availableAlignmentSpace);
- } else if (bottomOrRight.isAuto()) {
- if (isHorizontal)
- child.setMarginBottom(availableAlignmentSpace);
- else
- child.setMarginRight(availableAlignmentSpace);
+ const RenderStyle& parentStyle = style();
+ Length marginBefore = child.style().marginBeforeUsing(&parentStyle);
+ Length marginAfter = child.style().marginAfterUsing(&parentStyle);
+ if (marginBefore.isAuto() && marginAfter.isAuto()) {
+ child.setMarginBefore(availableAlignmentSpace / 2, &parentStyle);
+ child.setMarginAfter(availableAlignmentSpace / 2, &parentStyle);
+ } else if (marginBefore.isAuto()) {
+ child.setMarginBefore(availableAlignmentSpace, &parentStyle);
+ } else if (marginAfter.isAuto()) {
+ child.setMarginAfter(availableAlignmentSpace, &parentStyle);
}
}
@@ -1681,6 +1649,7 @@
unsigned childEndLine = coordinate.rows.resolvedFinalPosition.next().toInt();
LayoutUnit endOfRow = m_rowPositions[childEndLine];
LayoutUnit childBreadth = child.logicalHeight() + child.marginLogicalHeight();
+ // In order to properly adjust the Self Alignment values we need to consider the offset between tracks.
if (childEndLine - childStartLine > 1 && childEndLine < m_rowPositions.size() - 1)
endOfRow -= offsetBetweenTracks(style().resolvedAlignContentDistribution(), m_rowPositions, childBreadth);
LayoutUnit offsetFromStartPosition = computeOverflowAlignmentOffset(RenderStyle::resolveAlignmentOverflow(style(), child.style()), endOfRow - startOfRow, childBreadth);
@@ -1710,6 +1679,7 @@
unsigned childEndLine = coordinate.columns.resolvedFinalPosition.next().toInt();
LayoutUnit endOfColumn = m_columnPositions[childEndLine];
LayoutUnit childBreadth = child.logicalWidth() + child.marginLogicalWidth();
+ // In order to properly adjust the Self Alignment values we need to consider the offset between tracks.
if (childEndLine - childStartLine > 1 && childEndLine < m_columnPositions.size() - 1)
endOfColumn -= offsetBetweenTracks(style().resolvedJustifyContentDistribution(), m_columnPositions, childBreadth);
LayoutUnit offsetFromStartPosition = computeOverflowAlignmentOffset(RenderStyle::resolveJustificationOverflow(style(), child.style()), endOfColumn - startOfColumn, childBreadth);
