[LFC] Compute both the collapsed and the non-collapsed margin values.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2018 03:05:16 +0000 (03:05 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2018 03:05:16 +0000 (03:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187129

Reviewed by Antti Koivisto.

For validation purposes only at this point.

* layout/FormattingContext.cpp:
(WebCore::Layout::FormattingContext::computeFloatingHeightAndMargin const):
(WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
* layout/FormattingContext.h:
* layout/FormattingContextGeometry.cpp:
(WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedVerticalGeometry):
(WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry):
(WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedHeightAndMargin):
(WebCore::Layout::FormattingContext::Geometry::inlineReplacedHeightAndMargin):
* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
* layout/blockformatting/BlockFormattingContextGeometry.cpp:
(WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
* layout/blockformatting/BlockMarginCollapse.cpp:
(WebCore::Layout::isMarginTopCollapsedWithParent):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginTop):

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

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

index 0c42e42..0c9239c 100644 (file)
@@ -1,5 +1,32 @@
 2018-06-27  Zalan Bujtas  <zalan@apple.com>
 
+        [LFC] Compute both the collapsed and the non-collapsed margin values.
+        https://bugs.webkit.org/show_bug.cgi?id=187129
+
+        Reviewed by Antti Koivisto.
+
+        For validation purposes only at this point.
+
+        * layout/FormattingContext.cpp:
+        (WebCore::Layout::FormattingContext::computeFloatingHeightAndMargin const):
+        (WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
+        * layout/FormattingContext.h:
+        * layout/FormattingContextGeometry.cpp:
+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedVerticalGeometry):
+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry):
+        (WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedHeightAndMargin):
+        (WebCore::Layout::FormattingContext::Geometry::inlineReplacedHeightAndMargin):
+        * layout/blockformatting/BlockFormattingContext.cpp:
+        (WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
+        * layout/blockformatting/BlockMarginCollapse.cpp:
+        (WebCore::Layout::isMarginTopCollapsedWithParent):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild):
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginTop):
+
+2018-06-27  Zalan Bujtas  <zalan@apple.com>
+
         [LFC] Align inFlowNonReplacedHeightAndMargin() style with the rest of the compute functions.
         https://bugs.webkit.org/show_bug.cgi?id=187126
 
index 7c58b66..08f35d9 100644 (file)
@@ -55,7 +55,7 @@ void FormattingContext::computeFloatingHeightAndMargin(LayoutContext& layoutCont
 {
     auto heightAndMargin = Geometry::floatingHeightAndMargin(layoutContext, layoutBox);
     displayBox.setContentBoxHeight(heightAndMargin.height);
-    displayBox.moveVertically(heightAndMargin.margin.top);
+    displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
     displayBox.setVerticalMargin(heightAndMargin.margin);
 }
 
@@ -79,7 +79,8 @@ void FormattingContext::computeOutOfFlowVerticalGeometry(LayoutContext& layoutCo
 {
     auto verticalGeometry = Geometry::outOfFlowVerticalGeometry(layoutContext, layoutBox);
     displayBox.setTop(verticalGeometry.top);
-    displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height);
+    displayBox.setContentBoxHeight(verticalGeometry.bottom - verticalGeometry.top);
+    ASSERT(!verticalGeometry.heightAndMargin.collapsedMargin);
     displayBox.setVerticalMargin(verticalGeometry.heightAndMargin.margin);
 }
 
index ea17c7b..05b8288 100644 (file)
@@ -96,6 +96,7 @@ protected:
         struct HeightAndMargin {
             LayoutUnit height;
             Display::Box::VerticalEdges margin;
+            std::optional<Display::Box::VerticalEdges> collapsedMargin;
         };
 
          struct HorizontalGeometry {
index 2466f2d..c62b233 100644 (file)
@@ -227,7 +227,7 @@ FormattingContext::Geometry::VerticalGeometry FormattingContext::Geometry::outOf
     ASSERT(marginBottom);
 
     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Height][Margin] -> out-of-flow non-replaced -> top(" << *top << "px) bottom("  << *bottom << "px) height(" << *height << "px) margin(" << *marginTop << "px, "  << *marginBottom << "px) layoutBox(" << &layoutBox << ")");
-    return { *top, *bottom, { *height, { *marginTop, *marginBottom } } };
+    return { *top, *bottom, { *height, { *marginTop, *marginBottom }, { } } };
 }
 
 FormattingContext::Geometry::HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry(LayoutContext& layoutContext, const Box& layoutBox)
@@ -440,7 +440,7 @@ FormattingContext::Geometry::VerticalGeometry FormattingContext::Geometry::outOf
         bottom = containingBlockHeight - (*top + *marginTop + borderTop + paddingTop + height + paddingBottom + borderBottom + *marginBottom); 
 
     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Height][Margin] -> out-of-flow replaced -> top(" << *top << "px) bottom("  << *bottom << "px) height(" << height << "px) margin(" << *marginTop << "px, "  << *marginBottom << "px) layoutBox(" << &layoutBox << ")");
