Attachment 356130 Details for Bug 183040 – Patch for landing

[patch] Patch for landing

bug-183040-20181130064511.patch (text/plain), 19.07 KB, created by Frédéric Wang (:fredw) on 2018-11-29 21:45:13 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Frédéric Wang (:fredw)

Created: 2018-11-29 21:45:13 PST

Size: 19.07 KB

patch

obsolete

>Subversion Revision: 238725
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b905292e27deb41144513652c4833266122fe0c9..f3ca220be3f930eac7f9a58912684a33db482b2f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2018-11-29  Frederic Wang  <fwang@igalia.com>
>+
>+        Separate paint and scroll offsets for RenderLayerBacking::m_scrollingContentsLayer
>+        https://bugs.webkit.org/show_bug.cgi?id=183040
>+
>+        Currently, scroll offset of RenderLayerBacking::m_scrollingContentsLayer is stored in the
>+        GraphicsLayer::m_offsetFromRenderer member used for paint offset. This patch separates these
>+        two concept by introducing a new GraphicsLayer::m_scrollOffset for the scroll offset. This
>+        makes the API a little bit cleaner, the code easier to understand and might avoid unnecessary
>+        repaints in composited scroll.
>+
>+        Reviewed by Simon Fraser.
>+
>+        No new tests, already covered by existing tests.
>+
>+        * platform/graphics/GraphicsLayer.cpp:
>+        (WebCore::GraphicsLayer::setScrollOffset): Setter function to update the scroll offset
>+        of the content layer inside its scrolling parent layer. Ask a repaint if it has changed and
>+        is requested by the caller.
>+        (WebCore::GraphicsLayer::paintGraphicsLayerContents): Take into account the scroll offset
>+        when painting.
>+        (WebCore::GraphicsLayer::dumpProperties const): Dump the scroll offset property.
>+        * platform/graphics/GraphicsLayer.h: Include ScrollableArea for the ScrollOffset typedef.
>+        Add member for the scroll offset of the content layer inside its scrolling parent layer.
>+        (WebCore::GraphicsLayer::scrollOffset const): Getter function.
>+        * rendering/RenderLayerBacking.cpp:
>+        (WebCore::RenderLayerBacking::updateGeometry): Do not include the scroll offset in the
>+        paint offset of m_scrollingContentsLayer since it is now taken into account in
>+        paintGraphicsLayerContents. Update the scroll offset of m_scrollingContentsLayer separately.
>+        Leave the paint offset of m_foregroundLayer unchanged.
>+        (WebCore::RenderLayerBacking::setContentsNeedDisplayInRect): Take into account the scroll
>+        offset of m_scrollingContentsLayer when calculating the dirty rect.
>+
> 2018-11-29  Simon Fraser  <simon.fraser@apple.com>
> 
>         Overflow scrolling layers need to be self-painting
>diff --git a/Source/WebCore/platform/graphics/GraphicsLayer.cpp b/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>index 7d8c0e2f1c913b6d976c3325eab666deddb01746..885cd2e0de0c2046d8c20489e984c76b7a9d0de9 100644
>--- a/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>+++ b/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>@@ -456,6 +456,18 @@ void GraphicsLayer::setOffsetFromRenderer(const FloatSize& offset, ShouldSetNeed
>         setNeedsDisplay();
> }
> 
>+void GraphicsLayer::setScrollOffset(const ScrollOffset& offset, ShouldSetNeedsDisplay shouldSetNeedsDisplay)
>+{
>+    if (offset == m_scrollOffset)
>+        return;
>+
>+    m_scrollOffset = offset;
>+
>+    // If the compositing layer offset changes, we need to repaint.
>+    if (shouldSetNeedsDisplay == SetNeedsDisplay)
>+        setNeedsDisplay();
>+}
>+
> void GraphicsLayer::setSize(const FloatSize& size)
> {
>     if (size == m_size)
>@@ -474,7 +486,7 @@ void GraphicsLayer::setBackgroundColor(const Color& color)
> 
> void GraphicsLayer::paintGraphicsLayerContents(GraphicsContext& context, const FloatRect& clip, GraphicsLayerPaintBehavior layerPaintBehavior)
> {
>-    FloatSize offset = offsetFromRenderer();
>+    FloatSize offset = offsetFromRenderer() - toFloatSize(scrollOffset());
>     context.translate(-offset);
> 
>     FloatRect clipRect(clip);
>@@ -780,6 +792,9 @@ void GraphicsLayer::dumpProperties(TextStream& ts, LayerTreeAsTextBehavior behav
>     if (!m_offsetFromRenderer.isZero())
>         ts << indent << "(offsetFromRenderer " << m_offsetFromRenderer << ")\n";
> 
>+    if (!m_scrollOffset.isZero())
>+        ts << indent << "(scrollOffset " << m_scrollOffset << ")\n";
>+
>     if (m_position != FloatPoint())
>         ts << indent << "(position " << m_position.x() << " " << m_position.y() << ")\n";
> 
>diff --git a/Source/WebCore/platform/graphics/GraphicsLayer.h b/Source/WebCore/platform/graphics/GraphicsLayer.h
>index fec08884c7a1d175d982b6fe14f46a16353eaf8e..31c727338caa7373c4576dd8778ae9d2f5d0e0c1 100644
>--- a/Source/WebCore/platform/graphics/GraphicsLayer.h
>+++ b/Source/WebCore/platform/graphics/GraphicsLayer.h
>@@ -35,6 +35,7 @@
> #include "GraphicsLayerClient.h"
> #include "Path.h"
> #include "PlatformLayer.h"
>+#include "ScrollableArea.h"
> #include "TransformOperations.h"
> #include "WindRule.h"
> #include <wtf/Function.h>
>@@ -313,6 +314,10 @@ public:
>     FloatSize offsetFromRenderer() const { return m_offsetFromRenderer; }
>     void setOffsetFromRenderer(const FloatSize&, ShouldSetNeedsDisplay = SetNeedsDisplay);
> 
>+    // Scroll offset of the content layer inside its scrolling parent layer.
>+    ScrollOffset scrollOffset() const { return m_scrollOffset; }
>+    void setScrollOffset(const ScrollOffset&, ShouldSetNeedsDisplay = SetNeedsDisplay);
>+
>     // The position of the layer (the location of its top-left corner in its parent)
>     const FloatPoint& position() const { return m_position; }
>     virtual void setPosition(const FloatPoint& p) { m_approximatePosition = std::nullopt; m_position = p; }
>@@ -651,6 +656,9 @@ protected:
>     // Offset from the owning renderer
>     FloatSize m_offsetFromRenderer;
>     
>+    // Scroll offset of the content layer inside its scrolling parent layer.
>+    ScrollOffset m_scrollOffset;
>+
>     // Position is relative to the parent GraphicsLayer
>     FloatPoint m_position;
> 
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index fb582ae396d7aa3686c9024d45fb1653bfe3930a..54984777de9bb673a92f840d66110e866501b4f0 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -1206,9 +1206,8 @@ void RenderLayerBacking::updateGeometry()
> 
>         m_scrollingContentsLayer->setSize(scrollSize);
>         // Scrolling the content layer does not need to trigger a repaint. The offset will be compensated away during painting.
>-        // FIXME: The paint offset and the scroll offset should really be separate concepts.
>-        LayoutSize scrollingContentsOffset = toLayoutSize(paddingBox.location() - toLayoutSize(scrollOffset));
>-        m_scrollingContentsLayer->setOffsetFromRenderer(scrollingContentsOffset, GraphicsLayer::DontSetNeedsDisplay);
>+        m_scrollingContentsLayer->setScrollOffset(scrollOffset, GraphicsLayer::DontSetNeedsDisplay);
>+        m_scrollingContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBox.location()), GraphicsLayer::DontSetNeedsDisplay);
> #else
>         m_scrollingContentsLayer->setPosition(-scrollOffset);
> 
>@@ -1227,18 +1226,17 @@ void RenderLayerBacking::updateGeometry()
>         if (scrollSize != m_scrollingContentsLayer->size() || paddingBoxOffsetChanged)
>             m_scrollingContentsLayer->setNeedsDisplay();
> 
>-        LayoutSize scrollingContentsOffset = toLayoutSize(paddingBox.location() - toLayoutSize(scrollOffset));
>-        if (scrollingContentsOffset != m_scrollingContentsLayer->offsetFromRenderer() || scrollSize != m_scrollingContentsLayer->size())
>+        if (toLayoutSize(paddingBox.location()) != m_scrollingContentsLayer->offsetFromRenderer() || scrollOffset != m_scrollingContentsLayer->scrollOffset() || scrollSize != m_scrollingContentsLayer->size())
>             compositor().scrollingLayerDidChange(m_owningLayer);
> 
>         m_scrollingContentsLayer->setSize(scrollSize);
>-        // FIXME: The paint offset and the scroll offset should really be separate concepts.
>-        m_scrollingContentsLayer->setOffsetFromRenderer(scrollingContentsOffset, GraphicsLayer::DontSetNeedsDisplay);
>+        m_scrollingContentsLayer->setScrollOffset(scrollOffset, GraphicsLayer::DontSetNeedsDisplay);
>+        m_scrollingContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBox.location()), GraphicsLayer::DontSetNeedsDisplay);
> #endif
> 
>         if (m_foregroundLayer) {
>             m_foregroundLayer->setSize(m_scrollingContentsLayer->size());
>-            m_foregroundLayer->setOffsetFromRenderer(m_scrollingContentsLayer->offsetFromRenderer());
>+            m_foregroundLayer->setOffsetFromRenderer(m_scrollingContentsLayer->offsetFromRenderer() - toLayoutSize(m_scrollingContentsLayer->scrollOffset()));
>         }
>     }
> 
>@@ -2509,7 +2507,7 @@ void RenderLayerBacking::setContentsNeedDisplayInRect(const LayoutRect& r, Graph
> 
>     if (m_scrollingContentsLayer && m_scrollingContentsLayer->drawsContent()) {
>         FloatRect layerDirtyRect = pixelSnappedRectForPainting;
>-        layerDirtyRect.move(-m_scrollingContentsLayer->offsetFromRenderer() - m_subpixelOffsetFromRenderer);
>+        layerDirtyRect.move(-m_scrollingContentsLayer->offsetFromRenderer() + toLayoutSize(m_scrollingContentsLayer->scrollOffset()) - m_subpixelOffsetFromRenderer);
> #if PLATFORM(IOS_FAMILY)
>         // Account for the fact that RenderLayerBacking::updateGeometry() bakes scrollOffset into offsetFromRenderer on iOS,
>         // but the repaint rect is computed without taking the scroll position into account (see shouldApplyClipAndScrollPositionForRepaint()).
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 5d288ac8ec165e856d76937a9577a72c6554b800..8a41c60d2ded46a9440ff52ee78036448d75bf9f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-11-29  Frederic Wang  <fwang@igalia.com>
>+
>+        Separate paint and scroll offsets for RenderLayerBacking::m_scrollingContentsLayer
>+        https://bugs.webkit.org/show_bug.cgi?id=183040
>+
>+        Reviewed by Simon Fraser.
>+
>+        Update expectations containing layer trees of RenderLayerBacking::m_scrollingContentsLayer, to separate offsetFromRenderer and
>+        scrollOffset. We have OLD offsetFromRenderer = NEW offsetFromRenderer - scrollOffset.
>+
>+        * compositing/ios/overflow-scroll-touch-tiles-expected.txt:
>+        * fast/scrolling/ios/overflow-scroll-touch-expected.txt:
>+        * fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt:
>+        * platform/ios/compositing/overflow/scrolling-without-painting-expected.txt:
>+        * platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt:
>+        * platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt:
>+        * platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt:
>+        * platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt:
>+        * platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt:
>+
> 2018-11-29  Simon Fraser  <simon.fraser@apple.com>
> 
>         Overflow scrolling layers need to be self-painting
>diff --git a/LayoutTests/compositing/ios/overflow-scroll-touch-tiles-expected.txt b/LayoutTests/compositing/ios/overflow-scroll-touch-tiles-expected.txt
>index 31732c68f6874e1ab33893aabd58b6c1d94ed033..068adce53c893712c50f778b289ee1b4a87d0816 100644
>--- a/LayoutTests/compositing/ios/overflow-scroll-touch-tiles-expected.txt
>+++ b/LayoutTests/compositing/ios/overflow-scroll-touch-tiles-expected.txt
>@@ -62,7 +62,8 @@
>               (bounds 400.00 300.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=2 height=-238)
>+                  (offsetFromRenderer width=2 height=2)
>+                  (scrollOffset (0,240))
>                   (bounds 400.00 2002.00)
>                   (usingTiledLayer 1)
>                   (drawsContent 1)
>diff --git a/LayoutTests/fast/scrolling/ios/overflow-scroll-touch-expected.txt b/LayoutTests/fast/scrolling/ios/overflow-scroll-touch-expected.txt
>index 9af741822a4ecf590785cf83cc4aaba61696e47e..765dc6f6569b5ce8e052503ee7b2b0e19394a34a 100644
>--- a/LayoutTests/fast/scrolling/ios/overflow-scroll-touch-expected.txt
>+++ b/LayoutTests/fast/scrolling/ios/overflow-scroll-touch-expected.txt
>@@ -21,7 +21,8 @@
>               (bounds 200.00 200.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-49)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,50))
>                   (bounds 200.00 400.00)
>                   (drawsContent 1)
>                 )
>@@ -41,7 +42,8 @@
>               (bounds 200.00 200.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-49)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,50))
>                   (bounds 200.00 400.00)
>                   (drawsContent 1)
>                 )
>diff --git a/LayoutTests/fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt b/LayoutTests/fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt
>index f612e2097067ddd41b1cf9b06cfa8c35e3e8fbd0..828928400c2b57ffaacd7735da85b33ca7cce50e 100644
>--- a/LayoutTests/fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt
>+++ b/LayoutTests/fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt
>@@ -15,7 +15,7 @@
>               (bounds 300.00 400.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=0 height=-30)
>+                  (scrollOffset (0,30))
>                   (bounds 300.00 900.00)
>                   (drawsContent 1)
>                   (children 1
>diff --git a/LayoutTests/platform/ios/compositing/overflow/scrolling-without-painting-expected.txt b/LayoutTests/platform/ios/compositing/overflow/scrolling-without-painting-expected.txt
>index b8773d20d74d2e49951f801db50dfc6289a90c0f..aefd8e79d3fe8929a8148e876482c04b28f6d843 100644
>--- a/LayoutTests/platform/ios/compositing/overflow/scrolling-without-painting-expected.txt
>+++ b/LayoutTests/platform/ios/compositing/overflow/scrolling-without-painting-expected.txt
>@@ -18,7 +18,8 @@
>               (bounds 200.00 200.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-24)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,25))
>                   (bounds 200.00 1025.00)
>                 )
>               )
>diff --git a/LayoutTests/platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt b/LayoutTests/platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt
>index 70b74c8f462aa000cc32544b83ce9172aa3c7ef8..a89c9927feba7646cb0a59a415ec98bbc393be88 100644
>--- a/LayoutTests/platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt
>+++ b/LayoutTests/platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt
>@@ -19,7 +19,8 @@
>               (bounds 204.00 124.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-49)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,50))
>                   (bounds 204.00 270.00)
>                   (drawsContent 1)
>                 )
>@@ -40,7 +41,8 @@
>               (bounds 204.00 124.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-49)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,50))
>                   (bounds 204.00 270.00)
>                   (drawsContent 1)
>                 )
>diff --git a/LayoutTests/platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt b/LayoutTests/platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt
>index 0c6acbb8a9ebb6e903b71c305c1763a945afa502..f6d8175091a18b5e6bfaf2c058542e8bfb4059dd 100644
>--- a/LayoutTests/platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt
>+++ b/LayoutTests/platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt
>@@ -19,7 +19,8 @@
>               (bounds 400.00 205.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=-364 height=2)
>+                  (offsetFromRenderer width=2 height=2)
>+                  (scrollOffset (366,0))
>                   (bounds 766.00 205.00)
>                   (drawsContent 1)
>                   (children 1
>diff --git a/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt b/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt
>index cefeb18874295666d7bf9881d365e207e661f53b..5c20344365c64939b80bd6c2fb735dd7efe301d5 100644
>--- a/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt
>+++ b/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt
>@@ -18,7 +18,8 @@
>               (bounds 300.00 400.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-29)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,30))
>                   (bounds 300.00 900.00)
>                   (drawsContent 1)
>                   (children 1
>diff --git a/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt b/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt
>index 2f2a3d216261e93332980be476d45ad520090b48..ff20cafe545b9724dcdd273b7bceb4e5e73221ec 100644
>--- a/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt
>+++ b/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt
>@@ -18,7 +18,8 @@
>               (bounds 300.00 400.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=11 height=-19)
>+                  (offsetFromRenderer width=11 height=11)
>+                  (scrollOffset (0,30))
>                   (bounds 300.00 900.00)
>                   (drawsContent 1)
>                   (children 1
>diff --git a/LayoutTests/platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt b/LayoutTests/platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt
>index 70b74c8f462aa000cc32544b83ce9172aa3c7ef8..a89c9927feba7646cb0a59a415ec98bbc393be88 100644
>--- a/LayoutTests/platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt
>+++ b/LayoutTests/platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt
>@@ -19,7 +19,8 @@
>               (bounds 204.00 124.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-49)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,50))
>                   (bounds 204.00 270.00)
>                   (drawsContent 1)
>                 )
>@@ -40,7 +41,8 @@
>               (bounds 204.00 124.00)
>               (children 1
>                 (GraphicsLayer
>-                  (offsetFromRenderer width=1 height=-49)
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,50))
>                   (bounds 204.00 270.00)
>                   (drawsContent 1)
>                 )

Actions: View | Formatted Diff | Diff

Attachments on bug 183040: 334454 | 340071 | 348912 | 354929 | 354943 | 354956 | 355336 | 355474 | 356130