[CSS Grid Layout] Fix the handling of infinity in track growth limits
authorsvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Sep 2014 13:41:57 +0000 (13:41 +0000)
committersvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Sep 2014 13:41:57 +0000 (13:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=137019

Reviewed by Darin Adler.

Source/WebCore:

The growth limit of content sized tracks is initialized to
infinity which is internally represented as -1. We were not
specialcasing this situation, and thus, -1 was used in the
computations as any other value. This change makes the code aware
of the existence of infinites (like when sorting tracks by growth
potential or when initializing the track growth limits).

There was another bug related to infinities. The code that was
replacing a infinite growth limit by a finite one was not using
the proper indexes so the tracks that were being updated were the
wrong ones.

Test: fast/css-grid-layout/grid-content-sized-columns-resolution.html

* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::computeUsedBreadthOfGridTracks):
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
(WebCore::sortByGridTrackGrowthPotential):
(WebCore::RenderGrid::distributeSpaceToTracks):

LayoutTests:

* fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt: Added.
* fast/css-grid-layout/grid-content-sized-columns-resolution.html: Added.
* fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174006 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGrid.cpp

index 1622582..f7b308a 100644 (file)
@@ -1,3 +1,14 @@
+2014-09-23  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Fix the handling of infinity in track growth limits
+        https://bugs.webkit.org/show_bug.cgi?id=137019
+
+        Reviewed by Darin Adler.
+
+        * fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt: Added.
+        * fast/css-grid-layout/grid-content-sized-columns-resolution.html: Added.
+        * fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution-expected.txt:
+
 2014-09-26  Lorenzo Tilve  <ltilve@igalia.com>
 
         [GTK] Fix support for the initial-letter CSS property to first-letter
