[webkit-changes] [239899] trunk/Source/WebCore

Sat, 12 Jan 2019 07:32:54 -0800

Title: [239899] trunk/Source/WebCore
Revision
239899
Author
[email protected]
Date
2019-01-12 07:30:55 -0800 (Sat, 12 Jan 2019)

Log Message

[LFC][BFC][MarginCollapsing] Move estimatedMarginBefore flag from state/display box to BlockFormattingContext
https://bugs.webkit.org/show_bug.cgi?id=193375

Reviewed by Antti Koivisto.

The estimated marginBefore is a pre-computed, temporary value. We need to keep it around until the final vertical margin value is computed.
Neither BlockFormattingState nor Display should hold temporary values.

* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBefore const):
(WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBeforeForAncestors const):
(WebCore::Layout::BlockFormattingContext::hasPrecomputedMarginBefore const):
(WebCore::Layout::BlockFormattingContext::computeFloatingPosition const):
(WebCore::Layout::BlockFormattingContext::computePositionToAvoidFloats const):
(WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const):
(WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const):
(WebCore::Layout::BlockFormattingContext::setEstimatedMarginBefore const):
(WebCore::Layout::BlockFormattingContext::hasEstimatedMarginBefore const):
(WebCore::Layout::hasPrecomputedMarginBefore): Deleted.
* layout/blockformatting/BlockFormattingContext.h:
(WebCore::Layout::BlockFormattingContext::removeEstimatedMarginBefore const):
(WebCore::Layout::BlockFormattingContext::estimatedMarginBefore const):
* layout/blockformatting/BlockFormattingState.h:
(WebCore::Layout::BlockFormattingState::setHasEstimatedMarginBefore): Deleted.
(WebCore::Layout::BlockFormattingState::clearHasEstimatedMarginBefore): Deleted.
(WebCore::Layout::BlockFormattingState::hasEstimatedMarginBefore const): Deleted.
* layout/displaytree/DisplayBox.cpp:
(WebCore::Display::Box::Box):
* layout/displaytree/DisplayBox.h:
(WebCore::Display::Box::setHasEstimatedMarginBefore):
(WebCore::Display::Box::invalidateEstimatedMarginBefore):
(WebCore::Display::Box::top const):
(WebCore::Display::Box::topLeft const):
(WebCore::Display::Box::setEstimatedMarginBefore): Deleted.
(WebCore::Display::Box::estimatedMarginBefore const): Deleted.
* page/FrameViewLayoutContext.cpp:
(WebCore::layoutUsingFormattingContext):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (239898 => 239899)


--- trunk/Source/WebCore/ChangeLog	2019-01-12 09:49:17 UTC (rev 239898)
+++ trunk/Source/WebCore/ChangeLog	2019-01-12 15:30:55 UTC (rev 239899)
@@ -1,3 +1,43 @@
+2019-01-12  Zalan Bujtas  <[email protected]>
+
+        [LFC][BFC][MarginCollapsing] Move estimatedMarginBefore flag from state/display box to BlockFormattingContext
+        https://bugs.webkit.org/show_bug.cgi?id=193375
+
+        Reviewed by Antti Koivisto.
+
+        The estimated marginBefore is a pre-computed, temporary value. We need to keep it around until the final vertical margin value is computed.
+        Neither BlockFormattingState nor Display should hold temporary values.
+
+        * layout/blockformatting/BlockFormattingContext.cpp:
+        (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBefore const):
+        (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBeforeForAncestors const):
+        (WebCore::Layout::BlockFormattingContext::hasPrecomputedMarginBefore const):
+        (WebCore::Layout::BlockFormattingContext::computeFloatingPosition const):
+        (WebCore::Layout::BlockFormattingContext::computePositionToAvoidFloats const):
+        (WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const):
+        (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const):
+        (WebCore::Layout::BlockFormattingContext::setEstimatedMarginBefore const):
+        (WebCore::Layout::BlockFormattingContext::hasEstimatedMarginBefore const):
+        (WebCore::Layout::hasPrecomputedMarginBefore): Deleted.
+        * layout/blockformatting/BlockFormattingContext.h:
+        (WebCore::Layout::BlockFormattingContext::removeEstimatedMarginBefore const):
+        (WebCore::Layout::BlockFormattingContext::estimatedMarginBefore const):
+        * layout/blockformatting/BlockFormattingState.h:
+        (WebCore::Layout::BlockFormattingState::setHasEstimatedMarginBefore): Deleted.
+        (WebCore::Layout::BlockFormattingState::clearHasEstimatedMarginBefore): Deleted.
+        (WebCore::Layout::BlockFormattingState::hasEstimatedMarginBefore const): Deleted.
+        * layout/displaytree/DisplayBox.cpp:
+        (WebCore::Display::Box::Box):
+        * layout/displaytree/DisplayBox.h:
+        (WebCore::Display::Box::setHasEstimatedMarginBefore):
+        (WebCore::Display::Box::invalidateEstimatedMarginBefore):
+        (WebCore::Display::Box::top const):
+        (WebCore::Display::Box::topLeft const):
+        (WebCore::Display::Box::setEstimatedMarginBefore): Deleted.
+        (WebCore::Display::Box::estimatedMarginBefore const): Deleted.
+        * page/FrameViewLayoutContext.cpp:
+        (WebCore::layoutUsingFormattingContext):
+
 2019-01-11  Myles C. Maxfield  <[email protected]>
 
         [WHLSL] Implement the NameResolver

Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp (239898 => 239899)


