Incomplete repaint on twitter.com when replying to a tweet
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 May 2012 20:49:15 +0000 (20:49 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 May 2012 20:49:15 +0000 (20:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=87553

Reviewed by Dean Jackson.

Manual test that adds a transform to a layer, forcing that
layer to gain backing store.

* ManualTests/compositing/requires-backing-change.html: Added.

Source/WebCore:

Reviewed by Dean Jackson.

Style changes can cause a compositing layer to change between
requiring its own backing store or not, e.g. with the addition
or removal of a transform.

When that happens, we need to repaint the ancesetor layer that
this layer was, or will be drawing into.

Factored some code out of layerWillBeRemoved() to be able to
also call it from setRequiresOwnBackingStore().

New manual test, ManualTests/compositing/requires-backing-change.html.
I was not able to get an automated pixel test to work.

* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::setRequiresOwnBackingStore):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateCompositingLayers): Remove trailing whitespace.
(WebCore::RenderLayerCompositor::repaintInCompositedAncestor):
(WebCore::RenderLayerCompositor::layerWillBeRemoved):
* rendering/RenderLayerCompositor.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@118828 268f45cc-cd09-0410-ab3c-d52691b4dbfc

ChangeLog
ManualTests/compositing/requires-backing-change.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerBacking.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index c7f04c0..1fcba26 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2012-05-29  Simon Fraser  <simon.fraser@apple.com>
+
+        Incomplete repaint on twitter.com when replying to a tweet
+        https://bugs.webkit.org/show_bug.cgi?id=87553
+
+        Reviewed by Dean Jackson.
+        
+        Manual test that adds a transform to a layer, forcing that
+        layer to gain backing store.
+
+        * ManualTests/compositing/requires-backing-change.html: Added.
+
 2012-05-29  David Barr  <davidbarr@chromium.org>
 
         Introduce ENABLE_CSS_IMAGE_RESOLUTION compile flag
diff --git a/ManualTests/compositing/requires-backing-change.html b/ManualTests/compositing/requires-backing-change.html
new file mode 100644 (file)
index 0000000..d065ac1
--- /dev/null
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+  <style>
+    #container, #container2 {
+      height: 100px;
+      width: 100px;
+      margin: 10px 0;
+      -webkit-perspective: 1000px;
+    }
+    
+    #container.transformed, #container2.transformed {
+      -webkit-transform: translateX(0);
+    }
+    
+    .box {
+      margin-bottom: 5px;
+      height: 100px;
+      width: 100px;
+      background-color: green;
+      opacity: 0.5;
+    }
+  </style>
+  <script>
+    function doTest()
+    {
+      window.setTimeout(function() {
+        document.getElementById('container').className = 'transformed';
+        document.getElementById('container2').className = '';
+        
+        if (window.layoutTestController)
+          layoutTestController.notifyDone();
+      }, 100);
+    }
+    window.addEventListener('load', doTest, false);
+  </script>
+</head>
+<body>
+  <p>All squares should have the same pale green color></p>
+  <div class="box"></div>
+  <div id="container">
+    <div class="box">
+    </div>
+  </div>
+
+  <div id="container2" class="transformed">
+    <div class="box">
+    </div>
+  </div>
+</body>
+</html>
index 89052ec..2a23e17 100644 (file)
@@ -1,3 +1,31 @@
+2012-05-29  Simon Fraser  <simon.fraser@apple.com>
+
+        Incomplete repaint on twitter.com when replying to a tweet
+        https://bugs.webkit.org/show_bug.cgi?id=87553
+
+        Reviewed by Dean Jackson.
+        
+        Style changes can cause a compositing layer to change between
+        requiring its own backing store or not, e.g. with the addition
+        or removal of a transform.
+        
+        When that happens, we need to repaint the ancesetor layer that
+        this layer was, or will be drawing into.
+        
+        Factored some code out of layerWillBeRemoved() to be able to
+        also call it from setRequiresOwnBackingStore().
+
+        New manual test, ManualTests/compositing/requires-backing-change.html.
+        I was not able to get an automated pixel test to work.
+
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::setRequiresOwnBackingStore):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateCompositingLayers): Remove trailing whitespace.
+        (WebCore::RenderLayerCompositor::repaintInCompositedAncestor):
+        (WebCore::RenderLayerCompositor::layerWillBeRemoved):
+        * rendering/RenderLayerCompositor.h:
+
 2012-05-29  David Hyatt  <hyatt@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=87771
index 1085eab..adce4c7 100644 (file)
@@ -1112,15 +1112,17 @@ bool RenderLayerBacking::paintsIntoWindow() const
     return false;
 }
 
