Changeset 154470 in webkit

Timestamp:

Aug 22, 2013 5:28:48 PM (11 years ago)

Author:

Simon Fraser

Message:

compositing/geometry/bounds-ignores-hidden-dynamic.html has incorrect initial rendering
​https://bugs.webkit.org/show_bug.cgi?id=119825

Source/WebCore:

Reviewed by Tim Horton.

r137526 and some earlier commits attempted to avoid unconditionally
repainting layers when their size changes, because this was causing
TiledBacking layers to repaint when the document size changed.

However, the approach required that we have good information about
whether size changes require a repaint, which in some cases is hard
to determine, especially when RenderLayer changes affect our
decisions about which layers are composited.

Fix by pushing the decision about whether to repaint on size change
into GraphicsLayer. The default is to repaint on size change,
but GraphicsLayer provides a function that can be overridden to
modify this behavior; GraphicsLayerCA does so to avoid repaints
when layers with TiledBackings get resized.

Test: compositing/repaint/repaint-on-layer-grouping-change.html

• WebCore.exp.in: WebKit2 needs GraphicsLayer::setSize, which is no longer inline.
• platform/graphics/GraphicsLayer.cpp:

(WebCore::GraphicsLayer::setOffsetFromRenderer):
(WebCore::GraphicsLayer::setSize):

• platform/graphics/GraphicsLayer.h:

(WebCore::GraphicsLayer::shouldRepaintOnSizeChange):

• platform/graphics/ca/GraphicsLayerCA.cpp:

(WebCore::GraphicsLayerCA::shouldRepaintOnSizeChange):

• platform/graphics/ca/GraphicsLayerCA.h:
• rendering/RenderLayerBacking.h: No longer need m_boundsConstrainedByClipping
• rendering/RenderLayerBacking.cpp:

(WebCore::RenderLayerBacking::RenderLayerBacking):
(WebCore::RenderLayerBacking::updateCompositedBounds):
(WebCore::RenderLayerBacking::updateGraphicsLayerGeometry): setSize takes
care of repainting for us now, so we can remove all the conditional code.

LayoutTests:

Reviewed by Tim Horton.

Test that dumps repaint rects on layers after visibility changes affect
the layer hierarchy.

• compositing/repaint/repaint-on-layer-grouping-change-expected.txt: Added.
• compositing/repaint/repaint-on-layer-grouping-change.html: Added.
• compositing/repaint/resize-repaint-expected.txt: Update result.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/compositing/repaint/repaint-on-layer-grouping-change-expected.txt (added)
• LayoutTests/compositing/repaint/repaint-on-layer-grouping-change.html (added)
• LayoutTests/compositing/repaint/resize-repaint-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/WebCore.exp.in (modified) (1 diff)
• Source/WebCore/platform/graphics/GraphicsLayer.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/GraphicsLayer.h (modified) (2 diffs)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerBacking.cpp (modified) (7 diffs)
• Source/WebCore/rendering/RenderLayerBacking.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-08-22  Simon Fraser  <simon.fraser@apple.com>
+
+        compositing/geometry/bounds-ignores-hidden-dynamic.html has incorrect initial rendering
+        https://bugs.webkit.org/show_bug.cgi?id=119825
+
+        Reviewed by Tim Horton.
+        
+        Test that dumps repaint rects on layers after visibility changes affect
+        the layer hierarchy.
+
+        * compositing/repaint/repaint-on-layer-grouping-change-expected.txt: Added.
+        * compositing/repaint/repaint-on-layer-grouping-change.html: Added.
+        * compositing/repaint/resize-repaint-expected.txt: Update result.
+
 2013-08-22  Christophe Dumez  <ch.dumez@sisa.samsung.com>
 

trunk/LayoutTests/compositing/repaint/resize-repaint-expected.txt

