[LFC][BFC] computeStaticPosition should include estimated computation as well.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2019 16:29:53 +0000 (16:29 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2019 16:29:53 +0000 (16:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193719

Reviewed by Antti Koivisto.

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240337 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
Source/WebCore/layout/blockformatting/BlockFormattingContext.h
Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp

index d37fb3e..91af4cb 100644 (file)
@@ -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 Antti Koivisto.
+
+        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  Simon Fraser  <simon.fraser@apple.com>
 
         Compositing updates need to reparent scrolling tree nodes with a changed ancestor
index 5317d75..b7fae72 100644 (file)
@@ -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();
index f98ef2c..c3e58f3 100644 (file)
@@ -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;
index f68dda3..677a0eb 100644 (file)
@@ -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 };
 }
index 2ef34df..8c893ce 100644 (file)
@@ -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))