<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[190633] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/190633">190633</a></dd>
<dt>Author</dt> <dd>jfernandez@igalia.com</dd>
<dt>Date</dt> <dd>2015-10-06 12:23:52 -0700 (Tue, 06 Oct 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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.
Source/WebCore:
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.
LayoutTests:
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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderGridcpp">trunk/Source/WebCore/rendering/RenderGrid.cpp</a></li>
</ul>
<h3>Removed Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangeexpectedtxt">trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangehtml">trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (190632 => 190633)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-10-06 19:13:46 UTC (rev 190632)
+++ trunk/LayoutTests/ChangeLog 2015-10-06 19:23:52 UTC (rev 190633)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2015-10-06 Javier Fernandez <jfernandez@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.
+
</ins><span class="cx"> 2015-10-02 Jon Honeycutt <jhoneycutt@apple.com>
</span><span class="cx">
</span><span class="cx"> Import some Blink layout tests.
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangeexpectedtxt"></a>
<div class="delfile"><h4>Deleted: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt (190632 => 190633)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,4 +0,0 @@
</span><del>-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
</del></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangehtml"></a>
<div class="delfile"><h4>Deleted: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html (190632 => 190633)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,37 +0,0 @@
</span><del>-<!DOCTYPE HTML>
-<link href="resources/grid.css" rel="stylesheet">
-<script src="../../resources/check-layout.js"></script>
-<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>
</del></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (190632 => 190633)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+2015-10-06 Javier Fernandez <jfernandez@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.
+
</ins><span class="cx"> 2015-10-06 Tim Horton <timothy_horton@apple.com>
</span><span class="cx">
</span><span class="cx"> Tile map shows a green rect when threaded scrolling is disabled
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderGridcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (190632 => 190633)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1270,8 +1270,6 @@
</span><span class="cx"> || ((!oldOverrideContainingBlockContentLogicalHeight || oldOverrideContainingBlockContentLogicalHeight.value() != overrideContainingBlockContentLogicalHeight)
</span><span class="cx"> && child->hasRelativeLogicalHeight()))
</span><span class="cx"> child->setNeedsLayout(MarkOnlyThis);
</span><del>- else
- resetAutoMarginsAndLogicalTopInColumnAxis(*child);
</del><span class="cx">
</span><span class="cx"> child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
</span><span class="cx"> child->setOverrideContainingBlockContentLogicalHeight(overrideContainingBlockContentLogicalHeight);
</span><span class="lines">@@ -1412,9 +1410,24 @@
</span><span class="cx"> return isHorizontalWritingMode() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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;
+}
+
</ins><span class="cx"> LayoutUnit RenderGrid::availableAlignmentSpaceForChildBeforeStretching(LayoutUnit gridAreaBreadthForChild, const RenderBox& child) const
</span><span class="cx"> {
</span><del>- return gridAreaBreadthForChild - marginLogicalHeightForChild(child);
</del><ins>+ // 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));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
</span><span class="lines">@@ -1481,57 +1494,24 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
</span><del>-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.
</del><span class="cx"> void RenderGrid::updateAutoMarginsInRowAxisIfNeeded(RenderBox& child)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(!child.isOutOfFlowPositioned());
</span><del>- ASSERT(child.overrideContainingBlockContentLogicalWidth());
</del><span class="cx">
</span><span class="cx"> LayoutUnit availableAlignmentSpace = child.overrideContainingBlockContentLogicalWidth().value() - child.logicalWidth();
</span><span class="cx"> if (availableAlignmentSpace <= 0)
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- 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);
</del><ins>+ 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);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1539,33 +1519,21 @@
</span><span class="cx"> void RenderGrid::updateAutoMarginsInColumnAxisIfNeeded(RenderBox& child)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(!child.isOutOfFlowPositioned());
</span><del>- ASSERT(child.overrideContainingBlockContentLogicalHeight());
</del><span class="cx">
</span><span class="cx"> LayoutUnit availableAlignmentSpace = child.overrideContainingBlockContentLogicalHeight().value() - child.logicalHeight();
</span><span class="cx"> if (availableAlignmentSpace <= 0)
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- 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);
</del><ins>+ 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);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1681,6 +1649,7 @@
</span><span class="cx"> unsigned childEndLine = coordinate.rows.resolvedFinalPosition.next().toInt();
</span><span class="cx"> LayoutUnit endOfRow = m_rowPositions[childEndLine];
</span><span class="cx"> LayoutUnit childBreadth = child.logicalHeight() + child.marginLogicalHeight();
</span><ins>+ // In order to properly adjust the Self Alignment values we need to consider the offset between tracks.
</ins><span class="cx"> if (childEndLine - childStartLine > 1 && childEndLine < m_rowPositions.size() - 1)
</span><span class="cx"> endOfRow -= offsetBetweenTracks(style().resolvedAlignContentDistribution(), m_rowPositions, childBreadth);
</span><span class="cx"> LayoutUnit offsetFromStartPosition = computeOverflowAlignmentOffset(RenderStyle::resolveAlignmentOverflow(style(), child.style()), endOfRow - startOfRow, childBreadth);
</span><span class="lines">@@ -1710,6 +1679,7 @@
</span><span class="cx"> unsigned childEndLine = coordinate.columns.resolvedFinalPosition.next().toInt();
</span><span class="cx"> LayoutUnit endOfColumn = m_columnPositions[childEndLine];
</span><span class="cx"> LayoutUnit childBreadth = child.logicalWidth() + child.marginLogicalWidth();
</span><ins>+ // In order to properly adjust the Self Alignment values we need to consider the offset between tracks.
</ins><span class="cx"> if (childEndLine - childStartLine > 1 && childEndLine < m_columnPositions.size() - 1)
</span><span class="cx"> endOfColumn -= offsetBetweenTracks(style().resolvedJustifyContentDistribution(), m_columnPositions, childBreadth);
</span><span class="cx"> LayoutUnit offsetFromStartPosition = computeOverflowAlignmentOffset(RenderStyle::resolveJustificationOverflow(style(), child.style()), endOfColumn - startOfColumn, childBreadth);
</span></span></pre>
</div>
</div>
</body>
</html>