Changeset 174643 in webkit


Ignore:
Timestamp:
Oct 13, 2014, 1:44:24 AM (11 years ago)
Author:
svillar@igalia.com
Message:

[CSS Grid Layout] Pass the valid set of tracks to grow beyond growth limits
https://bugs.webkit.org/show_bug.cgi?id=137253

Reviewed by Darin Adler.

Source/WebCore:

Section 10.4 of the specs describe how to resolve content based
track sizing functions. Among others it describes the "distribute
extra space" algorithm. The 3rd bullet of that algorithm specifies
how to distribute (and also the target tracks) the extra space
once all the tracks have reached their growth limits.

Our implementation had 2 problems. First we were not passing a
valid subset of tracks (instead we were always using all of
them). Now we use a function that filters the right tracks to be
the target of the extra space distribution depending on whether
we're computing the min track function or the max track function.

Secondly the algorithm that was distributing the extra space was
not using that list of passed in tracks (it iterated over all of
them). From now on it will use the set of tracks selected using
the filter function described above.

  • rendering/RenderGrid.cpp:

(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctionsForItems):
(WebCore::RenderGrid::distributeSpaceToTracks):

  • rendering/RenderGrid.h:
  • rendering/style/GridTrackSize.h:

(WebCore::GridTrackSize::hasMinContentMinTrackBreadthAndMinOrMaxContentMaxTrackBreadth):
(WebCore::GridTrackSize::hasMaxContentMinTrackBreadthAndMaxContentMaxTrackBreadth):

