Changeset 252716 in webkit

Timestamp:

Nov 20, 2019, 3:52:40 PM (7 years ago)

Author:

Alan Bujtas

Message:

Flexbox sizing logic triggers full repaint on the flex items.
​https://bugs.webkit.org/show_bug.cgi?id=204380
<rdar://problem/57236404>

Reviewed by Simon Fraser.

Source/WebCore:

RenderFlexibleBox::applyStretchAlignmentToChild explicitly sets the child renderer's height to 0 before issuing layout on it.
This confuses the child's repaint logic and could trigger unnecessary repaints on complete subtrees.

Many ::layout functions plant a LayoutRepainter stack object to track and report paint invalidations.
It works as long as the renderer's geometry change happens within the scope of this LayoutRepainter.
When the parent (RenderFlexibleBox) mutates the renderer's geometry, the LayoutRepainter object sees
this already mutated state as the initial state and will happily issue repaints even when the final
geometry remains the same.

This patch addresses the redundant repaint by pushing the height reset from the parent down to the child, inside the LayoutRepainter scope.

• rendering/RenderBlock.h: Restrict it to RenderBlock level for now. It might need to go all the way up to RenderBox.

(WebCore::RenderBlock::shouldResetChildLogicalHeightBeforeLayout const):

• rendering/RenderBlockFlow.cpp:

(WebCore::RenderBlockFlow::layoutBlock):

• rendering/RenderBlockFlow.h:
• rendering/RenderBox.cpp:

(WebCore::RenderBox::resetLogicalHeightBeforeLayoutIfNeeded):

• rendering/RenderBox.h:

(WebCore::RenderBox::shouldResetLogicalHeightBeforeLayout const):

• rendering/RenderDeprecatedFlexibleBox.cpp:

(WebCore::RenderDeprecatedFlexibleBox::layoutBlock):

• rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::layoutBlock):
(WebCore::RenderFlexibleBox::applyStretchAlignmentToChild):

• rendering/RenderFlexibleBox.h:
• rendering/RenderGrid.cpp:

(WebCore::RenderGrid::layoutBlock):

• rendering/RenderGrid.h:
• rendering/RenderTable.cpp:

(WebCore::RenderTable::layout):

• rendering/RenderTable.h:

LayoutTests:

• fast/repaint/align-items-change-expected.txt: progression.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/repaint/align-items-change-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.h (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockFlow.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockFlow.h (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.h (modified) (1 diff)
• Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.h (modified) (1 diff)
• Source/WebCore/rendering/RenderTable.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderTable.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-11-20  Zalan Bujtas  <zalan@apple.com>
+
+        Flexbox sizing logic triggers full repaint on the flex items.
+        https://bugs.webkit.org/show_bug.cgi?id=204380
+        <rdar://problem/57236404>
+
+        Reviewed by Simon Fraser.
+
+        * fast/repaint/align-items-change-expected.txt: progression.
+
 2019-11-20  Justin Fan  <justin_fan@apple.com>
 

trunk/LayoutTests/fast/repaint/align-items-change-expected.txt

@@ -2 +2 @@
 
 (repaint rects
-  (rect 0 52 100 300)
-  (rect 0 51 100 1)
-  (rect 100 52 100 300)
-  (rect 100 51 100 1)
+  (rect 0 154 100 198)
+  (rect 0 153 100 1)
+  (rect 100 204 100 148)
+  (rect 100 203 100 1)
   (rect 0 52 200 300)
 )

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-11-20  Zalan Bujtas  <zalan@apple.com>
+
+        Flexbox sizing logic triggers full repaint on the flex items.
+        https://bugs.webkit.org/show_bug.cgi?id=204380
+        <rdar://problem/57236404>
+
+        Reviewed by Simon Fraser.
+
+        RenderFlexibleBox::applyStretchAlignmentToChild explicitly sets the child renderer's height to 0 before issuing layout on it.
+        This confuses the child's repaint logic and could trigger unnecessary repaints on complete subtrees.
+
+        Many ::layout functions plant a LayoutRepainter stack object to track and report paint invalidations.
+        It works as long as the renderer's geometry change happens within the scope of this LayoutRepainter.
+        When the parent (RenderFlexibleBox) mutates the renderer's geometry, the LayoutRepainter object sees
+        this already mutated state as the initial state and will happily issue repaints even when the final
+        geometry remains the same.
+
+        This patch addresses the redundant repaint by pushing the height reset from the parent down to the child, inside the LayoutRepainter scope.
+
+        * rendering/RenderBlock.h: Restrict it to RenderBlock level for now. It might need to go all the way up to RenderBox.
+        (WebCore::RenderBlock::shouldResetChildLogicalHeightBeforeLayout const):
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::layoutBlock):
+        * rendering/RenderBlockFlow.h:
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::resetLogicalHeightBeforeLayoutIfNeeded):
+        * rendering/RenderBox.h:
+        (WebCore::RenderBox::shouldResetLogicalHeightBeforeLayout const):
+        * rendering/RenderDeprecatedFlexibleBox.cpp:
+        (WebCore::RenderDeprecatedFlexibleBox::layoutBlock):
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutBlock):
+        (WebCore::RenderFlexibleBox::applyStretchAlignmentToChild):
+        * rendering/RenderFlexibleBox.h:
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::layoutBlock):
+        * rendering/RenderGrid.h:
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::layout):
+        * rendering/RenderTable.h:
+
 2019-11-20  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/rendering/RenderBlock.h