--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp	2019-01-12 09:49:17 UTC (rev 239898)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp	2019-01-12 15:30:55 UTC (rev 239899)
@@ -188,7 +188,7 @@
 {
     auto& layoutState = this->layoutState();
     auto estimatedMarginBefore = MarginCollapse::estimatedMarginBefore(layoutState, layoutBox);
-    blockFormattingState().setHasEstimatedMarginBefore(layoutBox);
+    setEstimatedMarginBefore(layoutBox, estimatedMarginBefore);
 
     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
     auto nonCollapsedValues = UsedVerticalMargin::NonCollapsedValues { estimatedMarginBefore.nonCollapsedValue, { } };
@@ -196,7 +196,7 @@
     auto verticalMargin = UsedVerticalMargin { nonCollapsedValues, collapsedValues };
     displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
 #if !ASSERT_DISABLED
-    displayBox.setEstimatedMarginBefore(estimatedMarginBefore);
+    displayBox.setHasEstimatedMarginBefore();
 #endif
 }
 
@@ -214,10 +214,9 @@
     // So when we get to the point where we intersect the box with the float to decide if the box needs to move, we don't yet have the final vertical position.
     //
     // The idea here is that as long as we don't cross the block formatting context boundary, we should be able to pre-compute the final top margin.
-    auto& formattingState = blockFormattingState();
     for (auto* ancestor = layoutBox.containingBlock(); ancestor && !ancestor->establishesBlockFormattingContext(); ancestor = ancestor->containingBlock()) {
         // FIXME: with incremental layout, we might actually have a valid (non-estimated) margin top as well.
-        if (formattingState.hasEstimatedMarginBefore(*ancestor))
+        if (hasEstimatedMarginBefore(*ancestor))
             return;
 
         computeEstimatedMarginBefore(*ancestor);
@@ -242,10 +241,10 @@
 }
 
 #ifndef NDEBUG
