Commit f67a8c17 authored by sergio@webkit.org's avatar sergio@webkit.org
Browse files

[CSS Grid Layout] Fix grid position resolution

https://bugs.webkit.org/show_bug.cgi?id=119801

Reviewed by Andreas Kling.

From Blink r148833, r148878, r150403 by <jchaffraix@chromium.org>

Source/WebCore:

Both grid-{column|row}-end and negative positions were not
properly handled in our grid position resolution code. We were
using the same code to resolve all the grid positions without
considering the edges of the grid.

Also refactored the grid size estimation in
resolveGridPositionsFromStyle() so we can use it for the grid size
estimation. The code no longer requires the grid to be filled at
that moment as the specs changed to use the "explicit grid" which
is independent of grid items (only depends on style).

Test: fast/css-grid-layout/grid-item-negative-position-resolution.html

* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::maximumIndexInDirection):
(WebCore::RenderGrid::resolveGridPositionsFromStyle):
(WebCore::adjustGridPositionForSide):
(WebCore::RenderGrid::resolveGridPositionFromStyle):
* rendering/RenderGrid.h:

LayoutTests:

Added a new test to check negative position resolution. Also added
several new test cases to check that we properly resolve grid
positions in the grid edges.

* fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt: Added.
* fast/css-grid-layout/grid-item-negative-position-resolution.html: Added.
* fast/css-grid-layout/grid-item-spanning-resolution-expected.txt:
* fast/css-grid-layout/grid-item-spanning-resolution.html:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@154731 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 0cfce663
2013-08-28 Sergio Villar Senin <svillar@igalia.com>
[CSS Grid Layout] Fix grid position resolution
https://bugs.webkit.org/show_bug.cgi?id=119801
Reviewed by Andreas Kling.
From Blink r148833, r148878, r150403 by <jchaffraix@chromium.org>
Added a new test to check negative position resolution. Also added
several new test cases to check that we properly resolve grid
positions in the grid edges.
* fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt: Added.
* fast/css-grid-layout/grid-item-negative-position-resolution.html: Added.
* fast/css-grid-layout/grid-item-spanning-resolution-expected.txt:
* fast/css-grid-layout/grid-item-spanning-resolution.html:
2013-08-28 Sergio Villar Senin <svillar@igalia.com>
 