@@ -316 +316 @@
     Optional<LayoutUnit> availableLogicalHeightForPercentageComputation() const;
     bool hasDefiniteLogicalHeight() const;
-    
+
+    virtual bool shouldResetChildLogicalHeightBeforeLayout(const RenderBox&) const { return false; }
+
 protected:
     RenderFragmentedFlow* locateEnclosingFragmentedFlow() const override;

trunk/Source/WebCore/rendering/RenderBlockFlow.cpp

@@ -473 +473 @@
     // FIXME: should this start out as borderAndPaddingLogicalHeight() + scrollbarLogicalHeight(),
     // for consistency with other render classes?
-    setLogicalHeight(0);
+    resetLogicalHeightBeforeLayoutIfNeeded();
 
     bool pageLogicalHeightChanged = false;

trunk/Source/WebCore/rendering/RenderBlockFlow.h

@@ -392 +392 @@
 
 protected:
+    bool shouldResetLogicalHeightBeforeLayout() const override { return true; }
+
     void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
     

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -568 +568 @@
 }
 
+void RenderBox::resetLogicalHeightBeforeLayoutIfNeeded()
+{
+    if (shouldResetLogicalHeightBeforeLayout() || (is<RenderBlock>(parent()) && downcast<RenderBlock>(*parent()).shouldResetChildLogicalHeightBeforeLayout(*this)))
+        setLogicalHeight(0_lu);
+}
+
 static void setupWheelEventMonitor(RenderLayer& layer)
 {

trunk/Source/WebCore/rendering/RenderBox.h

@@ -655 +655 @@
     bool createsNewFormattingContext() const;
 
+    virtual bool shouldResetLogicalHeightBeforeLayout() const { return false; }
+    void resetLogicalHeightBeforeLayoutIfNeeded();
+
     virtual ItemPosition selfAlignmentNormalBehavior(const RenderBox* = nullptr) const { return ItemPosition::Stretch; }
 

trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp

@@ -286 +286 @@
         LayoutStateMaintainer statePusher(*this, locationOffset(), hasTransform() || hasReflection() || style().isFlippedBlocksWritingMode());
 
+        resetLogicalHeightBeforeLayoutIfNeeded();
         preparePaginationBeforeBlockLayout(relayoutChildren);
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -263 +263 @@
     LayoutRepainter repainter(*this, checkForRepaintDuringLayout());
 
+    resetLogicalHeightBeforeLayoutIfNeeded();
     m_relaidOutChildren.clear();
     
@@ -1840 +1841 @@
             child.setOverrideContentLogicalHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight());
         if (childNeedsRelayout) {
-            child.setLogicalHeight(0_lu);
+            SetForScope<bool> resetChildLogicalHeight(m_shouldResetChildLogicalHeightBeforeLayout, true);
             // We cache the child's intrinsic content logical height to avoid it being
             // reset to the stretched height.
-            // FIXME: This is fragile. RendertBoxes should be smart enough to
+            // FIXME: This is fragile. RenderBoxes should be smart enough to
             // determine their intrinsic content logical height correctly even when
             // there's an overrideHeight.

trunk/Source/WebCore/rendering/RenderFlexibleBox.h

@@ -92 +92 @@
     void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
     void computePreferredLogicalWidths() override;
+
+    bool shouldResetChildLogicalHeightBeforeLayout(const RenderBox&) const override { return m_shouldResetChildLogicalHeightBeforeLayout; }
 
 private:
@@ -212 +214 @@
     mutable SizeDefiniteness m_hasDefiniteHeight { SizeDefiniteness::Unknown };
     bool m_inLayout { false };
+    bool m_shouldResetChildLogicalHeightBeforeLayout { false };
 };
 

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -220 +220 @@
         }
         m_baselineItemsCached = true;
-        setLogicalHeight(0);
+        resetLogicalHeightBeforeLayoutIfNeeded();
         updateLogicalWidth();
 

trunk/Source/WebCore/rendering/RenderGrid.h

@@ -187 +187 @@
     LayoutUnit translateRTLCoordinate(LayoutUnit) const;
 
+    bool shouldResetLogicalHeightBeforeLayout() const override { return true; }
+
     Grid m_grid;
 

trunk/Source/WebCore/rendering/RenderTable.cpp

@@ -434 +434 @@
         LayoutUnit oldLogicalWidth = logicalWidth();
         LayoutUnit oldLogicalHeight = logicalHeight();
-        setLogicalHeight(0);
+        resetLogicalHeightBeforeLayoutIfNeeded();
         updateLogicalWidth();
 

trunk/Source/WebCore/rendering/RenderTable.h

@@ -362 +362 @@
     }
 
+    bool shouldResetLogicalHeightBeforeLayout() const override { return true; }
+
     LayoutUnit m_hSpacing;
     LayoutUnit m_vSpacing;