-    return { *top, *bottom, { height, { *marginTop, *marginBottom } } };
+    return { *top, *bottom, { height, { *marginTop, *marginBottom }, { } } };
 }
 
 FormattingContext::Geometry::HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry(LayoutContext& layoutContext, const Box& layoutBox)
@@ -570,7 +570,7 @@ FormattingContext::Geometry::HeightAndMargin FormattingContext::Geometry::floati
         height = contentHeightForFormattingContextRoot(layoutContext, layoutBox);
 
     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> floating non-replaced -> height(" << *height << "px) margin(" << *marginTop << "px, " << *marginBottom << "px) -> layoutBox(" << &layoutBox << ")");
-    return FormattingContext::Geometry::HeightAndMargin { *height, { *marginTop, *marginBottom } };
+    return FormattingContext::Geometry::HeightAndMargin { *height, { *marginTop, *marginBottom }, { } };
 }
 
 FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::floatingNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
@@ -699,7 +699,7 @@ FormattingContext::Geometry::HeightAndMargin FormattingContext::Geometry::inline
     ASSERT(height);
 
     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow replaced -> height(" << *height << "px) margin(" << margin.top << "px, " << margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
-    return { *height, margin };
+    return { *height, margin, { } };
 }
 
 FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox,
index 1d342bb..4357735 100644 (file)
@@ -192,7 +192,7 @@ void BlockFormattingContext::computeInFlowHeightAndMargin(LayoutContext& layoutC
 {
     auto heightAndMargin = Geometry::inFlowHeightAndMargin(layoutContext, layoutBox);
     displayBox.setContentBoxHeight(heightAndMargin.height);
-    displayBox.moveVertically(heightAndMargin.margin.top);
+    displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
     displayBox.setVerticalMargin(heightAndMargin.margin);
 }
 
index d27bdfc..7edd2cd 100644 (file)
@@ -79,20 +79,21 @@ FormattingContext::Geometry::HeightAndMargin BlockFormattingContext::Geometry::i
         auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->contentBoxWidth();
         auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
 
-        auto marginTop = FormattingContext::Geometry::computedValueIfNotAuto(style.marginTop(), containingBlockWidth).value_or(0);
-        auto marginBottom = FormattingContext::Geometry::computedValueIfNotAuto(style.marginBottom(), containingBlockWidth).value_or(0);
+        Display::Box::VerticalEdges nonCollapsedMargin = { FormattingContext::Geometry::computedValueIfNotAuto(style.marginTop(), containingBlockWidth).value_or(0),
+            FormattingContext::Geometry::computedValueIfNotAuto(style.marginBottom(), containingBlockWidth).value_or(0) }; 
+        Display::Box::VerticalEdges collapsedMargin = { MarginCollapse::marginTop(layoutContext, layoutBox), MarginCollapse::marginBottom(layoutContext, layoutBox) };
         auto borderAndPaddingTop = displayBox.borderTop() + displayBox.paddingTop();
         
         if (!style.logicalHeight().isAuto())
-            return { style.logicalHeight().value(), { marginTop, marginBottom } };
+            return { style.logicalHeight().value(), nonCollapsedMargin, collapsedMargin };
 
         if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
-            return { 0, { marginTop, marginBottom } };
+            return { 0, nonCollapsedMargin, collapsedMargin };
 
         // 1. the bottom edge of the last line box, if the box establishes a inline formatting context with one or more lines
         if (layoutBox.establishesInlineFormattingContext()) {
             // height = lastLineBox().bottom();
-            return { 0, { marginTop, marginBottom } };
+            return { 0, nonCollapsedMargin, collapsedMargin };
         }
 
         // 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin...
@@ -101,7 +102,7 @@ FormattingContext::Geometry::HeightAndMargin BlockFormattingContext::Geometry::i
         if (!MarginCollapse::isMarginBottomCollapsedWithParent(*lastInFlowChild)) {
             auto* lastInFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*lastInFlowChild);
             ASSERT(lastInFlowDisplayBox);
-            return { lastInFlowDisplayBox->bottom() + lastInFlowDisplayBox->marginBottom() - borderAndPaddingTop, { marginTop, marginBottom } };
+            return { lastInFlowDisplayBox->bottom() + lastInFlowDisplayBox->marginBottom() - borderAndPaddingTop, nonCollapsedMargin, collapsedMargin };
         }
 
         // 3. the bottom border edge of the last in-flow child whose top margin doesn't collapse with the element's bottom margin
