Changeset 199728 in webkit


Ignore:
Timestamp:
Apr 19, 2016 8:48:00 AM (8 years ago)
Author:
svillar@igalia.com
Message:

[css-grid] Use the margin box for non-auto minimum sizes
​https://bugs.webkit.org/show_bug.cgi?id=156711

Reviewed by Darin Adler.

Source/WebCore:

When computing the min-size of items with non-auto minimum height/width we are incorrectly
returning the size of the border box, and thus incorrectly ignoring the margins of the item.

This is a follow up patch of r199153 were we added the missing border and paddings for
heights. Contrary to that, we were not including margins for both axis.

This CL requires 3 different interrelated changes:

  • Add the margins to the min-size returned by minSizeForChild (might require a layout).
  • Refactor and extract width computations from logicalHeightForChild(); not totally

mandatory but pretty logical and helpful.

  • Use a new update function to isolate the computation of the override width.

Test: fast/css-grid-layout/min-width-margin-box.html

  • rendering/RenderBox.cpp:

(WebCore::RenderBox::computeInlineDirectionMargins): Added const to a parameter.

  • rendering/RenderBox.h:
  • rendering/RenderGrid.cpp:

(WebCore::RenderGrid::computeTrackSizesForDirection): Initialize the sizingOperation.
(WebCore::RenderGrid::computeIntrinsicLogicalWidths): Ditto.
(WebCore::RenderGrid::computeIntrinsicLogicalHeight): Ditto.
(WebCore::RenderGrid::logicalHeightForChild): Renamed from logicalContentHeightForChild as
it no longer returns the content size but the outer size.
(WebCore::RenderGrid::minSizeForChild):
(WebCore::RenderGrid::updateOverrideContainingBlockContentLogicalWidthForChild): Extracted
from logicalHeightForChild().
(WebCore::RenderGrid::minContentForChild): Update override width if needed.
(WebCore::RenderGrid::maxContentForChild): Ditto.
(WebCore::RenderGrid::computeMarginLogicalSizeForChild): Generalized from
computeMarginLogicalHeightForChild(), it can now compute also margins for the inline
direction.
(WebCore::RenderGrid::availableAlignmentSpaceForChildBeforeStretching):
(WebCore::RenderGrid::logicalContentHeightForChild): Deleted.
(WebCore::RenderGrid::computeMarginLogicalHeightForChild): Deleted.

  • rendering/RenderGrid.h:

