Attachment 342434 Details for Bug 186525 – Patch

[patch] Patch

bug-186525-20180611083012.patch (text/plain), 15.72 KB, created by zalan on 2018-06-11 08:30:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-06-11 08:30:13 PDT

Size: 15.72 KB

patch

obsolete

>Subversion Revision: 232715
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 5d426f101dae1a1201c5213ac732088ccf672842..612fb67262b3adac4cdbebdaec1ef08fd028408a 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2018-06-11  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Remove redundant position functions for out-of-flow elements
>+        https://bugs.webkit.org/show_bug.cgi?id=186525
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Position is computed as part of the Horizontal/Vertical geometry computation.
>+        (see outOfFlow(Non)ReplacedHorizontalGeometry/outOfFlow(Non)ReplacedVerticalGeometry functions)
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::layoutOutOfFlowDescendants const):
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowPosition const): Deleted.
>+        * layout/FormattingContext.h:
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::outOfFlowNonReplacedPosition): Deleted.
>+        (WebCore::Layout::outOfFlowReplacedPosition): Deleted.
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowPosition): Deleted.
>+
> 2018-06-11  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Merge top, bottom, height and vertical margin computation for out-of-flow replaced elements
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index 6aeb4c2ee7f51093800fa12de3966e1290486bd2..9930dc8d3e949030a8e1217d266f7eeaed7d8cad 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -49,11 +49,6 @@ FormattingContext::~FormattingContext()
> {
> }
> 
>-void FormattingContext::computeOutOfFlowPosition(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>-{
>-    displayBox.setTopLeft(Geometry::outOfFlowPosition(layoutContext, layoutBox));
>-}
>-
> void FormattingContext::computeFloatingHeightAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
> {
>     auto heightAndMargin = Geometry::floatingHeightAndMargin(layoutContext, layoutBox);
>@@ -126,7 +121,6 @@ void FormattingContext::layoutOutOfFlowDescendants(LayoutContext& layoutContext)
>         formattingContext->layout(layoutContext, layoutContext.establishedFormattingState(layoutBox, *formattingContext));
> 
>         computeOutOfFlowVerticalGeometry(layoutContext, layoutBox, displayBox);
>-        computeOutOfFlowPosition(layoutContext, layoutBox, displayBox);
>     }
> }
> 
>diff --git a/Source/WebCore/layout/FormattingContext.h b/Source/WebCore/layout/FormattingContext.h
>index 2f9afa6b75e7756e48884b6fad07e46e3c071a87..353bec1aed43e6133a991f94e56059794675772f 100644
>--- a/Source/WebCore/layout/FormattingContext.h
>+++ b/Source/WebCore/layout/FormattingContext.h
>@@ -111,8 +111,6 @@ protected:
>         static HeightAndMargin floatingHeightAndMargin(LayoutContext&, const Box&);
>         static WidthAndMargin floatingWidthAndMargin(LayoutContext&, const Box&);
> 
>-        static LayoutPoint outOfFlowPosition(LayoutContext&, const Box&);
>-
>         static HeightAndMargin inlineReplacedHeightAndMargin(LayoutContext&, const Box&);
>         static WidthAndMargin inlineReplacedWidthAndMargin(LayoutContext&, const Box&, std::optional<LayoutUnit> precomputedMarginLeft = { },
>             std::optional<LayoutUnit> precomputedMarginRight = { });
>@@ -138,7 +136,6 @@ protected:
>     };
> 
> private:
>-    void computeOutOfFlowPosition(LayoutContext&, const Box&, Display::Box&) const;
>     void computeOutOfFlowVerticalGeometry(LayoutContext&, const Box&, Display::Box&) const;
>     void computeOutOfFlowHorizontalGeometry(LayoutContext&, const Box&, Display::Box&) const;
> 
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index f5afc613675355e78b544e1880578078320c25d0..d54518e95c2fd72f5994090c9ca3dcc5f360fc61 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -535,194 +535,6 @@ FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::floatin
>     return inlineReplacedWidthAndMargin(layoutContext, layoutBox, computedNonCollapsedHorizontalMarginValues.left, computedNonCollapsedHorizontalMarginValues.right);
> }
> 
>-static LayoutPoint outOfFlowNonReplacedPosition(LayoutContext& layoutContext, const Box& layoutBox)
>-{
>-    ASSERT(layoutBox.isOutOfFlowPositioned() && !layoutBox.replaced());
>-
>-    // 10.3.7 Absolutely positioned, non-replaced elements (left/right)
>-    // 10.6.4 Absolutely positioned, non-replaced elements (top/bottom)
>-
>-    // At this point we've the size computed.
>-    auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
>-    auto size = displayBox.size();
>-    auto& style = layoutBox.style();
>-
>-    // 10.6.4 Absolutely positioned, non-replaced elements
>-    auto top = style.logicalTop();
>-    auto bottom = style.logicalBottom();
>-    auto containingBlockHeight = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->height();
>-
>-    // 'top' + 'margin-top' + 'border-top-width' + 'padding-top' + 'height' + 'padding-bottom' + 'border-bottom-width' + 'margin-bottom' + 'bottom'
>-    // = height of containing block
>-    //
>-    // 1. 'top' and 'height' are 'auto' and 'bottom' is not 'auto', then the height is based on the content per 10.6.7,
>-    //     set 'auto' values for 'margin-top' and 'margin-bottom' to 0, and solve for 'top'
>-    // 2. 'top' and 'bottom' are 'auto' and 'height' is not 'auto', then set 'top' to the static position, set 'auto' values for
>-    //    'margin-top' and 'margin-bottom' to 0, and solve for 'bottom'
>-    // 3. 'height' and 'bottom' are 'auto' and 'top' is not 'auto', then the height is based on the content per 10.6.7, set 'auto'
>-    //     values for 'margin-top' and 'margin-bottom' to 0, and solve for 'bottom'
>-    // 4. 'top' is 'auto', 'height' and 'bottom' are not 'auto', then set 'auto' values for 'margin-top' and 'margin-bottom' to 0, and solve for 'top'
>-    // 5. 'height' is 'auto', 'top' and 'bottom' are not 'auto', then 'auto' values for 'margin-top' and 'margin-bottom' are set to 0 and solve for 'height'
>-    // 6. 'bottom' is 'auto', 'top' and 'height' are not 'auto', then set 'auto' values for 'margin-top' and 'margin-bottom' to 0 and solve for 'bottom'
>-    LayoutUnit computedTopValue;
>-    if (top.isAuto() && !bottom.isAuto()) {
>-        // #1 #4
>-        auto marginTop = displayBox.marginTop();
>-        auto marginBottom = displayBox.marginBottom();
>-    
>-        auto paddingTop = displayBox.paddingTop();
>-        auto paddingBottom = displayBox.paddingBottom();
>-
>-        auto borderTop = displayBox.borderTop();
>-        auto borderBottom = displayBox.borderBottom();
>-
>-        computedTopValue = containingBlockHeight - (marginTop + borderTop + paddingTop + size.height() + paddingBottom + borderBottom + marginBottom + bottom.value());
>-    } else if (top.isAuto() && bottom.isAuto()) {
>-        // #2
>-        // Already computed as part of the computeStaticPosition();
>-        computedTopValue = displayBox.top();
>-    } else {
>-        // #3 #5 #6 have top != auto
>-        computedTopValue = valueForLength(top, containingBlockHeight);
>-    }
>-
>-
>-    // 10.3.7 Absolutely positioned, non-replaced elements
>-    auto left = style.logicalLeft();
>-    auto right = style.logicalRight();
>-    auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->width();
>-
>-    // 'left' + 'margin-left' + 'border-left-width' + 'padding-left' + 'width' + 'padding-right' + 'border-right-width' + 'margin-right' + 'right'
>-    // = width of containing block
>-    //
>-    // If all three of 'left', 'width', and 'right' are 'auto': First set any 'auto' values for 'margin-left' and 'margin-right' to 0.
>-    // Then, if the 'direction' property of the element establishing the static-position containing block is 'ltr' set 'left' to the static
>-    // position and apply rule number three below; otherwise, set 'right' to the static position and apply rule number one below.
>-
>-    // 1. 'left' and 'width' are 'auto' and 'right' is not 'auto', then the width is shrink-to-fit. Then solve for 'left'
>-    // 2. 'left' and 'right' are 'auto' and 'width' is not 'auto', then if the 'direction' property of the element establishing the static-position 
>-    //    containing block is 'ltr' set 'left' to the static position, otherwise set 'right' to the static position.
>-    //    Then solve for 'left' (if 'direction is 'rtl') or 'right' (if 'direction' is 'ltr').
>-    // 3. 'width' and 'right' are 'auto' and 'left' is not 'auto', then the width is shrink-to-fit . Then solve for 'right'
>-    // 4. 'left' is 'auto', 'width' and 'right' are not 'auto', then solve for 'left'
>-    // 5. 'width' is 'auto', 'left' and 'right' are not 'auto', then solve for 'width'
>-    // 6. 'right' is 'auto', 'left' and 'width' are not 'auto', then solve for 'right'
>-    LayoutUnit computedLeftValue;
>-    if (left.isAuto() && !right.isAuto()) {
>-        // #1 #4
>-        auto marginLeft = displayBox.marginLeft();
>-        auto marginRight = displayBox.marginRight();
>-    
>-        auto paddingLeft = displayBox.paddingLeft();
>-        auto paddingRight = displayBox.paddingRight();
>-
>-        auto borderLeft = displayBox.borderLeft();
>-        auto borderRight = displayBox.borderRight();
>-
>-        computedLeftValue = containingBlockWidth - (marginLeft + borderLeft + paddingLeft + size.width() + paddingRight + borderRight + marginRight + right.value());
>-    } else if (left.isAuto() && right.isAuto()) {
>-        // #2
>-        // FIXME: rtl
>-        computedLeftValue = displayBox.left();
>-    } else {
>-        // #3 #5 #6 have left != auto
>-        computedLeftValue = valueForLength(left, containingBlockWidth);
>-    }
>-
>-    return { computedLeftValue, computedTopValue };
>-}
>-
>-static LayoutPoint outOfFlowReplacedPosition(LayoutContext& layoutContext, const Box& layoutBox)
>-{
>-    ASSERT(layoutBox.isOutOfFlowPositioned() && layoutBox.replaced());
>-
>-    // 10.6.5 Absolutely positioned, replaced elements (top/bottom)
>-    // 10.3.8 Absolutely positioned, replaced elements (left/right)
>-
>-    // At this point we've the size computed.
>-    auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
>-    auto size = displayBox.size();
>-    auto& style = layoutBox.style();
>-
>-    // 10.6.5 Absolutely positioned, replaced elements
>-    //
>-    // This situation is similar to the previous one, except that the element has an intrinsic height. The sequence of substitutions is now:
>-    // The used value of 'height' is determined as for inline replaced elements. If 'margin-top' or 'margin-bottom' is specified as 'auto'
>-    // its used value is determined by the rules below.
>-    //
>-    // 1. If both 'top' and 'bottom' have the value 'auto', replace 'top' with the element's static position.
>-    // 2. If 'bottom' is 'auto', replace any 'auto' on 'margin-top' or 'margin-bottom' with '0'.
>-    // 3. If at this point both 'margin-top' and 'margin-bottom' are still 'auto', solve the equation under the extra constraint that the two margins must get equal values.
>-    // 4. If at this point there is only one 'auto' left, solve the equation for that value.
>-    // 5. If at this point the values are over-constrained, ignore the value for 'bottom' and solve for that value.
>-    auto top = style.logicalTop();
>-    auto bottom = style.logicalBottom();
>-    auto containingBlockHeight = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->height();
>-    LayoutUnit computedTopValue;
>-
>-    if (!top.isAuto())
>-        computedTopValue = valueForLength(top, containingBlockHeight);
>-    else if (bottom.isAuto()) {
>-        // #1
>-        computedTopValue = displayBox.top();
>-    } else {
>-        // #4
>-        auto marginTop = displayBox.marginTop();
>-        auto marginBottom = displayBox.marginBottom();
>-
>-        auto paddingTop = displayBox.paddingTop();
>-        auto paddingBottom = displayBox.paddingBottom();
>-
>-        auto borderTop = displayBox.borderTop();
>-        auto borderBottom = displayBox.borderBottom();
>-
>-        computedTopValue = containingBlockHeight - (marginTop + borderTop + paddingTop + size.height() + paddingBottom + borderBottom + marginBottom + bottom.value());
>-    }
>-
>-
>-    // 10.3.8 Absolutely positioned, replaced elements
>-    //
>-    // In this case, section 10.3.7 applies up through and including the constraint equation, but the rest of section 10.3.7 is replaced by the following rules:
>-    //
>-    // The used value of 'width' is determined as for inline replaced elements. 
>-    //
>-    // 1. If 'margin-left' or 'margin-right' is specified as 'auto' its used value is determined by the rules below.
>-    // 2. If both 'left' and 'right' have the value 'auto', then if the 'direction' property of the element establishing the
>-    //    static-position containing block is 'ltr', set 'left' to the static position; else if 'direction' is 'rtl', set 'right' to the static position.
>-    // 3. If 'left' or 'right' are 'auto', replace any 'auto' on 'margin-left' or 'margin-right' with '0'.
>-    // 4. If at this point both 'margin-left' and 'margin-right' are still 'auto', solve the equation under the extra constraint
>-    //    that the two margins must get equal values, unless this would make them negative, in which case when the direction of
>-    //    the containing block is 'ltr' ('rtl'), set 'margin-left' ('margin-right') to zero and solve for 'margin-right' ('margin-left').
>-    // 5. If at this point there is an 'auto' left, solve the equation for that value.
>-    // 6. If at this point the values are over-constrained, ignore the value for either 'left' (in case the 'direction'
>-    //    property of the containing block is 'rtl') or 'right' (in case 'direction' is 'ltr') and solve for that value.
>-    auto left = style.logicalLeft();
>-    auto right = style.logicalRight();
>-    auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->width();
>-    LayoutUnit computedLeftValue;
>-
>-    if (!left.isAuto())   
>-        computedLeftValue = valueForLength(left, containingBlockWidth);
>-    else if (right.isAuto()) {
>-        // FIXME: take direction into account
>-        computedLeftValue = displayBox.left();
>-    } else {
>-        // #5
>-        auto marginLeft = displayBox.marginLeft();
>-        auto marginRight = displayBox.marginRight();
>-    
>-        auto paddingLeft = displayBox.paddingLeft();
>-        auto paddingRight = displayBox.paddingRight();
>-
>-        auto borderLeft = displayBox.borderLeft();
>-        auto borderRight = displayBox.borderRight();
>-
>-        computedLeftValue = containingBlockWidth - (marginLeft + borderLeft + paddingLeft + size.width() + paddingRight + borderRight + marginRight + right.value());
>-    }
>-
>-    return { computedLeftValue, computedTopValue };
>-}
>-
> FormattingContext::Geometry::VerticalGeometry FormattingContext::Geometry::outOfFlowVerticalGeometry(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isOutOfFlowPositioned());
>@@ -759,15 +571,6 @@ FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::floatin
>     return floatingReplacedWidthAndMargin(layoutContext, layoutBox);
> }
> 
>-LayoutPoint FormattingContext::Geometry::outOfFlowPosition(LayoutContext& layoutContext, const Box& layoutBox)
>-{
>-    ASSERT(layoutBox.isOutOfFlowPositioned());
>-
>-    if (!layoutBox.replaced())
>-        return outOfFlowNonReplacedPosition(layoutContext, layoutBox);
>-    return outOfFlowReplacedPosition(layoutContext, layoutBox);
>-}
>-
> FormattingContext::Geometry::HeightAndMargin FormattingContext::Geometry::inlineReplacedHeightAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT((layoutBox.isOutOfFlowPositioned() || layoutBox.isFloatingPositioned() || layoutBox.isInFlow()) && layoutBox.replaced());

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186525: 342434