WebKit Bugzilla
Attachment 358974 Details for
Bug 193375
: [LFC][BFC][MarginCollapsing] Move estimatedMarginBefore flag from state/display box to BlockFormattingContext
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193375-20190111191553.patch (text/plain), 14.86 KB, created by
zalan
on 2019-01-11 19:16:05 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
zalan
Created:
2019-01-11 19:16:05 PST
Size:
14.86 KB
patch
obsolete
>Subversion Revision: 239863 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index da837e764364a8367dd180a8a456daa05946636a..0284be90941712134bad8bf3e6c83680bf7510df 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,43 @@ >+2019-01-11 Zalan Bujtas <zalan@apple.com> >+ >+ [LFC][BFC][MarginCollapsing] Move estimatedMarginBefore flag from state/display box to BlockFormattingContext >+ https://bugs.webkit.org/show_bug.cgi?id=193375 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ The estimated marginBefore is a pre-computed, temporary value. We need to keep it around until the final vertical margin value is computed. >+ Neither BlockFormattingState nor Display should hold temporary values. >+ >+ * layout/blockformatting/BlockFormattingContext.cpp: >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBefore const): >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBeforeForAncestors const): >+ (WebCore::Layout::BlockFormattingContext::hasPrecomputedMarginBefore const): >+ (WebCore::Layout::BlockFormattingContext::computeFloatingPosition const): >+ (WebCore::Layout::BlockFormattingContext::computePositionToAvoidFloats const): >+ (WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const): >+ (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const): >+ (WebCore::Layout::BlockFormattingContext::setEstimatedMarginBefore const): >+ (WebCore::Layout::BlockFormattingContext::hasEstimatedMarginBefore const): >+ (WebCore::Layout::hasPrecomputedMarginBefore): Deleted. >+ * layout/blockformatting/BlockFormattingContext.h: >+ (WebCore::Layout::BlockFormattingContext::removeEstimatedMarginBefore const): >+ (WebCore::Layout::BlockFormattingContext::estimatedMarginBefore const): >+ * layout/blockformatting/BlockFormattingState.h: >+ (WebCore::Layout::BlockFormattingState::setHasEstimatedMarginBefore): Deleted. >+ (WebCore::Layout::BlockFormattingState::clearHasEstimatedMarginBefore): Deleted. >+ (WebCore::Layout::BlockFormattingState::hasEstimatedMarginBefore const): Deleted. >+ * layout/displaytree/DisplayBox.cpp: >+ (WebCore::Display::Box::Box): >+ * layout/displaytree/DisplayBox.h: >+ (WebCore::Display::Box::setHasEstimatedMarginBefore): >+ (WebCore::Display::Box::invalidateEstimatedMarginBefore): >+ (WebCore::Display::Box::top const): >+ (WebCore::Display::Box::topLeft const): >+ (WebCore::Display::Box::setEstimatedMarginBefore): Deleted. >+ (WebCore::Display::Box::estimatedMarginBefore const): Deleted. >+ * page/FrameViewLayoutContext.cpp: >+ (WebCore::layoutUsingFormattingContext): >+ > 2019-01-11 Zalan Bujtas <zalan@apple.com> > > [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through. >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >index c3b1d823124069b01218067b4dcce52ca54bbfda..765baf9b932ff5a403f670539fa0fa99c5a98c27 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >@@ -188,7 +188,7 @@ void BlockFormattingContext::computeEstimatedMarginBefore(const Box& layoutBox) > { > auto& layoutState = this->layoutState(); > auto estimatedMarginBefore = MarginCollapse::estimatedMarginBefore(layoutState, layoutBox); >- blockFormattingState().setHasEstimatedMarginBefore(layoutBox); >+ setEstimatedMarginBefore(layoutBox, estimatedMarginBefore); > > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); > auto nonCollapsedValues = UsedVerticalMargin::NonCollapsedValues { estimatedMarginBefore.nonCollapsedValue, { } }; >@@ -196,7 +196,7 @@ void BlockFormattingContext::computeEstimatedMarginBefore(const Box& layoutBox) > auto verticalMargin = UsedVerticalMargin { nonCollapsedValues, collapsedValues }; > displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin)); > #if !ASSERT_DISABLED >- displayBox.setEstimatedMarginBefore(estimatedMarginBefore); >+ displayBox.setHasEstimatedMarginBefore(); > #endif > } > >@@ -214,10 +214,9 @@ void BlockFormattingContext::computeEstimatedMarginBeforeForAncestors(const Box& > // 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. >- auto& formattingState = blockFormattingState(); > for (auto* ancestor = layoutBox.containingBlock(); ancestor && !ancestor->establishesBlockFormattingContext(); ancestor = ancestor->containingBlock()) { > // FIXME: with incremental layout, we might actually have a valid (non-estimated) margin top as well. >- if (formattingState.hasEstimatedMarginBefore(*ancestor)) >+ if (hasEstimatedMarginBefore(*ancestor)) > return; > > computeEstimatedMarginBefore(*ancestor); >@@ -242,10 +241,10 @@ void BlockFormattingContext::precomputeVerticalPositionForFormattingRootIfNeeded > } > > #ifndef NDEBUG >-static bool hasPrecomputedMarginBefore(const BlockFormattingState& formattingState, const Box& layoutBox) >+bool BlockFormattingContext::hasPrecomputedMarginBefore(const Box& layoutBox) const > { > for (auto* ancestor = layoutBox.containingBlock(); ancestor && !ancestor->establishesBlockFormattingContext(); ancestor = ancestor->containingBlock()) { >- if (formattingState.hasEstimatedMarginBefore(*ancestor)) >+ if (hasEstimatedMarginBefore(*ancestor)) > continue; > return false; > } >@@ -257,7 +256,7 @@ void BlockFormattingContext::computeFloatingPosition(const FloatingContext& floa > { > auto& layoutState = this->layoutState(); > ASSERT(layoutBox.isFloatingPositioned()); >- ASSERT(hasPrecomputedMarginBefore(blockFormattingState(), layoutBox)); >+ ASSERT(hasPrecomputedMarginBefore(layoutBox)); > > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); > // 8.3.1 Collapsing margins >@@ -277,7 +276,7 @@ void BlockFormattingContext::computePositionToAvoidFloats(const FloatingContext& > ASSERT(layoutBox.establishesBlockFormattingContext()); > ASSERT(!layoutBox.isFloatingPositioned()); > ASSERT(!layoutBox.hasFloatClear()); >- ASSERT(hasPrecomputedMarginBefore(blockFormattingState(), layoutBox)); >+ ASSERT(hasPrecomputedMarginBefore(layoutBox)); > > if (floatingContext.floatingState().isEmpty()) > return; >@@ -296,7 +295,7 @@ void BlockFormattingContext::computeVerticalPositionForFloatClear(const Floating > // For formatting roots, we already precomputed final position. > if (!layoutBox.establishesFormattingContext()) > computeEstimatedMarginBeforeForAncestors(layoutBox); >- ASSERT(hasPrecomputedMarginBefore(blockFormattingState(), layoutBox)); >+ ASSERT(hasPrecomputedMarginBefore(layoutBox)); > > if (auto verticalPositionWithClearance = floatingContext.verticalPositionWithClearance(layoutBox)) > layoutState.displayBoxForLayoutBox(layoutBox).setTop(*verticalPositionWithClearance); >@@ -384,18 +383,15 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const > > // Out of flow boxes don't need vertical adjustment after margin collapsing. > if (layoutBox.isOutOfFlowPositioned()) { >+ ASSERT(!hasEstimatedMarginBefore(layoutBox)); > displayBox.setContentBoxHeight(heightAndMargin.height); > displayBox.setVerticalMargin(verticalMargin); > return; > } > >- auto& formattingState = this->blockFormattingState(); >- if (formattingState.hasEstimatedMarginBefore(layoutBox)) { >- formattingState.clearHasEstimatedMarginBefore(layoutBox); >- ASSERT(displayBox.estimatedMarginBefore()->usedValue() == verticalMargin.before()); >- } else >- displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin)); >- >+ ASSERT(!hasEstimatedMarginBefore(layoutBox) || estimatedMarginBefore(layoutBox).usedValue() == verticalMargin.before()); >+ removeEstimatedMarginBefore(layoutBox); >+ displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin)); > // Adjust the previous sibling's margin bottom now that this box's vertical margin is computed. > if (MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox)) > MarginCollapse::updateCollapsedMarginAfter(layoutState, *layoutBox.previousInFlowSibling(), verticalMargin); >@@ -524,6 +520,20 @@ LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing > return containingBlockContentBoxTop + verticalMargin.before(); > } > >+void BlockFormattingContext::setEstimatedMarginBefore(const Box& layoutBox, const EstimatedMarginBefore& estimatedMarginBefore) const >+{ >+ // Can't cross formatting context boundary. >+ ASSERT(&layoutState().formattingStateForBox(layoutBox) == &formattingState()); >+ m_estimatedMarginBeforeList.set(&layoutBox, estimatedMarginBefore); >+} >+ >+bool BlockFormattingContext::hasEstimatedMarginBefore(const Box& layoutBox) const >+{ >+ // Can't cross formatting context boundary. >+ ASSERT(&layoutState().formattingStateForBox(layoutBox) == &formattingState()); >+ return m_estimatedMarginBeforeList.contains(&layoutBox); >+} >+ > } > } > >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >index a2cfdad2017fae77103e8081c311e6f556608e10..8c837b0abfe33ec66f3702d16f6956d8f27988b3 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >@@ -28,6 +28,8 @@ > #if ENABLE(LAYOUT_FORMATTING_CONTEXT) > > #include "FormattingContext.h" >+#include "MarginTypes.h" >+#include <wtf/HashMap.h> > #include <wtf/IsoMalloc.h> > > namespace WebCore { >@@ -126,6 +128,17 @@ private: > static bool shouldIgnoreMarginBefore(const LayoutState&, const Box&); > static bool shouldIgnoreMarginAfter(const LayoutState&, const Box&); > }; >+ >+ void setEstimatedMarginBefore(const Box&, const EstimatedMarginBefore&) const; >+ void removeEstimatedMarginBefore(const Box& layoutBox) const { m_estimatedMarginBeforeList.remove(&layoutBox); } >+ bool hasEstimatedMarginBefore(const Box&) const; >+#ifndef NDEBUG >+ EstimatedMarginBefore estimatedMarginBefore(const Box& layoutBox) const { return m_estimatedMarginBeforeList.get(&layoutBox); } >+ bool hasPrecomputedMarginBefore(const Box&) const; >+#endif >+ >+private: >+ mutable HashMap<const Box*, EstimatedMarginBefore> m_estimatedMarginBeforeList; > }; > > } >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingState.h b/Source/WebCore/layout/blockformatting/BlockFormattingState.h >index bb456b5927fefe45c637831d5b24f717092bad2a..b28ef695f148c3f91afabd838c98b7776df436ac 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingState.h >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingState.h >@@ -28,7 +28,6 @@ > #if ENABLE(LAYOUT_FORMATTING_CONTEXT) > > #include "FormattingState.h" >-#include "MarginTypes.h" > #include <wtf/IsoMalloc.h> > > namespace WebCore { >@@ -48,13 +47,8 @@ public: > bool hasPositiveAndNegativeVerticalMargin(const Box& layoutBox) const { return m_positiveAndNegativeVerticalMargin.contains(&layoutBox); } > PositiveAndNegativeVerticalMargin positiveAndNegativeVerticalMargin(const Box& layoutBox) const { return m_positiveAndNegativeVerticalMargin.get(&layoutBox); } > >- void setHasEstimatedMarginBefore(const Box& layoutBox) { m_estimatedMarginBeforeList.add(&layoutBox); } >- void clearHasEstimatedMarginBefore(const Box& layoutBox) { m_estimatedMarginBeforeList.remove(&layoutBox); } >- bool hasEstimatedMarginBefore(const Box& layoutBox) const { return m_estimatedMarginBeforeList.contains(&layoutBox); } >- > private: > HashMap<const Box*, PositiveAndNegativeVerticalMargin> m_positiveAndNegativeVerticalMargin; >- HashSet<const Box*> m_estimatedMarginBeforeList; > }; > > } >diff --git a/Source/WebCore/layout/displaytree/DisplayBox.cpp b/Source/WebCore/layout/displaytree/DisplayBox.cpp >index 631ef71b983c715ad7e2ead54c0c152ec95102ea..0eeeaecd374aabdb3aeef00c8f3124a6cae0af39 100644 >--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp >+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp >@@ -74,7 +74,7 @@ Box::Box(const Box& other) > , m_hasValidPadding(other.m_hasValidPadding) > , m_hasValidContentHeight(other.m_hasValidContentHeight) > , m_hasValidContentWidth(other.m_hasValidContentWidth) >- , m_estimatedMarginBefore(other.m_estimatedMarginBefore) >+ , m_hasEstimatedMarginBefore(other.m_hasEstimatedMarginBefore) > #endif > { > } >diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h >index 9aefb18d7759735820f34b0a0333302e5dc72719..16782c44ebcb765869f1691174131572b17df065 100644 >--- a/Source/WebCore/layout/displaytree/DisplayBox.h >+++ b/Source/WebCore/layout/displaytree/DisplayBox.h >@@ -175,8 +175,7 @@ public: > Rect contentBox() const; > > #if !ASSERT_DISABLED >- void setEstimatedMarginBefore(Layout::EstimatedMarginBefore marginBefore) { m_estimatedMarginBefore = marginBefore; } >- Optional<Layout::EstimatedMarginBefore> estimatedMarginBefore() const { return m_estimatedMarginBefore; } >+ void setHasEstimatedMarginBefore() { m_hasEstimatedMarginBefore = true; } > #endif > > private: >@@ -207,7 +206,7 @@ private: > void invalidateMargin(); > void invalidateBorder() { m_hasValidBorder = false; } > void invalidatePadding() { m_hasValidPadding = false; } >- void invalidateEstimatedMarginBefore() { m_estimatedMarginBefore = { }; } >+ void invalidateEstimatedMarginBefore() { m_hasEstimatedMarginBefore = false; } > > void setHasValidTop() { m_hasValidTop = true; } > void setHasValidLeft() { m_hasValidLeft = true; } >@@ -248,7 +247,7 @@ private: > bool m_hasValidPadding { false }; > bool m_hasValidContentHeight { false }; > bool m_hasValidContentWidth { false }; >- Optional<Layout::EstimatedMarginBefore> m_estimatedMarginBefore; >+ bool m_hasEstimatedMarginBefore { false }; > #endif > }; > >@@ -443,7 +442,7 @@ inline Box::Rect::operator LayoutRect() const > > inline LayoutUnit Box::top() const > { >- ASSERT(m_hasValidTop && (m_estimatedMarginBefore || m_hasValidVerticalMargin)); >+ ASSERT(m_hasValidTop && (m_hasEstimatedMarginBefore || m_hasValidVerticalMargin)); > return m_topLeft.y(); > } > >@@ -455,7 +454,7 @@ inline LayoutUnit Box::left() const > > inline LayoutPoint Box::topLeft() const > { >- ASSERT(m_hasValidTop && (m_estimatedMarginBefore || m_hasValidVerticalMargin)); >+ ASSERT(m_hasValidTop && (m_hasEstimatedMarginBefore || m_hasValidVerticalMargin)); > ASSERT(m_hasValidLeft && m_hasValidHorizontalMargin); > return m_topLeft; > }
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 193375
: 358974