Changeset 154731 in webkit


Ignore:
Timestamp:
Aug 28, 2013 3:26:10 AM (11 years ago)
Author:
sergio@webkit.org
Message:

[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:
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r154730 r154731  
     12013-08-28  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [CSS Grid Layout] Fix grid position resolution
     4        https://bugs.webkit.org/show_bug.cgi?id=119801
     5
     6        Reviewed by Andreas Kling.
     7
     8        From Blink r148833, r148878, r150403 by <jchaffraix@chromium.org>
     9
     10        Added a new test to check negative position resolution. Also added
     11        several new test cases to check that we properly resolve grid
     12        positions in the grid edges.
     13
     14        * fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt: Added.
     15        * fast/css-grid-layout/grid-item-negative-position-resolution.html: Added.
     16        * fast/css-grid-layout/grid-item-spanning-resolution-expected.txt:
     17        * fast/css-grid-layout/grid-item-spanning-resolution.html:
     18
    1192013-08-28  Sergio Villar Senin  <svillar@igalia.com>
    220

  • trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution-expected.txt

    r154044 r154731  
    1212PASS
    1313PASS
     14PASS

  • trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution.html

    r154044 r154731  
    1212    width: 400px;
    1313    height: 300px;
     14}
     15
     16#bigGrid {
     17    -webkit-grid-definition-columns: 25% 25% 25% 25%;
     18    -webkit-grid-definition-rows: 25% 25% 25% 25%;
     19    height: 100px;
     20    width: 200px;
    1421}
    1522
    … …  
    3239    -webkit-grid-row: 1;
    3340    -webkit-grid-column: 1 / 5;
     41}
     42
     43.secondRowSecondColumnNoSpan {
     44    -webkit-grid-column: 2 / 3;
     45    -webkit-grid-row: 2 / 3;
     46}
     47
     48.thirdRowThirdColumnNoSpan {
     49    -webkit-grid-column: 3 / 4;
     50    -webkit-grid-row: 3 / 4;
    3451}
    3552</style>
    … …  
    101118<div style="position: relative">
    102119<div class="grid" data-expected-width="400" data-expected-height="300">
    103     <div class="sizedToGridArea autoSecondRowAutoFirstColumn" data-offset-x="0" data-offset-y="90" data-expected-width="160" data-expected-height="210"></div>
     120    <div class="sizedToGridArea autoSecondRowAutoFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="160" data-expected-height="90"></div>
    104121</div>
    105122</div>
    106123
     124<div style="position: relative">
     125<div class="grid" id="bigGrid" data-expected-width="200" data-expected-height="100">
     126    <div class="sizedToGridArea secondRowSecondColumnNoSpan" data-offset-x="50" data-offset-y="25" data-expected-width="50" data-expected-height="25"></div>
     127    <div class="sizedToGridArea thirdRowThirdColumnNoSpan" data-offset-x="100" data-offset-y="50" data-expected-width="50" data-expected-height="25"></div>
     128</div>
    107129</body>
    108130</html>

  • trunk/Source/WebCore/ChangeLog

    r154730 r154731  
     12013-08-28  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [CSS Grid Layout] Fix grid position resolution
     4        https://bugs.webkit.org/show_bug.cgi?id=119801
     5
     6        Reviewed by Andreas Kling.
     7
     8        From Blink r148833, r148878, r150403 by <jchaffraix@chromium.org>
     9
     10        Both grid-{column|row}-end and negative positions were not
     11        properly handled in our grid position resolution code. We were
     12        using the same code to resolve all the grid positions without
     13        considering the edges of the grid.
     14
     15        Also refactored the grid size estimation in
     16        resolveGridPositionsFromStyle() so we can use it for the grid size
     17        estimation. The code no longer requires the grid to be filled at
     18        that moment as the specs changed to use the "explicit grid" which
     19        is independent of grid items (only depends on style).
     20
     21        Test: fast/css-grid-layout/grid-item-negative-position-resolution.html
     22
     23        * rendering/RenderGrid.cpp:
     24        (WebCore::RenderGrid::maximumIndexInDirection):
     25        (WebCore::RenderGrid::resolveGridPositionsFromStyle):
     26        (WebCore::adjustGridPositionForSide):
     27        (WebCore::RenderGrid::resolveGridPositionFromStyle):
     28        * rendering/RenderGrid.h:
     29
    1302013-08-28  Sergio Villar Senin  <svillar@igalia.com>
    231

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

    r154730 r154731  
    328328}
    329329
    330 static size_t estimatedGridSizeForPosition(const GridPosition& position)
    331 {
    332     if (position.isAuto())
    333         return 1;
    334 
    335     // Negative explicit values never grow the grid as they are clamped against
    336     // the explicit grid's size. Thus we don't special case them here.
    337     return std::max(position.integerPosition(), 1);
    338 }
    339 
    340330size_t RenderGrid::explicitGridColumnCount() const
    341331{
    … …  
    353343
    354344    for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
    355         // This function bypasses the cache (cachedGridCoordinate()) as it is used to build it.
    356         // Also we can't call resolveGridPositionsFromStyle here as it assumes that the grid is build and we are in
    357         // the middle of building it. However we should be able to share more code with the previous logic (FIXME).
    358         const GridPosition& initialPosition = (direction == ForColumns) ? child->style()->gridItemColumnStart() : child->style()->gridItemRowStart();
    359         const GridPosition& finalPosition = (direction == ForColumns) ? child->style()->gridItemColumnEnd() : child->style()->gridItemRowEnd();
    360 
    361         size_t estimatedSizeForInitialPosition = estimatedGridSizeForPosition(initialPosition);
    362         size_t estimatedSizeForFinalPosition = estimatedGridSizeForPosition(finalPosition);
    363         ASSERT(estimatedSizeForInitialPosition);
    364         ASSERT(estimatedSizeForFinalPosition);
    365 
    366         maximumIndex = std::max(maximumIndex, estimatedSizeForInitialPosition);
    367         maximumIndex = std::max(maximumIndex, estimatedSizeForFinalPosition);
     345        OwnPtr<GridSpan> positions = resolveGridPositionsFromStyle(child, direction);
     346
     347        // |positions| is null if we need to run the auto-placement algorithm. Our estimation ignores
     348        // this case as the auto-placement algorithm will grow the grid as needed.
     349        if (!positions)
     350            continue;
     351
     352        maximumIndex = std::max(maximumIndex, positions->finalPositionIndex + 1);
    368353    }
    369354
    … …  
    731716PassOwnPtr<RenderGrid::GridSpan> RenderGrid::resolveGridPositionsFromStyle(const RenderBox* gridItem, TrackSizingDirection direction) const
    732717{
    733     ASSERT(gridWasPopulated());
    734 
    735718    const GridPosition& initialPosition = (direction == ForColumns) ? gridItem->style()->gridItemColumnStart() : gridItem->style()->gridItemRowStart();
    736719    const GridPositionSide initialPositionSide = (direction == ForColumns) ? ColumnStartSide : RowStartSide;
    … …  
    768751}
    769752
     753static size_t adjustGridPositionForSide(size_t resolvedPosition, RenderGrid::GridPositionSide side)
     754{
     755    // An item finishing on the N-th line belongs to the N-1-th cell.
     756    if (side == RenderGrid::ColumnEndSide || side == RenderGrid::RowEndSide)
     757        return resolvedPosition ? resolvedPosition - 1 : 0;
     758
     759    return resolvedPosition;
     760}
     761
    770762size_t RenderGrid::resolveGridPositionFromStyle(const GridPosition& position, GridPositionSide side) const
    771763{
    772     ASSERT(gridWasPopulated());
    773 
    774764    // FIXME: Handle other values for grid-{row,column} like ranges or line names.
    775765    switch (position.type()) {
    776766    case IntegerPosition: {
     767        ASSERT(position.integerPosition());
    777768        if (position.isPositive())
    778             return position.integerPosition() - 1;
    779 
    780         size_t resolvedPosition = abs(position.integerPosition());
    781         // FIXME: This returns one less than the expected result for side == ColumnStartSide or RowStartSide as we don't properly convert
    782         // the grid line to its grid track. However this avoids the issue of growing the grid when inserting the item (e.g. -1 / auto).
     769            return adjustGridPositionForSide(position.integerPosition() - 1, side);
     770
     771        size_t resolvedPosition = abs(position.integerPosition()) - 1;
    783772        const size_t endOfTrack = (side == ColumnStartSide || side == ColumnEndSide) ? explicitGridColumnCount() : explicitGridRowCount();
    784773
    … …  
    787776            return 0;
    788777
    789         return endOfTrack - resolvedPosition;
     778        return adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
    790779    }
    791780    case AutoPosition:

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

    r154044 r154731  
    4444    virtual bool avoidsFloats() const OVERRIDE { return true; }
    4545    virtual bool canCollapseAnonymousBlockChild() const OVERRIDE { return false; }
     46
     47    enum GridPositionSide {
     48        ColumnStartSide,
     49        ColumnEndSide,
     50        RowStartSide,
     51        RowEndSide
     52    };
    4653
    4754private:
    … …  
    123130    GridSpan resolveGridPositionsFromAutoPlacementPosition(const RenderBox*, TrackSizingDirection, size_t) const;
    124131    PassOwnPtr<GridSpan> resolveGridPositionsFromStyle(const RenderBox*, TrackSizingDirection) const;
    125     enum GridPositionSide {
    126         ColumnStartSide,
    127         ColumnEndSide,
    128         RowStartSide,
    129         RowEndSide
    130     };
    131132    size_t resolveGridPositionFromStyle(const GridPosition&, GridPositionSide) const;
    132133
Note: See TracChangeset for help on using the changeset viewer.