[LFC][BFC] In-flow positioned logic is really formatting context dependent.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Nov 2018 21:29:12 +0000 (21:29 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Nov 2018 21:29:12 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191512

Reviewed by Simon Fraser.

Move block formatting context specific code from FormattingContext to BlockFormattingContext.

* layout/FormattingContext.cpp:
(WebCore::Layout::FormattingContext::placeInFlowPositionedChildren const): Deleted.
* layout/FormattingContext.h:
* layout/FormattingContextGeometry.cpp:
(WebCore::Layout::FormattingContext::Geometry::inFlowPositionedPositionOffset):
* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::placeInFlowPositionedChildren const):
(WebCore::Layout::BlockFormattingContext::computeInFlowPositionedPosition const): Deleted.
* layout/blockformatting/BlockFormattingContext.h:
* layout/blockformatting/BlockFormattingContextGeometry.cpp:
(WebCore::Layout::BlockFormattingContext::Geometry::inFlowPositionedPosition): Deleted.
* layout/inlineformatting/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::computeInFlowPositionedPosition const): Deleted.
* layout/inlineformatting/InlineFormattingContext.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238077 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/BlockFormattingContext.h
Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
Source/WebCore/layout/inlineformatting/InlineFormattingContext.h

index 17cce8f..9956481 100644 (file)
@@ -1,3 +1,27 @@
+2018-11-11  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][BFC] In-flow positioned logic is really formatting context dependent.
+        https://bugs.webkit.org/show_bug.cgi?id=191512
+
+        Reviewed by Simon Fraser.
+
+        Move block formatting context specific code from FormattingContext to BlockFormattingContext.
+
+        * layout/FormattingContext.cpp:
+        (WebCore::Layout::FormattingContext::placeInFlowPositionedChildren const): Deleted.
+        * layout/FormattingContext.h:
+        * layout/FormattingContextGeometry.cpp:
+        (WebCore::Layout::FormattingContext::Geometry::inFlowPositionedPositionOffset):
+        * layout/blockformatting/BlockFormattingContext.cpp:
+        (WebCore::Layout::BlockFormattingContext::placeInFlowPositionedChildren const):
+        (WebCore::Layout::BlockFormattingContext::computeInFlowPositionedPosition const): Deleted.
+        * layout/blockformatting/BlockFormattingContext.h:
+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowPositionedPosition): Deleted.
+        * layout/inlineformatting/InlineFormattingContext.cpp:
+        (WebCore::Layout::InlineFormattingContext::computeInFlowPositionedPosition const): Deleted.
+        * layout/inlineformatting/InlineFormattingContext.h:
+
 2018-11-11  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Address post-review comments after r237955
index 9a7094a..98e1728 100644 (file)
@@ -130,21 +130,6 @@ void FormattingContext::computeBorderAndPadding(const Box& layoutBox) const
     displayBox.setPadding(Geometry::computedPadding(layoutState, layoutBox));
 }
 