-static bool hasPrecomputedMarginBefore(const BlockFormattingState& formattingState, const Box& layoutBox)
+bool BlockFormattingContext::hasPrecomputedMarginBefore(const Box& layoutBox) const
 {
     for (auto* ancestor = layoutBox.containingBlock(); ancestor && !ancestor->establishesBlockFormattingContext(); ancestor = ancestor->containingBlock()) {
-        if (formattingState.hasEstimatedMarginBefore(*ancestor))
+        if (hasEstimatedMarginBefore(*ancestor))
             continue;
         return false;
     }
@@ -257,7 +256,7 @@
 {
     auto& layoutState = this->layoutState();
     ASSERT(layoutBox.isFloatingPositioned());
-    ASSERT(hasPrecomputedMarginBefore(blockFormattingState(), layoutBox));
+    ASSERT(hasPrecomputedMarginBefore(layoutBox));
 
     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
     // 8.3.1 Collapsing margins
@@ -277,7 +276,7 @@
     ASSERT(layoutBox.establishesBlockFormattingContext());
     ASSERT(!layoutBox.isFloatingPositioned());
     ASSERT(!layoutBox.hasFloatClear());
-    ASSERT(hasPrecomputedMarginBefore(blockFormattingState(), layoutBox));
+    ASSERT(hasPrecomputedMarginBefore(layoutBox));
 
     if (floatingContext.floatingState().isEmpty())
         return;
@@ -296,7 +295,7 @@
     // For formatting roots, we already precomputed final position.
     if (!layoutBox.establishesFormattingContext())
         computeEstimatedMarginBeforeForAncestors(layoutBox);
-    ASSERT(hasPrecomputedMarginBefore(blockFormattingState(), layoutBox));
+    ASSERT(hasPrecomputedMarginBefore(layoutBox));
 
     if (auto verticalPositionWithClearance = floatingContext.verticalPositionWithClearance(layoutBox))
         layoutState.displayBoxForLayoutBox(layoutBox).setTop(*verticalPositionWithClearance);
@@ -384,18 +383,15 @@
 
     // Out of flow boxes don't need vertical adjustment after margin collapsing.
     if (layoutBox.isOutOfFlowPositioned()) {
+        ASSERT(!hasEstimatedMarginBefore(layoutBox));
         displayBox.setContentBoxHeight(heightAndMargin.height);
         displayBox.setVerticalMargin(verticalMargin);
         return;
     }
 
-    auto& formattingState = this->blockFormattingState();
-    if (formattingState.hasEstimatedMarginBefore(layoutBox)) {
-        formattingState.clearHasEstimatedMarginBefore(layoutBox);
-        ASSERT(displayBox.estimatedMarginBefore()->usedValue() == verticalMargin.before());
-    } else
-        displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
-
+    ASSERT(!hasEstimatedMarginBefore(layoutBox) || estimatedMarginBefore(layoutBox).usedValue() == verticalMargin.before());
+    removeEstimatedMarginBefore(layoutBox);
+    displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
     // Adjust the previous sibling's margin bottom now that this box's vertical margin is computed.
     if (MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox))
         MarginCollapse::updateCollapsedMarginAfter(layoutState, *layoutBox.previousInFlowSibling(), verticalMargin);
@@ -524,7 +520,21 @@
     return containingBlockContentBoxTop + verticalMargin.before();
 }
 
+void BlockFormattingContext::setEstimatedMarginBefore(const Box& layoutBox, const EstimatedMarginBefore& estimatedMarginBefore) const
+{
+    // Can't cross formatting context boundary.
+    ASSERT(&layoutState().formattingStateForBox(layoutBox) == &formattingState());
+    m_estimatedMarginBeforeList.set(&layoutBox, estimatedMarginBefore);
 }
+
+bool BlockFormattingContext::hasEstimatedMarginBefore(const Box& layoutBox) const
+{
+    // Can't cross formatting context boundary.
+    ASSERT(&layoutState().formattingStateForBox(layoutBox) == &formattingState());
+    return m_estimatedMarginBeforeList.contains(&layoutBox);
 }
 
+}
+}
+
 #endif

Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h (239898 => 239899)


--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h	2019-01-12 09:49:17 UTC (rev 239898)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h	2019-01-12 15:30:55 UTC (rev 239899)
@@ -28,6 +28,8 @@
 #if ENABLE(LAYOUT_FORMATTING_CONTEXT)
 
 #include "FormattingContext.h"
+#include "MarginTypes.h"
+#include <wtf/HashMap.h>
 #include <wtf/IsoMalloc.h>
 
 namespace WebCore {
@@ -126,6 +128,17 @@
         static bool shouldIgnoreMarginBefore(const LayoutState&, const Box&);
         static bool shouldIgnoreMarginAfter(const LayoutState&, const Box&);
     };
+
+    void setEstimatedMarginBefore(const Box&, const EstimatedMarginBefore&) const;
+    void removeEstimatedMarginBefore(const Box& layoutBox) const { m_estimatedMarginBeforeList.remove(&layoutBox); }
+    bool hasEstimatedMarginBefore(const Box&) const;
+#ifndef NDEBUG
+    EstimatedMarginBefore estimatedMarginBefore(const Box& layoutBox) const { return m_estimatedMarginBeforeList.get(&layoutBox); }
+    bool hasPrecomputedMarginBefore(const Box&) const;
+#endif
+
+private:
+    mutable HashMap<const Box*, EstimatedMarginBefore> m_estimatedMarginBeforeList;
 };
 
 }

Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingState.h (239898 => 239899)


--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingState.h	2019-01-12 09:49:17 UTC (rev 239898)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingState.h	2019-01-12 15:30:55 UTC (rev 239899)
@@ -28,7 +28,6 @@
 #if ENABLE(LAYOUT_FORMATTING_CONTEXT)
 
 #include "FormattingState.h"