[CSS Grid Layout] infinity should be defined as a negative value
Test that negative grid positions are correctly resolved.
PASS
PASS
PASS
PASS
<!DOCTYPE html>
<html>
<script>
if (window.testRunner)
testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
</script>
<link href="resources/grid.css" rel="stylesheet">
<style>
.grid {
-webkit-grid-definition-columns: 50px 100px;
-webkit-grid-definition-rows: 50px 100px;
/* To detect how much we extend the grid. */
-webkit-grid-auto-columns: 200px;
-webkit-grid-auto-rows: 200px;
/* Make the grid shrink-to-fit. */
position: absolute;
}
.negativeStartPositionGrowGridInColumnDirection {
-webkit-grid-column: -1 / auto;
-webkit-grid-row: 1;
}
.negativeStartPositionGrowGridInRowDirection {
-webkit-grid-column: 1;
-webkit-grid-row: -1 / auto;
}
.negativeEndPositionStartNegativeInColumnDirection {
-webkit-grid-column: -3 / -1;
-webkit-grid-row: 1;
}
.negativeEndPositionStartNegativeInRowDirection {
-webkit-grid-column: -5 / -2;
-webkit-grid-row: 1;
}
</style>
<script src="../../resources/check-layout.js"></script>
<body onload="checkLayout('.grid');">
<p>Test that negative grid positions are correctly resolved.</p>
<div style="position: relative">
<div class="grid" data-expected-width="350" data-expected-height="150">
<div class="sizedToGridArea negativeStartPositionGrowGridInColumnDirection" data-offset-x="150" data-offset-y="0" data-expected-width="200" data-expected-height="50"></div>
</div>
</div>
<div style="position: relative">
<div class="grid" data-expected-width="150" data-expected-height="350">
<div class="sizedToGridArea negativeStartPositionGrowGridInRowDirection" data-offset-x="0" data-offset-y="150" data-expected-width="50" data-expected-height="200"></div>
</div>
</div>
<div style="position: relative">
<div class="grid" data-expected-width="150" data-expected-height="150">
<div class="sizedToGridArea negativeEndPositionStartNegativeInColumnDirection" data-offset-x="0" data-offset-y="0" data-expected-width="150" data-expected-height="50"></div>
</div>
</div>
<div style="position: relative">
<div class="grid" data-expected-width="150" data-expected-height="150">
<div class="sizedToGridArea negativeEndPositionStartNegativeInRowDirection" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="50"></div>
</div>
</div>
</body>
</html>
......@@ -13,6 +13,13 @@ if (window.testRunner)
height: 300px;
}
#bigGrid {
-webkit-grid-definition-columns: 25% 25% 25% 25%;
-webkit-grid-definition-rows: 25% 25% 25% 25%;
height: 100px;
width: 200px;
}
.negativeOverflowRowFirstColumn {
-webkit-grid-row: 1 / -5;
-webkit-grid-column: 1;
......@@ -32,6 +39,16 @@ if (window.testRunner)
-webkit-grid-row: 1;
-webkit-grid-column: 1 / 5;
}
.secondRowSecondColumnNoSpan {
-webkit-grid-column: 2 / 3;
-webkit-grid-row: 2 / 3;
}
.thirdRowThirdColumnNoSpan {
-webkit-grid-column: 3 / 4;
-webkit-grid-row: 3 / 4;
}
</style>
<script src="../../resources/check-layout.js"></script>
<body onload="checkLayout('.grid');">
......@@ -100,9 +117,14 @@ if (window.testRunner)
<div style="position: relative">
<div class="grid" data-expected-width="400" data-expected-height="300">
<div class="sizedToGridArea autoSecondRowAutoFirstColumn" data-offset-x="0" data-offset-y="90" data-expected-width="160" data-expected-height="210"></div>
<div class="sizedToGridArea autoSecondRowAutoFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="160" data-expected-height="90"></div>
</div>
</div>
<div style="position: relative">
<div class="grid" id="bigGrid" data-expected-width="200" data-expected-height="100">
<div class="sizedToGridArea secondRowSecondColumnNoSpan" data-offset-x="50" data-offset-y="25" data-expected-width="50" data-expected-height="25"></div>
<div class="sizedToGridArea thirdRowThirdColumnNoSpan" data-offset-x="100" data-offset-y="50" data-expected-width="50" data-expected-height="25"></div>
</div>
</body>
</html>
2013-08-28 Sergio Villar Senin <svillar@igalia.com>
[CSS Grid Layout] Fix grid position resolution
https://bugs.webkit.org/show_bug.cgi?id=119801
Reviewed by Andreas Kling.
From Blink r148833, r148878, r150403 by <jchaffraix@chromium.org>
Both grid-{column|row}-end and negative positions were not
properly handled in our grid position resolution code. We were
using the same code to resolve all the grid positions without
considering the edges of the grid.
Also refactored the grid size estimation in
resolveGridPositionsFromStyle() so we can use it for the grid size
estimation. The code no longer requires the grid to be filled at
that moment as the specs changed to use the "explicit grid" which
is independent of grid items (only depends on style).
Test: fast/css-grid-layout/grid-item-negative-position-resolution.html
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::maximumIndexInDirection):
(WebCore::RenderGrid::resolveGridPositionsFromStyle):
(WebCore::adjustGridPositionForSide):
(WebCore::RenderGrid::resolveGridPositionFromStyle):
* rendering/RenderGrid.h:
2013-08-28 Sergio Villar Senin <svillar@igalia.com>
 