@@ -111,29 +112,27 @@ FormattingContext::Geometry::HeightAndMargin BlockFormattingContext::Geometry::i
         if (inFlowChild) {
             auto* inFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*inFlowChild);
             ASSERT(inFlowDisplayBox);
-            return { inFlowDisplayBox->top() + inFlowDisplayBox->borderBox().height() - borderAndPaddingTop, { marginTop, marginBottom } };
+            return { inFlowDisplayBox->top() + inFlowDisplayBox->borderBox().height() - borderAndPaddingTop, nonCollapsedMargin, collapsedMargin };
         }
 
         // 4. zero, otherwise
-        return { 0, { marginTop, marginBottom } };
+        return { 0, nonCollapsedMargin, collapsedMargin };
     };
 
     auto heightAndMargin = compute();
-    auto collapsedMarginTop = MarginCollapse::marginTop(layoutContext, layoutBox);
-    auto collapsedMarginBottom =  MarginCollapse::marginBottom(layoutContext, layoutBox);
 
     if (!isStretchedToViewport(layoutContext, layoutBox)) {
-        LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << collapsedMarginTop << "px, " << collapsedMarginBottom << "px) -> layoutBox(" << &layoutBox << ")");
-        return { heightAndMargin.height, { collapsedMarginTop, collapsedMarginBottom } };
+        LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
+        return heightAndMargin;
     }
 
     auto initialContainingBlockHeight = layoutContext.displayBoxForLayoutBox(initialContainingBlock(layoutBox))->contentBoxHeight();
     // Stretch but never overstretch with the margins.
-    if (heightAndMargin.height + collapsedMarginTop + collapsedMarginBottom < initialContainingBlockHeight)
-        heightAndMargin.height = initialContainingBlockHeight - collapsedMarginTop - collapsedMarginBottom;
+    if (heightAndMargin.height + heightAndMargin.margin.top + heightAndMargin.margin.bottom < initialContainingBlockHeight)
+        heightAndMargin.height = initialContainingBlockHeight - heightAndMargin.margin.top - heightAndMargin.margin.bottom;
 
-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << collapsedMarginTop << "px, " << collapsedMarginBottom << "px) -> layoutBox(" << &layoutBox << ")");
-    return { heightAndMargin.height, { collapsedMarginTop, collapsedMarginBottom } };
+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
+    return heightAndMargin;
 }
 
 FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox,
index 42a9eb8..ea6caa8 100644 (file)
@@ -78,7 +78,7 @@ static bool isMarginBottomCollapsedWithSibling(const Box& layoutBox)
     return layoutBox.style().bottom().isAuto();
 }
 
-static bool isMarginTopCollapsedWithParent(const Box& layoutBox, const Display::Box& displayBox)
+static bool isMarginTopCollapsedWithParent(const LayoutContext& layoutContext, const Box& layoutBox)
 {
     // The first inflow child could propagate its top margin to parent.
     // https://www.w3.org/TR/CSS21/box.html#collapsing-margins
@@ -99,13 +99,14 @@ static bool isMarginTopCollapsedWithParent(const Box& layoutBox, const Display::
         return false;
 
     // Margins of the root element's box do not collapse.
-    if (layoutBox.isInitialContainingBlock())
+    if (parent.isDocumentBox() || parent.isInitialContainingBlock())
         return false;
 
-    if (displayBox.borderTop())
+    auto& parentDisplayBox = *layoutContext.displayBoxForLayoutBox(parent);
+    if (parentDisplayBox.borderTop())
         return false;
 
-    if (!displayBox.paddingTop())
+    if (parentDisplayBox.paddingTop())
         return false;
 
     return true;
@@ -118,8 +119,7 @@ LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstCh
         return 0;
 
     auto& firstInFlowChild = *downcast<Container>(layoutBox).firstInFlowChild();
-    auto& fistInflowDisplayBox = *layoutContext.displayBoxForLayoutBox(firstInFlowChild);
-    if (!isMarginTopCollapsedWithParent(firstInFlowChild, fistInflowDisplayBox))
+    if (!isMarginTopCollapsedWithParent(layoutContext, firstInFlowChild))
         return 0;
 
     // Collect collapsed margin top recursively.
@@ -162,8 +162,7 @@ LayoutUnit BlockFormattingContext::MarginCollapse::marginTop(const LayoutContext
         return 0;
 
     // TODO: take _hasAdjoiningMarginTopAndBottom() into account.
-    auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
-    if (isMarginTopCollapsedWithParent(layoutBox, displayBox))
+    if (isMarginTopCollapsedWithParent(layoutContext, layoutBox))
         return 0;
 
     // Floats and out of flow positioned boxes do not collapse their margins.