Attachment 347079 Details for Bug 188555 – Patch

[patch] Patch

bug-188555-20180814090524.patch (text/plain), 17.65 KB, created by zalan on 2018-08-14 09:05:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-14 09:05:24 PDT

Size: 17.65 KB

patch

obsolete

>Subversion Revision: 234847
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 3d958956f18c555f10b568a1ead5b8ddfc74974b..5d8d52cb48626c98039dba8ef9960c025bc60f32 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2018-08-14  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floating] Add support for negative clearance.
>+        https://bugs.webkit.org/show_bug.cgi?id=188555
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        1. Compute clearance to avoid float(s) (border box needs to avoid floats)
>+        2. Reset vertical margins to non-collapsed values.
>+        4. Adjust clearance with the new margins.
>+        5. Take the adjusted clearance and move the box vertically if needed.
>+
>+        Test: fast/block/block-only/margin-collapse-with-clearance.html
>+
>+        * layout/FloatingContext.cpp:
>+        (WebCore::Layout::FloatingContext::verticalPositionWithClearance const):
>+        * layout/FloatingState.cpp:
>+        (WebCore::Layout::FloatingState::FloatItem::FloatItem):
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::mapBoxToAncestor):
>+        (WebCore::Layout::FormattingContext::mapTopLeftToAncestor):
>+        (WebCore::Layout::FormattingContext::mapCoordinateToAncestor):
>+        (WebCore::Layout::FormattingContext::mapToAncestor): Deleted.
>+        * layout/FormattingContext.h:
>+        * layout/LayoutUnits.h:
>+        (WebCore::Layout::Position::Position):
>+        (WebCore::Layout::Position::moveBy):
>+        * layout/displaytree/DisplayBox.h:
>+
> 2018-08-14  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Adjust vertical position with non-collapsed previous sibling margin.
>diff --git a/Source/WebCore/layout/FloatingContext.cpp b/Source/WebCore/layout/FloatingContext.cpp
>index 340a7e169701fb0a239b1c88744bcd3ba6a71710..f117ff1b7211fa4c328be96e2d9a5546a2dff89a 100644
>--- a/Source/WebCore/layout/FloatingContext.cpp
>+++ b/Source/WebCore/layout/FloatingContext.cpp
>@@ -150,13 +150,40 @@ std::optional<LayoutUnit> FloatingContext::verticalPositionWithClearance(const B
>         // 1. The amount necessary to place the border edge of the block even with the bottom outer edge of the lowest float that is to be cleared.
>         // 2. The amount necessary to place the top border edge of the block at its hypothetical position.
> 
>-        auto absoluteDisplayBox = FormattingContext::mapToAncestor(layoutContext(), layoutBox, downcast<Container>(m_floatingState.root()));
>-        if (floatBottom <= absoluteDisplayBox.rectWithMargin().top())
>+        auto& layoutContext = this->layoutContext();
>+        auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
>+        auto absoluteTop = FormattingContext::mapTopLeftToAncestor(layoutContext, layoutBox, downcast<Container>(m_floatingState.root())).y;
>+        auto clearance = *floatBottom - absoluteTop;
>+        if (clearance <= 0)
>             return { };
> 
>+        // Clearance inhibits margin collapsing. Let's reset the relevant adjoining margins.
>+        if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
>+            auto& previousInFlowDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowSibling);
>+
>+            // Previous inlflow's bottom margin is collapsed.
>+            ASSERT(!previousInFlowDisplayBox.marginBottom());
>+            auto collapsedMargin = displayBox.marginTop();
>+
>+            // Reset previous's bottom and current's top to non-collapsing.
>+            previousInFlowDisplayBox.setVerticalMargin({ previousInFlowDisplayBox.marginTop(), previousInFlowDisplayBox.nonCollapsedMarginBottom() });
>+            displayBox.setVerticalMargin({ displayBox.nonCollapsedMarginTop(), displayBox.marginBottom() });
>+
>+            auto nonCollapsedMargin = previousInFlowDisplayBox.marginBottom() + displayBox.marginTop();
>+            auto marginOffset = nonCollapsedMargin - collapsedMargin;
>+            // Move the box to the position where it would be with non-collapsed margins.
>+            absoluteTop += marginOffset;
>+
>+            // Having negative clearance is also normal. It just means that the box with the non-collapsed margins is now lower than it needs to be.
>+            clearance -= marginOffset;
>+        }
>+        // Now adjust the box's position with the clearance.
>+        absoluteTop += clearance;
>+        ASSERT(*floatBottom == absoluteTop);
>+
>         // The return vertical position is in the containing block's coordinate system.
>-        auto* containingBlockDisplayBox = layoutContext().displayBoxForLayoutBox(*layoutBox.containingBlock());
>-        return *floatBottom - containingBlockDisplayBox->top();
>+        auto containingBlockAbsoluteTop = FormattingContext::mapTopLeftToAncestor(layoutContext, *layoutBox.containingBlock(), downcast<Container>(m_floatingState.root())).y;
>+        return absoluteTop - containingBlockAbsoluteTop;
>     };
> 
>     auto clear = layoutBox.style().clear();
>diff --git a/Source/WebCore/layout/FloatingState.cpp b/Source/WebCore/layout/FloatingState.cpp
>index 91d7c2e6a4039449ff18b651709d1737ba7ff414..41d3ecedcbf53a300c8f195b991d345b6fdbe008 100644
>--- a/Source/WebCore/layout/FloatingState.cpp
>+++ b/Source/WebCore/layout/FloatingState.cpp
>@@ -41,8 +41,8 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(FloatingState);
> FloatingState::FloatItem::FloatItem(const Box& layoutBox, const FloatingState& floatingState)
>     : m_layoutBox(makeWeakPtr(const_cast<Box&>(layoutBox)))
>     , m_containingBlock(makeWeakPtr(const_cast<Container&>(*layoutBox.containingBlock())))
>-    , m_absoluteDisplayBox(FormattingContext::mapToAncestor(floatingState.layoutContext(), layoutBox, downcast<Container>(floatingState.root())))
>-    , m_containingBlockAbsoluteDisplayBox(FormattingContext::mapToAncestor(floatingState.layoutContext(), *m_containingBlock, downcast<Container>(floatingState.root())))
>+    , m_absoluteDisplayBox(FormattingContext::mapBoxToAncestor(floatingState.layoutContext(), layoutBox, downcast<Container>(floatingState.root())))
>+    , m_containingBlockAbsoluteDisplayBox(FormattingContext::mapBoxToAncestor(floatingState.layoutContext(), *m_containingBlock, downcast<Container>(floatingState.root())))
> {
> }
> 
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index 1a05b7a2cc7fe7607854a6204fe5ea39f63a499c..4163e76193fb1b2d9b5b74d9c49dae5adbb8485b 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -142,7 +142,7 @@ void FormattingContext::layoutOutOfFlowDescendants(LayoutContext& layoutContext,
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "End: layout out-of-flow descendants -> context: " << &layoutContext << " root: " << &root());
> }
> 
>-Display::Box FormattingContext::mapToAncestor(const LayoutContext& layoutContext, const Box& layoutBox, const Container& ancestor)
>+Display::Box FormattingContext::mapBoxToAncestor(const LayoutContext& layoutContext, const Box& layoutBox, const Container& ancestor)
> {
>     ASSERT(layoutBox.isDescendantOf(ancestor));
> 
>@@ -164,6 +164,27 @@ Display::Box FormattingContext::mapToAncestor(const LayoutContext& layoutContext
>     return mappedDisplayBox;
> }
> 
>+Position FormattingContext::mapTopLeftToAncestor(const LayoutContext& layoutContext, const Box& layoutBox, const Container& ancestor)
>+{
>+    ASSERT(layoutBox.isDescendantOf(ancestor));
>+    return mapCoordinateToAncestor(layoutContext, layoutContext.displayBoxForLayoutBox(layoutBox)->topLeft(), *layoutBox.containingBlock(), ancestor);
>+}
>+
>+Position FormattingContext::mapCoordinateToAncestor(const LayoutContext& layoutContext, Position position, const Container& containingBlock, const Container& ancestor)
>+{
>+    auto mappedPosition = position;
>+    auto* container = &containingBlock;
>+    for (; container && container != &ancestor; container = container->containingBlock())
>+        mappedPosition.moveBy(layoutContext.displayBoxForLayoutBox(*container)->topLeft());
>+
>+    if (!container) {
>+        ASSERT_NOT_REACHED();
>+        return position;
>+    }
>+
>+    return mappedPosition;
>+}
>+
> #ifndef NDEBUG
> void FormattingContext::validateGeometryConstraintsAfterLayout(const LayoutContext& layoutContext) const
> {
>diff --git a/Source/WebCore/layout/FormattingContext.h b/Source/WebCore/layout/FormattingContext.h
>index 57139a5e6847d81a0755319ec4461d38ae4ff026..aeb0e2f9a3a0770d0e7643228e2ef10913ea1218 100644
>--- a/Source/WebCore/layout/FormattingContext.h
>+++ b/Source/WebCore/layout/FormattingContext.h
>@@ -59,7 +59,9 @@ public:
>     };
>     virtual InstrinsicWidthConstraints instrinsicWidthConstraints(LayoutContext&, const Box&) const = 0;
> 
>-    static Display::Box mapToAncestor(const LayoutContext&, const Box&, const Container& ancestor);
>+    static Display::Box mapBoxToAncestor(const LayoutContext&, const Box&, const Container& ancestor);
>+    static Position mapTopLeftToAncestor(const LayoutContext&, const Box&, const Container& ancestor);
>+    static Position mapCoordinateToAncestor(const LayoutContext&, Position, const Container& containingBlock, const Container& ancestor);
> 
> protected:
>     struct LayoutPair {
>diff --git a/Source/WebCore/layout/LayoutUnits.h b/Source/WebCore/layout/LayoutUnits.h
>index e2a8375eef95003d9b3d414dad4845bb54566986..d39e50a4b6fb65ea8d706097501fede524e472ba 100644
>--- a/Source/WebCore/layout/LayoutUnits.h
>+++ b/Source/WebCore/layout/LayoutUnits.h
>@@ -38,9 +38,32 @@ struct Position {
>     // FIXME: Use LayoutUnit<Horizontal> to avoid top/left vs. x/y confusion.
>     LayoutUnit x; // left
>     LayoutUnit y; // top
>+
>+    Position() = default;
>+    Position(LayoutUnit, LayoutUnit);
>+    Position(LayoutPoint);
>+    void moveBy(LayoutPoint);
>     operator LayoutPoint() const { return { x, y }; }
> };
> 
>+inline Position::Position(LayoutPoint point)
>+    : x(point.x())
>+    , y(point.y())
>+{
>+}
>+
>+inline Position::Position(LayoutUnit x, LayoutUnit y)
>+    : x(x)
>+    , y(y)
>+{
>+}
>+
>+inline void Position::moveBy(LayoutPoint offset)
>+{
>+    x += offset.x();
>+    y += offset.y();
>+}
>+
> // Margin, border, padding
> struct HorizontalEdges {
>     LayoutUnit left;
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index f7e0c6b64098110ff1ef5bb25a4617a9b89feab5..fd096d3f33214df1632670dd205fff094378a14e 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -38,6 +38,7 @@ class RenderStyle;
> namespace Layout {
> class BlockFormattingContext;
> class FormattingContext;
>+class FloatingContext;
> class LayoutContext;
> }
> 
>@@ -48,6 +49,7 @@ class Box {
> public:
>     friend class Layout::BlockFormattingContext;
>     friend class Layout::FormattingContext;
>+    friend class Layout::FloatingContext;
>     friend class Layout::LayoutContext;
> 
>     Box(const Box&);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 049918be0739f183bfab5c3a430b1593e53f275d..3cf2d14bfa925e704877c22f2a4c8e592924e3ac 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-08-14  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floating] Add support for negative clearance.
>+        https://bugs.webkit.org/show_bug.cgi?id=188555
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * LayoutReloaded/misc/LFC-passing-tests.txt:
>+
> 2018-08-14  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Adjust vertical position with non-collapsing previous sibling margin.
>diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>index 51d16c24c0f39814d6ee24b54bdf8450d9f6efe1..5d294a0c93bfc7f0fcf3ee26cc4eefa97f619d82 100644
>--- a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>+++ b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>@@ -49,3 +49,4 @@ fast/block/block-only/floating-multiple-lefts.html
> fast/block/block-only/floating-and-next-previous-inflow-with-margin.html
> fast/block/block-only/floating-left-and-right-with-clearance.html
> fast/block/block-only/float-and-siblings-with-margins.html
>+fast/block/block-only/margin-collapse-with-clearance.html
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e563eb32f20e5c1c8fd8577897a8876fc05fbae1..4b91f7d3e713c53cb3e3419719d155dcd890b4fe 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-08-14  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floating] Add support for negative clearance.
>+        https://bugs.webkit.org/show_bug.cgi?id=188555
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/block/block-only/margin-collapse-with-clearance-expected.txt: Added.
>+        * fast/block/block-only/margin-collapse-with-clearance.html: Added.
>+
> 2018-08-14  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Adjust vertical position with non-collapsing previous sibling margin.
>diff --git a/LayoutTests/fast/block/block-only/margin-collapse-with-clearance-expected.txt b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..767ec80572a62d32567472f84d947aef232ee117
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance-expected.txt
>@@ -0,0 +1,33 @@
>+layer at (0,0) size 800x600
>+  RenderView at (0,0) size 800x600
>+layer at (0,0) size 800x450
>+  RenderBlock {HTML} at (0,0) size 800x450
>+    RenderBody {BODY} at (8,8) size 784x434
>+      RenderBlock {DIV} at (0,0) size 502x62 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (1,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,62) size 502x62 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (449,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,124) size 502x62 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,43) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,186) size 502x62 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (1,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (449,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,248) size 502x62 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (1,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,310) size 502x62 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (449,23) size 52x22 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,45) size 12x12 [border: (1px solid #000000)]
>+      RenderBlock {DIV} at (0,372) size 502x62 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,1) size 12x12 [border: (1px solid #000000)]
>+        RenderBlock (floating) {DIV} at (1,23) size 52x7 [border: (1px solid #000000)]
>+        RenderBlock {DIV} at (1,30) size 12x12 [border: (1px solid #000000)]
>diff --git a/LayoutTests/fast/block/block-only/margin-collapse-with-clearance.html b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..baf64cbade7c2d1a9e1802739b62fdef31887411
>--- /dev/null
>+++ b/LayoutTests/fast/block/block-only/margin-collapse-with-clearance.html
>@@ -0,0 +1,47 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<style>
>+div {
>+	border: 1px solid black;
>+}
>+</style>
>+</head>
>+<body>
>+<div style="width: 500px; height: 60px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: left; width: 50px; height: 20px;"></div>
>+	<div style="clear: left; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 60px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: right; width: 50px; height: 20px;"></div>
>+	<div style="clear: right; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 60px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 60px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: left; width: 50px; height: 20px;"></div>
>+	<div style="float: right; width: 50px; height: 20px;"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 60px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: left; width: 50px; height: 20px;"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 60px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: right; width: 50px; height: 20px;"></div>
>+	<div style="clear: both; width: 10px; height: 10px; margin-top: 30px"></div>
>+</div>
>+<div style="width: 500px; height: 60px;">
>+	<div style="width: 10px; height: 10px; margin-bottom: 10px"></div>
>+	<div style="float: left; width: 50px; height: 5px;"></div>
>+	<div style="clear: left; width: 10px; height: 10px; margin-top: 10px"></div>
>+</div>
>+</body>
>+</html>
>\ No newline at end of file

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188555: 347079