WebKit Bugzilla
Attachment 348239 Details for
Bug 189035
: [LFC][Floating] Remove redundant FloatAvoider functions.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-189035-20180827164601.patch (text/plain), 13.56 KB, created by
zalan
on 2018-08-27 16:46:02 PDT
(
hide
)
Description:
Patch
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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
koivisto
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189035
: 348239