diff --git a/LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt b/LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt
new file mode 100644 (file)
index 0000000..66fbf44
--- /dev/null
@@ -0,0 +1,27 @@
+PASS window.getComputedStyle(gridMinContentFixedAndAuto, '').getPropertyValue('-webkit-grid-template-columns') is "15px 75px"
+PASS window.getComputedStyle(gridAutoAndAuto, '').getPropertyValue('-webkit-grid-template-columns') is "45px 45px"
+PASS window.getComputedStyle(gridMinContentAndMinContentFixed, '').getPropertyValue('-webkit-grid-template-columns') is "20px 30px"
+PASS window.getComputedStyle(gridMaxContentAndMinContent, '').getPropertyValue('-webkit-grid-template-columns') is "70px 20px"
+PASS window.getComputedStyle(gridFixedMinContentAndMaxContent, '').getPropertyValue('-webkit-grid-template-columns') is "10px 80px"
+PASS window.getComputedStyle(gridFixedMaxContentAndMinContent, '').getPropertyValue('-webkit-grid-template-columns') is "60px 30px"
+PASS window.getComputedStyle(gridMinContentAndMaxContentFixed, '').getPropertyValue('-webkit-grid-template-columns') is "20px 70px"
+PASS window.getComputedStyle(gridMaxContentFixedAndAuto, '').getPropertyValue('-webkit-grid-template-columns') is "65px 25px"
+PASS window.getComputedStyle(gridAutoMinContent, '').getPropertyValue('-webkit-grid-template-columns') is "70px 20px"
+PASS window.getComputedStyle(gridAutoMaxContent, '').getPropertyValue('-webkit-grid-template-columns') is "20px 70px"
+PASS window.getComputedStyle(gridMaxContentAndMinContentFixed, '').getPropertyValue('-webkit-grid-template-columns') is "70px 20px"
+PASS window.getComputedStyle(gridMaxContentAndMaxContentFixed, '').getPropertyValue('-webkit-grid-template-columns') is "55px 35px"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
+XXXX XXXX
diff --git a/LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution.html b/LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution.html
new file mode 100644 (file)
index 0000000..9cd0743
--- /dev/null
@@ -0,0 +1,120 @@
+<!DOCTYPE html>
+<head>
+<link href="resources/grid.css" rel="stylesheet"/>
+<style>
+.grid {
+    font: 10px/1 Ahem;
+}
+.gridMinContentFixedAndAuto {
+    -webkit-grid-template-columns: minmax(-webkit-min-content, 15px) auto;
+}
+.gridMaxContentFixedAndAuto {
+    -webkit-grid-template-columns: minmax(-webkit-max-content, 15px) auto;
+}
+.gridAutoAndAuto {
+    -webkit-grid-template-columns: auto auto;
+}
+.gridMinContentAndMinContentFixed {
+    -webkit-grid-template-columns: -webkit-min-content minmax(-webkit-min-content, 30px);
+}
+.gridMinContentAndMaxContentFixed {
+    -webkit-grid-template-columns: -webkit-min-content minmax(-webkit-max-content, 30px);
+}
+.gridMaxContentAndMinContent {
+    -webkit-grid-template-columns: -webkit-max-content -webkit-min-content;
+}
+.gridFixedMinContentAndMaxContent {
+    -webkit-grid-template-columns: minmax(10px, -webkit-min-content) -webkit-max-content;
+}
+.gridFixedMaxContentAndMinContent {
+    -webkit-grid-template-columns: minmax(10px, -webkit-max-content) -webkit-min-content;
+}
+.gridAutoMinContent {
+    -webkit-grid-template-columns: auto -webkit-min-content;
+}
+.gridAutoMaxContent {
+    -webkit-grid-template-columns: auto -webkit-max-content;
+}
+.gridMaxContentAndMinContentFixed {
+    -webkit-grid-template-columns: -webkit-max-content minmax(-webkit-min-content, 35px);
+}
+.gridMaxContentAndMaxContentFixed {
+    -webkit-grid-template-columns: -webkit-max-content minmax(-webkit-max-content, 35px);
+}
+</style>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<div class="grid gridMinContentFixedAndAuto" id="gridMinContentFixedAndAuto">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="grid gridAutoAndAuto" id="gridAutoAndAuto">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="grid gridMinContentAndMinContentFixed" id="gridMinContentAndMinContentFixed">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="grid gridMaxContentAndMinContent" id="gridMaxContentAndMinContent">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="grid gridFixedMinContentAndMaxContent" id="gridFixedMinContentAndMaxContent">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="grid gridFixedMaxContentAndMinContent" id="gridFixedMaxContentAndMinContent">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="grid gridMinContentAndMaxContentFixed" id="gridMinContentAndMaxContentFixed">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="constrainedContainer">
+    <div class="grid gridMaxContentFixedAndAuto" id="gridMaxContentFixedAndAuto">
+       <div class="firstRowBothColumn">XXXX XXXX</div>
+    </div>
+</div>
+
+<div class="grid gridAutoMinContent" id="gridAutoMinContent">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="grid gridAutoMaxContent" id="gridAutoMaxContent">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<div class="constrainedContainer">
+    <div class="grid gridMaxContentAndMinContentFixed" id="gridMaxContentAndMinContentFixed">
+       <div class="firstRowBothColumn">XXXX XXXX</div>
+    </div>
+</div>
+
+<div class="grid gridMaxContentAndMaxContentFixed" id="gridMaxContentAndMaxContentFixed">
+    <div class="firstRowBothColumn">XXXX XXXX</div>
+</div>
+
+<script>
+function testGridColumnsValues(id, computedColumnValue)
+{
+    shouldBeEqualToString("window.getComputedStyle(" + id + ", '').getPropertyValue('-webkit-grid-template-columns')", computedColumnValue);
+}
+
+testGridColumnsValues("gridMinContentFixedAndAuto", "15px 75px");
+testGridColumnsValues("gridAutoAndAuto", "45px 45px");
+testGridColumnsValues("gridMinContentAndMinContentFixed", "20px 30px");
+testGridColumnsValues("gridMaxContentAndMinContent", "70px 20px");
+testGridColumnsValues("gridFixedMinContentAndMaxContent", "10px 80px");
+testGridColumnsValues("gridFixedMaxContentAndMinContent", "60px 30px");
+testGridColumnsValues("gridMinContentAndMaxContentFixed", "20px 70px");
+testGridColumnsValues("gridMaxContentFixedAndAuto", "65px 25px");
+testGridColumnsValues("gridAutoMinContent", "70px 20px");
+testGridColumnsValues("gridAutoMaxContent", "20px 70px");
+testGridColumnsValues("gridMaxContentAndMinContentFixed", "70px 20px");
+testGridColumnsValues("gridMaxContentAndMaxContentFixed", "55px 35px");
+</script>
+</body>
+</html>
index 77753cc..6509835 100644 (file)
@@ -1,9 +1,9 @@
 PASS grid-template-columns is 60px 20px after grid row swap.
 PASS grid-template-columns is 76px 50px after grid row swap.
 PASS grid-template-columns is 60px 16px after grid row swap.
-PASS grid-template-columns is 68px 16px after grid row swap.
-PASS grid-template-columns is 76px 8px after grid row swap.
-PASS grid-template-columns is 68px 16px after grid row swap.
+PASS grid-template-columns is 60px 16px after grid row swap.
+PASS grid-template-columns is 76px 0px after grid row swap.
+PASS grid-template-columns is 60px 16px after grid row swap.
 PASS grid-template-columns is 60px 16px after grid row swap.
 PASS grid-template-columns is 60px 16px after grid row swap.
 PASS successfullyParsed is true
