Changeset 201510 in webkit


Ignore:
Timestamp:
May 31, 2016 8:17:03 AM (8 years ago)
Author:
svillar@igalia.com
Message:

[css-grid] Empty grid without explicit tracks shouldn't have any size
​https://bugs.webkit.org/show_bug.cgi?id=155197

Reviewed by Darin Adler.

Source/WebCore:

The internal representation of the grid is a Vector of Vector representing rows and
columns. Because of that it was not possible to have columns without having at least one
row. That forced us to have a 1x1 internal representation of the grid even if it was
actually empty. That works for most of the cases except when the grid is actually empty.

By changing the way we compute the sizes we can overcome that implementation
restriction. This allowed us also to thighten the conditions under we could use the
GridIterator. From now on it won't be possible to use it on empty grids so callers should
enforce that restriction.

A new bool was added to verify that placeItemsOnGrid() has been already called. The previous
code was relying on the fact that there were items in the internal representation, which is
wrong, as there might be no items in the grid.

Test: fast/css-grid-layout/empty-grid.html

  • rendering/RenderGrid.cpp:

(WebCore::RenderGrid::GridIterator::GridIterator): Added ASSERTs.
(WebCore::RenderGrid::GridIterator::nextGridItem): Ditto.
(WebCore::RenderGrid::GridIterator::isEmptyAreaEnough): Ditto.
(WebCore::RenderGrid::GridIterator::nextEmptyGridArea): Ditto.
(WebCore::RenderGrid::gridColumnCount): Use the style to resolve the number of columns if
the internal representation is empty.
(WebCore::RenderGrid::gridRowCount):
(WebCore::RenderGrid::guttersSize): Allow to pass 0 as span, this permits using the return
value of gridColumnCount|gridRowCount directly to call this method.
(WebCore::RenderGrid::computeIntrinsicLogicalWidths): Use m_gridIsDirty.
(WebCore::RenderGrid::computeUsedBreadthOfGridTracks): Do not examine the contents of grid
tracks if there are no items in the grid.
(WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions): Ditto.
(WebCore::RenderGrid::placeItemsOnGrid): Set m_gridIsDirty to false.
(WebCore::RenderGrid::populateExplicitGridAndOrderIterator):
(WebCore::RenderGrid::clearGrid):
(WebCore::RenderGrid::populateGridPositionsForDirection):

  • rendering/RenderGrid.h: Moved gridColumnCount/gridRowCount to cpp file.

LayoutTests:

Make sure that empty grids (and grids with one empty axis) are properly handled. Do also
verify that removing all the items from a grid also generates an correct empty grid.

  • fast/css-grid-layout/empty-grid-expected.txt: Added.
  • fast/css-grid-layout/empty-grid.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r201501 r201510  
     12016-05-25  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-grid] Empty grid without explicit tracks shouldn't have any size
     4        https://bugs.webkit.org/show_bug.cgi?id=155197
     5
     6        Reviewed by Darin Adler.
     7
     8        Make sure that empty grids (and grids with one empty axis) are properly handled. Do also
     9        verify that removing all the items from a grid also generates an correct empty grid.
     10
     11        * fast/css-grid-layout/empty-grid-expected.txt: Added.
     12        * fast/css-grid-layout/empty-grid.html: Added.
     13
    1142016-05-30  Per Arne Vollan  <pvollan@apple.com>
    215

  • trunk/Source/WebCore/ChangeLog

    r201504 r201510  
     12016-05-25  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-grid] Empty grid without explicit tracks shouldn't have any size
     4        https://bugs.webkit.org/show_bug.cgi?id=155197
     5
     6        Reviewed by Darin Adler.
     7
     8        The internal representation of the grid is a Vector of Vector representing rows and
     9        columns. Because of that it was not possible to have columns without having at least one
     10        row. That forced us to have a 1x1 internal representation of the grid even if it was
     11        actually empty. That works for most of the cases except when the grid is actually empty.
     12
     13        By changing the way we compute the sizes we can overcome that implementation
     14        restriction. This allowed us also to thighten the conditions under we could use the
     15        GridIterator. From now on it won't be possible to use it on empty grids so callers should
     16        enforce that restriction.
     17
     18        A new bool was added to verify that placeItemsOnGrid() has been already called. The previous
     19        code was relying on the fact that there were items in the internal representation, which is
     20        wrong, as there might be no items in the grid.
     21
     22        Test: fast/css-grid-layout/empty-grid.html
     23
     24        * rendering/RenderGrid.cpp:
     25        (WebCore::RenderGrid::GridIterator::GridIterator): Added ASSERTs.
     26        (WebCore::RenderGrid::GridIterator::nextGridItem): Ditto.
     27        (WebCore::RenderGrid::GridIterator::isEmptyAreaEnough): Ditto.
     28        (WebCore::RenderGrid::GridIterator::nextEmptyGridArea): Ditto.
     29        (WebCore::RenderGrid::gridColumnCount): Use the style to resolve the number of columns if
     30        the internal representation is empty.
     31        (WebCore::RenderGrid::gridRowCount):
     32        (WebCore::RenderGrid::guttersSize): Allow to pass 0 as span, this permits using the return
     33        value of gridColumnCount|gridRowCount directly to call this method.
     34        (WebCore::RenderGrid::computeIntrinsicLogicalWidths): Use m_gridIsDirty.
     35        (WebCore::RenderGrid::computeUsedBreadthOfGridTracks): Do not examine the contents of grid
     36        tracks if there are no items in the grid.
     37        (WebCore::RenderGrid::resolveContentBasedTrackSizingFunctions): Ditto.
     38        (WebCore::RenderGrid::placeItemsOnGrid): Set m_gridIsDirty to false.
     39        (WebCore::RenderGrid::populateExplicitGridAndOrderIterator):
     40        (WebCore::RenderGrid::clearGrid):
     41        (WebCore::RenderGrid::populateGridPositionsForDirection):
     42        * rendering/RenderGrid.h: Moved gridColumnCount/gridRowCount to cpp file.
     43
    1442016-05-30  Brady Eidson  <beidson@apple.com>
    245

  • trunk/Source/WebCore/rendering/RenderGrid.cpp

    r201498 r201510  
    140140        , m_childIndex(0)
    141141    {
     142        ASSERT(!m_grid.isEmpty());
     143        ASSERT(!m_grid[0].isEmpty());
    142144        ASSERT(m_rowIndex < m_grid.size());
    143145        ASSERT(m_columnIndex < m_grid[0].size());
    … …  
    146148    RenderBox* nextGridItem()
    147149    {
    148         if (!m_grid.size())
    149             return 0;
     150        ASSERT(!m_grid.isEmpty());
     151        ASSERT(!m_grid[0].isEmpty());
    150152
    151153        unsigned& varyingTrackIndex = (m_direction == ForColumns) ? m_rowIndex : m_columnIndex;
    … …  
    163165    bool isEmptyAreaEnough(unsigned rowSpan, unsigned columnSpan) const
    164166    {
     167        ASSERT(!m_grid.isEmpty());
     168        ASSERT(!m_grid[0].isEmpty());
     169
    165170        // Ignore cells outside current grid as we will grow it later if needed.
    166171        unsigned maxRows = std::min<unsigned>(m_rowIndex + rowSpan, m_grid.size());
    … …  
    181186    std::unique_ptr<GridArea> nextEmptyGridArea(unsigned fixedTrackSpan, unsigned varyingTrackSpan)
    182187    {
    183         ASSERT(fixedTrackSpan >= 1 && varyingTrackSpan >= 1);
     188        ASSERT(!m_grid.isEmpty());
     189        ASSERT(!m_grid[0].isEmpty());
     190        ASSERT(fixedTrackSpan >= 1);
     191        ASSERT(varyingTrackSpan >= 1);
    184192
    185193        if (m_grid.isEmpty())
    … …  
    320328}
    321329
     330unsigned RenderGrid::gridColumnCount() const
     331{
     332    ASSERT(!m_gridIsDirty);
     333    // Due to limitations in our internal representation, we cannot know the number of columns from
     334    // m_grid *if* there is no row (because m_grid would be empty). That's why in that case we need
     335    // to get it from the style. Note that we know for sure that there are't any implicit tracks,
     336    // because not having rows implies that there are no "normal" children (out-of-flow children are
     337    // not stored in m_grid).
     338    return m_grid.size() ? m_grid[0].size() : GridPositionsResolver::explicitGridColumnCount(style(), m_autoRepeatColumns);
     339}
     340
     341unsigned RenderGrid::gridRowCount() const
     342{
     343    ASSERT(!m_gridIsDirty);
     344    return m_grid.size();
     345}
     346
    322347LayoutUnit RenderGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizingData) const
    323348{
    … …  
    421446LayoutUnit RenderGrid::guttersSize(GridTrackSizingDirection direction, unsigned span) const
    422447{
    423     ASSERT(span >= 1);
    424 
    425     if (span == 1)
     448    if (span <= 1)
    426449        return { };
    427450
    … …  
    437460void RenderGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
    438461{
    439     bool wasPopulated = gridWasPopulated();
     462    bool wasPopulated = !m_gridIsDirty;
    440463    if (!wasPopulated)
    441464        const_cast<RenderGrid*>(this)->placeItemsOnGrid();
    … …  
    590613            flexFraction = std::max(flexFraction, normalizedFlexFraction(tracks[trackIndex], gridTrackSize(direction, trackIndex).maxTrackBreadth().flex()));
    591614
    592         for (unsigned i = 0; i < flexibleSizedTracksIndex.size(); ++i) {
    593             GridIterator iterator(m_grid, direction, flexibleSizedTracksIndex[i]);
    594             while (RenderBox* gridItem = iterator.nextGridItem()) {
    595                 const GridSpan span = cachedGridSpan(*gridItem, direction);
    596 
    597                 // Do not include already processed items.
    598                 if (i > 0 && span.startLine() <= flexibleSizedTracksIndex[i - 1])
    599                     continue;
    600 
    601                 flexFraction = std::max(flexFraction, findFlexFactorUnitSize(tracks, span, direction, maxContentForChild(*gridItem, direction, sizingData)));
     615        if (!m_gridItemArea.isEmpty()) {
     616            for (unsigned i = 0; i < flexibleSizedTracksIndex.size(); ++i) {
     617                GridIterator iterator(m_grid, direction, flexibleSizedTracksIndex[i]);
     618                while (auto* gridItem = iterator.nextGridItem()) {
     619                    GridSpan& span = cachedGridSpan(*gridItem, direction);
     620
     621                    // Do not include already processed items.
     622                    if (i > 0 && span.startLine() <= flexibleSizedTracksIndex[i - 1])
     623                        continue;
     624
     625                    flexFraction = std::max(flexFraction, findFlexFactorUnitSize(tracks, span, direction, maxContentForChild(*gridItem, direction, sizingData)));
     626                }
    602627            }
    603628        }
    … …  
    899924    sizingData.itemsSortedByIncreasingSpan.shrink(0);
    900925    HashSet<RenderBox*> itemsSet;
    901     for (auto trackIndex : sizingData.contentSizedTracksIndex) {
    902         GridIterator iterator(m_grid, direction, trackIndex);
    903         GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackIndex] : sizingData.rowTracks[trackIndex];
    904 
    905         while (RenderBox* gridItem = iterator.nextGridItem()) {
    906             if (itemsSet.add(gridItem).isNewEntry) {
    907                 const GridSpan& span = cachedGridSpan(*gridItem, direction);
    908                 if (span.integerSpan() == 1)
    909                     resolveContentBasedTrackSizingFunctionsForNonSpanningItems(direction, span, *gridItem, track, sizingData);
    910                 else if (!spanningItemCrossesFlexibleSizedTracks(span, direction))
    911                     sizingData.itemsSortedByIncreasingSpan.append(GridItemWithSpan(*gridItem, span));
     926    if (!m_gridItemArea.isEmpty()) {
     927        for (auto trackIndex : sizingData.contentSizedTracksIndex) {
     928            GridIterator iterator(m_grid, direction, trackIndex);
     929            GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackIndex] : sizingData.rowTracks[trackIndex];
     930
     931            while (auto* gridItem = iterator.nextGridItem()) {
     932                if (itemsSet.add(gridItem).isNewEntry) {
     933                    GridSpan& span = cachedGridSpan(*gridItem, direction);
     934                    if (span.integerSpan() == 1)
     935                        resolveContentBasedTrackSizingFunctionsForNonSpanningItems(direction, span, *gridItem, track, sizingData);
     936                    else if (!spanningItemCrossesFlexibleSizedTracks(span, direction))
     937                        sizingData.itemsSortedByIncreasingSpan.append(GridItemWithSpan(*gridItem, span));
     938                }
    912939            }
    913940        }
    914     }
    915     std::sort(sizingData.itemsSortedByIncreasingSpan.begin(), sizingData.itemsSortedByIncreasingSpan.end());
     941        std::sort(sizingData.itemsSortedByIncreasingSpan.begin(), sizingData.itemsSortedByIncreasingSpan.end());
     942    }
    916943
    917944    auto it = sizingData.itemsSortedByIncreasingSpan.begin();
    … …  
    12961323void RenderGrid::placeItemsOnGrid()
    12971324{
    1298     ASSERT(!gridWasPopulated());
     1325    ASSERT(m_gridIsDirty);
    12991326    ASSERT(m_gridItemArea.isEmpty());
    13001327
    … …  
    13031330
    13041331    populateExplicitGridAndOrderIterator();
     1332    m_gridIsDirty = false;
    13051333
    13061334    Vector<RenderBox*> autoMajorAxisAutoGridItems;
    … …  
    13501378    OrderIteratorPopulator populator(m_orderIterator);
    13511379    m_smallestRowStart = m_smallestColumnStart = 0;
    1352     unsigned maximumRowIndex = std::max<unsigned>(1, GridPositionsResolver::explicitGridRowCount(style(), m_autoRepeatRows));
    1353     unsigned maximumColumnIndex = std::max<unsigned>(1, GridPositionsResolver::explicitGridColumnCount(style(), m_autoRepeatColumns));
     1380    unsigned maximumRowIndex = GridPositionsResolver::explicitGridRowCount(style(), m_autoRepeatRows);
     1381    unsigned maximumColumnIndex = GridPositionsResolver::explicitGridColumnCount(style(), m_autoRepeatColumns);
    13541382
    13551383    for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
    … …  
    15111539    m_grid.clear();
    15121540    m_gridItemArea.clear();
     1541    m_gridIsDirty = true;
    15131542}
    15141543
    … …  
    17681797    unsigned numberOfLines = numberOfTracks + 1;
    17691798    unsigned lastLine = numberOfLines - 1;
    1770     unsigned nextToLastLine = numberOfLines - 2;
     1799
    17711800    ContentAlignmentData offset = computeContentPositionAndDistributionOffset(direction, sizingData.freeSpaceForDirection(direction).value(), numberOfTracks);
    17721801    LayoutUnit trackGap = guttersSize(direction, 2);
    … …  
    17751804    auto borderAndPadding = isRowAxis ? borderAndPaddingLogicalLeft() : borderAndPaddingBefore();
    17761805    positions[0] = borderAndPadding + offset.positionOffset;
    1777     for (unsigned i = 0; i < nextToLastLine; ++i)
    1778         positions[i + 1] = positions[i] + offset.distributionOffset + tracks[i].baseSize() + trackGap;
    1779     positions[lastLine] = positions[nextToLastLine] + tracks[nextToLastLine].baseSize();
     1806    if (numberOfLines > 1) {
     1807        unsigned nextToLastLine = numberOfLines - 2;
     1808        for (unsigned i = 0; i < nextToLastLine; ++i)
     1809            positions[i + 1] = positions[i] + offset.distributionOffset + tracks[i].baseSize() + trackGap;
     1810        positions[lastLine] = positions[nextToLastLine] + tracks[nextToLastLine].baseSize();
     1811    }
    17801812    auto& offsetBetweenTracks = isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;
    17811813    offsetBetweenTracks = offset.distributionOffset;

  • trunk/Source/WebCore/rendering/RenderGrid.h

    r201379 r201510  
    174174#endif
    175175
    176     bool gridWasPopulated() const { return !m_grid.isEmpty() && !m_grid[0].isEmpty(); }
    177 
    178176    bool spanningItemCrossesFlexibleSizedTracks(const GridSpan&, GridTrackSizingDirection) const;
    179177
    180     unsigned gridColumnCount() const
    181     {
    182         ASSERT(gridWasPopulated());
    183         return m_grid[0].size();
    184     }
    185     unsigned gridRowCount() const
    186     {
    187         ASSERT(gridWasPopulated());
    188         return m_grid.size();
    189     }
     178    unsigned gridColumnCount() const;
     179    unsigned gridRowCount() const;
    190180
    191181    bool hasDefiniteLogicalSize(GridTrackSizingDirection) const;
    … …  
    208198    unsigned m_autoRepeatColumns { 0 };
    209199    unsigned m_autoRepeatRows { 0 };
     200
     201    bool m_gridIsDirty { true };
    210202};
    211203
Note: See TracChangeset for help on using the changeset viewer.