LayoutTests:

  • fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt:
  • fast/css-grid-layout/grid-content-sized-columns-resolution.html:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r174640 r174643  
     12014-09-30  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [CSS Grid Layout] Pass the valid set of tracks to grow beyond growth limits
     4        https://bugs.webkit.org/show_bug.cgi?id=137253
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt:
     9        * fast/css-grid-layout/grid-content-sized-columns-resolution.html:
     10
    1112014-10-12  Mike West  <mkwst@chromium.org>
    212
  • trunk/LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution-expected.txt

    r174006 r174643  
    1111PASS window.getComputedStyle(gridMaxContentAndMinContentFixed, '').getPropertyValue('-webkit-grid-template-columns') is "70px 20px"
    1212PASS window.getComputedStyle(gridMaxContentAndMaxContentFixed, '').getPropertyValue('-webkit-grid-template-columns') is "55px 35px"
     13PASS window.getComputedStyle(gridMinContentFixedAndAutoAboveLimits, '').getPropertyValue('-webkit-grid-template-columns') is "15px 95px"
     14PASS window.getComputedStyle(gridMaxContentFixedAndAutoAboveLimits, '').getPropertyValue('-webkit-grid-template-columns') is "65px 85px"
     15PASS window.getComputedStyle(gridMinContentFixedAndFixedFixedAndAuto, '').getPropertyValue('-webkit-grid-template-columns') is "20px 20px 60px"
     16PASS window.getComputedStyle(gridAutoAndFixedFixedAndMaxContentFixed, '').getPropertyValue('-webkit-grid-template-columns') is "40px 20px 90px"
    1317PASS successfullyParsed is true
    1418
     
    2630XXXX XXXX
    2731XXXX XXXX
     32XXXX XXXX
     33XXXXXXXXXXX
     34XXXX XXXX
     35XXXXXXXXXXXXXXX
     36XXXX XXXX
     37XXXXXXXXXX
     38XXXX XXXX
     39XXXXXXXXXXXXXXX
  • trunk/LayoutTests/fast/css-grid-layout/grid-content-sized-columns-resolution.html

    r174006 r174643  
    4141.gridMaxContentAndMaxContentFixed {
    4242    -webkit-grid-template-columns: -webkit-max-content minmax(-webkit-max-content, 35px);
     43}
     44.gridMinContentFixedAndFixedFixedAndAuto {
     45    -webkit-grid-template-columns: minmax(-webkit-min-content, 20px) minmax(20px, 30px) auto;
     46}
     47.gridAutoAndFixedFixedAndMaxContentFixed {
     48    -webkit-grid-template-columns: auto minmax(20px, 30px) minmax(-webkit-max-content, 20px);
    4349}
    4450</style>
     
    98104</div>
    99105
     106<!-- The next four force the grid to grow only a particular subset of tracks above the limits -->
     107<div class="constrainedContainer">
     108    <div class="grid gridMinContentFixedAndAuto" id="gridMinContentFixedAndAutoAboveLimits">
     109        <div class="firstRowBothColumn">XXXX XXXX</div>
     110        <div class="firstRowBothColumn">XXXXXXXXXXX</div>
     111    </div>
     112</div>
     113
     114<div class="constrainedContainer">
     115    <div class="grid gridMaxContentFixedAndAuto" id="gridMaxContentFixedAndAutoAboveLimits">
     116        <div class="firstRowBothColumn">XXXX XXXX</div>
     117        <div class="firstRowBothColumn">XXXXXXXXXXXXXXX</div>
     118    </div>
     119</div>
     120
     121<div class="constrainedContainer">
     122    <div class="grid gridMinContentFixedAndFixedFixedAndAuto" id="gridMinContentFixedAndFixedFixedAndAuto">
     123        <div class="firstRowBothColumn">XXXX XXXX</div>
     124        <div class="firstRowBothColumn">XXXXXXXXXX</div>
     125    </div>
     126</div>
     127
     128<div class="constrainedContainer">
     129    <div class="grid gridAutoAndFixedFixedAndMaxContentFixed" id="gridAutoAndFixedFixedAndMaxContentFixed">
     130        <div class="firstRowBothColumn">XXXX XXXX</div>
     131        <div class="firstRowBothColumn">XXXXXXXXXXXXXXX</div>
     132    </div>
     133</div>
     134
    100135<script>
    101136function testGridColumnsValues(id, computedColumnValue)
     
    116151testGridColumnsValues("gridMaxContentAndMinContentFixed", "70px 20px");
    117152testGridColumnsValues("gridMaxContentAndMaxContentFixed", "55px 35px");
     153
     154testGridColumnsValues("gridMinContentFixedAndAutoAboveLimits", "15px 95px");
     155testGridColumnsValues("gridMaxContentFixedAndAutoAboveLimits", "65px 85px");
     156testGridColumnsValues("gridMinContentFixedAndFixedFixedAndAuto", "20px 20px 60px");
     157testGridColumnsValues("gridAutoAndFixedFixedAndMaxContentFixed", "40px 20px 90px");
    118158</script>
    119159</body>
  • trunk/Source/WebCore/ChangeLog

    r174640 r174643  
     12014-09-30  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [CSS Grid Layout] Pass the valid set of tracks to grow beyond growth limits
     4        https://bugs.webkit.org/show_bug.cgi?id=137253
     5
     6        Reviewed by Darin Adler.
     7
     8        Section 10.4 of the specs describe how to resolve content based
     9        track sizing functions. Among others it describes the "distribute
     10        extra space" algorithm. The 3rd bullet of that algorithm specifies
     11        how to distribute (and also the target tracks) the extra space
     12        once all the tracks have reached their growth limits.
     13
     14        Our implementation had 2 problems. First we were not passing a
     15        valid subset of tracks (instead we were always using all of
     16        them). Now we use a function that filters the right tracks to be
     17        the target of the extra space distribution depending on whether
     18        we're computing the min track function or the max track function.
     19
     20        Secondly the algorithm that was distributing the extra space was
     21        not using that list of passed in tracks (it iterated over all of
     22        them). From now on it will use the set of tracks selected using
     23        the filter function described above.
     24
     25        * rendering/RenderGrid.cpp:
     26        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions):
     27        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctionsForItems):
     28        (WebCore::RenderGrid::distributeSpaceToTracks):
     29        * rendering/RenderGrid.h:
     30        * rendering/style/GridTrackSize.h:
     31        (WebCore::GridTrackSize::hasMinContentMinTrackBreadthAndMinOrMaxContentMaxTrackBreadth):
     32        (WebCore::GridTrackSize::hasMaxContentMinTrackBreadthAndMaxContentMaxTrackBreadth):
     33
    1342014-10-12  Mike West  <mkwst@chromium.org>
    235
  • trunk/Source/WebCore/rendering/RenderGrid.cpp

    r174101 r174643  
    183183    Vector<LayoutUnit> distributeTrackVector;
    184184    Vector<GridTrack*> filteredTracks;
     185    Vector<size_t> growAboveMaxBreadthTrackIndexes;
    185186};
    186187
     
    569570
    570571        for (auto& itemWithSpan : itemsSortedByIncreasingSpan) {
    571             resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMinOrMaxContentMinTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
    572             resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMaxContentMinTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth);
     572            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMinOrMaxContentMinTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth, &GridTrackSize::hasMinContentMinTrackBreadthAndMinOrMaxContentMaxTrackBreadth);
     573            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMaxContentMinTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::usedBreadth, &GridTrack::growUsedBreadth, &GridTrackSize::hasMaxContentMinTrackBreadthAndMaxContentMaxTrackBreadth);
    573574            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMinOrMaxContentMaxTrackBreadth, &RenderGrid::minContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
    574575            resolveContentBasedTrackSizingFunctionsForItems(direction, sizingData, itemWithSpan, &GridTrackSize::hasMaxContentMaxTrackBreadth, &RenderGrid::maxContentForChild, &GridTrack::maxBreadthIfNotInfinite, &GridTrack::growMaxBreadth);
     
    581582}
    582583
    583 void RenderGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection direction, GridSizingData& sizingData, GridItemWithSpan& gridItemWithSpan, FilterFunction filterFunction, SizingFunction sizingFunction, AccumulatorGetter trackGetter, AccumulatorGrowFunction trackGrowthFunction)
     584void RenderGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection direction, GridSizingData& sizingData, GridItemWithSpan& gridItemWithSpan, FilterFunction filterFunction, SizingFunction sizingFunction, AccumulatorGetter trackGetter, AccumulatorGrowFunction trackGrowthFunction, FilterFunction growAboveMaxBreadthFilterFunction)
    584585{
    585586    const GridCoordinate& coordinate = gridItemWithSpan.coordinate();
     
    588589
    589590    sizingData.filteredTracks.shrink(0);
     591    sizingData.growAboveMaxBreadthTrackIndexes.shrink(0);
    590592    for (GridResolvedPosition trackIndex = initialTrackPosition; trackIndex <= finalTrackPosition; ++trackIndex) {
    591593        const GridTrackSize& trackSize = gridTrackSize(direction, trackIndex.toInt());
     
    595597        GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackIndex.toInt()] : sizingData.rowTracks[trackIndex.toInt()];
    596598        sizingData.filteredTracks.append(&track);
     599
     600        if (growAboveMaxBreadthFilterFunction && (trackSize.*growAboveMaxBreadthFilterFunction)())
     601            sizingData.growAboveMaxBreadthTrackIndexes.append(sizingData.filteredTracks.size() - 1);
    597602    }
    598603
     
    606611    }
    607612
    608     // FIXME: We should pass different values for |tracksForGrowthAboveMaxBreadth|.
    609613    // Specs mandate to floor additionalBreadthSpace (extra-space in specs) to 0. Instead we directly avoid the function
    610614    // call in those cases as it will be a noop in terms of track sizing.
    611615    if (additionalBreadthSpace > 0)
    612         distributeSpaceToTracks(sizingData.filteredTracks, &sizingData.filteredTracks, trackGetter, trackGrowthFunction, sizingData, additionalBreadthSpace);
     616        distributeSpaceToTracks(sizingData.filteredTracks, &sizingData.growAboveMaxBreadthTrackIndexes, trackGetter, trackGrowthFunction, sizingData, additionalBreadthSpace);
    613617}
    614618
     
    626630}
    627631
    628 void RenderGrid::distributeSpaceToTracks(Vector<GridTrack*>& tracks, Vector<GridTrack*>* tracksForGrowthAboveMaxBreadth, AccumulatorGetter trackGetter, AccumulatorGrowFunction trackGrowthFunction, GridSizingData& sizingData, LayoutUnit& availableLogicalSpace)
     632void RenderGrid::distributeSpaceToTracks(Vector<GridTrack*>& tracks, Vector<size_t>* growAboveMaxBreadthTrackIndexes, AccumulatorGetter trackGetter, AccumulatorGrowFunction trackGrowthFunction, GridSizingData& sizingData, LayoutUnit& availableLogicalSpace)
    629633{
    630634    ASSERT(availableLogicalSpace > 0);
     
    651655    }
    652656
    653     if (availableLogicalSpace > 0 && tracksForGrowthAboveMaxBreadth) {
    654         tracksSize = tracksForGrowthAboveMaxBreadth->size();
    655         for (size_t i = 0; i < tracksSize; ++i) {
    656             LayoutUnit growthShare = availableLogicalSpace / (tracksSize - i);
    657             sizingData.distributeTrackVector[i] += growthShare;
     657    if (availableLogicalSpace > 0 && growAboveMaxBreadthTrackIndexes) {
     658        size_t indexesSize = growAboveMaxBreadthTrackIndexes->size();
     659        size_t tracksGrowingAboveMaxBreadthSize = indexesSize ? indexesSize : tracksSize;
     660        // If we have a non-null empty vector of track indexes to grow above max breadth means that we should grow all
     661        // affected tracks.
     662        for (size_t i = 0; i < tracksGrowingAboveMaxBreadthSize; ++i) {
     663            LayoutUnit growthShare = availableLogicalSpace / (tracksGrowingAboveMaxBreadthSize - i);
     664            size_t distributeTrackIndex = indexesSize ? growAboveMaxBreadthTrackIndexes->at(i) : i;
     665            sizingData.distributeTrackVector[distributeTrackIndex] += growthShare;
    658666            availableLogicalSpace -= growthShare;
    659667        }
  • trunk/Source/WebCore/rendering/RenderGrid.h

    r174603 r174643  
    9292    typedef void (GridTrack::* AccumulatorGrowFunction)(LayoutUnit);
    9393    typedef bool (GridTrackSize::* FilterFunction)() const;
    94     void resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection, GridSizingData&, GridItemWithSpan&, FilterFunction, SizingFunction, AccumulatorGetter, AccumulatorGrowFunction);
    95     void distributeSpaceToTracks(Vector<GridTrack*>&, Vector<GridTrack*>* tracksForGrowthAboveMaxBreadth, AccumulatorGetter, AccumulatorGrowFunction, GridSizingData&, LayoutUnit& availableLogicalSpace);
     94    void resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection, GridSizingData&, GridItemWithSpan&, FilterFunction, SizingFunction, AccumulatorGetter, AccumulatorGrowFunction, FilterFunction growAboveMaxBreadthFilterFunction = nullptr);
     95    void distributeSpaceToTracks(Vector<GridTrack*>&, Vector<size_t>* growAboveMaxBreadthTrackIndexes, AccumulatorGetter, AccumulatorGrowFunction, GridSizingData&, LayoutUnit& availableLogicalSpace);
    9696
    9797    double computeNormalizedFractionBreadth(Vector<GridTrack>&, const GridSpan& tracksSpan, GridTrackSizingDirection, LayoutUnit availableLogicalSpace) const;
  • trunk/Source/WebCore/rendering/style/GridTrackSize.h

    r174057 r174643  
    116116    bool hasMinOrMaxContentMaxTrackBreadth() const { return maxTrackBreadth().isLength() && (maxTrackBreadth().length().isMinContent() || maxTrackBreadth().length().isMaxContent()); }
    117117    bool hasMaxContentMaxTrackBreadth() const { return maxTrackBreadth().isLength() && maxTrackBreadth().length().isMaxContent(); }
     118    bool hasMinContentMinTrackBreadthAndMinOrMaxContentMaxTrackBreadth() const { return minTrackBreadth().isLength() && minTrackBreadth().length().isMinContent() && hasMinOrMaxContentMaxTrackBreadth(); }
     119    bool hasMaxContentMinTrackBreadthAndMaxContentMaxTrackBreadth() const { return hasMaxContentMinTrackBreadth() && hasMaxContentMaxTrackBreadth(); }
     120
    118121
    119122private:
Note: See TracChangeset for help on using the changeset viewer.