index 29c8b2e..0edcee2 100644 (file)
@@ -1,3 +1,30 @@
+2014-09-23  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Fix the handling of infinity in track growth limits
+        https://bugs.webkit.org/show_bug.cgi?id=137019
+
+        Reviewed by Darin Adler.
+
+        The growth limit of content sized tracks is initialized to
+        infinity which is internally represented as -1. We were not
+        specialcasing this situation, and thus, -1 was used in the
+        computations as any other value. This change makes the code aware
+        of the existence of infinites (like when sorting tracks by growth
+        potential or when initializing the track growth limits).
+
+        There was another bug related to infinities. The code that was
+        replacing a infinite growth limit by a finite one was not using
+        the proper indexes so the tracks that were being updated were the
+        wrong ones.
+
+        Test: fast/css-grid-layout/grid-content-sized-columns-resolution.html
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::computeUsedBreadthOfGridTracks):
+        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
+        (WebCore::sortByGridTrackGrowthPotential):
+        (WebCore::RenderGrid::distributeSpaceToTracks):
+
 2014-09-26  Lorenzo Tilve  <ltilve@igalia.com>
 
         [GTK] Fix support for the initial-letter CSS property to first-letter
index e51f16b..616cb03 100644 (file)
@@ -307,7 +307,8 @@ void RenderGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection directi
         track.m_usedBreadth = computeUsedBreadthOfMinLength(direction, minTrackBreadth);
         track.m_maxBreadth = computeUsedBreadthOfMaxLength(direction, maxTrackBreadth, track.m_usedBreadth);
 
-        track.m_maxBreadth = std::max(track.m_maxBreadth, track.m_usedBreadth);
+        if (track.m_maxBreadth != infinity)
+            track.m_maxBreadth = std::max(track.m_maxBreadth, track.m_usedBreadth);
 
         if (trackSize.isContentSized())
             sizingData.contentSizedTracksIndex.append(i);
@@ -559,8 +560,8 @@ void RenderGrid::resolveContentBasedTrackSizingFunctions(GridTrackSizingDirectio
 {
     // FIXME: Split the grid tracks into groups that doesn't overlap a <flex> grid track.
 
-    for (size_t i = 0; i < sizingData.contentSizedTracksIndex.size(); ++i) {
-        GridIterator iterator(m_grid, direction, sizingData.contentSizedTracksIndex[i]);
+    for (auto trackIndex : sizingData.contentSizedTracksIndex) {
+        GridIterator iterator(m_grid, direction, trackIndex);
         HashSet<RenderBox*> itemsSet;
         Vector<GridItemWithSpan> itemsSortedByIncreasingSpan;
 
@@ -577,7 +578,7 @@ void RenderGrid::resolveContentBasedTrackSizingFunctions(GridTrackSizingDirectio
             resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMaxContentMaxTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
         }
 
-        GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[i] : sizingData.rowTracks[i];
+        GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackIndex] : sizingData.rowTracks[trackIndex];
         if (track.m_maxBreadth == infinity)
             track.m_maxBreadth = track.m_usedBreadth;
     }
@@ -617,6 +618,14 @@ void RenderGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizing
 
 static bool sortByGridTrackGrowthPotential(const GridTrack* track1, const GridTrack* track2)
 {
+    // This check ensures that we respect the irreflexivity property of the strict weak ordering required by std::sort
+    // (forall x: NOT x < x).
+    if (track1->m_maxBreadth == infinity && track2->m_maxBreadth == infinity)
+        return false;
+
+    if (track1->m_maxBreadth == infinity || track2->m_maxBreadth == infinity)
+        return track2->m_maxBreadth == infinity;
+
     return (track1->m_maxBreadth - track1->m_usedBreadth) < (track2->m_maxBreadth - track2->m_usedBreadth);
 }
 
@@ -631,11 +640,14 @@ void RenderGrid::distributeSpaceToTracks(Vector<GridTrack*>& tracks, Vector<Grid
     for (size_t i = 0; i < tracksSize; ++i) {
         GridTrack& track = *tracks[i];
         LayoutUnit trackBreadth = (tracks[i]->*trackGetter)();
-        LayoutUnit trackGrowthPotential = track.m_maxBreadth - trackBreadth;
+        bool infiniteGrowthPotential = track.m_maxBreadth == infinity;
+        LayoutUnit trackGrowthPotential = infiniteGrowthPotential ? track.m_maxBreadth : track.m_maxBreadth - trackBreadth;
         sizingData.distributeTrackVector[i] = trackBreadth;
-        if (trackGrowthPotential > 0) {
+        // Let's avoid computing availableLogicalSpaceShare as much as possible as it's a hot spot in performance tests.
+        if (trackGrowthPotential > 0 || infiniteGrowthPotential) {
             LayoutUnit availableLogicalSpaceShare = availableLogicalSpace / (tracksSize - i);
-            LayoutUnit growthShare = std::min(availableLogicalSpaceShare, trackGrowthPotential);
+            LayoutUnit growthShare = infiniteGrowthPotential ? availableLogicalSpaceShare : std::min(availableLogicalSpaceShare, trackGrowthPotential);
+            ASSERT(growthShare > 0);
             // We should never shrink any grid track or else we can't guarantee we abide by our min-sizing function.
             sizingData.distributeTrackVector[i] += growthShare;
             availableLogicalSpace -= growthShare;