WebKit Bugzilla
Attachment 359886 Details for
Bug 193719
: [LFC][BFC] computeStaticPosition should include estimated computation as well.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193719-20190123075922.patch (text/plain), 14.53 KB, created by
zalan
on 2019-01-23 07:59:29 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
zalan
Created:
2019-01-23 07:59:29 PST
Size:
14.53 KB
patch
obsolete
>Subversion Revision: 240253 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 2eae79f395066af7e2c903d762804982d23fdb2d..1949473c1af9b477287dc1321354d70c5684ff36 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,36 @@ >+2019-01-23 Zalan Bujtas <zalan@apple.com> >+ >+ [LFC][BFC] computeStaticPosition should include estimated computation as well. >+ https://bugs.webkit.org/show_bug.cgi?id=193719 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Consolidate all static position (non-estimated, estimated) computation in BlockFormattingContext::computeStaticPosition. >+ It requires to compute width/horizontal margin first, since vertical top estimation needs valid horizontal widths (margin-top: 5% is computed using >+ the containing block's width). >+ This is also in preparation for moving 'clear' positioning to computeStaticPosition. >+ >+ * layout/blockformatting/BlockFormattingContext.cpp: >+ (WebCore::Layout::BlockFormattingContext::layout const): >+ (WebCore::Layout::BlockFormattingContext::layoutFormattingContextRoot const): >+ (WebCore::Layout::BlockFormattingContext::computeStaticPosition const): >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedVerticalPosition const): >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedVerticalPositionForAncestors const): >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedVerticalPositionForFormattingRoot const): >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedVerticalPositionForFloatClear const): >+ (WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const): >+ (WebCore::Layout::BlockFormattingContext::computeWidthAndMargin const): >+ (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const): >+ (WebCore::Layout::BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing const): >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBefore const): Deleted. >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBeforeForAncestors const): Deleted. >+ (WebCore::Layout::BlockFormattingContext::precomputeVerticalPositionForFormattingRootIfNeeded const): Deleted. >+ * layout/blockformatting/BlockFormattingContext.h: >+ * layout/blockformatting/BlockFormattingContextGeometry.cpp: >+ (WebCore::Layout::BlockFormattingContext::Geometry::staticPosition): >+ * layout/blockformatting/BlockMarginCollapse.cpp: >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithFirstInFlowChildMarginBefore): >+ > 2019-01-22 Zalan Bujtas <zalan@apple.com> > > [LFC][Floats] Decouple clearance computation and margin collapsing reset. >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >index 5317d75c690b28ad00b41bd3acf1111f829ef136..b7fae7213e8b81809716ee78540f807c33d55667 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >@@ -88,9 +88,9 @@ void BlockFormattingContext::layout() const > } > > LOG_WITH_STREAM(FormattingContextLayout, stream << "[Compute] -> [Position][Border][Padding][Width][Margin] -> for layoutBox(" << &layoutBox << ")"); >- computeStaticPosition(layoutBox); > computeBorderAndPadding(layoutBox); > computeWidthAndMargin(layoutBox); >+ computeStaticPosition(layoutBox); > if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowOrFloatingChild()) > break; > layoutQueue.append(downcast<Container>(layoutBox).firstInFlowOrFloatingChild()); >@@ -128,11 +128,9 @@ void BlockFormattingContext::layoutFormattingContextRoot(FloatingContext& floati > { > // Start laying out this formatting root in the formatting contenxt it lives in. > LOG_WITH_STREAM(FormattingContextLayout, stream << "[Compute] -> [Position][Border][Padding][Width][Margin] -> for layoutBox(" << &layoutBox << ")"); >- computeStaticPosition(layoutBox); > computeBorderAndPadding(layoutBox); > computeWidthAndMargin(layoutBox); >- >- precomputeVerticalPositionForFormattingRootIfNeeded(layoutBox); >+ computeStaticPosition(layoutBox); > // Swich over to the new formatting context (the one that the root creates). > auto formattingContext = layoutState().createFormattingContext(layoutBox); > formattingContext->layout(); >@@ -182,9 +180,13 @@ void BlockFormattingContext::computeStaticPosition(const Box& layoutBox) const > { > auto& layoutState = this->layoutState(); > layoutState.displayBoxForLayoutBox(layoutBox).setTopLeft(Geometry::staticPosition(layoutState, layoutBox)); >+ if (layoutBox.hasFloatClear()) >+ computeEstimatedVerticalPositionForFloatClear(layoutBox); >+ else if (layoutBox.establishesFormattingContext()) >+ computeEstimatedVerticalPositionForFormattingRoot(layoutBox); > } > >-void BlockFormattingContext::computeEstimatedMarginBefore(const Box& layoutBox) const >+void BlockFormattingContext::computeEstimatedVerticalPosition(const Box& layoutBox) const > { > auto& layoutState = this->layoutState(); > auto estimatedMarginBefore = MarginCollapse::estimatedMarginBefore(layoutState, layoutBox); >@@ -194,13 +196,14 @@ void BlockFormattingContext::computeEstimatedMarginBefore(const Box& layoutBox) > auto nonCollapsedValues = UsedVerticalMargin::NonCollapsedValues { estimatedMarginBefore.nonCollapsedValue, { } }; > auto collapsedValues = UsedVerticalMargin::CollapsedValues { estimatedMarginBefore.collapsedValue, { }, estimatedMarginBefore.isCollapsedThrough }; > auto verticalMargin = UsedVerticalMargin { nonCollapsedValues, collapsedValues }; >+ displayBox.setVerticalMargin(verticalMargin); > displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin)); > #if !ASSERT_DISABLED > displayBox.setHasEstimatedMarginBefore(); > #endif > } > >-void BlockFormattingContext::computeEstimatedMarginBeforeForAncestors(const Box& layoutBox) const >+void BlockFormattingContext::computeEstimatedVerticalPositionForAncestors(const Box& layoutBox) const > { > // We only need to estimate margin top for float related layout (formatting context roots avoid floats). > ASSERT(layoutBox.isFloatingPositioned() || layoutBox.hasFloatClear() || layoutBox.establishesBlockFormattingContext() || layoutBox.establishesInlineFormattingContext()); >@@ -213,33 +216,37 @@ void BlockFormattingContext::computeEstimatedMarginBeforeForAncestors(const 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. >+ // 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 position. > 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 (hasEstimatedMarginBefore(*ancestor)) > return; >- >- computeEstimatedMarginBefore(*ancestor); >+ computeEstimatedVerticalPosition(*ancestor); > } > } > >-void BlockFormattingContext::precomputeVerticalPositionForFormattingRootIfNeeded(const Box& layoutBox) const >+void BlockFormattingContext::computeEstimatedVerticalPositionForFormattingRoot(const Box& layoutBox) const > { > ASSERT(layoutBox.establishesFormattingContext()); >+ ASSERT(!layoutBox.hasFloatClear()); > >- auto avoidsFloats = layoutBox.isFloatingPositioned() || layoutBox.establishesBlockFormattingContext() || layoutBox.hasFloatClear(); >+ auto avoidsFloats = layoutBox.isFloatingPositioned() || layoutBox.establishesBlockFormattingContext(); > if (avoidsFloats) >- computeEstimatedMarginBeforeForAncestors(layoutBox); >+ computeEstimatedVerticalPositionForAncestors(layoutBox); > > // If the inline formatting root is also the root for the floats (happens when the root box also establishes a block formatting context) > // the floats are in the coordinate system of this root. No need to find the final vertical position. > auto inlineContextInheritsFloats = layoutBox.establishesInlineFormattingContext() && !layoutBox.establishesBlockFormattingContext(); > if (inlineContextInheritsFloats) { >- computeEstimatedMarginBefore(layoutBox); >- computeEstimatedMarginBeforeForAncestors(layoutBox); >+ computeEstimatedVerticalPosition(layoutBox); >+ computeEstimatedVerticalPositionForAncestors(layoutBox); > } > } > >+void BlockFormattingContext::computeEstimatedVerticalPositionForFloatClear(const Box&) const >+{ >+} >+ > #ifndef NDEBUG > bool BlockFormattingContext::hasPrecomputedMarginBefore(const Box& layoutBox) const > { >@@ -293,7 +300,7 @@ void BlockFormattingContext::computeVerticalPositionForFloatClear(const Floating > > // For formatting roots, we already precomputed final position. > if (!layoutBox.establishesFormattingContext()) >- computeEstimatedMarginBeforeForAncestors(layoutBox); >+ computeEstimatedVerticalPositionForAncestors(layoutBox); > ASSERT(hasPrecomputedMarginBefore(layoutBox)); > > // 1. Compute and adjust the vertical position with clearance. >@@ -365,7 +372,6 @@ void BlockFormattingContext::computeWidthAndMargin(const Box& layoutBox) const > > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); > displayBox.setContentBoxWidth(widthAndMargin.width); >- displayBox.moveHorizontally(widthAndMargin.usedMargin.start); > displayBox.setHorizontalMargin(widthAndMargin.usedMargin); > displayBox.setHorizontalComputedMargin(widthAndMargin.computedMargin); > } >@@ -406,7 +412,7 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const > } > > // 1. Compute collapsed margins. >- // 2. Adjust vertical position using the collaped values >+ // 2. Adjust vertical position using the collapsed values > // 3. Adjust previous in-flow sibling margin after using this margin. > auto collapsedMargin = MarginCollapse::collapsedVerticalValues(layoutState, layoutBox, heightAndMargin.nonCollapsedMargin); > auto verticalMargin = UsedVerticalMargin { heightAndMargin.nonCollapsedMargin, collapsedMargin }; >@@ -499,7 +505,7 @@ LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing > { > ASSERT(!layoutBox.isOutOfFlowPositioned()); > // Now that we've computed the final margin before, let's shift the box's vertical position. >- // 1. Check if the margin before collapses with the previous box's margin after. if not -> return previous box's bottom inlcuding margin after + marginBefore >+ // 1. Check if the margin before collapses with the previous box's margin after. if not -> return previous box's bottom including margin after + marginBefore > // 2. Check if the previous box's margins collapse through. If not -> return previous box' bottom excluding margin after + marginBefore (they are supposed to be equal) > // 3. Go to previous box and start from step #1 until we hit the parent box. > auto& layoutState = this->layoutState(); >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >index f98ef2c1b5e10fb073a10c18ef9b730a8035a36d..c3e58f32a9365951cc72401eefc9c3009c36dc7d 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >@@ -63,10 +63,10 @@ private: > void computePositionToAvoidFloats(const FloatingContext&, const Box&) const; > void computeVerticalPositionForFloatClear(const FloatingContext&, const Box&) const; > >- void computeEstimatedMarginBeforeForAncestors(const Box&) const; >- void computeEstimatedMarginBefore(const Box&) const; >- >- void precomputeVerticalPositionForFormattingRootIfNeeded(const Box&) const; >+ void computeEstimatedVerticalPosition(const Box&) const; >+ void computeEstimatedVerticalPositionForAncestors(const Box&) const; >+ void computeEstimatedVerticalPositionForFormattingRoot(const Box&) const; >+ void computeEstimatedVerticalPositionForFloatClear(const Box&) const; > > InstrinsicWidthConstraints instrinsicWidthConstraints() const override; > LayoutUnit adjustedVerticalPositionAfterMarginCollapsing(const Box&, const UsedVerticalMargin&) const; >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp >index f68dda3cbd317ed801410418b8ad7ca497b5cd18..677a0eb1b855434d6fd037617581714e523f6f40 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp >@@ -236,7 +236,7 @@ Point BlockFormattingContext::Geometry::staticPosition(const LayoutState& layout > } else > top = containingBlockDisplayBox.contentBoxTop(); > >- auto left = containingBlockDisplayBox.contentBoxLeft(); >+ auto left = containingBlockDisplayBox.contentBoxLeft() + layoutState.displayBoxForLayoutBox(layoutBox).marginStart(); > LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position] -> static -> top(" << top << "px) left(" << left << "px) layoutBox(" << &layoutBox << ")"); > return { left, top }; > } >diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >index 2ef34df5a4c2127e1e1ccf251904ec38f79e32ed..8c893ce8e7de07256b53102cdc627c0e51b562a7 100644 >--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >@@ -187,10 +187,6 @@ bool BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithFirstInFlo > if (establishesBlockFormattingContext(layoutBox)) > return false; > >- // Margins of elements that establish new block formatting contexts do not collapse with their in-flow children. >- if (establishesBlockFormattingContext(layoutBox)) >- return false; >- > // The top margin of an in-flow block element collapses with its first in-flow block-level > // child's top margin if the element has no top border... > if (hasBorderBefore(layoutBox))
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
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193719
: 359886