-#include "MarginTypes.h"
 #include <wtf/IsoMalloc.h>
 
 namespace WebCore {
@@ -48,13 +47,8 @@
     bool hasPositiveAndNegativeVerticalMargin(const Box& layoutBox) const { return m_positiveAndNegativeVerticalMargin.contains(&layoutBox); }
     PositiveAndNegativeVerticalMargin positiveAndNegativeVerticalMargin(const Box& layoutBox) const { return m_positiveAndNegativeVerticalMargin.get(&layoutBox); }
 
-    void setHasEstimatedMarginBefore(const Box& layoutBox) { m_estimatedMarginBeforeList.add(&layoutBox); }
-    void clearHasEstimatedMarginBefore(const Box& layoutBox) { m_estimatedMarginBeforeList.remove(&layoutBox); }
-    bool hasEstimatedMarginBefore(const Box& layoutBox) const { return m_estimatedMarginBeforeList.contains(&layoutBox); }
-
 private:
     HashMap<const Box*, PositiveAndNegativeVerticalMargin> m_positiveAndNegativeVerticalMargin;
-    HashSet<const Box*> m_estimatedMarginBeforeList;
 };
 
 }

Modified: trunk/Source/WebCore/layout/displaytree/DisplayBox.cpp (239898 => 239899)


--- trunk/Source/WebCore/layout/displaytree/DisplayBox.cpp	2019-01-12 09:49:17 UTC (rev 239898)
+++ trunk/Source/WebCore/layout/displaytree/DisplayBox.cpp	2019-01-12 15:30:55 UTC (rev 239899)
@@ -74,7 +74,7 @@
     , m_hasValidPadding(other.m_hasValidPadding)
     , m_hasValidContentHeight(other.m_hasValidContentHeight)
     , m_hasValidContentWidth(other.m_hasValidContentWidth)
-    , m_estimatedMarginBefore(other.m_estimatedMarginBefore)
+    , m_hasEstimatedMarginBefore(other.m_hasEstimatedMarginBefore)
 #endif
 {
 }

Modified: trunk/Source/WebCore/layout/displaytree/DisplayBox.h (239898 => 239899)


--- trunk/Source/WebCore/layout/displaytree/DisplayBox.h	2019-01-12 09:49:17 UTC (rev 239898)
+++ trunk/Source/WebCore/layout/displaytree/DisplayBox.h	2019-01-12 15:30:55 UTC (rev 239899)
@@ -175,8 +175,7 @@
     Rect contentBox() const;
 
 #if !ASSERT_DISABLED
-    void setEstimatedMarginBefore(Layout::EstimatedMarginBefore marginBefore) { m_estimatedMarginBefore = marginBefore; }
-    Optional<Layout::EstimatedMarginBefore> estimatedMarginBefore() const { return m_estimatedMarginBefore; }
+    void setHasEstimatedMarginBefore() { m_hasEstimatedMarginBefore = true; }
 #endif
 
 private:
@@ -207,7 +206,7 @@
     void invalidateMargin();
     void invalidateBorder() { m_hasValidBorder = false; }
     void invalidatePadding() { m_hasValidPadding = false; }
-    void invalidateEstimatedMarginBefore() { m_estimatedMarginBefore = { }; }
+    void invalidateEstimatedMarginBefore() { m_hasEstimatedMarginBefore = false; }
 
     void setHasValidTop() { m_hasValidTop = true; }
     void setHasValidLeft() { m_hasValidLeft = true; }
@@ -248,7 +247,7 @@
     bool m_hasValidPadding { false };
     bool m_hasValidContentHeight { false };
     bool m_hasValidContentWidth { false };
-    Optional<Layout::EstimatedMarginBefore> m_estimatedMarginBefore;
+    bool m_hasEstimatedMarginBefore { false };
 #endif
 };
 
@@ -443,7 +442,7 @@
 
 inline LayoutUnit Box::top() const
 {
-    ASSERT(m_hasValidTop && (m_estimatedMarginBefore || m_hasValidVerticalMargin));
+    ASSERT(m_hasValidTop && (m_hasEstimatedMarginBefore || m_hasValidVerticalMargin));
     return m_topLeft.y();
 }
 
@@ -455,7 +454,7 @@
 
 inline LayoutPoint Box::topLeft() const
 {
-    ASSERT(m_hasValidTop && (m_estimatedMarginBefore || m_hasValidVerticalMargin));
+    ASSERT(m_hasValidTop && (m_hasEstimatedMarginBefore || m_hasValidVerticalMargin));
     ASSERT(m_hasValidLeft && m_hasValidHorizontalMargin);
     return m_topLeft;
 }

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to