Changeset 234927 in webkit


Ignore:
Timestamp:
Aug 16, 2018 8:43:53 AM (6 years ago)
Author:
Alan Bujtas
Message:

[lFC][Floating] Add estimated margin top computation.
​https://bugs.webkit.org/show_bug.cgi?id=188619

Reviewed by Antti Koivisto.

In order to figure out whether a box should avoid a float, we need to know the final positions of both (ignore relative positioning for now).
In block formatting context the final position for a normal flow box includes

  1. the static position and
  2. the corresponding (non)collapsed margins.

Now the vertical margins are computed when all the descendants are finalized, because the margin values might be depending on the height of the box
(and the height might be based on the content).
So when we get to the point where we intersect the box with the float to decide if the box needs to move, we don't yet have the final vertical position.

The idea here is that as long as we don't cross the block formatting context boundary, we should be able to pre-compute the final top margin.
(if this holds true for all the cases, the estimated prefix could be removed and just use marginTop() instead.)

Covered by existing block-only tests.

  • layout/blockformatting/BlockFormattingContext.cpp:

(WebCore::Layout::BlockFormattingContext::layout const):
(WebCore::Layout::BlockFormattingContext::computeEstimatedMarginTop const):
(WebCore::Layout::BlockFormattingContext::computeEstimatedMarginTopForAncestors const):
(WebCore::Layout::BlockFormattingContext::computeFloatingPosition const):
(WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const):
(WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):

  • layout/blockformatting/BlockFormattingContext.h:
  • layout/blockformatting/BlockMarginCollapse.cpp:

(WebCore::Layout::BlockFormattingContext::MarginCollapse::estimatedMarginTop):

  • layout/displaytree/DisplayBox.cpp:

(WebCore::Display::Box::Box):

  • layout/displaytree/DisplayBox.h:

(WebCore::Display::Box::setHasValidTop):
(WebCore::Display::Box::setHasValidLeft):
(WebCore::Display::Box::top const):
(WebCore::Display::Box::left const):
(WebCore::Display::Box::topLeft const):
(WebCore::Display::Box::setTopLeft):
(WebCore::Display::Box::setTop):
(WebCore::Display::Box::setLeft):
(WebCore::Display::Box::setVerticalMargin):
(WebCore::Display::Box::setEstimatedMarginTop):
(WebCore::Display::Box::estimatedMarginTop const):

Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r234925 r234927  
     12018-08-16  Zalan Bujtas  <zalan@apple.com>
     2
     3        [lFC][Floating] Add estimated margin top computation.
     4        https://bugs.webkit.org/show_bug.cgi?id=188619
     5
     6        Reviewed by Antti Koivisto.
     7
     8        In order to figure out whether a box should avoid a float, we need to know the final positions of both (ignore relative positioning for now).
     9        In block formatting context the final position for a normal flow box includes
     10        1. the static position and
     11        2. the corresponding (non)collapsed margins.
     12        Now the vertical margins are computed when all the descendants are finalized, because the margin values might be depending on the height of the box
     13        (and the height might be based on the content).
     14        So when we get to the point where we intersect the box with the float to decide if the box needs to move, we don't yet have the final vertical position.
     15
     16        The idea here is that as long as we don't cross the block formatting context boundary, we should be able to pre-compute the final top margin.
     17        (if this holds true for all the cases, the estimated prefix could be removed and just use marginTop() instead.)
     18
     19        Covered by existing block-only tests.
     20
     21        * layout/blockformatting/BlockFormattingContext.cpp:
     22        (WebCore::Layout::BlockFormattingContext::layout const):
     23        (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginTop const):
     24        (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginTopForAncestors const):
     25        (WebCore::Layout::BlockFormattingContext::computeFloatingPosition const):
     26        (WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const):
     27        (WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
     28        * layout/blockformatting/BlockFormattingContext.h:
     29        * layout/blockformatting/BlockMarginCollapse.cpp:
     30        (WebCore::Layout::BlockFormattingContext::MarginCollapse::estimatedMarginTop):
     31        * layout/displaytree/DisplayBox.cpp:
     32        (WebCore::Display::Box::Box):
     33        * layout/displaytree/DisplayBox.h:
     34        (WebCore::Display::Box::setHasValidTop):
     35        (WebCore::Display::Box::setHasValidLeft):
     36        (WebCore::Display::Box::top const):
     37        (WebCore::Display::Box::left const):
     38        (WebCore::Display::Box::topLeft const):
     39        (WebCore::Display::Box::setTopLeft):
     40        (WebCore::Display::Box::setTop):
     41        (WebCore::Display::Box::setLeft):
     42        (WebCore::Display::Box::setVerticalMargin):
     43        (WebCore::Display::Box::setEstimatedMarginTop):
     44        (WebCore::Display::Box::estimatedMarginTop const):
     45
    1462018-08-16  Zalan Bujtas  <zalan@apple.com>
    247

  • trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp

    r234847 r234927  
    114114            // Finalize position with clearance.
    115115            if (layoutBox.hasFloatClear())
    116                 computeVerticalPositionForFloatClear(floatingContext, layoutBox, displayBox);
     116                computeVerticalPositionForFloatClear(layoutContext, floatingContext, layoutBox, displayBox);
    117117            if (!is<Container>(layoutBox))
    118118                continue;
    … …  
    157157}
    158158
     159void BlockFormattingContext::computeEstimatedMarginTop(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
     160{
     161    auto estimatedMarginTop = MarginCollapse::estimatedMarginTop(layoutContext, layoutBox);
     162    displayBox.setEstimatedMarginTop(estimatedMarginTop);
     163    displayBox.moveVertically(estimatedMarginTop);
     164}
     165
     166void BlockFormattingContext::computeEstimatedMarginTopForAncestors(LayoutContext& layoutContext, const Box& layoutBox) const
     167{
     168    // We only need to estimate margin top for float related layout.
     169    ASSERT(layoutBox.isFloatingPositioned() || layoutBox.hasFloatClear());
     170
     171    // In order to figure out whether a box should avoid a float, we need to know the final positions of both (ignore relative positioning for now).
     172    // In block formatting context the final position for a normal flow box includes
     173    // 1. the static position and
     174    // 2. the corresponding (non)collapsed margins.
     175    // Now the vertical margins are computed when all the descendants are finalized, because the margin values might be depending on the height of the box
     176    // (and the height might be based on the content).
     177    // So when we get to the point where we intersect the box with the float to decide if the box needs to move, we don't yet have the final vertical position.
     178    //
     179    // The idea here is that as long as we don't cross the block formatting context boundary, we should be able to pre-compute the final top margin.
     180
     181    for (auto* ancestor = layoutBox.containingBlock(); ancestor && !ancestor->establishesBlockFormattingContext(); ancestor = ancestor->containingBlock()) {
     182        auto* displayBox = layoutContext.displayBoxForLayoutBox(*ancestor);
     183        ASSERT(displayBox);
     184        // FIXME: with incremental layout, we might actually have a valid (non-estimated) margin top as well.
     185        if (displayBox->estimatedMarginTop())
     186            return;
     187
     188        computeEstimatedMarginTop(layoutContext, *ancestor, *displayBox);
     189    }
     190}
     191
    159192void BlockFormattingContext::computeFloatingPosition(LayoutContext& layoutContext, FloatingContext& floatingContext, const Box& layoutBox, Display::Box& displayBox) const
    160193{
    … …  
    167200        displayBox.moveVertically(previousDisplayBox.nonCollapsedMarginBottom() - previousDisplayBox.marginBottom());
    168201    }
     202    computeEstimatedMarginTopForAncestors(layoutContext, layoutBox);
    169203    displayBox.setTopLeft(floatingContext.positionForFloat(layoutBox));
    170204    floatingContext.floatingState().append(layoutBox);
    171205}
    172206
    173 void BlockFormattingContext::computeVerticalPositionForFloatClear(const FloatingContext& floatingContext, const Box& layoutBox, Display::Box& displayBox) const
     207void BlockFormattingContext::computeVerticalPositionForFloatClear(LayoutContext& layoutContext, const FloatingContext& floatingContext, const Box& layoutBox, Display::Box& displayBox) const
    174208{
    175209    ASSERT(layoutBox.hasFloatClear());
     210    if (floatingContext.floatingState().isEmpty())
     211        return;
     212
     213    computeEstimatedMarginTopForAncestors(layoutContext, layoutBox);
    176214    if (auto verticalPositionWithClearance = floatingContext.verticalPositionWithClearance(layoutBox))
    177215        displayBox.setTop(*verticalPositionWithClearance);
    … …  
    209247    auto heightAndMargin = Geometry::inFlowHeightAndMargin(layoutContext, layoutBox);
    210248    displayBox.setContentBoxHeight(heightAndMargin.height);
    211     displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
    212     displayBox.setVerticalMargin(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin));
     249    auto marginValue = heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin);
     250    displayBox.setVerticalMargin(marginValue);
    213251    displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin);
     252
     253    // This box has already been moved by the estimated vertical margin. No need to move it again.
     254    if (!displayBox.estimatedMarginTop())
     255        displayBox.moveVertically(marginValue.top);
    214256}
    215257

  • trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h

    r234847 r234927  
    5858    void computeStaticPosition(LayoutContext&, const Box&, Display::Box&) const override;
    5959    void computeFloatingPosition(LayoutContext&, FloatingContext&, const Box&, Display::Box&) const;
    60     void computeVerticalPositionForFloatClear(const FloatingContext&, const Box&, Display::Box&) const;
     60    void computeVerticalPositionForFloatClear(LayoutContext&, const FloatingContext&, const Box&, Display::Box&) const;
    6161    void computeInFlowPositionedPosition(LayoutContext&, const Box&, Display::Box&) const override;
    6262    void computeInFlowWidthAndMargin(LayoutContext&, const Box&, Display::Box&) const;
    6363    void computeInFlowHeightAndMargin(LayoutContext&, const Box&, Display::Box&) const;
     64    void computeEstimatedMarginTop(LayoutContext&, const Box&, Display::Box&) const;
     65    void computeEstimatedMarginTopForAncestors(LayoutContext&, const Box&) const;
    6466
    6567    FormattingContext::InstrinsicWidthConstraints instrinsicWidthConstraints(LayoutContext&, const Box&) const override;
    … …  
    8890    public:
    8991        static LayoutUnit marginTop(const LayoutContext&, const Box&);
     92        static LayoutUnit estimatedMarginTop(const LayoutContext&, const Box&);
    9093        static LayoutUnit marginBottom(const LayoutContext&, const Box&);
    9194

  • trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp

    r234786 r234927  
    333333}
    334334
     335LayoutUnit BlockFormattingContext::MarginCollapse::estimatedMarginTop(const LayoutContext& layoutContext, const Box& layoutBox)
     336{
     337    ASSERT(layoutBox.isBlockLevelBox());
     338    // Can't estimate vertical margins for out of flow boxes (and we shouldn't need to do it for float boxes).
     339    ASSERT(layoutBox.isInFlow());
     340    // Can't cross block formatting context boundary.
     341    ASSERT(!layoutBox.establishesBlockFormattingContext());
     342
     343    // Let's just use the normal path for now.
     344    return marginTop(layoutContext, layoutBox);
     345}
     346
    335347}
    336348}

  • trunk/Source/WebCore/layout/displaytree/DisplayBox.cpp

    r234476 r234927  
    6060    , m_margin(other.m_margin)
    6161    , m_verticalNonCollapsedMargin(other.m_verticalNonCollapsedMargin)
     62    , m_estimatedMarginTop(other.m_estimatedMarginTop)
    6263    , m_border(other.m_border)
    6364    , m_padding(other.m_padding)
    6465#if !ASSERT_DISABLED
     66    , m_hasValidTop(other.m_hasValidTop)
     67    , m_hasValidLeft(other.m_hasValidLeft)
    6568    , m_hasValidHorizontalMargin(other.m_hasValidHorizontalMargin)
    6669    , m_hasValidVerticalMargin(other.m_hasValidVerticalMargin)

  • trunk/Source/WebCore/layout/displaytree/DisplayBox.h

    r234925 r234927  
    118118    ~Box();
    119119
    120     LayoutUnit top() const { return m_topLeft.y(); }
    121     LayoutUnit left() const { return m_topLeft.x(); }
     120    LayoutUnit top() const;
     121    LayoutUnit left() const;
    122122    LayoutUnit bottom() const { return top() + height(); }
    123123    LayoutUnit right() const { return left() + width(); }
    124124
    125     LayoutPoint topLeft() const { return m_topLeft; }
     125    LayoutPoint topLeft() const;
    126126    LayoutPoint bottomRight() const { return { right(), bottom() }; }
    127127
    … …  
    139139    LayoutUnit nonCollapsedMarginTop() const;
    140140    LayoutUnit nonCollapsedMarginBottom() const;
     141    std::optional<LayoutUnit> estimatedMarginTop() const { return m_estimatedMarginTop; }
    141142
    142143    LayoutUnit borderTop() const;
    … …  
    173174    };
    174175
    175     void setTopLeft(const LayoutPoint& topLeft) { m_topLeft = topLeft; }
    176     void setTop(LayoutUnit top) { m_topLeft.setY(top); }
    177     void setLeft(LayoutUnit left) { m_topLeft.setX(left); }
     176    void setTopLeft(const LayoutPoint&);
     177    void setTop(LayoutUnit);
     178    void setLeft(LayoutUnit);
    178179    void moveHorizontally(LayoutUnit offset) { m_topLeft.move(offset, { }); }
    179180    void moveVertically(LayoutUnit offset) { m_topLeft.move({ }, offset); }
    … …  
    185186    void setVerticalMargin(Layout::VerticalEdges);
    186187    void setVerticalNonCollapsedMargin(Layout::VerticalEdges);
     188    void setEstimatedMarginTop(LayoutUnit marginTop) { m_estimatedMarginTop = marginTop; }
    187189
    188190    void setBorder(Layout::Edges);
    … …  
    194196    void invalidatePadding() { m_hasValidPadding = false; }
    195197
     198    void setHasValidTop() { m_hasValidTop = true; }
     199    void setHasValidLeft() { m_hasValidLeft = true; }
    196200    void setHasValidVerticalMargin() { m_hasValidVerticalMargin = true; }
    197201    void setHasValidVerticalNonCollapsedMargin() { m_hasValidVerticalNonCollapsedMargin = true; }
    … …  
    213217    Layout::Edges m_margin;
    214218    Layout::VerticalEdges m_verticalNonCollapsedMargin;
     219    std::optional<LayoutUnit> m_estimatedMarginTop;
    215220
    216221    Layout::Edges m_border;
    … …  
    218223
    219224#if !ASSERT_DISABLED
     225    bool m_hasValidTop { false };
     226    bool m_hasValidLeft { false };
    220227    bool m_hasValidHorizontalMargin { false };
    221228    bool m_hasValidVerticalMargin { false };
    … …  
    417424}
    418425
     426inline LayoutUnit Box::top() const
     427{
     428    ASSERT(m_hasValidTop && (m_estimatedMarginTop || m_hasValidVerticalMargin));
     429    return m_topLeft.y();
     430}
     431
     432inline LayoutUnit Box::left() const
     433{
     434    ASSERT(m_hasValidLeft && m_hasValidHorizontalMargin);
     435    return m_topLeft.x();
     436}
     437
     438inline LayoutPoint Box::topLeft() const
     439{
     440    ASSERT(m_hasValidTop && (m_estimatedMarginTop || m_hasValidVerticalMargin));
     441    ASSERT(m_hasValidLeft && m_hasValidHorizontalMargin);
     442    return m_topLeft;
     443}
     444
     445inline void Box::setTopLeft(const LayoutPoint& topLeft)
     446{
     447#if !ASSERT_DISABLED
     448    setHasValidTop();
     449    setHasValidLeft();
     450#endif
     451    m_topLeft = topLeft;
     452}
     453
     454inline void Box::setTop(LayoutUnit top)
     455{
     456#if !ASSERT_DISABLED
     457    setHasValidTop();
     458#endif
     459    m_topLeft.setY(top);
     460}
     461
     462inline void Box::setLeft(LayoutUnit left)
     463{
     464#if !ASSERT_DISABLED
     465    setHasValidLeft();
     466#endif
     467    m_topLeft.setX(left);
     468}
     469
    419470inline void Box::setContentBoxHeight(LayoutUnit height)
    420471{
    … …  
    458509    setHasValidVerticalMargin();
    459510#endif
     511    ASSERT(!m_estimatedMarginTop || *m_estimatedMarginTop == margin.top);
    460512    m_margin.vertical = margin;
    461513}
Note: See TracChangeset for help on using the changeset viewer.