Attachment 348239 Details for Bug 189035 – Patch

[patch] Patch

bug-189035-20180827164601.patch (text/plain), 13.56 KB, created by zalan on 2018-08-27 16:46:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-27 16:46:02 PDT

Size: 13.56 KB

patch

obsolete

>Subversion Revision: 235352
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0b036e4abcbf6173a8c1516f914009c8a9c8d0a1..71a69facb8b4113117ac0ff60cd93b5f1a12256b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-08-27  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floating] Remove redundant FloatAvoider functions.
>+        https://bugs.webkit.org/show_bug.cgi?id=189035
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        and move some code from FloatContext to FloatAvoider.
>+
>+        * layout/floats/FloatAvoider.cpp:
>+        (WebCore::Layout::FloatAvoider::initializePosition):
>+        (WebCore::Layout::FloatAvoider::rect const):
>+        (WebCore::Layout::FloatAvoider::setVerticalConstraint):
>+        (WebCore::Layout::FloatAvoider::setHorizontalConstraints):
>+        (WebCore::Layout::FloatAvoider::resetHorizontalConstraint):
>+        (WebCore::Layout::FloatAvoider::initialVerticalPosition const):
>+        (WebCore::Layout::FloatAvoider::initialHorizontalPosition const):
>+        (WebCore::Layout::FloatAvoider::rectInContainingBlock const):
>+        (WebCore::Layout::FloatAvoider::setLeft): Deleted.
>+        (WebCore::Layout::FloatAvoider::setTopLeft): Deleted.
>+        (WebCore::Layout::FloatAvoider::resetVertically): Deleted.
>+        (WebCore::Layout::FloatAvoider::resetHorizontally): Deleted.
>+        (WebCore::Layout::FloatAvoider::topLeftInContainingBlock const): Deleted.
>+        * layout/floats/FloatAvoider.h:
>+        (WebCore::Layout::FloatAvoider::top const): Deleted.
>+        (WebCore::Layout::FloatAvoider::left const): Deleted.
>+        (WebCore::Layout::FloatAvoider::marginTop const): Deleted.
>+        (WebCore::Layout::FloatAvoider::marginLeft const): Deleted.
>+        (WebCore::Layout::FloatAvoider::marginBottom const): Deleted.
>+        (WebCore::Layout::FloatAvoider::marginRight const): Deleted.
>+        (WebCore::Layout::FloatAvoider::rectWithMargin const): Deleted.
>+        (WebCore::Layout::FloatAvoider::setTop): Deleted.
>+        * layout/floats/FloatingContext.cpp:
>+        (WebCore::Layout::FloatingContext::positionForFloat const):
>+        (WebCore::Layout::FloatingContext::floatingPosition const):
>+        (WebCore::Layout::FloatingPair::horizontalConstraints const):
>+        (WebCore::Layout::FloatingPair::horiztonalPosition const): Deleted.
>+
> 2018-08-26  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] FloatBox -> FloatAvoider
>diff --git a/Source/WebCore/layout/floats/FloatAvoider.cpp b/Source/WebCore/layout/floats/FloatAvoider.cpp
>index 16b9375cc2e923631305001ae1b4f9997da8ac21..b8b9f5c5c419bf461a6fadf8debec6e4198115d7 100644
>--- a/Source/WebCore/layout/floats/FloatAvoider.cpp
>+++ b/Source/WebCore/layout/floats/FloatAvoider.cpp
>@@ -49,8 +49,7 @@ FloatAvoider::FloatAvoider(const Box& layoutBox, const FloatingState& floatingSt
> 
> void FloatAvoider::initializePosition()
> {
>-    resetVertically();
>-    resetHorizontally();
>+    m_absoluteDisplayBox.setTopLeft({ initialHorizontalPosition(), initialVerticalPosition() });
> }
> 
> bool FloatAvoider::isLeftAligned() const
>@@ -58,29 +57,45 @@ bool FloatAvoider::isLeftAligned() const
>     return m_layoutBox->isLeftFloatingPositioned();
> }
> 
>-void FloatAvoider::setLeft(PositionInContextRoot left)
>+Display::Box::Rect FloatAvoider::rect() const
> {
>+    return m_absoluteDisplayBox.rectWithMargin();
>+}
>+
>+void FloatAvoider::setVerticalConstraint(PositionInContextRoot verticalPosition)
>+{ 
>+    m_absoluteDisplayBox.setTop(verticalPosition + m_absoluteDisplayBox.marginTop());
>+}
>+
>+void FloatAvoider::setHorizontalConstraints(HorizontalConstraints horizontalConstraints)
>+{
>+    if ((isLeftAligned() && !horizontalConstraints.left) || (!isLeftAligned() && !horizontalConstraints.right)) {
>+        resetHorizontalConstraint();
>+        return;
>+    }
>+
>+    auto positionCandidate = isLeftAligned() ? *horizontalConstraints.left : *horizontalConstraints.right - rect().width();
>     // Horizontal position is constrained by the containing block's content box.
>     // Compute the horizontal position for the new floating by taking both the contining block and the current left/right floats into account.
>     auto containingBlockContentBoxLeft = m_containingBlockAbsoluteDisplayBox.left() + m_containingBlockAbsoluteDisplayBox.contentBoxLeft();
>     auto containingBlockContentBoxRight = containingBlockContentBoxLeft + m_containingBlockAbsoluteDisplayBox.contentBoxWidth();
> 
>+    positionCandidate += m_absoluteDisplayBox.marginLeft();
>     // Align it with the containing block's left edge first.
>-    left = std::max(containingBlockContentBoxLeft + marginLeft(), left);
>+    positionCandidate = std::max(containingBlockContentBoxLeft + m_absoluteDisplayBox.marginLeft(), positionCandidate);
>     // Make sure it does not overflow the containing block on the right.
>     auto marginBoxSize = m_absoluteDisplayBox.marginBox().width();
>-    left = std::min(left, containingBlockContentBoxRight - marginBoxSize + marginLeft());
>+    positionCandidate = std::min(positionCandidate, containingBlockContentBoxRight - marginBoxSize + m_absoluteDisplayBox.marginLeft());
> 
>-    m_absoluteDisplayBox.setLeft(left);
>+    m_absoluteDisplayBox.setLeft(positionCandidate);
> }
> 
>-void FloatAvoider::setTopLeft(PointInContextRoot topLeft)
>+void FloatAvoider::resetHorizontalConstraint()
> {
>-    setTop(topLeft.y);
>-    setLeft(topLeft.x);
>+    m_absoluteDisplayBox.setLeft(initialHorizontalPosition());
> }
> 
>-void FloatAvoider::resetVertically()
>+PositionInContextRoot FloatAvoider::initialVerticalPosition() const
> {
>     // Incoming float cannot be placed higher than existing floats (margin box of the last float).
>     // Take the static position (where the box would go if it wasn't floating) and adjust it with the last float.
>@@ -89,10 +104,10 @@ void FloatAvoider::resetVertically()
>         top = std::max(top, lastFloat->rectWithMargin().top());
>     top += m_absoluteDisplayBox.marginTop();
> 
>-    m_absoluteDisplayBox.setTop(top);
>+    return top;
> }
> 
>-void FloatAvoider::resetHorizontally()
>+PositionInContextRoot FloatAvoider::initialHorizontalPosition() const
> {
>     // Align the box with the containing block's content box.
>     auto containingBlockContentBoxLeft = m_containingBlockAbsoluteDisplayBox.left() + m_containingBlockAbsoluteDisplayBox.contentBoxLeft();
>@@ -101,16 +116,21 @@ void FloatAvoider::resetHorizontally()
>     auto left = isLeftAligned() ? containingBlockContentBoxLeft : containingBlockContentBoxRight - m_absoluteDisplayBox.marginBox().width();
>     left += m_absoluteDisplayBox.marginLeft();
> 
>-    m_absoluteDisplayBox.setLeft(left);
>+    return left;
> }
> 
>-PointInContainingBlock FloatAvoider::topLeftInContainingBlock() const
>+Display::Box::Rect FloatAvoider::rectInContainingBlock() const
> {
>     // From formatting root coordinate system back to containing block's.
>     if (m_layoutBox->containingBlock() == &m_floatingState.root())
>-        return m_absoluteDisplayBox.topLeft();
>-
>-    return { m_absoluteDisplayBox.left() - m_containingBlockAbsoluteDisplayBox.left(), m_absoluteDisplayBox.top() - m_containingBlockAbsoluteDisplayBox.top() };
>+        return m_absoluteDisplayBox.rect();
>+
>+    return {
>+        m_absoluteDisplayBox.top() - m_containingBlockAbsoluteDisplayBox.top(),
>+        m_absoluteDisplayBox.left() - m_containingBlockAbsoluteDisplayBox.left(),
>+        m_absoluteDisplayBox.width(),
>+        m_absoluteDisplayBox.height()
>+    };
> }
> 
> }
>diff --git a/Source/WebCore/layout/floats/FloatAvoider.h b/Source/WebCore/layout/floats/FloatAvoider.h
>index 305f4a6c9d5bd3b7a9abb1a9a9f088c067657b62..da0f69c067c8e9d9f36751cb41f041328be23f06 100644
>--- a/Source/WebCore/layout/floats/FloatAvoider.h
>+++ b/Source/WebCore/layout/floats/FloatAvoider.h
>@@ -45,28 +45,23 @@ class FloatAvoider {
> public:
>     FloatAvoider(const Box&, const FloatingState&, const LayoutContext&);
> 
>-    PositionInContextRoot top() const { return m_absoluteDisplayBox.top(); }
>-    PositionInContextRoot left() const { return m_absoluteDisplayBox.left(); }
>-    PointInContainingBlock topLeftInContainingBlock() const;
>+    Display::Box::Rect rect() const;
>+    Display::Box::Rect rectInContainingBlock() const;
> 
>-    LayoutUnit marginTop() const { return m_absoluteDisplayBox.marginTop(); }
>-    LayoutUnit marginLeft() const { return m_absoluteDisplayBox.marginLeft(); }
>-    LayoutUnit marginBottom() const { return m_absoluteDisplayBox.marginBottom(); }
>-    LayoutUnit marginRight() const { return m_absoluteDisplayBox.marginRight(); }
>-
>-    Display::Box::Rect rectWithMargin() const { return m_absoluteDisplayBox.rectWithMargin(); }
>-
>-    void setTop(PositionInContextRoot top) { m_absoluteDisplayBox.setTop(top); }
>-    void setLeft(PositionInContextRoot);
>-    void setTopLeft(PointInContextRoot);
>-
>-    void resetHorizontally();
>-    void resetVertically();
>+    struct HorizontalConstraints {
>+        std::optional<PositionInContextRoot> left;
>+        std::optional<PositionInContextRoot> right;
>+    };
>+    void setHorizontalConstraints(HorizontalConstraints);
>+    void setVerticalConstraint(PositionInContextRoot);
> 
>+private:
>     bool isLeftAligned() const;
> 
>-private:
>     void initializePosition();
>+    PositionInContextRoot initialHorizontalPosition() const;
>+    PositionInContextRoot initialVerticalPosition() const;
>+    void resetHorizontalConstraint();
> 
>     WeakPtr<Box> m_layoutBox;
>     const FloatingState& m_floatingState;
>diff --git a/Source/WebCore/layout/floats/FloatingContext.cpp b/Source/WebCore/layout/floats/FloatingContext.cpp
>index 72f9d7712e343f68647d3664ddd62a279a82083e..87d6f4a1406152d04765d188d08e64e144559605 100644
>--- a/Source/WebCore/layout/floats/FloatingContext.cpp
>+++ b/Source/WebCore/layout/floats/FloatingContext.cpp
>@@ -68,7 +68,7 @@ public:
>     const FloatingState::FloatItem* right() const;
>     bool intersects(const Display::Box::Rect&) const;
>     PositionInContextRoot verticalPosition() const { return m_verticalPosition; }
>-    std::optional<PositionInContextRoot> horiztonalPosition(Float) const;
>+    FloatAvoider::HorizontalConstraints horizontalConstraints() const;
>     PositionInContextRoot bottom() const;
>     bool operator==(const FloatingPair&) const;
> 
>@@ -139,7 +139,7 @@ PointInContainingBlock FloatingContext::positionForFloat(const Box& layoutBox) c
>     // Find the top most position where the float box fits.
>     FloatAvoider alignedBox = { layoutBox, m_floatingState, layoutContext() };
>     floatingPosition(alignedBox);
>-    return alignedBox.topLeftInContainingBlock();
>+    return alignedBox.rectInContainingBlock().topLeft();
> }
> 
> std::optional<PositionInContainingBlock> FloatingContext::verticalPositionWithClearance(const Box& layoutBox) const
>@@ -216,25 +216,18 @@ std::optional<PositionInContainingBlock> FloatingContext::verticalPositionWithCl
> void FloatingContext::floatingPosition(FloatAvoider& floatAvoider) const
> {
>     std::optional<PositionInContextRoot> bottomMost;
>-    auto initialLeft = floatAvoider.left();
>     auto end = Layout::end(m_floatingState);
>-    for (auto iterator = begin(m_floatingState, floatAvoider.rectWithMargin().top()); iterator != end; ++iterator) {
>+    for (auto iterator = begin(m_floatingState, floatAvoider.rect().top()); iterator != end; ++iterator) {
>         ASSERT(!(*iterator).isEmpty());
>         auto floats = *iterator;
> 
>-        floatAvoider.setTop(floats.verticalPosition() + floatAvoider.marginTop());
>         // Move the box horizontally so that it either
>         // 1. aligns with the current floating pair
>         // 2. or with the containing block's content box if there's no float to align with at this vertical position.
>-        if (auto horiztonalPosition = floats.horiztonalPosition(floatAvoider.isLeftAligned() ? Float::Left : Float::Right)) {
>-            if (!floatAvoider.isLeftAligned())
>-                horiztonalPosition = *horiztonalPosition - floatAvoider.rectWithMargin().width();
>-            floatAvoider.setLeft(*horiztonalPosition + floatAvoider.marginLeft());
>-        } else
>-            floatAvoider.resetHorizontally();
>-
>+        floatAvoider.setHorizontalConstraints(floats.horizontalConstraints());
>+        floatAvoider.setVerticalConstraint(floats.verticalPosition());
>         // Check if the box fits at this position.
>-        if (!floats.intersects(floatAvoider.rectWithMargin()))
>+        if (!floats.intersects(floatAvoider.rect()))
>             return;
> 
>         bottomMost = floats.bottom();
>@@ -246,7 +239,8 @@ void FloatingContext::floatingPosition(FloatAvoider& floatAvoider) const
>         return;
> 
>     // Passed all the floats and still does not fit? Push it below the last float.
>-    floatAvoider.setTopLeft({ initialLeft, *bottomMost + floatAvoider.marginTop() });
>+    floatAvoider.setVerticalConstraint(*bottomMost);
>+    floatAvoider.setHorizontalConstraints({ });
> }
> 
> FloatingPair::FloatingPair(const FloatingState::FloatList& floats)
>@@ -306,15 +300,18 @@ bool FloatingPair::operator ==(const FloatingPair& other) const
>     return m_leftIndex == other.m_leftIndex && m_rightIndex == other.m_rightIndex;
> }
> 
>-std::optional<PositionInContextRoot> FloatingPair::horiztonalPosition(Float floatType) const
>+FloatAvoider::HorizontalConstraints FloatingPair::horizontalConstraints() const
> {
>-    if (floatType == Float::Left && left())
>-        return left()->rectWithMargin().right();
>+    std::optional<PositionInContextRoot> leftEdge;
>+    std::optional<PositionInContextRoot> rightEdge;
> 
>-    if (floatType == Float::Right && right())
>-        return right()->rectWithMargin().left();
>+    if (left())
>+        leftEdge = left()->rectWithMargin().right();
> 
>-    return { };
>+    if (right())
>+        rightEdge = right()->rectWithMargin().left();
>+
>+    return { leftEdge, rightEdge };
> }
> 
> PositionInContextRoot FloatingPair::bottom() const

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 189035: 348239