Attachment 359886 Details for Bug 193719 – Patch

[patch] Patch

bug-193719-20190123075922.patch (text/plain), 14.53 KB, created by zalan on 2019-01-23 07:59:29 PST

(hide)

Description:

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))

Actions: View | Formatted Diff | Diff

Attachments on bug 193719: 359886