-void RenderLayerBacking::setRequiresOwnBackingStore(bool flag)
+void RenderLayerBacking::setRequiresOwnBackingStore(bool requiresOwnBacking)
 {
-    if (flag == m_requiresOwnBackingStore)
+    if (requiresOwnBacking == m_requiresOwnBackingStore)
         return;
     
     // This affects the answer to paintsIntoCompositedAncestor(), which in turn affects
     // cached clip rects, so when it changes we have to clear clip rects on descendants.
     m_owningLayer->clearClipRectsIncludingDescendants(PaintingClipRects);
-    m_requiresOwnBackingStore = flag;
+    m_requiresOwnBackingStore = requiresOwnBacking;
+    
+    compositor()->repaintInCompositedAncestor(m_owningLayer, compositedBounds());
 }
 
 void RenderLayerBacking::setContentsNeedDisplay()
index 2a9d2ab..6b269fd 100644 (file)
@@ -420,7 +420,7 @@ void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
         LOG(Compositing, "\nUpdate %d of %s. Overlap testing is %s\n", m_rootLayerUpdateCount, isMainFrame ? "main frame" : frame->tree()->uniqueName().string().utf8().data(),
             m_compositingConsultsOverlap ? "on" : "off");
     }
-#endif        
+#endif
 
     if (needHierarchyUpdate) {
         // Update the hierarchy of the compositing layers.
@@ -578,6 +578,28 @@ void RenderLayerCompositor::repaintOnCompositingChange(RenderLayer* layer)
     }
 }
 
+// This method assumes that layout is up-to-date, unlike repaintOnCompositingChange().
+void RenderLayerCompositor::repaintInCompositedAncestor(RenderLayer* layer, const LayoutRect& rect)
+{
+    RenderLayer* compositedAncestor = layer->enclosingCompositingLayerForRepaint(false /*exclude self*/);
+    if (compositedAncestor) {
+        ASSERT(compositedAncestor->backing());
+
+        LayoutPoint offset;
+        layer->convertToLayerCoords(compositedAncestor, offset);
+
+        LayoutRect repaintRect = rect;
+        repaintRect.moveBy(offset);
+
+        compositedAncestor->setBackingNeedsRepaintInRect(repaintRect);
+    }
+
+    // The contents of this layer may be moving from a GraphicsLayer to the window,
+    // so we need to make sure the window system synchronizes those changes on the screen.
+    if (compositedAncestor == m_renderView->layer())
+        m_renderView->frameView()->setNeedsOneShotDrawingSynchronization();
+}
+
 // The bounds of the GraphicsLayer created for a compositing layer is the union of the bounds of all the descendant
 // RenderLayers that are rendered by the composited RenderLayer.
 IntRect RenderLayerCompositor::calculateCompositedBounds(const RenderLayer* layer, const RenderLayer* ancestorLayer)
@@ -597,24 +619,9 @@ void RenderLayerCompositor::layerWillBeRemoved(RenderLayer* parent, RenderLayer*
     if (!child->isComposited() || parent->renderer()->documentBeingDestroyed())
         return;
 
-    setCompositingParent(child, 0);
-    
-    RenderLayer* compLayer = parent->enclosingCompositingLayerForRepaint();
-    if (compLayer) {
-        ASSERT(compLayer->backing());
-        LayoutRect compBounds = child->backing()->compositedBounds();
-
-        LayoutPoint offset;
-        child->convertToLayerCoords(compLayer, offset);
-        compBounds.moveBy(offset);
-
-        compLayer->setBackingNeedsRepaintInRect(compBounds);
-
-        // The contents of this layer may be moving from a GraphicsLayer to the window,
-        // so we need to make sure the window system synchronizes those changes on the screen.
-        m_renderView->frameView()->setNeedsOneShotDrawingSynchronization();
-    }
+    repaintInCompositedAncestor(child, child->backing()->compositedBounds());
 
+    setCompositingParent(child, 0);
     setCompositingLayersNeedRebuild();
 }
 
index 2738493..9df3a65 100644 (file)
@@ -126,6 +126,8 @@ public:
     // Repaint the appropriate layers when the given RenderLayer starts or stops being composited.
     void repaintOnCompositingChange(RenderLayer*);
     
+    void repaintInCompositedAncestor(RenderLayer*, const LayoutRect&);
+    
     // Notify us that a layer has been added or removed
     void layerWasAdded(RenderLayer* parent, RenderLayer* child);
     void layerWillBeRemoved(RenderLayer* parent, RenderLayer* child);