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 c7f04c066c07e5ce88c54fdcc916225f3624de69..1fcba26b88c5f6ae00850fc15388b4292f98b288 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 89052ecdebed981701628694be47b10e10004a28..2a23e1714ea9b35a84a74c17f6dde405e11f02e3 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 1085eabc0053523c781ad97ee6434cecfa909557..adce4c7eeb88095476540b0d738a174cd531abdf 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 2a9d2ab07db76a4b7467163a4fcfed9e65268885..6b269fdc4ea616790fca1dca993c11ab2431af5c 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 2738493521b0252fb0fd5ad4b4717efaacdb6994..9df3a654c69425a4eb75c3e5594d4cdbd300ce18 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);