[CSS Grid Layout] infinity should be defined as a negative value
......@@ -327,16 +327,6 @@ const GridTrackSize& RenderGrid::gridTrackSize(TrackSizingDirection direction, s
return trackStyles[i];
}
static size_t estimatedGridSizeForPosition(const GridPosition& position)
{
if (position.isAuto())
return 1;
// Negative explicit values never grow the grid as they are clamped against
// the explicit grid's size. Thus we don't special case them here.
return std::max(position.integerPosition(), 1);
}
size_t RenderGrid::explicitGridColumnCount() const
{
return style()->gridColumns().size();
......@@ -352,19 +342,14 @@ size_t RenderGrid::maximumIndexInDirection(TrackSizingDirection direction) const
size_t maximumIndex = std::max<size_t>(1, (direction == ForColumns) ? explicitGridColumnCount() : explicitGridRowCount());
for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
// This function bypasses the cache (cachedGridCoordinate()) as it is used to build it.
// Also we can't call resolveGridPositionsFromStyle here as it assumes that the grid is build and we are in
// the middle of building it. However we should be able to share more code with the previous logic (FIXME).
const GridPosition& initialPosition = (direction == ForColumns) ? child->style()->gridItemColumnStart() : child->style()->gridItemRowStart();
const GridPosition& finalPosition = (direction == ForColumns) ? child->style()->gridItemColumnEnd() : child->style()->gridItemRowEnd();
OwnPtr<GridSpan> positions = resolveGridPositionsFromStyle(child, direction);
size_t estimatedSizeForInitialPosition = estimatedGridSizeForPosition(initialPosition);
size_t estimatedSizeForFinalPosition = estimatedGridSizeForPosition(finalPosition);
ASSERT(estimatedSizeForInitialPosition);
ASSERT(estimatedSizeForFinalPosition);
// |positions| is null if we need to run the auto-placement algorithm. Our estimation ignores
// this case as the auto-placement algorithm will grow the grid as needed.
if (!positions)
continue;
maximumIndex = std::max(maximumIndex, estimatedSizeForInitialPosition);
maximumIndex = std::max(maximumIndex, estimatedSizeForFinalPosition);
maximumIndex = std::max(maximumIndex, positions->finalPositionIndex + 1);
}
return maximumIndex;
......@@ -730,8 +715,6 @@ RenderGrid::GridSpan RenderGrid::resolveGridPositionsFromAutoPlacementPosition(c
PassOwnPtr<RenderGrid::GridSpan> RenderGrid::resolveGridPositionsFromStyle(const RenderBox* gridItem, TrackSizingDirection direction) const
{
ASSERT(gridWasPopulated());
const GridPosition& initialPosition = (direction == ForColumns) ? gridItem->style()->gridItemColumnStart() : gridItem->style()->gridItemRowStart();
const GridPositionSide initialPositionSide = (direction == ForColumns) ? ColumnStartSide : RowStartSide;
const GridPosition& finalPosition = (direction == ForColumns) ? gridItem->style()->gridItemColumnEnd() : gridItem->style()->gridItemRowEnd();
......@@ -767,26 +750,32 @@ PassOwnPtr<RenderGrid::GridSpan> RenderGrid::resolveGridPositionsFromStyle(const
return adoptPtr(new GridSpan(resolvedInitialPosition, resolvedFinalPosition));
}
size_t RenderGrid::resolveGridPositionFromStyle(const GridPosition& position, GridPositionSide side) const
static size_t adjustGridPositionForSide(size_t resolvedPosition, RenderGrid::GridPositionSide side)
{
ASSERT(gridWasPopulated());
// An item finishing on the N-th line belongs to the N-1-th cell.
if (side == RenderGrid::ColumnEndSide || side == RenderGrid::RowEndSide)
return resolvedPosition ? resolvedPosition - 1 : 0;
return resolvedPosition;
}
size_t RenderGrid::resolveGridPositionFromStyle(const GridPosition& position, GridPositionSide side) const
{
// FIXME: Handle other values for grid-{row,column} like ranges or line names.
switch (position.type()) {
case IntegerPosition: {
ASSERT(position.integerPosition());
if (position.isPositive())
return position.integerPosition() - 1;
return adjustGridPositionForSide(position.integerPosition() - 1, side);
size_t resolvedPosition = abs(position.integerPosition());
// FIXME: This returns one less than the expected result for side == ColumnStartSide or RowStartSide as we don't properly convert
// the grid line to its grid track. However this avoids the issue of growing the grid when inserting the item (e.g. -1 / auto).
size_t resolvedPosition = abs(position.integerPosition()) - 1;
const size_t endOfTrack = (side == ColumnStartSide || side == ColumnEndSide) ? explicitGridColumnCount() : explicitGridRowCount();
// Per http://lists.w3.org/Archives/Public/www-style/2013Mar/0589.html, we clamp negative value to the first line.
if (endOfTrack < resolvedPosition)
return 0;
return endOfTrack - resolvedPosition;
return adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
}
case AutoPosition:
// 'auto' depends on the opposite position for resolution (e.g. grid-row: auto / 1).
......
......@@ -44,6 +44,13 @@ public:
virtual bool avoidsFloats() const OVERRIDE { return true; }
virtual bool canCollapseAnonymousBlockChild() const OVERRIDE { return false; }
enum GridPositionSide {
ColumnStartSide,
ColumnEndSide,
RowStartSide,
RowEndSide
};
private:
virtual bool isRenderGrid() const OVERRIDE { return true; }
virtual void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const OVERRIDE;
......@@ -122,12 +129,6 @@ private:
GridSpan resolveGridPositionsFromAutoPlacementPosition(const RenderBox*, TrackSizingDirection, size_t) const;
PassOwnPtr<GridSpan> resolveGridPositionsFromStyle(const RenderBox*, TrackSizingDirection) const;
enum GridPositionSide {
ColumnStartSide,
ColumnEndSide,
RowStartSide,
RowEndSide
};
size_t resolveGridPositionFromStyle(const GridPosition&, GridPositionSide) const;
LayoutUnit gridAreaBreadthForChild(const RenderBox* child, TrackSizingDirection, const Vector<GridTrack>&) const;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment