Title: [238727] trunk
Revision
238727
Author
commit-qu...@webkit.org
Date
2018-11-29 22:35:36 -0800 (Thu, 29 Nov 2018)

Log Message

Separate paint and scroll offsets for RenderLayerBacking::m_scrollingContentsLayer
https://bugs.webkit.org/show_bug.cgi?id=183040

Source/WebCore:

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.

Patch by Frederic Wang <fw...@igalia.com> on 2018-11-29
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.

LayoutTests:

Patch by Frederic Wang <fw...@igalia.com> on 2018-11-29
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:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (238726 => 238727)


--- trunk/LayoutTests/ChangeLog	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/ChangeLog	2018-11-30 06:35:36 UTC (rev 238727)
@@ -1,3 +1,23 @@
+2018-11-29  Frederic Wang  <fw...@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.fra...@apple.com>
 
         Overflow scrolling layers need to be self-painting

Modified: trunk/LayoutTests/compositing/ios/overflow-scroll-touch-tiles-expected.txt (238726 => 238727)


--- trunk/LayoutTests/compositing/ios/overflow-scroll-touch-tiles-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/compositing/ios/overflow-scroll-touch-tiles-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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)

Modified: trunk/LayoutTests/fast/scrolling/ios/overflow-scroll-touch-expected.txt (238726 => 238727)


--- trunk/LayoutTests/fast/scrolling/ios/overflow-scroll-touch-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/fast/scrolling/ios/overflow-scroll-touch-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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)
                 )

Modified: trunk/LayoutTests/fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt (238726 => 238727)


--- trunk/LayoutTests/fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/fast/scrolling/ios/subpixel-overflow-scrolling-with-ancestor-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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

Modified: trunk/LayoutTests/platform/ios/compositing/overflow/scrolling-without-painting-expected.txt (238726 => 238727)


--- trunk/LayoutTests/platform/ios/compositing/overflow/scrolling-without-painting-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/platform/ios/compositing/overflow/scrolling-without-painting-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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)
                 )
               )

Modified: trunk/LayoutTests/platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt (238726 => 238727)


--- trunk/LayoutTests/platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/platform/ios/compositing/overflow/textarea-scroll-touch-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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)
                 )

Modified: trunk/LayoutTests/platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt (238726 => 238727)


--- trunk/LayoutTests/platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/platform/ios/compositing/rtl/rtl-scrolling-with-transformed-descendants-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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

Modified: trunk/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt (238726 => 238727)


--- trunk/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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

Modified: trunk/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt (238726 => 238727)


--- trunk/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/platform/ios/fast/scrolling/ios/overflow-scrolling-ancestor-clip-size-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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

Modified: trunk/LayoutTests/platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt (238726 => 238727)


--- trunk/LayoutTests/platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/LayoutTests/platform/ios/fast/scrolling/ios/textarea-scroll-touch-expected.txt	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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)
                 )

Modified: trunk/Source/WebCore/ChangeLog (238726 => 238727)


--- trunk/Source/WebCore/ChangeLog	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/Source/WebCore/ChangeLog	2018-11-30 06:35:36 UTC (rev 238727)
@@ -1,3 +1,36 @@
+2018-11-29  Frederic Wang  <fw...@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.fra...@apple.com>
 
         Overflow scrolling layers need to be self-painting

Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp (238726 => 238727)


--- trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp	2018-11-30 06:35:36 UTC (rev 238727)
@@ -456,6 +456,18 @@
         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::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 @@
     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";
 

Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayer.h (238726 => 238727)


--- trunk/Source/WebCore/platform/graphics/GraphicsLayer.h	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayer.h	2018-11-30 06:35:36 UTC (rev 238727)
@@ -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 @@
     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 @@
     // 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;
 

Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (238726 => 238727)


--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2018-11-30 06:31:04 UTC (rev 238726)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2018-11-30 06:35:36 UTC (rev 238727)
@@ -1206,9 +1206,8 @@
 
         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 @@
         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 @@
 
     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()).
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to