-void FormattingContext::placeInFlowPositionedChildren(const Container& container) const
-{
-    // If this container also establishes a formatting context, then positioning already has happend in that the formatting context.
-    if (container.establishesFormattingContext() && &container != &root())
-        return;
-
-    LOG_WITH_STREAM(FormattingContextLayout, stream << "Start: move in-flow positioned children -> parent: " << &container);
-    for (auto& layoutBox : childrenOfType<Box>(container)) {
-        if (!layoutBox.isInFlowPositioned())
-            continue;
-        computeInFlowPositionedPosition(layoutBox);
-    }
-    LOG_WITH_STREAM(FormattingContextLayout, stream << "End: move in-flow positioned children -> parent: " << &container);
-}
-
 void FormattingContext::layoutOutOfFlowDescendants(const Box& layoutBox) const
 {
     // Initial containing block by definition is a containing block.
index 16c764a..6769a69 100644 (file)
@@ -70,12 +70,9 @@ protected:
     const Box& root() const { return *m_root; }
 
     virtual void computeStaticPosition(const Box&) const = 0;
-    virtual void computeInFlowPositionedPosition(const Box&) const = 0;
 
     void computeBorderAndPadding(const Box&) const;
 
-    void placeInFlowPositionedChildren(const Container&) const;
-
 #ifndef NDEBUG
     virtual void validateGeometryConstraintsAfterLayout() const;
 #endif
@@ -93,6 +90,8 @@ protected:
         static WidthAndMargin inlineReplacedWidthAndMargin(const LayoutState&, const Box&, std::optional<LayoutUnit> usedWidth = { }, 
             std::optional<LayoutUnit> precomputedMarginLeft = { }, std::optional<LayoutUnit> precomputedMarginRight = { });
 
+        static LayoutSize inFlowPositionedPositionOffset(const LayoutState&, const Box&);
+
         static HeightAndMargin complicatedCases(const LayoutState&, const Box&, std::optional<LayoutUnit> usedHeight = { });
         static LayoutUnit shrinkToFitWidth(LayoutState&, const Box&);
 
index 8080343..3da4253 100644 (file)
@@ -947,6 +947,82 @@ WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(const L
     return { *width, { marginLeft, marginRight }, { nonComputedMarginLeft, nonComputedMarginRight } };
 }
 
+LayoutSize FormattingContext::Geometry::inFlowPositionedPositionOffset(const LayoutState& layoutState, const Box& layoutBox)
+{
+    ASSERT(layoutBox.isInFlowPositioned());
+
+    // 9.4.3 Relative positioning
+    //
+    // The 'top' and 'bottom' properties move relatively positioned element(s) up or down without changing their size.
+    // Top' moves the boxes down, and 'bottom' moves them up. Since boxes are not split or stretched as a result of 'top' or 'bottom', the used values are always: top = -bottom.
+    //
+    // 1. If both are 'auto', their used values are both '0'.
+    // 2. If one of them is 'auto', it becomes the negative of the other.
+    // 3. If neither is 'auto', 'bottom' is ignored (i.e., the used value of 'bottom' will be minus the value of 'top').
+
+    auto& style = layoutBox.style();
+    auto& containingBlock = *layoutBox.containingBlock();
+    auto containingBlockWidth = layoutState.displayBoxForLayoutBox(containingBlock).contentBoxWidth();
+
+    auto top = computedValueIfNotAuto(style.logicalTop(), containingBlockWidth);
+    auto bottom = computedValueIfNotAuto(style.logicalBottom(), containingBlockWidth);
+
+    if (!top && !bottom) {
+        // #1
+        top = bottom = { 0 };
+    } else if (!top) {
+        // #2
+        top = -*bottom;
+    } else if (!bottom) {
+        // #3
+        bottom = -*top;
+    } else {
+        // #4
+        bottom = std::nullopt;
+    }
+
+    // For relatively positioned elements, 'left' and 'right' move the box(es) horizontally, without changing their size.
+    // 'Left' moves the boxes to the right, and 'right' moves them to the left.
+    // Since boxes are not split or stretched as a result of 'left' or 'right', the used values are always: left = -right.
+    //
+    // 1. If both 'left' and 'right' are 'auto' (their initial values), the used values are '0' (i.e., the boxes stay in their original position).
+    // 2. If 'left' is 'auto', its used value is minus the value of 'right' (i.e., the boxes move to the left by the value of 'right').
+    // 3. If 'right' is specified as 'auto', its used value is minus the value of 'left'.
+    // 4. If neither 'left' nor 'right' is 'auto', the position is over-constrained, and one of them has to be ignored.
+    //    If the 'direction' property of the containing block is 'ltr', the value of 'left' wins and 'right' becomes -'left'.
+    //    If 'direction' of the containing block is 'rtl', 'right' wins and 'left' is ignored.
+
+    auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
+    auto right = computedValueIfNotAuto(style.logicalRight(), containingBlockWidth);
+
+    if (!left && !right) {
+        // #1
+        left = right = { 0 };
+    } else if (!left) {
+        // #2
+        left = -*right;
+    } else if (!right) {
+        // #3
+        right = -*left;
+    } else {
+        // #4
+        auto isLeftToRightDirection = containingBlock.style().isLeftToRightDirection();
+        if (isLeftToRightDirection)
+            right = -*left;
+        else
+            left = std::nullopt;
+    }
+
+    ASSERT(!bottom || *top == -*bottom);
+    ASSERT(!left || *left == -*right);
+
+    auto topPositionOffset = *top;
+    auto leftPositionOffset = left.value_or(-*right);
+
+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position] -> positioned inflow -> top offset(" << topPositionOffset << "px) left offset(" << leftPositionOffset << "px) layoutBox(" << &layoutBox << ")");
+    return { leftPositionOffset, topPositionOffset };
+}
+
 Edges FormattingContext::Geometry::computedBorder(const LayoutState&, const Box& layoutBox)
 {
     auto& style = layoutBox.style();
index 8fc4d7f..5550a2b 100644 (file)
@@ -154,6 +154,30 @@ void BlockFormattingContext::layoutFormattingContextRoot(FloatingContext& floati
     formattingContext->layoutOutOfFlowDescendants(layoutBox);
 }
 
+void BlockFormattingContext::placeInFlowPositionedChildren(const Container& container) const
+{
+    LOG_WITH_STREAM(FormattingContextLayout, stream << "Start: move in-flow positioned children -> parent: " << &container);
+    for (auto& layoutBox : childrenOfType<Box>(container)) {
+        if (!layoutBox.isInFlowPositioned())
+            continue;
+
+        auto computeInFlowPositionedPosition = [&](auto& layoutBox) {
+            auto& layoutState = this->layoutState();
+            auto positionOffset = Geometry::inFlowPositionedPositionOffset(layoutState, layoutBox);
+
+            auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
+            auto topLeft = displayBox.topLeft();
+
+            topLeft.move(positionOffset);
+
+            displayBox.setTopLeft(topLeft);
+        };
+
+        computeInFlowPositionedPosition(layoutBox);
+    }
+    LOG_WITH_STREAM(FormattingContextLayout, stream << "End: move in-flow positioned children -> parent: " << &container);
+}
+
 void BlockFormattingContext::computeStaticPosition(const Box& layoutBox) const
 {
     auto& layoutState = this->layoutState();
@@ -274,12 +298,6 @@ void BlockFormattingContext::computeVerticalPositionForFloatClear(const Floating
         layoutState.displayBoxForLayoutBox(layoutBox).setTop(*verticalPositionWithClearance);
 }
 
-void BlockFormattingContext::computeInFlowPositionedPosition(const Box& layoutBox) const
-{
-    auto& layoutState = this->layoutState();
-    layoutState.displayBoxForLayoutBox(layoutBox).setTopLeft(Geometry::inFlowPositionedPosition(layoutState, layoutBox));
-}
-
 void BlockFormattingContext::computeWidthAndMargin(const Box& layoutBox) const
 {
     auto& layoutState = this->layoutState();
index e615ecc..9a9a0c6 100644 (file)
@@ -51,6 +51,7 @@ public:
 
 private:
     void layoutFormattingContextRoot(FloatingContext&, const Box&) const;
+    void placeInFlowPositionedChildren(const Container&) const;
 
     void computeWidthAndMargin(const Box&) const;
     void computeHeightAndMargin(const Box&) const;
@@ -60,7 +61,6 @@ private:
     void computePositionToAvoidFloats(const FloatingContext&, const Box&) const;
     void computeVerticalPositionForFloatClear(const FloatingContext&, const Box&) const;
 
-    void computeInFlowPositionedPosition(const Box&) const override;
     void computeEstimatedMarginTopForAncestors(const Box&) const;
     void computeEstimatedMarginTop(const Box&) const;
 
@@ -75,7 +75,6 @@ private:
         static WidthAndMargin inFlowWidthAndMargin(const LayoutState&, const Box&, std::optional<LayoutUnit> usedWidth = { });
 
         static Position staticPosition(const LayoutState&, const Box&);
-        static Position inFlowPositionedPosition(const LayoutState&, const Box&);
 
         static bool instrinsicWidthConstraintsNeedChildrenWidth(const Box&);
         static InstrinsicWidthConstraints instrinsicWidthConstraints(const LayoutState&, const Box&);
index 8b88ef7..b5244bc 100644 (file)
@@ -287,83 +287,6 @@ Position BlockFormattingContext::Geometry::staticPosition(const LayoutState& lay
     return { left, top };
 }
 
-Position BlockFormattingContext::Geometry::inFlowPositionedPosition(const LayoutState& layoutState, const Box& layoutBox)
-{
-    ASSERT(layoutBox.isInFlowPositioned());
-
-    // 9.4.3 Relative positioning
-    //
-    // The 'top' and 'bottom' properties move relatively positioned element(s) up or down without changing their size.
-    // Top' moves the boxes down, and 'bottom' moves them up. Since boxes are not split or stretched as a result of 'top' or 'bottom', the used values are always: top = -bottom.
-    //
-    // 1. If both are 'auto', their used values are both '0'.
-    // 2. If one of them is 'auto', it becomes the negative of the other.
-    // 3. If neither is 'auto', 'bottom' is ignored (i.e., the used value of 'bottom' will be minus the value of 'top').
-
-    auto& style = layoutBox.style();
-    auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
-    auto& containingBlock = *layoutBox.containingBlock();
-    auto containingBlockWidth = layoutState.displayBoxForLayoutBox(containingBlock).contentBoxWidth();
-
-    auto top = computedValueIfNotAuto(style.logicalTop(), containingBlockWidth);
-    auto bottom = computedValueIfNotAuto(style.logicalBottom(), containingBlockWidth);
-
-    if (!top && !bottom) {
-        // #1
-        top = bottom = { 0 };
-    } else if (!top) {
-        // #2
-        top = -*bottom;
-    } else if (!bottom) {
-        // #3
-        bottom = -*top;
-    } else {
-        // #4
-        bottom = std::nullopt;
-    }
-
-    // For relatively positioned elements, 'left' and 'right' move the box(es) horizontally, without changing their size.
-    // 'Left' moves the boxes to the right, and 'right' moves them to the left.
-    // Since boxes are not split or stretched as a result of 'left' or 'right', the used values are always: left = -right.
-    //
-    // 1. If both 'left' and 'right' are 'auto' (their initial values), the used values are '0' (i.e., the boxes stay in their original position).
-    // 2. If 'left' is 'auto', its used value is minus the value of 'right' (i.e., the boxes move to the left by the value of 'right').
-    // 3. If 'right' is specified as 'auto', its used value is minus the value of 'left'.
-    // 4. If neither 'left' nor 'right' is 'auto', the position is over-constrained, and one of them has to be ignored.
-    //    If the 'direction' property of the containing block is 'ltr', the value of 'left' wins and 'right' becomes -'left'.
-    //    If 'direction' of the containing block is 'rtl', 'right' wins and 'left' is ignored.
-
-    auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
-    auto right = computedValueIfNotAuto(style.logicalRight(), containingBlockWidth);
-
-    if (!left && !right) {
-        // #1
-        left = right = { 0 };
-    } else if (!left) {
-        // #2
-        left = -*right;
-    } else if (!right) {
-        // #3
-        right = -*left;
-    } else {
-        // #4
-        auto isLeftToRightDirection = containingBlock.style().isLeftToRightDirection();
-        if (isLeftToRightDirection)
-            right = -*left;
-        else
-            left = std::nullopt;
-    }
-
-    ASSERT(!bottom || *top == -*bottom);
-    ASSERT(!left || *left == -*right);
-
-    auto newTopPosition = displayBox.top() + *top;
-    auto newLeftPosition = displayBox.left() + left.value_or(-*right);
-
-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position] -> positioned inflow -> top(" << newTopPosition << "px) left(" << newLeftPosition << "px) layoutBox(" << &layoutBox << ")");
-    return { newLeftPosition, newTopPosition };
-}
-
 HeightAndMargin BlockFormattingContext::Geometry::inFlowHeightAndMargin(const LayoutState& layoutState, const Box& layoutBox, std::optional<LayoutUnit> usedHeight)
 {
     ASSERT(layoutBox.isInFlow());
index 832318f..d7b463e 100644 (file)
@@ -284,10 +284,6 @@ void InlineFormattingContext::computeStaticPosition(const Box&) const
 {
 }
 
-void InlineFormattingContext::computeInFlowPositionedPosition(const Box&) const
-{
-}
-
 void InlineFormattingContext::collectInlineContent(InlineRunProvider& inlineRunProvider) const
 {
     if (!is<Container>(root()))
index 724792a..034175f 100644 (file)
@@ -118,7 +118,6 @@ private:
     void computeWidthAndMargin(const Box&) const;
     void computeFloatPosition(const FloatingContext&, Line&, const Box&) const;
     void computeStaticPosition(const Box&) const override;
-    void computeInFlowPositionedPosition(const Box&) const override;
 
     void collectInlineContent(InlineRunProvider&) const;
     InstrinsicWidthConstraints instrinsicWidthConstraints() const override;