Changeset 174006 in webkit

Timestamp:

Sep 26, 2014 6:41:57 AM (10 years ago)

Author:

svillar@igalia.com

Message:

[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.

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt (added)
• LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution.html (added)
• LayoutTests/fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/fast/css-grid-layout/grid-item-order-in-content-sized-columns-resolution-expected.txt

@@ -2 +2 @@
 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.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -308 +308 @@
         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())
@@ -560 +561 @@
     // 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;
@@ -578 +579 @@
         }
 
-        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;
@@ -618 +619 @@
 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);
 }
@@ -632 +641 @@
         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;