Attachment 341823 Details for Bug 186217 – Patch

[patch] Patch

bug-186217-20180601204026.patch (text/plain), 36.33 KB, created by zalan on 2018-06-01 20:40:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-06-01 20:40:27 PDT

Size: 36.33 KB

patch

obsolete

>Subversion Revision: 232394
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ff60d3759cdc1236638e66a7ac7578cc06904e6c..4801ad324041316539b4a518b4a7cc639b3f33ee 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,71 @@
>+2018-06-01  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Merge width and horizontal margin computation
>+        https://bugs.webkit.org/show_bug.cgi?id=186217
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We could certainly compute width and maring separately, but to match each steps defined
>+        at https://www.w3.org/TR/CSS22/visudet.html, we should just merge width and horizontal maring computations
>+        (followed by height and vertical margins).
>+        Use 0 computed marings for now.
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::computeFloatingWidthAndMargin const):
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowWidthAndMargin const):
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowHeight const):
>+        (WebCore::Layout::FormattingContext::layoutOutOfFlowDescendants const):
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowWidth const): Deleted.
>+        (WebCore::Layout::FormattingContext::computeFloatingWidth const): Deleted.
>+        * layout/FormattingContext.h:
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::inlineReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::computedPadding):
>+        (WebCore::Layout::outOfFlowNonReplacedWidth): Deleted.
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedWidth): Deleted.
>+        (WebCore::Layout::floatingNonReplacedWidth): Deleted.
>+        (WebCore::Layout::FormattingContext::Geometry::floatingReplacedWidth): Deleted.
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowWidth): Deleted.
>+        (WebCore::Layout::FormattingContext::Geometry::floatingWidth): Deleted.
>+        (WebCore::Layout::FormattingContext::Geometry::inlineReplacedWidth): Deleted.
>+        * layout/LayoutContext.cpp:
>+        (WebCore::Layout::LayoutContext::initializeRoot):
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::layout const):
>+        (WebCore::Layout::BlockFormattingContext::computeWidthAndMargin const):
>+        (WebCore::Layout::BlockFormattingContext::computeInFlowWidthAndMargin const):
>+        (WebCore::Layout::BlockFormattingContext::computeWidth const): Deleted.
>+        (WebCore::Layout::BlockFormattingContext::computeInFlowWidth const): Deleted.
>+        (WebCore::Layout::BlockFormattingContext::computeMargin const): Deleted.
>+        * layout/blockformatting/BlockFormattingContext.h:
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowWidthAndMargin):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidth): Deleted.
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowWidth): Deleted.
>+        (WebCore::Layout::BlockFormattingContext::Geometry::computedMargin): Deleted.
>+        * layout/displaytree/DisplayBox.cpp:
>+        (WebCore::Display::Box::marginBox const):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::setHasValidVerticalMargin):
>+        (WebCore::Display::Box::setHasValidHorizontalMargin):
>+        (WebCore::Display::Box::invalidateMargin):
>+        (WebCore::Display::Box::setHorizontalMargin):
>+        (WebCore::Display::Box::setVerticalMargin):
>+        (WebCore::Display::Box::marginTop const):
>+        (WebCore::Display::Box::marginLeft const):
>+        (WebCore::Display::Box::marginBottom const):
>+        (WebCore::Display::Box::marginRight const):
>+        (WebCore::Display::Box::Edges::Edges): Deleted.
>+        (WebCore::Display::Box::setHasValidMargin): Deleted.
>+        (WebCore::Display::Box::setMargin): Deleted.
>+
> 2018-06-01  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Simplify the formatting class implementation by pushing down some of the logic to the Geometry class
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index a8a38f5a48e7719b61daf685211d69570dd6fac2..a48fce3d2a6504be9204ba4e485c727646f87766 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -59,19 +59,23 @@ void FormattingContext::computeFloatingHeight(LayoutContext& layoutContext, cons
>     displayBox.setHeight(Geometry::floatingHeight(layoutContext, layoutBox));
> }
> 
>-void FormattingContext::computeOutOfFlowWidth(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>+void FormattingContext::computeFloatingWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
> {
>-    displayBox.setWidth(Geometry::outOfFlowWidth(layoutContext, layoutBox));
>+    auto widthAndMargin = Geometry::floatingWidthAndMargin(layoutContext, layoutBox);
>+    displayBox.setWidth(widthAndMargin.width);
>+    displayBox.setHorizontalMargin(widthAndMargin.margin);
> }
> 
>-void FormattingContext::computeOutOfFlowHeight(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>+void FormattingContext::computeOutOfFlowWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
> {
>-    displayBox.setHeight(Geometry::outOfFlowHeight(layoutContext, layoutBox));
>+    auto widthAndMargin = Geometry::outOfFlowWidthAndMargin(layoutContext, layoutBox);
>+    displayBox.setWidth(widthAndMargin.width);
>+    displayBox.setHorizontalMargin(widthAndMargin.margin);
> }
> 
>-void FormattingContext::computeFloatingWidth(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>+void FormattingContext::computeOutOfFlowHeight(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
> {
>-    displayBox.setWidth(Geometry::floatingWidth(layoutContext, layoutBox));
>+    displayBox.setHeight(Geometry::outOfFlowHeight(layoutContext, layoutBox));
> }
> 
> void FormattingContext::computeBorderAndPadding(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>@@ -107,7 +111,7 @@ void FormattingContext::layoutOutOfFlowDescendants(LayoutContext& layoutContext)
>         // of a hypothetical box that would have been the first box of the element if its specified 'position' value had been 'static' and
>         // its specified 'float' had been 'none' and its specified 'clear' had been 'none'.
>         computeStaticPosition(layoutContext, layoutBox, displayBox);
>-        computeOutOfFlowWidth(layoutContext, layoutBox, displayBox);
>+        computeOutOfFlowWidthAndMargin(layoutContext, layoutBox, displayBox);
> 
>         ASSERT(layoutBox.establishesFormattingContext());
>         auto formattingContext = layoutContext.formattingContext(layoutBox);
>diff --git a/Source/WebCore/layout/FormattingContext.h b/Source/WebCore/layout/FormattingContext.h
>index cad7ead5c3584f3b624d38375022c73ec333bc61..9d40016e8ac89d9b86a43dcdc632d1cda4841580 100644
>--- a/Source/WebCore/layout/FormattingContext.h
>+++ b/Source/WebCore/layout/FormattingContext.h
>@@ -67,7 +67,7 @@ protected:
>     virtual void computeInFlowPositionedPosition(LayoutContext&, const Box&, Display::Box&) const = 0;
> 
>     void computeFloatingHeight(LayoutContext&, const Box&, Display::Box&) const;
>-    void computeFloatingWidth(LayoutContext&, const Box&, Display::Box&) const;
>+    void computeFloatingWidthAndMargin(LayoutContext&, const Box&, Display::Box&) const;
> 
>     void computeBorderAndPadding(LayoutContext&, const Box&, Display::Box&) const;
> 
>@@ -81,31 +81,41 @@ protected:
>     // This class implements generic positioning and sizing.
>     class Geometry {
>     public:
>+        struct WidthAndMargin {
>+            LayoutUnit width;
>+            Display::Box::HorizontalEdges margin;
>+        };
>+
>         static LayoutUnit outOfFlowHeight(LayoutContext&, const Box&);
>-        static LayoutUnit outOfFlowWidth(LayoutContext&, const Box&);
>+        static WidthAndMargin outOfFlowWidthAndMargin(LayoutContext&, const Box&);
> 
>         static LayoutUnit floatingHeight(LayoutContext&, const Box&);
>-        static LayoutUnit floatingWidth(LayoutContext&, const Box&);
>+        static WidthAndMargin floatingWidthAndMargin(LayoutContext&, const Box&);
> 
>         static LayoutPoint outOfFlowPosition(LayoutContext&, const Box&);
> 
>         static LayoutUnit inlineReplacedHeight(LayoutContext&, const Box&);
>-        static LayoutUnit inlineReplacedWidth(LayoutContext&, const Box&);
>+        static WidthAndMargin inlineReplacedWidthAndMargin(LayoutContext&, const Box&);
> 
>         static Display::Box::Edges computedBorder(LayoutContext&, const Box&);
>         static std::optional<Display::Box::Edges> computedPadding(LayoutContext&, const Box&);
> 
>     private:
>         static LayoutUnit outOfFlowReplacedHeight(LayoutContext&, const Box&);
>-        static LayoutUnit outOfFlowReplacedWidth(LayoutContext&, const Box&);
>+        static WidthAndMargin outOfFlowReplacedWidthAndMargin(LayoutContext&, const Box&);
>+
>+        static WidthAndMargin outOfFlowNonReplacedWidthAndMargin(LayoutContext&, const Box&);
>+
>         static LayoutUnit floatingReplacedHeight(LayoutContext&, const Box&);
>-        static LayoutUnit floatingReplacedWidth(LayoutContext&, const Box&);
>+        static WidthAndMargin floatingReplacedWidthAndMargin(LayoutContext&, const Box&);
>+
>+        static WidthAndMargin floatingNonReplacedWidthAndMargin(LayoutContext&, const Box&);
>     };
> 
> private:
>     void computeOutOfFlowPosition(LayoutContext&, const Box&, Display::Box&) const;
>     void computeOutOfFlowHeight(LayoutContext&, const Box&, Display::Box&) const;
>-    void computeOutOfFlowWidth(LayoutContext&, const Box&, Display::Box&) const;
>+    void computeOutOfFlowWidthAndMargin(LayoutContext&, const Box&, Display::Box&) const;
> 
>     WeakPtr<Box> m_root;
> };
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index 0cd7ad5f6cc60c25c3d6b558d3bebb1c20f8c3f0..fedfed08658a48e7f81776e1e49fb08aba5b9a9a 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -128,7 +128,7 @@ static LayoutUnit outOfFlowNonReplacedHeight(LayoutContext& layoutContext, const
>     return computedHeightValue;
> }
> 
>-static LayoutUnit outOfFlowNonReplacedWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::outOfFlowNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isOutOfFlowPositioned() && !layoutBox.replaced());
>     
>@@ -183,7 +183,7 @@ static LayoutUnit outOfFlowNonReplacedWidth(LayoutContext& layoutContext, const
>         ASSERT_NOT_REACHED();
>     }
> 
>-    return computedWidthValue;
>+    return WidthAndMargin { computedWidthValue, { } };
> }
> 
> LayoutUnit FormattingContext::Geometry::outOfFlowReplacedHeight(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -195,13 +195,13 @@ LayoutUnit FormattingContext::Geometry::outOfFlowReplacedHeight(LayoutContext& l
>     return inlineReplacedHeight(layoutContext, layoutBox);
> }
> 
>-LayoutUnit FormattingContext::Geometry::outOfFlowReplacedWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::outOfFlowReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isOutOfFlowPositioned() && layoutBox.replaced());
>     // 10.3.8 Absolutely positioned, replaced elements
>     //
>     // The used value of 'width' is determined as for inline replaced elements.
>-    return inlineReplacedWidth(layoutContext, layoutBox);
>+    return inlineReplacedWidthAndMargin(layoutContext, layoutBox);
> }
> 
> static LayoutUnit floatingNonReplacedHeight(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -216,14 +216,15 @@ static LayoutUnit floatingNonReplacedHeight(LayoutContext& layoutContext, const
>     return height.isAuto() ? contentHeightForFormattingContextRoot(layoutContext, layoutBox) : LayoutUnit(height.value());
> }
> 
>-static LayoutUnit floatingNonReplacedWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::floatingNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isFloatingPositioned() && !layoutBox.replaced());
>     // 10.3.5 Floating, non-replaced elements
> 
>     // If 'width' is computed as 'auto', the used value is the "shrink-to-fit" width.
>     auto width = layoutBox.style().logicalWidth();
>-    return width.isAuto() ? shrinkToFitWidth(layoutContext, layoutBox) : LayoutUnit(width.value());
>+    auto computedWidthValue = width.isAuto() ? shrinkToFitWidth(layoutContext, layoutBox) : LayoutUnit(width.value());
>+    return FormattingContext::Geometry::WidthAndMargin { computedWidthValue, { } };
> }
> 
> LayoutUnit FormattingContext::Geometry::floatingReplacedHeight(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -234,13 +235,13 @@ LayoutUnit FormattingContext::Geometry::floatingReplacedHeight(LayoutContext& la
>     return inlineReplacedHeight(layoutContext, layoutBox);
> }
> 
>-LayoutUnit FormattingContext::Geometry::floatingReplacedWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::floatingReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isFloatingPositioned() && layoutBox.replaced());
>     // 10.3.6 Floating, replaced elements
>     //
>     // The used value of 'width' is determined as for inline replaced elements.
>-    return inlineReplacedWidth(layoutContext, layoutBox);
>+    return inlineReplacedWidthAndMargin(layoutContext, layoutBox);
> }
> 
> static LayoutPoint outOfFlowNonReplacedPosition(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -440,13 +441,13 @@ LayoutUnit FormattingContext::Geometry::outOfFlowHeight(LayoutContext& layoutCon
>     return outOfFlowReplacedHeight(layoutContext, layoutBox);
> }
> 
>-LayoutUnit FormattingContext::Geometry::outOfFlowWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::outOfFlowWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isOutOfFlowPositioned());
> 
>     if (!layoutBox.replaced())
>-        return outOfFlowNonReplacedWidth(layoutContext, layoutBox);
>-    return outOfFlowReplacedWidth(layoutContext, layoutBox);
>+        return outOfFlowNonReplacedWidthAndMargin(layoutContext, layoutBox);
>+    return outOfFlowReplacedWidthAndMargin(layoutContext, layoutBox);
> }
> 
> LayoutUnit FormattingContext::Geometry::floatingHeight(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -458,13 +459,13 @@ LayoutUnit FormattingContext::Geometry::floatingHeight(LayoutContext& layoutCont
>     return floatingReplacedHeight(layoutContext, layoutBox);
> }
> 
>-LayoutUnit FormattingContext::Geometry::floatingWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::floatingWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isFloatingPositioned());
> 
>     if (!layoutBox.replaced())
>-        return floatingNonReplacedWidth(layoutContext, layoutBox);
>-    return floatingReplacedWidth(layoutContext, layoutBox);
>+        return floatingNonReplacedWidthAndMargin(layoutContext, layoutBox);
>+    return floatingReplacedWidthAndMargin(layoutContext, layoutBox);
> }
> 
> LayoutPoint FormattingContext::Geometry::outOfFlowPosition(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -518,7 +519,7 @@ LayoutUnit FormattingContext::Geometry::inlineReplacedHeight(LayoutContext&, con
>     return computedHeightValue;
> }
> 
>-LayoutUnit FormattingContext::Geometry::inlineReplacedWidth(LayoutContext&, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(LayoutContext&, const Box& layoutBox)
> {
>     ASSERT((layoutBox.isOutOfFlowPositioned() || layoutBox.isFloatingPositioned() || layoutBox.isInFlow()) && layoutBox.replaced());
>     // 10.3.2 Inline, replaced elements
>@@ -564,17 +565,15 @@ LayoutUnit FormattingContext::Geometry::inlineReplacedWidth(LayoutContext&, cons
>         computedWidthValue = 300;
>     }
> 
>-    return computedWidthValue;
>+    return WidthAndMargin { computedWidthValue, { } };
> }
> 
> Display::Box::Edges FormattingContext::Geometry::computedBorder(LayoutContext&, const Box& layoutBox)
> {
>     auto& style = layoutBox.style();
>     return {
>-        style.borderTop().boxModelWidth(),
>-        style.borderLeft().boxModelWidth(),
>-        style.borderBottom().boxModelWidth(),
>-        style.borderRight().boxModelWidth()
>+        { style.borderLeft().boxModelWidth(), style.borderRight().boxModelWidth() },
>+        { style.borderTop().boxModelWidth(), style.borderBottom().boxModelWidth() }
>     };
> }
> 
>@@ -585,12 +584,10 @@ std::optional<Display::Box::Edges> FormattingContext::Geometry::computedPadding(
> 
>     auto& style = layoutBox.style();
>     auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->width();
>-    return Display::Box::Edges(
>-        valueForLength(style.paddingTop(), containingBlockWidth),
>-        valueForLength(style.paddingLeft(), containingBlockWidth),
>-        valueForLength(style.paddingBottom(), containingBlockWidth),
>-        valueForLength(style.paddingRight(), containingBlockWidth)
>-    );
>+    return Display::Box::Edges {
>+        { valueForLength(style.paddingLeft(), containingBlockWidth), valueForLength(style.paddingRight(), containingBlockWidth) },
>+        { valueForLength(style.paddingTop(), containingBlockWidth), valueForLength(style.paddingBottom(), containingBlockWidth) }
>+    };
> }
> 
> }
>diff --git a/Source/WebCore/layout/LayoutContext.cpp b/Source/WebCore/layout/LayoutContext.cpp
>index e015b5197ae31ffb9c9b919931bfa22866fea922..9308b2019774868955ab053148b7197ba34e05bb 100644
>--- a/Source/WebCore/layout/LayoutContext.cpp
>+++ b/Source/WebCore/layout/LayoutContext.cpp
>@@ -58,22 +58,19 @@ void LayoutContext::initializeRoot(const Container& root, const LayoutSize& cont
>     // Root is always at 0 0 with no margin 
>     displayBox.setTopLeft({ });
>     displayBox.setSize(containerSize);
>-    displayBox.setMargin({ });
>+    displayBox.setHorizontalMargin({ });
>+    displayBox.setVerticalMargin({ });
> 
>     auto& style = root.style();
>     // FIXME: m_root could very well be a formatting context root with ancestors and resolvable border and padding (as opposed to the topmost root)
>     displayBox.setBorder({
>-        style.borderTop().boxModelWidth(),
>-        style.borderLeft().boxModelWidth(),
>-        style.borderBottom().boxModelWidth(),
>-        style.borderRight().boxModelWidth()
>+        { style.borderLeft().boxModelWidth(), style.borderRight().boxModelWidth() },
>+        { style.borderTop().boxModelWidth(), style.borderBottom().boxModelWidth() }
>     });
> 
>     displayBox.setPadding({
>-        valueForLength(style.paddingTop(), containerSize.width()),
>-        valueForLength(style.paddingLeft(), containerSize.width()),
>-        valueForLength(style.paddingBottom(), containerSize.width()),
>-        valueForLength(style.paddingRight(), containerSize.width())
>+        { valueForLength(style.paddingLeft(), containerSize.width()), valueForLength(style.paddingRight(), containerSize.width()) },
>+        { valueForLength(style.paddingTop(), containerSize.width()), valueForLength(style.paddingBottom(), containerSize.width()) }
>     });
> 
>     m_formattingContextRootListForLayout.add(&root);
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index 4e174d9cbdfd9264b6348ec7b180145a57dc1bff..8f58a22f66b55294a7251b7892dc3e7c139a6602 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -73,10 +73,9 @@ void BlockFormattingContext::layout(LayoutContext& layoutContext, FormattingStat
>             auto& layoutBox = layoutPair.layoutBox;
>             auto& displayBox = layoutPair.displayBox;
>             
>-            computeMargin(layoutContext, layoutBox, displayBox);
>             computeBorderAndPadding(layoutContext, layoutBox, displayBox);
>+            computeWidthAndMargin(layoutContext, layoutBox, displayBox);
>             computeStaticPosition(layoutContext, layoutBox, displayBox);
>-            computeWidth(layoutContext, layoutBox, displayBox);
>             if (layoutBox.establishesFormattingContext()) {
>                 auto formattingContext = layoutContext.formattingContext(layoutBox);
>                 formattingContext->layout(layoutContext, layoutContext.establishedFormattingState(layoutBox, *formattingContext));
>@@ -139,13 +138,13 @@ void BlockFormattingContext::computeInFlowPositionedPosition(LayoutContext& layo
>     displayBox.setTopLeft(Geometry::inFlowPositionedPosition(layoutContext, layoutBox));
> }
> 
>-void BlockFormattingContext::computeWidth(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>+void BlockFormattingContext::computeWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
> {
>     if (layoutBox.isInFlow())
>-        return computeInFlowWidth(layoutContext, layoutBox, displayBox);
>+        return computeInFlowWidthAndMargin(layoutContext, layoutBox, displayBox);
> 
>     if (layoutBox.isFloatingPositioned())
>-        return computeFloatingWidth(layoutContext, layoutBox, displayBox);
>+        return computeFloatingWidthAndMargin(layoutContext, layoutBox, displayBox);
> 
>     ASSERT_NOT_REACHED();
> }
>@@ -166,14 +165,11 @@ void BlockFormattingContext::computeInFlowHeight(LayoutContext& layoutContext, c
>     displayBox.setHeight(Geometry::inFlowHeight(layoutContext, layoutBox));
> }
> 
>-void BlockFormattingContext::computeInFlowWidth(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>+void BlockFormattingContext::computeInFlowWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
> {
>-    displayBox.setWidth(Geometry::inFlowWidth(layoutContext, layoutBox));
>-}
>-
>-void BlockFormattingContext::computeMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>-{
>-    displayBox.setMargin(Geometry::computedMargin(layoutContext, layoutBox));
>+    auto widthAndMargin = Geometry::inFlowWidthAndMargin(layoutContext, layoutBox);
>+    displayBox.setWidth(widthAndMargin.width);
>+    displayBox.setHorizontalMargin(widthAndMargin.margin);
> }
> 
> }
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>index a1353502c9623b81ef5ff92f3cd6fd87ffb0db40..9bbadcc3026250c4fee770c3b55651333a071df2 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>@@ -51,29 +51,26 @@ public:
>     Ref<FloatingState> createOrFindFloatingState(LayoutContext&) const override;
> 
> private:
>-    void computeWidth(LayoutContext&, const Box&, Display::Box&) const;
>+    void computeWidthAndMargin(LayoutContext&, const Box&, Display::Box&) const;
>     void computeHeight(LayoutContext&, const Box&, Display::Box&) const;
> 
>     void computeStaticPosition(LayoutContext&, const Box&, Display::Box&) const override;
>     void computeInFlowPositionedPosition(LayoutContext&, const Box&, Display::Box&) const override;
>-    void computeInFlowWidth(LayoutContext&, const Box&, Display::Box&) const;
>+    void computeInFlowWidthAndMargin(LayoutContext&, const Box&, Display::Box&) const;
>     void computeInFlowHeight(LayoutContext&, const Box&, Display::Box&) const;
>-    void computeMargin(LayoutContext&, const Box&, Display::Box&) const;
> 
>     // This class implements positioning and sizing for boxes participating in a block formatting context.
>     class Geometry {
>     public:
>         static LayoutUnit inFlowHeight(LayoutContext&, const Box&);
>-        static LayoutUnit inFlowWidth(LayoutContext&, const Box&);
>+        static FormattingContext::Geometry::WidthAndMargin inFlowWidthAndMargin(LayoutContext&, const Box&);
> 
>         static LayoutPoint staticPosition(LayoutContext&, const Box&);
>         static LayoutPoint inFlowPositionedPosition(LayoutContext&, const Box&);
> 
>-        static Display::Box::Edges computedMargin(LayoutContext&, const Box&);
>-
>     private:
>         static LayoutUnit inFlowNonReplacedHeight(LayoutContext&, const Box&);
>-        static LayoutUnit inFlowNonReplacedWidth(LayoutContext&, const Box&);
>+        static FormattingContext::Geometry::WidthAndMargin inFlowNonReplacedWidthAndMargin(LayoutContext&, const Box&);
>     };
>     
>     // This class implements margin collapsing for block formatting context.
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index cc72cf2ac4bf9ed76fa9b23d448d814e7f3b1c5f..8661ea016871bb60afd24d5ccae9d9b185f6bf5e 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -113,7 +113,7 @@ LayoutUnit BlockFormattingContext::Geometry::inFlowNonReplacedHeight(LayoutConte
>     return std::max(computedHeight, initialContainingBlockHeight);
> }
> 
>-LayoutUnit BlockFormattingContext::Geometry::inFlowNonReplacedWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isInFlow() && !layoutBox.replaced());
> 
>@@ -143,14 +143,14 @@ LayoutUnit BlockFormattingContext::Geometry::inFlowNonReplacedWidth(LayoutContex
>         } else
>             computedWidthValue = valueForLength(width, containingBlockWidth);
> 
>-        return computedWidthValue;
>+        return FormattingContext::Geometry::WidthAndMargin { computedWidthValue, { } };
>     };
> 
>-    auto computedWidth = compute();
>+    auto computedWidthAndMarginValue = compute();
>     if (!isStretchedToViewport(layoutContext, layoutBox))
>-        return computedWidth;
>+        return computedWidthAndMarginValue;
>     auto initialContainingBlockWidth = layoutContext.displayBoxForLayoutBox(initialContainingBlock(layoutBox))->contentBox().width();
>-    return std::max(computedWidth, initialContainingBlockWidth);
>+    return FormattingContext::Geometry::WidthAndMargin { std::max(computedWidthAndMarginValue.width, initialContainingBlockWidth), { } };
> }
> 
> LayoutPoint BlockFormattingContext::Geometry::staticPosition(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -247,28 +247,15 @@ LayoutUnit BlockFormattingContext::Geometry::inFlowHeight(LayoutContext& layoutC
>     return FormattingContext::Geometry::inlineReplacedHeight(layoutContext, layoutBox);
> }
> 
>-LayoutUnit BlockFormattingContext::Geometry::inFlowWidth(LayoutContext& layoutContext, const Box& layoutBox)
>+FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::inFlowWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isInFlow());
> 
>     if (!layoutBox.replaced())
>-        return inFlowNonReplacedWidth(layoutContext, layoutBox);
>+        return inFlowNonReplacedWidthAndMargin(layoutContext, layoutBox);
>     // 10.3.4 Block-level, replaced elements in normal flow
>     // The used value of 'width' is determined as for inline replaced elements
>-    return FormattingContext::Geometry::inlineReplacedWidth(layoutContext, layoutBox);
>-}
>-
>-Display::Box::Edges BlockFormattingContext::Geometry::computedMargin(LayoutContext& layoutContext, const Box& layoutBox)
>-{
>-    auto& style = layoutBox.style();
>-    auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->width();
>-
>-    return Display::Box::Edges(
>-        BlockFormattingContext::MarginCollapse::marginTop(layoutBox),
>-        valueForLength(style.marginLeft(), containingBlockWidth),
>-        BlockFormattingContext::MarginCollapse::marginBottom(layoutBox),
>-        valueForLength(style.marginRight(), containingBlockWidth)
>-    );
>+    return FormattingContext::Geometry::inlineReplacedWidthAndMargin(layoutContext, layoutBox);
> }
> 
> }
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.cpp b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>index df22d8fc44f29b8fb25bf9c419bc853dd37849d5..88758ed8bf1c1ce3865cfa2fb2df3d2038b15d0c 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>@@ -52,13 +52,14 @@ Box::Style::Style(const RenderStyle& style)
> 
> Box::Rect Box::marginBox() const
> {
>-    ASSERT(m_hasValidMargin);
>+    ASSERT(m_hasValidHorizontalMargin);
>+    ASSERT(m_hasValidVerticalMargin);
>     auto marginBox = borderBox();
> 
>-    marginBox.shiftLeftTo(marginBox.left() - m_margin.left);
>-    marginBox.shiftTopTo(marginBox.top() - m_margin.top);
>-    marginBox.shiftRightTo(marginBox.right() + m_margin.right);
>-    marginBox.shiftBottomTo(marginBox.bottom() + m_margin.bottom);
>+    marginBox.shiftLeftTo(marginBox.left() - m_margin.horizontal.left);
>+    marginBox.shiftTopTo(marginBox.top() - m_margin.vertical.top);
>+    marginBox.shiftRightTo(marginBox.right() + m_margin.horizontal.right);
>+    marginBox.shiftBottomTo(marginBox.bottom() + m_margin.vertical.bottom);
> 
>     return marginBox;
> }
>@@ -83,10 +84,10 @@ Box::Rect Box::paddingBox() const
>     ASSERT(m_hasValidBorder);
>     auto paddingBox = borderBox();
> 
>-    paddingBox.shiftLeftTo(paddingBox.left() + m_border.left);
>-    paddingBox.shiftTopTo(paddingBox.top() + m_border.top);
>-    paddingBox.shiftRightTo(paddingBox.right() - m_border.right);
>-    paddingBox.shiftBottomTo(paddingBox.bottom() - m_border.bottom);
>+    paddingBox.shiftLeftTo(paddingBox.left() + m_border.horizontal.left);
>+    paddingBox.shiftTopTo(paddingBox.top() + m_border.vertical.top);
>+    paddingBox.shiftRightTo(paddingBox.right() - m_border.horizontal.right);
>+    paddingBox.shiftBottomTo(paddingBox.bottom() - m_border.vertical.bottom);
> 
>     return paddingBox;
> }
>@@ -103,10 +104,10 @@ Box::Rect Box::contentBox() const
>     ASSERT(m_hasValidPadding);
>     auto contentBox = paddingBox();
> 
>-    contentBox.shiftLeftTo(contentBox.left() + m_padding.left);
>-    contentBox.shiftTopTo(contentBox.top() + m_padding.top);
>-    contentBox.shiftRightTo(contentBox.right() - m_padding.right);
>-    contentBox.shiftBottomTo(contentBox.bottom() - m_padding.bottom);
>+    contentBox.shiftLeftTo(contentBox.left() + m_padding.horizontal.left);
>+    contentBox.shiftTopTo(contentBox.top() + m_padding.vertical.top);
>+    contentBox.shiftRightTo(contentBox.right() - m_padding.horizontal.right);
>+    contentBox.shiftBottomTo(contentBox.bottom() - m_padding.vertical.bottom);
> 
>     return contentBox;
> }
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index 8c1e261e4c01993a702ce22a9c0a5d5558850dea..f717aa5c47e9ad8b80bc92340e2d69f87dc1baf6 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -160,30 +160,34 @@ private:
>     void setHeight(LayoutUnit height) { m_rect.setHeight(height); }
>     void setSize(const LayoutSize& size) { m_rect.setSize(size); }
> 
>-    struct Edges {
>-        Edges() = default;
>-        Edges(LayoutUnit top, LayoutUnit left, LayoutUnit bottom, LayoutUnit right)
>-            : top(top)
>-            , left(left)
>-            , bottom(bottom)
>-            , right(right)
>-            { }
>+    struct HorizontalEdges {
>+        LayoutUnit left;
>+        LayoutUnit right;
>+    };
> 
>+    struct VerticalEdges {
>         LayoutUnit top;
>-        LayoutUnit left;
>         LayoutUnit bottom;
>-        LayoutUnit right;
>     };
>-    void setMargin(Edges);
>+
>+    struct Edges {
>+        HorizontalEdges horizontal;
>+        VerticalEdges vertical;
>+    };
>+
>+    void setHorizontalMargin(HorizontalEdges);
>+    void setVerticalMargin(VerticalEdges);
>     void setBorder(Edges);
>     void setPadding(Edges);
> 
> #if !ASSERT_DISABLED
>-    void invalidateMargin() { m_hasValidMargin = false; }
>+    void invalidateMargin();
>     void invalidateBorder() { m_hasValidBorder = false; }
>     void invalidatePadding() { m_hasValidPadding = false; }
> 
>-    void setHasValidMargin() { m_hasValidMargin = true; }
>+    void setHasValidVerticalMargin() { m_hasValidVerticalMargin = true; }
>+    void setHasValidHorizontalMargin() { m_hasValidHorizontalMargin = true; }
>+
>     void setHasValidBorder() { m_hasValidBorder = true; }
>     void setHasValidPadding() { m_hasValidPadding = true; }
> #endif
>@@ -197,7 +201,8 @@ private:
>     Edges m_padding;
> 
> #if !ASSERT_DISABLED
>-    bool m_hasValidMargin { false };
>+    bool m_hasValidHorizontalMargin { false };
>+    bool m_hasValidVerticalMargin { false };
>     bool m_hasValidBorder { false };
>     bool m_hasValidPadding { false };
> #endif
>@@ -221,6 +226,12 @@ inline void Box::Rect::setHasValidSize()
>     m_hasValidWidth = true;
>     m_hasValidHeight = true;
> }
>+
>+inline void Box::invalidateMargin()
>+{
>+    m_hasValidHorizontalMargin = false;
>+    m_hasValidVerticalMargin = false;
>+}
> #endif
> 
> inline LayoutUnit Box::Rect::top() const
>@@ -376,12 +387,20 @@ inline Box::Rect::operator LayoutRect() const
>     return m_rect;
> }
> 
>-inline void Box::setMargin(Edges margin)
>+inline void Box::setHorizontalMargin(HorizontalEdges margin)
>+{
>+#if !ASSERT_DISABLED
>+    setHasValidHorizontalMargin();
>+#endif
>+    m_margin.horizontal = margin;
>+}
>+
>+inline void Box::setVerticalMargin(VerticalEdges margin)
> {
> #if !ASSERT_DISABLED
>-    setHasValidMargin();
>+    setHasValidVerticalMargin();
> #endif
>-    m_margin = margin;
>+    m_margin.vertical = margin;
> }
> 
> inline void Box::setBorder(Edges border)
>@@ -402,74 +421,74 @@ inline void Box::setPadding(Edges padding)
> 
> inline LayoutUnit Box::marginTop() const
> {
>-    ASSERT(m_hasValidMargin);
>-    return m_margin.top;
>+    ASSERT(m_hasValidVerticalMargin);
>+    return m_margin.vertical.top;
> }
> 
> inline LayoutUnit Box::marginLeft() const
> {
>-    ASSERT(m_hasValidMargin);
>-    return m_margin.left;
>+    ASSERT(m_hasValidHorizontalMargin);
>+    return m_margin.horizontal.left;
> }
> 
> inline LayoutUnit Box::marginBottom() const
> {
>-    ASSERT(m_hasValidMargin);
>-    return m_margin.bottom;
>+    ASSERT(m_hasValidVerticalMargin);
>+    return m_margin.vertical.bottom;
> }
> 
> inline LayoutUnit Box::marginRight() const
> {
>-    ASSERT(m_hasValidMargin);
>-    return m_margin.right;
>+    ASSERT(m_hasValidHorizontalMargin);
>+    return m_margin.horizontal.right;
> }
> 
> inline LayoutUnit Box::paddingTop() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.top;
>+    return m_padding.vertical.top;
> }
> 
> inline LayoutUnit Box::paddingLeft() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.left;
>+    return m_padding.horizontal.left;
> }
> 
> inline LayoutUnit Box::paddingBottom() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.bottom;
>+    return m_padding.vertical.bottom;
> }
> 
> inline LayoutUnit Box::paddingRight() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.right;
>+    return m_padding.horizontal.right;
> }
> 
> inline LayoutUnit Box::borderTop() const
> {
>     ASSERT(m_hasValidBorder);
>-    return m_border.top;
>+    return m_border.vertical.top;
> }
> 
> inline LayoutUnit Box::borderLeft() const
> {
>     ASSERT(m_hasValidBorder);
>-    return m_border.left;
>+    return m_border.horizontal.left;
> }
> 
> inline LayoutUnit Box::borderBottom() const
> {
>     ASSERT(m_hasValidBorder);
>-    return m_border.bottom;
>+    return m_border.vertical.bottom;
> }
> 
> inline LayoutUnit Box::borderRight() const
> {
>     ASSERT(m_hasValidBorder);
>-    return m_border.right;
>+    return m_border.horizontal.right;
> }
> 
> }

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186217: 341813 | 341814 | 341823