@@ -15 +15 @@
           (drawsContent 1)
           (repaint rects
-            (rect 2.00 104.00 398.00 1.00)
-            (rect 0.00 104.00 402.00 1.00)
+            (rect 0.00 0.00 402.00 207.00)
           )
         )

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-08-22  Simon Fraser  <simon.fraser@apple.com>
+
+        compositing/geometry/bounds-ignores-hidden-dynamic.html has incorrect initial rendering
+        https://bugs.webkit.org/show_bug.cgi?id=119825
+
+        Reviewed by Tim Horton.
+        
+        r137526 and some earlier commits attempted to avoid unconditionally
+        repainting layers when their size changes, because this was causing
+        TiledBacking layers to repaint when the document size changed.
+        
+        However, the approach required that we have good information about
+        whether size changes require a repaint, which in some cases is hard
+        to determine, especially when RenderLayer changes affect our
+        decisions about which layers are composited.
+        
+        Fix by pushing the decision about whether to repaint on size change
+        into GraphicsLayer. The default is to repaint on size change,
+        but GraphicsLayer provides a function that can be overridden to
+        modify this behavior; GraphicsLayerCA does so to avoid repaints
+        when layers with TiledBackings get resized.
+
+        Test: compositing/repaint/repaint-on-layer-grouping-change.html
+
+        * WebCore.exp.in: WebKit2 needs GraphicsLayer::setSize, which is no longer inline.
+        * platform/graphics/GraphicsLayer.cpp:
+        (WebCore::GraphicsLayer::setOffsetFromRenderer):
+        (WebCore::GraphicsLayer::setSize):
+        * platform/graphics/GraphicsLayer.h:
+        (WebCore::GraphicsLayer::shouldRepaintOnSizeChange):
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::shouldRepaintOnSizeChange):
+        * platform/graphics/ca/GraphicsLayerCA.h:
+        * rendering/RenderLayerBacking.h: No longer need m_boundsConstrainedByClipping
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::RenderLayerBacking):
+        (WebCore::RenderLayerBacking::updateCompositedBounds):
+        (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry): setSize takes
+        care of repainting for us now, so we can remove all the conditional code.
+
 2013-08-22  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/WebCore.exp.in

@@ -1373 +1373 @@
 __ZNK7WebCore13ContainerNode14childNodeCountEv
 __ZNK7WebCore13ContainerNode9childNodeEj
+__ZN7WebCore13GraphicsLayer7setSizeERKNS_9FloatSizeE
 __ZNK7WebCore13GraphicsLayer18accumulatedOpacityEv
 __ZNK7WebCore13GraphicsLayer18getDebugBorderInfoERNS_5ColorERf

trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp

@@ -308 +308 @@
 }
 
+void GraphicsLayer::setSize(const FloatSize& size)
+{
+    if (size == m_size)
+        return;
+    
+    m_size = size;
+
+    if (shouldRepaintOnSizeChange())
+        setNeedsDisplay();
+}
+
 void GraphicsLayer::setBackgroundColor(const Color& color)
 {

trunk/Source/WebCore/platform/graphics/GraphicsLayer.h

@@ -290 +290 @@
     // The size of the layer.
     const FloatSize& size() const { return m_size; }
-    virtual void setSize(const FloatSize& size) { m_size = size; }
+    virtual void setSize(const FloatSize&);
 
     // The boundOrigin affects the offset at which content is rendered, and sublayers are positioned.
@@ -501 +501 @@
     // rotations of >= 180 degrees
     static int validateTransformOperations(const KeyframeValueList&, bool& hasBigRotation);
+
+    virtual bool shouldRepaintOnSizeChange() const { return drawsContent(); }
 
     virtual void setOpacityInternal(float) { }

trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -673 +673 @@
 }
 
+bool GraphicsLayerCA::shouldRepaintOnSizeChange() const
+{
+    return drawsContent() && !tiledBacking();
+}
+
 bool GraphicsLayerCA::addAnimation(const KeyframeValueList& valueList, const IntSize& boxSize, const Animation* anim, const String& animationName, double timeOffset)
 {

trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h

@@ -176 +176 @@
 
     virtual double backingStoreMemoryEstimate() const;
+
+    virtual bool shouldRepaintOnSizeChange() const OVERRIDE;
 
     void updateOpacityOnLayer();

trunk/Source/WebCore/rendering/RenderLayerBacking.cpp

@@ -111 +111 @@
     , m_scrollLayerID(0)
     , m_artificiallyInflatedBounds(false)
-    , m_boundsConstrainedByClipping(false)
     , m_isMainFrameRenderViewLayer(false)
     , m_usingTiledCacheLayer(false)
@@ -449 +448 @@
 
         layerBounds.intersect(clippingBounds);
-        m_boundsConstrainedByClipping = true;
-    } else
-        m_boundsConstrainedByClipping = false;
+    }
     
     // If the element has a transform-origin that has fixed lengths, and the renderer has zero size,
@@ -706 +703 @@
 
     m_graphicsLayer->setPosition(FloatPoint(relativeCompositingBounds.location() - graphicsLayerParentLocation));
+    m_graphicsLayer->setSize(contentsSize);
     m_graphicsLayer->setOffsetFromRenderer(toIntSize(localCompositingBounds.location()));
 
-    FloatSize oldSize = m_graphicsLayer->size();
-    if (oldSize != contentsSize) {
-        m_graphicsLayer->setSize(contentsSize);
-        // Usually invalidation will happen via layout etc, but if we've affected the layer
-        // size by constraining relative to a clipping ancestor or the viewport, we
-        // have to invalidate to avoid showing stretched content.
-        if (m_boundsConstrainedByClipping)
-            m_graphicsLayer->setNeedsDisplay();
-    }
     if (!m_isMainFrameRenderViewLayer) {
         // For non-root layers, background is always painted by the primary graphics layer.
@@ -733 +722 @@
     
     if (m_maskLayer) {
-        if (m_maskLayer->size() != m_graphicsLayer->size()) {
-            m_maskLayer->setSize(m_graphicsLayer->size());
-            m_maskLayer->setNeedsDisplay();
-        }
+        m_maskLayer->setSize(m_graphicsLayer->size());
         m_maskLayer->setPosition(FloatPoint());
         m_maskLayer->setOffsetFromRenderer(m_graphicsLayer->offsetFromRenderer());
@@ -793 +779 @@
 
         m_foregroundLayer->setPosition(foregroundPosition);
-        if (foregroundSize != m_foregroundLayer->size()) {
-            m_foregroundLayer->setSize(foregroundSize);
-            m_foregroundLayer->setNeedsDisplay();
-        }
+        m_foregroundLayer->setSize(foregroundSize);
         m_foregroundLayer->setOffsetFromRenderer(foregroundOffset);
     }
@@ -809 +792 @@
         }
         m_backgroundLayer->setPosition(backgroundPosition);
-        if (backgroundSize != m_backgroundLayer->size()) {
-            m_backgroundLayer->setSize(backgroundSize);
-            m_backgroundLayer->setNeedsDisplay();
-        }
+        m_backgroundLayer->setSize(backgroundSize);
         m_backgroundLayer->setOffsetFromRenderer(m_graphicsLayer->offsetFromRenderer());
     }
@@ -856 +836 @@
 
         if (m_foregroundLayer) {
-            if (m_foregroundLayer->size() != m_scrollingContentsLayer->size())
-                m_foregroundLayer->setSize(m_scrollingContentsLayer->size());
-            m_foregroundLayer->setNeedsDisplay();
+            m_foregroundLayer->setSize(m_scrollingContentsLayer->size());
             m_foregroundLayer->setOffsetFromRenderer(m_scrollingContentsLayer->offsetFromRenderer());
         }

trunk/Source/WebCore/rendering/RenderLayerBacking.h

@@ -302 +302 @@
 
     bool m_artificiallyInflatedBounds; // bounds had to be made non-zero to make transform-origin work
-    bool m_boundsConstrainedByClipping;
     bool m_isMainFrameRenderViewLayer;
     bool m_usingTiledCacheLayer;