LayoutTests:

  • fast/css-grid-layout/min-height-border-box.html:
  • fast/css-grid-layout/min-width-margin-box-expected.txt: Added.
  • fast/css-grid-layout/min-width-margin-box.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r199726 r199728  
     12016-04-19  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-grid] Use the margin box for non-auto minimum sizes
     4        https://bugs.webkit.org/show_bug.cgi?id=156711
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/css-grid-layout/min-height-border-box.html:
     9        * fast/css-grid-layout/min-width-margin-box-expected.txt: Added.
     10        * fast/css-grid-layout/min-width-margin-box.html: Added.
     11
    1122016-04-19  Michael Saboff  <msaboff@apple.com>
    213

  • trunk/LayoutTests/fast/css-grid-layout/min-height-border-box.html

    r199153 r199728  
    1414
    1515.container { width: 150px; }
    16 .borderPadding {
     16.borderPaddingMargin {
    1717    border-width: 2px 3px 5px 9px;
    1818    border-color: blue;
    1919    border-style: solid;
    2020    padding: 4px 9px 12px 20px;
     21    margin: 7px 10px 12px 14px;
    2122}
    2223
    … …  
    3536
    3637<div class="container">
    37     <div class="grid" data-expected-width="150" data-expected-height="58">
    38         <div class="item borderPadding" data-expected-width="140" data-expected-height="48">XXX</div>
     38    <div class="grid" data-expected-width="150" data-expected-height="77">
     39        <div class="item borderPaddingMargin" data-expected-width="116" data-expected-height="48">XXX</div>
    3940    </div>
    4041</div>
    … …  
    4849
    4950<div class="container">
    50     <div class="grid" data-expected-width="150" data-expected-height="33">
    51         <div class="item borderPadding" style="min-height: 0px;" data-expected-width="140" data-expected-height="23">XXX</div>
     51    <div class="grid" data-expected-width="150" data-expected-height="52">
     52        <div class="item borderPaddingMargin" style="min-height: 0px;" data-expected-width="116" data-expected-height="23">XXX</div>
    5253    </div>
    5354</div>
    … …  
    6162
    6263<div class="container">
    63     <div class="grid" data-expected-width="150" data-expected-height="33">
    64         <div class="item alignSelfStart borderPadding" style="min-height: 0px;" data-expected-width="140" data-expected-height="48">XXX</div>
     64    <div class="grid" data-expected-width="150" data-expected-height="52">
     65        <div class="item alignSelfStart borderPaddingMargin" style="min-height: 0px;" data-expected-width="116" data-expected-height="48">XXX</div>
    6566    </div>
    6667</div>
    … …  
    7576<div class="container">
    7677    <div class="grid" style="height: 15px;" data-expected-width="150" data-expected-height="25">
    77         <div class="item borderPadding" style="min-height: 0px;" data-expected-width="140" data-expected-height="23">XXX</div>
     78        <div class="item borderPaddingMargin" style="min-height: 0px;" data-expected-width="116" data-expected-height="23">XXX</div>
    7879    </div>
    7980</div>
    … …  
    8889<div class="container">
    8990    <div class="grid" style="height: 15px;" data-expected-width="150" data-expected-height="25">
    90         <div class="item alignSelfStart borderPadding" style="min-height: 0px;" data-expected-width="140" data-expected-height="48">XXX</div>
     91        <div class="item alignSelfStart borderPaddingMargin" style="min-height: 0px;" data-expected-width="116" data-expected-height="48">XXX</div>
    9192    </div>
    9293</div>
    … …  
    100101
    101102<div class="container">
    102     <div class="grid" data-expected-width="150" data-expected-height="63">
    103         <div class="item borderPadding" style="min-height: 30px;" data-expected-width="140" data-expected-height="53">XXX</div>
     103    <div class="grid" data-expected-width="150" data-expected-height="82">
     104        <div class="item borderPaddingMargin" style="min-height: 30px;" data-expected-width="116" data-expected-height="53">XXX</div>
    104105    </div>
    105106</div>
    … …  
    113114
    114115<div class="container">
    115     <div class="grid" data-expected-width="150" data-expected-height="63">
    116         <div class="item alignSelfStart borderPadding" style="min-height: 30px;" data-expected-width="140" data-expected-height="53">XXX</div>
     116    <div class="grid" data-expected-width="150" data-expected-height="82">
     117        <div class="item alignSelfStart borderPaddingMargin" style="min-height: 30px;" data-expected-width="116" data-expected-height="53">XXX</div>
    117118    </div>
    118119</div>

  • trunk/Source/WebCore/ChangeLog

    r199727 r199728  
     12016-04-19  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-grid] Use the margin box for non-auto minimum sizes
     4        https://bugs.webkit.org/show_bug.cgi?id=156711
     5
     6        Reviewed by Darin Adler.
     7
     8        When computing the min-size of items with non-auto minimum height/width we are incorrectly
     9        returning the size of the border box, and thus incorrectly ignoring the margins of the item.
     10
     11        This is a follow up patch of r199153 were we added the missing border and paddings for
     12        heights. Contrary to that, we were not including margins for both axis.
     13
     14        This CL requires 3 different interrelated changes:
     15        - Add the margins to the min-size returned by minSizeForChild (might require a layout).
     16        - Refactor and extract width computations from logicalHeightForChild(); not totally
     17        mandatory but pretty logical and helpful.
     18        - Use a new update function to isolate the computation of the override width.
     19
     20        Test: fast/css-grid-layout/min-width-margin-box.html
     21
     22        * rendering/RenderBox.cpp:
     23        (WebCore::RenderBox::computeInlineDirectionMargins): Added const to a parameter.
     24        * rendering/RenderBox.h:
     25        * rendering/RenderGrid.cpp:
     26        (WebCore::RenderGrid::computeTrackSizesForDirection): Initialize the sizingOperation.
     27        (WebCore::RenderGrid::computeIntrinsicLogicalWidths): Ditto.
     28        (WebCore::RenderGrid::computeIntrinsicLogicalHeight): Ditto.
     29        (WebCore::RenderGrid::logicalHeightForChild): Renamed from logicalContentHeightForChild as
     30        it no longer returns the content size but the outer size.
     31        (WebCore::RenderGrid::minSizeForChild):
     32        (WebCore::RenderGrid::updateOverrideContainingBlockContentLogicalWidthForChild): Extracted
     33        from logicalHeightForChild().
     34        (WebCore::RenderGrid::minContentForChild): Update override width if needed.
     35        (WebCore::RenderGrid::maxContentForChild): Ditto.
     36        (WebCore::RenderGrid::computeMarginLogicalSizeForChild): Generalized from
     37        computeMarginLogicalHeightForChild(), it can now compute also margins for the inline
     38        direction.
     39        (WebCore::RenderGrid::availableAlignmentSpaceForChildBeforeStretching):
     40        (WebCore::RenderGrid::logicalContentHeightForChild): Deleted.
     41        (WebCore::RenderGrid::computeMarginLogicalHeightForChild): Deleted.
     42        * rendering/RenderGrid.h:
     43
    1442016-04-19  Carlos Garcia Campos  <cgarcia@igalia.com>
    245

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

    r198943 r199728  
    26152615}
    26162616
    2617 void RenderBox::computeInlineDirectionMargins(RenderBlock& containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const
     2617void RenderBox::computeInlineDirectionMargins(const RenderBlock& containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const
    26182618{
    26192619    const RenderStyle& containingBlockStyle = containingBlock.style();

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

    r198843 r199728  
    367367    // Resolve auto margins in the inline direction of the containing block so that objects can be pushed to the start, middle or end
    368368    // of the containing block.
    369     void computeInlineDirectionMargins(RenderBlock& containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const;
     369    void computeInlineDirectionMargins(const RenderBlock& containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const;
    370370
    371371    // Used to resolve margins in the containing block's block-flow direction.

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

    r199657 r199728  
    230230    void setFreeSpaceForDirection(GridTrackSizingDirection, Optional<LayoutUnit> freeSpace);
    231231
     232    enum SizingOperation { TrackSizing, IntrinsicSizeComputation };
     233    SizingOperation sizingOperation { TrackSizing };
     234
    232235private:
    233236    Optional<LayoutUnit> freeSpaceForColumns;
    … …  
    332335    LayoutUnit totalGuttersSize = guttersSize(direction, direction == ForRows ? gridRowCount() : gridColumnCount());
    333336    sizingData.setFreeSpaceForDirection(direction, freeSpace - totalGuttersSize);
     337    sizingData.sizingOperation = GridSizingData::TrackSizing;
    334338
    335339    LayoutUnit baseSizes, growthLimits;
    … …  
    433437    GridSizingData sizingData(gridColumnCount(), gridRowCount());
    434438    sizingData.setFreeSpaceForDirection(ForColumns, Nullopt);
     439    sizingData.sizingOperation = GridSizingData::IntrinsicSizeComputation;
    435440    const_cast<RenderGrid*>(this)->computeUsedBreadthOfGridTracks(ForColumns, sizingData, minLogicalWidth, maxLogicalWidth);
    436441
    … …  
    451456    ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData));
    452457    sizingData.setFreeSpaceForDirection(ForRows, Nullopt);
     458    sizingData.sizingOperation = GridSizingData::IntrinsicSizeComputation;
    453459    LayoutUnit minHeight, maxHeight;
    454460    computeUsedBreadthOfGridTracks(ForRows, sizingData, minHeight, maxHeight);
    … …  
    714720}
    715721
    716 LayoutUnit RenderGrid::logicalContentHeightForChild(RenderBox& child, GridSizingData& sizingData)
     722LayoutUnit RenderGrid::logicalHeightForChild(RenderBox& child, GridSizingData& sizingData)
    717723{
    718724    Optional<LayoutUnit> oldOverrideContainingBlockContentLogicalWidth = child.hasOverrideContainingBlockLogicalWidth() ? child.overrideContainingBlockContentLogicalWidth() : LayoutUnit();
    … …  
    725731        child.clearOverrideLogicalContentHeight();
    726732
    727     child.setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
    728733    // If |child| has a relative logical height, we shouldn't let it override its intrinsic height, which is
    729734    // what we are interested in here. Thus we need to set the override logical height to Nullopt (no possible resolution).
    … …  
    748753        return minContentForChild(child, direction, sizingData);
    749754
    750     if (isRowAxis)
    751         return child.computeLogicalWidthInRegionUsing(MinSize, childMinSize, contentLogicalWidth(), *this, nullptr);
    752 
    753     return child.computeLogicalHeightUsing(MinSize, childMinSize, Nullopt).valueOr(0);
     755    bool overrideLogicalWidthHasChanged = updateOverrideContainingBlockContentLogicalWidthForChild(child, sizingData);
     756    if (isRowAxis) {
     757        LayoutUnit marginLogicalWidth = sizingData.sizingOperation == GridSizingData::TrackSizing ? computeMarginLogicalSizeForChild(ForColumns, child) : marginIntrinsicLogicalWidthForChild(child);
     758        return child.computeLogicalWidthInRegionUsing(MinSize, childMinSize, child.overrideContainingBlockContentLogicalWidth().valueOr(0), *this, nullptr) + marginLogicalWidth;
     759    }
     760
     761    if (overrideLogicalWidthHasChanged)
     762        child.setNeedsLayout(MarkOnlyThis);
     763    child.layoutIfNeeded();
     764    return child.computeLogicalHeightUsing(MinSize, childMinSize, Nullopt).valueOr(0) + child.marginLogicalHeight() + child.scrollbarLogicalHeight();
     765}
     766
     767bool RenderGrid::updateOverrideContainingBlockContentLogicalWidthForChild(RenderBox& child, GridSizingData& sizingData)
     768{
     769    LayoutUnit overrideWidth = gridAreaBreadthForChild(child, ForColumns, sizingData.columnTracks);
     770    if (child.hasOverrideContainingBlockLogicalWidth() && child.overrideContainingBlockContentLogicalWidth() == overrideWidth)
     771        return false;
     772
     773    child.setOverrideContainingBlockContentLogicalWidth(overrideWidth);
     774    return true;
    754775}
    755776
    … …  
    772793    }
    773794
    774     return logicalContentHeightForChild(child, sizingData);
     795    if (updateOverrideContainingBlockContentLogicalWidthForChild(child, sizingData))
     796        child.setNeedsLayout(MarkOnlyThis);
     797    return logicalHeightForChild(child, sizingData);
    775798}
    776799
    … …  
    793816    }
    794817
    795     return logicalContentHeightForChild(child, sizingData);
     818    if (updateOverrideContainingBlockContentLogicalWidthForChild(child, sizingData))
     819        child.setNeedsLayout(MarkOnlyThis);
     820    return logicalHeightForChild(child, sizingData);
    796821}
    797822
    … …  
    16901715}
    16911716
    1692 LayoutUnit RenderGrid::computeMarginLogicalHeightForChild(const RenderBox& child) const
     1717LayoutUnit RenderGrid::computeMarginLogicalSizeForChild(GridTrackSizingDirection direction, const RenderBox& child) const
    16931718{
    16941719    if (!child.style().hasMargin())
    16951720        return 0;
    16961721
    1697     LayoutUnit marginBefore;
    1698     LayoutUnit marginAfter;
    1699     child.computeBlockDirectionMargins(*this, marginBefore, marginAfter);
    1700 
    1701     return marginBefore + marginAfter;
     1722    LayoutUnit marginStart;
     1723    LayoutUnit marginEnd;
     1724    if (direction == ForColumns)
     1725        child.computeInlineDirectionMargins(*this, child.containingBlockLogicalWidthForContentInRegion(nullptr), child.logicalWidth(), marginStart, marginEnd);
     1726    else
     1727        child.computeBlockDirectionMargins(*this, marginStart, marginEnd);
     1728
     1729    return marginStart + marginEnd;
    17021730}
    17031731
    … …  
    17071735    // children are laid out, so we can't use the child cached values. Hence, we need to
    17081736    // compute margins in order to determine the available height before stretching.
    1709     return gridAreaBreadthForChild - (child.needsLayout() ? computeMarginLogicalHeightForChild(child) : marginLogicalHeightForChild(child));
     1737    return gridAreaBreadthForChild - (child.needsLayout() ? computeMarginLogicalSizeForChild(ForRows, child) : marginLogicalHeightForChild(child));
    17101738}
    17111739

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

    r199657 r199728  
    131131    GridTrackSize gridTrackSize(GridTrackSizingDirection, unsigned) const;
    132132
    133     LayoutUnit logicalContentHeightForChild(RenderBox&, GridSizingData&);
     133    bool updateOverrideContainingBlockContentLogicalWidthForChild(RenderBox&, GridSizingData&);
     134    LayoutUnit logicalHeightForChild(RenderBox&, GridSizingData&);
    134135    LayoutUnit minSizeForChild(RenderBox&, GridTrackSizingDirection, GridSizingData&);
    135136    LayoutUnit minContentForChild(RenderBox&, GridTrackSizingDirection, GridSizingData&);
    … …  
    153154    bool needToStretchChildLogicalHeight(const RenderBox&) const;
    154155    LayoutUnit marginLogicalHeightForChild(const RenderBox&) const;
    155     LayoutUnit computeMarginLogicalHeightForChild(const RenderBox&) const;
     156    LayoutUnit computeMarginLogicalSizeForChild(GridTrackSizingDirection, const RenderBox&) const;
    156157    LayoutUnit availableAlignmentSpaceForChildBeforeStretching(LayoutUnit gridAreaBreadthForChild, const RenderBox&) const;
    157158    void applyStretchAlignmentToChildIfNeeded(RenderBox&);
Note: See TracChangeset for help on using the changeset viewer.