Attachment 341423 Details for Bug 186019 – Patch

[patch] Patch

bug-186019-20180527092126.patch (text/plain), 18.97 KB, created by zalan on 2018-05-27 09:21:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-05-27 09:21:27 PDT

Size: 18.97 KB

patch

obsolete

>Subversion Revision: 232226
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6b088b9d834e3e0565973ecc1141071d89e052d3..b607ea82efff4106273b0ac70dbc2da65e8ad2f2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,78 @@
>+2018-05-27  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Add Rect interface to Display::Box
>+        https://bugs.webkit.org/show_bug.cgi?id=186019
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Having a dedicated Rect class and moving the geometry invalidation to it enables us to call
>+        displayBox.contentBox().left() even when the widht/height are not computed yet.
>+        Also having Rect with top/left/bottom/right interface is more aligned with the rest of the layout code than x/y/maxX/maxY.
>+
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::contentHeightForFormattingContextRoot):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::staticPosition):
>+        * layout/displaytree/DisplayBox.cpp:
>+        (WebCore::Display::Box::marginBox const):
>+        (WebCore::Display::Box::borderBox const):
>+        (WebCore::Display::Box::paddingBox const):
>+        (WebCore::Display::Box::contentBox const):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::Rect::invalidateTop):
>+        (WebCore::Display::Box::Rect::invalidateLeft):
>+        (WebCore::Display::Box::Rect::invalidateWidth):
>+        (WebCore::Display::Box::Rect::invalidateHeight):
>+        (WebCore::Display::Box::Rect::hasValidPosition const):
>+        (WebCore::Display::Box::Rect::hasValidSize const):
>+        (WebCore::Display::Box::Rect::hasValidGeometry const):
>+        (WebCore::Display::Box::rect const):
>+        (WebCore::Display::Box::top const):
>+        (WebCore::Display::Box::left const):
>+        (WebCore::Display::Box::bottom const):
>+        (WebCore::Display::Box::right const):
>+        (WebCore::Display::Box::topLeft const):
>+        (WebCore::Display::Box::bottomRight const):
>+        (WebCore::Display::Box::size const):
>+        (WebCore::Display::Box::width const):
>+        (WebCore::Display::Box::height const):
>+        (WebCore::Display::Box::setTopLeft):
>+        (WebCore::Display::Box::setTop):
>+        (WebCore::Display::Box::setLeft):
>+        (WebCore::Display::Box::setWidth):
>+        (WebCore::Display::Box::setHeight):
>+        (WebCore::Display::Box::Rect::invalidatePosition):
>+        (WebCore::Display::Box::Rect::setHasValidPosition):
>+        (WebCore::Display::Box::Rect::Rect):
>+        (WebCore::Display::Box::Rect::top const):
>+        (WebCore::Display::Box::Rect::left const):
>+        (WebCore::Display::Box::Rect::bottom const):
>+        (WebCore::Display::Box::Rect::right const):
>+        (WebCore::Display::Box::Rect::topLeft const):
>+        (WebCore::Display::Box::Rect::bottomRight const):
>+        (WebCore::Display::Box::Rect::size const):
>+        (WebCore::Display::Box::Rect::width const):
>+        (WebCore::Display::Box::Rect::height const):
>+        (WebCore::Display::Box::Rect::setTopLeft):
>+        (WebCore::Display::Box::Rect::setTop):
>+        (WebCore::Display::Box::Rect::setLeft):
>+        (WebCore::Display::Box::Rect::setWidth):
>+        (WebCore::Display::Box::Rect::setHeight):
>+        (WebCore::Display::Box::Rect::shiftLeftTo):
>+        (WebCore::Display::Box::Rect::shiftRightTo):
>+        (WebCore::Display::Box::Rect::shiftTopTo):
>+        (WebCore::Display::Box::Rect::shiftBottomTo):
>+        (WebCore::Display::Box::Rect::expand):
>+        (WebCore::Display::Box::invalidateTop): Deleted.
>+        (WebCore::Display::Box::invalidateLeft): Deleted.
>+        (WebCore::Display::Box::invalidateWidth): Deleted.
>+        (WebCore::Display::Box::invalidateHeight): Deleted.
>+        (WebCore::Display::Box::hasValidPosition const): Deleted.
>+        (WebCore::Display::Box::hasValidSize const): Deleted.
>+        (WebCore::Display::Box::hasValidGeometry const): Deleted.
>+        (WebCore::Display::Box::invalidatePosition): Deleted.
>+        (WebCore::Display::Box::setHasValidPosition): Deleted.
>+
> 2018-05-26  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Implement margin computation
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index 2f3c954fcfde3162714167d28fe41fd300c2203d..8c74ec541817ed09883a06f0792d6c7b3ca73093 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -53,8 +53,8 @@ static LayoutUnit contentHeightForFormattingContextRoot(LayoutContext& layoutCon
>     auto* firstDisplayBox = layoutContext.displayBoxForLayoutBox(*formattingRootContainer.firstInFlowChild());
>     auto* lastDisplayBox = layoutContext.displayBoxForLayoutBox(*formattingRootContainer.lastInFlowChild());
> 
>-    auto top = firstDisplayBox->marginBox().y();
>-    auto bottom = lastDisplayBox->marginBox().maxY();
>+    auto top = firstDisplayBox->marginBox().top();
>+    auto bottom = lastDisplayBox->marginBox().bottom();
>     // FIXME: add floating support.
>     return bottom - top;
> }
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index d5be6ddb9c1a699cf4b7e35a3c0c5d94246265db..2531e903639c844daf5fbaa4502cddcfa7a21a6a 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -125,8 +125,8 @@ LayoutPoint BlockFormattingContext::Geometry::staticPosition(LayoutContext& layo
>     // In a block formatting context, each box's left outer edge touches the left edge of the containing block (for right-to-left formatting, right edges touch).
>     auto containingBlockContentBox = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->contentBox();
>     // Start from the top of the container's content box.
>-    auto top = containingBlockContentBox.y();
>-    auto left = containingBlockContentBox.x();
>+    auto top = containingBlockContentBox.top();
>+    auto left = containingBlockContentBox.left();
>     if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
>         auto& previousInFlowDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowSibling);
>         top = previousInFlowDisplayBox.bottom() + previousInFlowDisplayBox.marginBottom();
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.cpp b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>index 8664e64cd646de0e8598dddbcd29a0f67ae3b08f..f4f373935135f69c59bcba82e8ebcb806f52d9bd 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>@@ -51,58 +51,58 @@ Box::Style::Style(const RenderStyle& style)
> 
> }
> 
>-LayoutRect Box::marginBox() const
>+Box::Rect Box::marginBox() const
> {
>     ASSERT(m_hasValidMargin);
>     auto marginBox = borderBox();
> 
>-    marginBox.shiftXEdgeTo(marginBox.x() + m_margin.left);
>-    marginBox.shiftYEdgeTo(marginBox.y() + m_margin.top);
>-    marginBox.shiftMaxXEdgeTo(marginBox.maxX() - m_margin.right);
>-    marginBox.shiftMaxYEdgeTo(marginBox.maxY() - m_margin.bottom);
>+    marginBox.shiftLeftTo(marginBox.left() + m_margin.left);
>+    marginBox.shiftBottomTo(marginBox.top() + m_margin.top);
>+    marginBox.shiftRightTo(marginBox.right() - m_margin.right);
>+    marginBox.shiftBottomTo(marginBox.bottom() - m_margin.bottom);
> 
>     return marginBox;
> }
> 
>-LayoutRect Box::borderBox() const
>+Box::Rect Box::borderBox() const
> {
>     if (m_style.boxSizing == BoxSizing::BorderBox)
>-        return LayoutRect( { }, size());
>+        return Box::Rect( { }, size());
> 
>     // Width is content box.
>     ASSERT(m_hasValidBorder);
>     ASSERT(m_hasValidPadding);
>     auto borderBoxSize = size();
>-    borderBoxSize.expand(borderLeft() + paddingLeft() + paddingRight() + borderRight() , borderTop() + paddingTop() + paddingBottom() + borderBottom());
>-    return LayoutRect( { }, borderBoxSize);
>+    borderBoxSize.expand(borderLeft() + paddingLeft() + paddingRight() + borderRight(), borderTop() + paddingTop() + paddingBottom() + borderBottom());
>+    return Box::Rect( { }, borderBoxSize);
> }
> 
>-LayoutRect Box::paddingBox() const
>+Box::Rect Box::paddingBox() const
> {
>     ASSERT(m_hasValidBorder);
>     auto paddingBox = borderBox();
> 
>-    paddingBox.shiftXEdgeTo(paddingBox.x() + m_border.left);
>-    paddingBox.shiftYEdgeTo(paddingBox.y() + m_border.top);
>-    paddingBox.shiftMaxXEdgeTo(paddingBox.maxX() - m_border.right);
>-    paddingBox.shiftMaxYEdgeTo(paddingBox.maxY() - m_border.bottom);
>+    paddingBox.shiftLeftTo(paddingBox.left() + m_border.left);
>+    paddingBox.shiftTopTo(paddingBox.top() + m_border.top);
>+    paddingBox.shiftRightTo(paddingBox.left() - m_border.right);
>+    paddingBox.shiftBottomTo(paddingBox.bottom() - m_border.bottom);
> 
>     return paddingBox;
> }
> 
>-LayoutRect Box::contentBox() const
>+Box::Rect Box::contentBox() const
> {
>     if (m_style.boxSizing == BoxSizing::ContentBox)
>-        return LayoutRect(LayoutPoint(0, 0), size());
>+        return Box::Rect(LayoutPoint(0, 0), size());
> 
>     // Width is border box.
>     ASSERT(m_hasValidPadding);
>     auto contentBox = paddingBox();
> 
>-    contentBox.shiftXEdgeTo(contentBox.x() + m_padding.left);
>-    contentBox.shiftYEdgeTo(contentBox.y() + m_padding.top);
>-    contentBox.shiftMaxXEdgeTo(contentBox.maxX() - m_padding.right);
>-    contentBox.shiftMaxYEdgeTo(contentBox.maxY() - m_padding.bottom);
>+    contentBox.shiftLeftTo(contentBox.left() + m_padding.left);
>+    contentBox.shiftTopTo(contentBox.top() + m_padding.top);
>+    contentBox.shiftBottomTo(contentBox.bottom() - m_padding.bottom);
>+    contentBox.shiftRightTo(contentBox.right() - m_padding.right);
> 
>     return contentBox;
> }
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index e56ec73fc16654580a68cf8566eecb851e2bd67a..3fa123187982d7745d3d10155d514e92a429f646 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -52,21 +52,73 @@ public:
>     friend class Layout::FormattingContext;
>     friend class Layout::LayoutContext;
> 
>+    class Rect {
>+    public:
>+        Rect() = default;
>+        Rect(const LayoutPoint&, const LayoutSize&);
>+        
>+        LayoutUnit top() const;
>+        LayoutUnit left() const;
>+        LayoutPoint topLeft() const;
>+
>+        LayoutUnit bottom() const;
>+        LayoutUnit right() const;        
>+        LayoutPoint bottomRight() const;
>+
>+        LayoutUnit width() const;
>+        LayoutUnit height() const;
>+        LayoutSize size() const;
>+
>+        void setTop(LayoutUnit);
>+        void setLeft(LayoutUnit);
>+        void setTopLeft(const LayoutPoint&);
>+        void setWidth(LayoutUnit);
>+        void setHeight(LayoutUnit);
>+
>+        void shiftLeftTo(LayoutUnit);
>+        void shiftRightTo(LayoutUnit);
>+        void shiftTopTo(LayoutUnit);
>+        void shiftBottomTo(LayoutUnit);
>+
>+        void expand(LayoutUnit, LayoutUnit);
>+
>+    private:
>+#if !ASSERT_DISABLED
>+        void invalidateTop() { m_hasValidTop = false; }
>+        void invalidateLeft() { m_hasValidLeft = false; }
>+        void invalidateWidth() { m_hasValidWidth = false; }
>+        void invalidateHeight() { m_hasValidHeight = false; }
>+        void invalidatePosition();
>+
>+        bool hasValidPosition() const { return m_hasValidTop && m_hasValidLeft; }
>+        bool hasValidSize() const { return m_hasValidWidth && m_hasValidHeight; }
>+        bool hasValidGeometry() const { return hasValidPosition() && hasValidSize(); }
>+    
>+        void setHasValidPosition();
>+
>+        bool m_hasValidTop { false };
>+        bool m_hasValidLeft { false };
>+        bool m_hasValidWidth { false };
>+        bool m_hasValidHeight { false };
>+#endif
>+        LayoutRect m_rect;
>+    };
>+
>     ~Box();
> 
>-    LayoutRect rect() const;
>+    Rect rect() const { return m_rect; }
> 
>-    LayoutUnit top() const;
>-    LayoutUnit left() const;
>-    LayoutUnit bottom() const;
>-    LayoutUnit right() const;
>+    LayoutUnit top() const { return m_rect.top(); }
>+    LayoutUnit left() const { return m_rect.left(); }
>+    LayoutUnit bottom() const { return m_rect.bottom(); }
>+    LayoutUnit right() const { return m_rect.right(); }
> 
>-    LayoutPoint topLeft() const;
>-    LayoutPoint bottomRight() const;
>+    LayoutPoint topLeft() const { return m_rect.topLeft(); }
>+    LayoutPoint bottomRight() const { return m_rect.bottomRight(); }
> 
>-    LayoutSize size() const;
>-    LayoutUnit width() const;
>-    LayoutUnit height() const;
>+    LayoutSize size() const { return m_rect.size(); }
>+    LayoutUnit width() const { return m_rect.width(); }
>+    LayoutUnit height() const { return m_rect.height(); }
> 
>     LayoutUnit marginTop() const;
>     LayoutUnit marginLeft() const;
>@@ -83,10 +135,10 @@ public:
>     LayoutUnit paddingBottom() const;
>     LayoutUnit paddingRight() const;
> 
>-    LayoutRect marginBox() const;
>-    LayoutRect borderBox() const;
>-    LayoutRect paddingBox() const;
>-    LayoutRect contentBox() const;
>+    Rect marginBox() const;
>+    Rect borderBox() const;
>+    Rect paddingBox() const;
>+    Rect contentBox() const;
> 
> private:
>     Box(const RenderStyle&);
>@@ -97,11 +149,11 @@ private:
>         BoxSizing boxSizing { BoxSizing::ContentBox };
>     };
> 
>-    void setTopLeft(const LayoutPoint&);
>-    void setTop(LayoutUnit);
>-    void setLeft(LayoutUnit);
>-    void setWidth(LayoutUnit);
>-    void setHeight(LayoutUnit);
>+    void setTopLeft(const LayoutPoint& topLeft) { m_rect.setTopLeft(topLeft); }
>+    void setTop(LayoutUnit top) { m_rect.setTop(top); }
>+    void setLeft(LayoutUnit left) { m_rect.setLeft(left); }
>+    void setWidth(LayoutUnit width) { m_rect.setWidth(width); }
>+    void setHeight(LayoutUnit height) { m_rect.setHeight(height); }
> 
>     struct Edges {
>         Edges() = default;
>@@ -122,21 +174,10 @@ private:
>     void setPadding(Edges);
> 
> #if !ASSERT_DISABLED
>-    void invalidateTop() { m_hasValidTop = false; }
>-    void invalidateLeft() { m_hasValidLeft = false; }
>-    void invalidateWidth() { m_hasValidWidth = false; }
>-    void invalidateHeight() { m_hasValidHeight = false; }
>-    void invalidatePosition();
>     void invalidateMargin() { m_hasValidMargin = false; }
>     void invalidateBorder() { m_hasValidBorder = false; }
>     void invalidatePadding() { m_hasValidPadding = false; }
> 
>-    bool hasValidPosition() const { return m_hasValidTop && m_hasValidLeft; }
>-    bool hasValidSize() const { return m_hasValidWidth && m_hasValidHeight; }
>-    bool hasValidGeometry() const { return hasValidPosition() && hasValidSize(); }
>-    
>-    void setHasValidPosition();
>-    
>     void setHasValidMargin();
>     void setHasValidBorder();
>     void setHasValidPadding();
>@@ -144,17 +185,13 @@ private:
> 
>     const Style m_style;
> 
>-    LayoutRect m_rect;
>+    Rect m_rect;
> 
>     Edges m_margin;
>     Edges m_border;
>     Edges m_padding;
> 
> #if !ASSERT_DISABLED
>-    bool m_hasValidTop { false };
>-    bool m_hasValidLeft { false };
>-    bool m_hasValidWidth { false };
>-    bool m_hasValidHeight { false };
>     bool m_hasValidMargin { false };
>     bool m_hasValidBorder { false };
>     bool m_hasValidPadding { false };
>@@ -162,80 +199,80 @@ private:
> };
> 
> #if !ASSERT_DISABLED
>-inline void Box::invalidatePosition()
>+inline void Box::Rect::invalidatePosition()
> {
>     invalidateTop();
>     invalidateLeft();
> }
> 
>-inline void Box::setHasValidPosition()
>+inline void Box::Rect::setHasValidPosition()
> {
>     m_hasValidTop = true;
>     m_hasValidLeft = true;
> }
> #endif
> 
>-inline LayoutRect Box::rect() const
>+inline Box::Rect::Rect(const LayoutPoint& topLeft, const LayoutSize& size)
>+    : m_rect(topLeft, size)
> {
>-    ASSERT(hasValidGeometry());
>-    return m_rect;
>+
> }
> 
>-inline LayoutUnit Box::top() const
>+inline LayoutUnit Box::Rect::top() const
> {
>     ASSERT(m_hasValidTop);
>     return m_rect.y();
> }
> 
>-inline LayoutUnit Box::left() const
>+inline LayoutUnit Box::Rect::left() const
> {
>     ASSERT(m_hasValidLeft);
>     return m_rect.x();
> }
> 
>-inline LayoutUnit Box::bottom() const
>+inline LayoutUnit Box::Rect::bottom() const
> {
>     ASSERT(m_hasValidTop && m_hasValidHeight);
>     return m_rect.maxY();
> }
> 
>-inline LayoutUnit Box::right() const
>+inline LayoutUnit Box::Rect::right() const
> {
>     ASSERT(m_hasValidLeft && m_hasValidWidth);
>     return m_rect.maxX();
> }
> 
>-inline LayoutPoint Box::topLeft() const
>+inline LayoutPoint Box::Rect::topLeft() const
> {
>     ASSERT(hasValidPosition());
>-    return m_rect.location();
>+    return m_rect.minXMinYCorner();
> }
> 
>-inline LayoutPoint Box::bottomRight() const
>+inline LayoutPoint Box::Rect::bottomRight() const
> {
>     ASSERT(hasValidGeometry());
>     return m_rect.maxXMaxYCorner();
> }
> 
>-inline LayoutSize Box::size() const
>+inline LayoutSize Box::Rect::size() const
> {
>     ASSERT(hasValidSize());
>     return m_rect.size();
> }
> 
>-inline LayoutUnit Box::width() const
>+inline LayoutUnit Box::Rect::width() const
> {
>     ASSERT(m_hasValidWidth);
>     return m_rect.width();
> }
> 
>-inline LayoutUnit Box::height() const
>+inline LayoutUnit Box::Rect::height() const
> {
>     ASSERT(m_hasValidHeight);
>     return m_rect.height();
> }
> 
>-inline void Box::setTopLeft(const LayoutPoint& topLeft)
>+inline void Box::Rect::setTopLeft(const LayoutPoint& topLeft)
> {
> #if !ASSERT_DISABLED
>     setHasValidPosition();
>@@ -243,7 +280,7 @@ inline void Box::setTopLeft(const LayoutPoint& topLeft)
>     m_rect.setLocation(topLeft);
> }
> 
>-inline void Box::setTop(LayoutUnit top)
>+inline void Box::Rect::setTop(LayoutUnit top)
> {
> #if !ASSERT_DISABLED
>     m_hasValidTop = true;
>@@ -251,7 +288,7 @@ inline void Box::setTop(LayoutUnit top)
>     m_rect.setY(top);
> }
> 
>-inline void Box::setLeft(LayoutUnit left)
>+inline void Box::Rect::setLeft(LayoutUnit left)
> {
> #if !ASSERT_DISABLED
>     m_hasValidLeft = true;
>@@ -259,7 +296,7 @@ inline void Box::setLeft(LayoutUnit left)
>     m_rect.setX(left);
> }
> 
>-inline void Box::setWidth(LayoutUnit width)
>+inline void Box::Rect::setWidth(LayoutUnit width)
> {
> #if !ASSERT_DISABLED
>     m_hasValidWidth = true;
>@@ -268,7 +305,7 @@ inline void Box::setWidth(LayoutUnit width)
>     m_rect.setWidth(width);
> }
> 
>-inline void Box::setHeight(LayoutUnit height)
>+inline void Box::Rect::setHeight(LayoutUnit height)
> {
> #if !ASSERT_DISABLED
>     m_hasValidHeight = true;
>@@ -277,6 +314,36 @@ inline void Box::setHeight(LayoutUnit height)
>     m_rect.setHeight(height);
> }
> 
>+inline void Box::Rect::shiftLeftTo(LayoutUnit left)
>+{
>+    ASSERT(m_hasValidLeft);
>+    m_rect.shiftXEdgeTo(left);
>+}
>+
>+inline void Box::Rect::shiftRightTo(LayoutUnit right)
>+{
>+    ASSERT(m_hasValidLeft && m_hasValidWidth);
>+    m_rect.shiftMaxXEdgeTo(right);
>+}
>+
>+inline void Box::Rect::shiftTopTo(LayoutUnit top)
>+{
>+    ASSERT(m_hasValidTop);
>+    m_rect.shiftYEdgeTo(top);
>+}
>+
>+inline void Box::Rect::shiftBottomTo(LayoutUnit bottom)
>+{
>+    ASSERT(m_hasValidTop && m_hasValidHeight);
>+    m_rect.shiftMaxYEdgeTo(bottom);
>+}
>+
>+inline void Box::Rect::expand(LayoutUnit width, LayoutUnit height)
>+{
>+    ASSERT(hasValidGeometry());
>+    m_rect.expand(width, height);
>+}
>+
> inline void Box::setMargin(Edges margin)
> {
> #if !ASSERT_DISABLED

Actions: View | Formatted Diff | Diff

Attachments on bug 186019: 341423