Fix repaint issues when resizing a window with centered content, for platforms with...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Dec 2012 22:11:27 +0000 (22:11 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Dec 2012 22:11:27 +0000 (22:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=105073

Reviewed by Dan Bernstein.

Add a manual test for window resize with a centered element.

* ManualTests/resize-repaint.html: Added.

Source/WebCore:

There were several issues with the "do full repaint" code path in
FrameView::layout(). These caused repaint issues when resizing the web view,
especially for platforms that use a tile cache.

First, the m_doFullRepaint flag wold get clobbered on resize-layouts, because
the call to adjustViewSize() re-enters layout(), and resets the m_doFullRepaint member
variable to false, even if the outer call had previously set it to true. This would
cause us to lose track of whether we needed to do a full repaint. The patch fixes
this by restoring m_doFullRepaint to the value it had before the call to adjustViewSize().

The second problem was that full repaints would not propagate to compositing
layers. They only repainted the RenderView, and on platforms that use a tile cache,
this only repaints the top portion of that tile cache. This was fixed by sending
a NeedsFullRepaintInBacking flag down into RenderLayer::updateLayerPositions(),
and using that to do a full repaint on all compositing layers.

Sending this new flag down into updateAfterLayout() prompted some boolean/flags
cleanup with propagated into several files. This also allowed me to no longer
include RenderLayerBacking.h in RenderLayerCompositor.h, but that required
header cleanup in several files.

Automated testing is not possible because WebKitTestRunner resizes the window
asynchronously (bug 105101). Added manual test.

* page/FrameView.cpp:
(WebCore::updateLayerPositionFlags):
(WebCore::FrameView::layout):
* page/scrolling/ScrollingCoordinator.cpp:
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateLayerPositions):
(WebCore::RenderLayer::updateCompositingLayersAfterScroll):
* rendering/RenderLayer.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateAfterLayout):
(WebCore::RenderLayerBacking::contentChanged):
* rendering/RenderLayerBacking.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateCompositingDescendantGeometry):
* rendering/RenderLayerCompositor.h:
* rendering/RenderObject.cpp:
* rendering/RenderView.cpp:

Source/WebKit/chromium:

Include RenderLayerBacking.h, which is no longer included by RenderLayerCompositor.h.

* tests/ScrollingCoordinatorChromiumTest.cpp:

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

16 files changed:
ChangeLog
ManualTests/resize-repaint.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderLayerBacking.cpp
Source/WebCore/rendering/RenderLayerBacking.h
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderView.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/ScrollingCoordinatorChromiumTest.cpp

index a69b8c964bc804823060e7890c8bf7e33ecff3a5..4d4e21a5b8b0fba7a7539ad4f68579326a5d17bb 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2012-12-15  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix repaint issues when resizing a window with centered content, for platforms with a tile cache
+        https://bugs.webkit.org/show_bug.cgi?id=105073
+
+        Reviewed by Dan Bernstein.
+
+        Add a manual test for window resize with a centered element.
+
+        * ManualTests/resize-repaint.html: Added.
+
 2012-12-13  Stephen White  <senorblanco@chromium.org>
 
         Added manual test for canvas setFont speed.
diff --git a/ManualTests/resize-repaint.html b/ManualTests/resize-repaint.html
new file mode 100644 (file)
index 0000000..7154af5
--- /dev/null
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        .container {
+            width: 700px;
+            height: 2000px;
+            margin: 0 auto;
+            background-color: silver;
+        }
+    </style>
+    <script>
+        function doTest()
+        {
+            window.scrollTo(0, 400);
+            window.resizeTo(1000, 600);
+        }
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="container"></div>
+<pre id="layers">Layer tree goes here</p>
+</body>
+</html>
index 5e40f448d60c70240de2eae4062b00f338f56a5c..dfa0d96ba81d54e59312380c3319b265a7cfb372 100644 (file)
@@ -1,3 +1,53 @@
+2012-12-15  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix repaint issues when resizing a window with centered content, for platforms with a tile cache
+        https://bugs.webkit.org/show_bug.cgi?id=105073
+
+        Reviewed by Dan Bernstein.
+
+        There were several issues with the "do full repaint" code path in
+        FrameView::layout(). These caused repaint issues when resizing the web view,
+        especially for platforms that use a tile cache.
+        
+        First, the m_doFullRepaint flag wold get clobbered on resize-layouts, because
+        the call to adjustViewSize() re-enters layout(), and resets the m_doFullRepaint member
+        variable to false, even if the outer call had previously set it to true. This would
+        cause us to lose track of whether we needed to do a full repaint. The patch fixes
+        this by restoring m_doFullRepaint to the value it had before the call to adjustViewSize().
+        
+        The second problem was that full repaints would not propagate to compositing
+        layers. They only repainted the RenderView, and on platforms that use a tile cache,
+        this only repaints the top portion of that tile cache. This was fixed by sending
+        a NeedsFullRepaintInBacking flag down into RenderLayer::updateLayerPositions(),
+        and using that to do a full repaint on all compositing layers.
+        
+        Sending this new flag down into updateAfterLayout() prompted some boolean/flags
+        cleanup with propagated into several files. This also allowed me to no longer
+        include RenderLayerBacking.h in RenderLayerCompositor.h, but that required
+        header cleanup in several files.
+
+        Automated testing is not possible because WebKitTestRunner resizes the window
+        asynchronously (bug 105101). Added manual test.
+
+        * page/FrameView.cpp:
+        (WebCore::updateLayerPositionFlags):
+        (WebCore::FrameView::layout):
+        * page/scrolling/ScrollingCoordinator.cpp:
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateLayerPositions):
+        (WebCore::RenderLayer::updateCompositingLayersAfterScroll):
+        * rendering/RenderLayer.h:
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::updateAfterLayout):
+        (WebCore::RenderLayerBacking::contentChanged):
+        * rendering/RenderLayerBacking.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateCompositingDescendantGeometry):
+        * rendering/RenderLayerCompositor.h:
+        * rendering/RenderObject.cpp:
+        * rendering/RenderView.cpp:
+
 2012-12-15  Anders Carlsson  <andersca@apple.com>
 
         Fix build.
index d3a1a494a588aeece74f8c6b7a216fdab2c13239..046560e417c4ef9154e69164c588621963d8bc29 100644 (file)
@@ -57,6 +57,7 @@
 #include "RenderFullScreen.h"
 #include "RenderIFrame.h"
 #include "RenderLayer.h"
+#include "RenderLayerBacking.h"
 #include "RenderPart.h"
 #include "RenderScrollbar.h"
 #include "RenderScrollbarPart.h"
@@ -131,8 +132,10 @@ static inline RenderView* rootRenderer(const FrameView* view)
 static RenderLayer::UpdateLayerPositionsFlags updateLayerPositionFlags(RenderLayer* layer, bool isRelayoutingSubtree, bool didFullRepaint)
 {
     RenderLayer::UpdateLayerPositionsFlags flags = RenderLayer::defaultFlags;
-    if (didFullRepaint)
+    if (didFullRepaint) {
         flags &= ~RenderLayer::CheckForRepaint;
+        flags |= RenderLayer::NeedsFullRepaintInBacking;
+    }
     if (isRelayoutingSubtree && layer->isPaginated())
         flags |= RenderLayer::UpdatePagination;
     return flags;
@@ -1213,9 +1216,13 @@ void FrameView::layout(bool allowSubtree)
         m_layoutRoot = 0;
     } // Reset m_layoutSchedulingEnabled to its previous value.
 
+    bool neededFullRepaint = m_doFullRepaint;
+
     if (!subtree && !toRenderView(root)->printing())
         adjustViewSize();
 
+    m_doFullRepaint = neededFullRepaint;
+
     // Now update the positions of all layers.
     beginDeferredRepaints();
     if (m_doFullRepaint)
index 50fe150024bd0e5c0880c7e39e7bbd00b30fd225..33fbe8b539a396651ca9d307775c80471869d0fa 100644 (file)
@@ -29,6 +29,7 @@
 
 #include "Frame.h"
 #include "FrameView.h"
+#include "GraphicsLayer.h"
 #include "IntRect.h"
 #include "Page.h"
 #include "PlatformWheelEvent.h"
index 03ca389a915a62567551ea6352a2fa0c9f755580..ceb967967c51bddbffb5b7589d3e57a93f3ff53d 100644 (file)
@@ -29,6 +29,7 @@
 
 #import "ScrollingCoordinatorMac.h"
 
+#include "GraphicsLayer.h"
 #include "Frame.h"
 #include "FrameView.h"
 #include "IntRect.h"
index 53a9d2d0557e290810038711083dedad41f11a85..ca1612857ecc75c5b4a30cdda483dc8a0221385a 100644 (file)
@@ -416,8 +416,14 @@ void RenderLayer::updateLayerPositions(RenderGeometryMap* geometryMap, UpdateLay
         child->updateLayerPositions(geometryMap, flags);
 
 #if USE(ACCELERATED_COMPOSITING)
-    if ((flags & UpdateCompositingLayers) && isComposited())
-        backing()->updateAfterLayout(RenderLayerBacking::CompositingChildren, isUpdateRoot);
+    if ((flags & UpdateCompositingLayers) && isComposited()) {
+        RenderLayerBacking::UpdateAfterLayoutFlags updateFlags = RenderLayerBacking::CompositingChildrenOnly;
+        if (flags & NeedsFullRepaintInBacking)
+            updateFlags |= RenderLayerBacking::NeedsFullRepaint;
+        if (isUpdateRoot)
+            updateFlags |= RenderLayerBacking::IsUpdateRoot;
+        backing()->updateAfterLayout(updateFlags);
+    }
 #endif
         
     // With all our children positioned, now update our marquee if we need to.
@@ -1922,10 +1928,8 @@ void RenderLayer::updateCompositingLayersAfterScroll()
         if (RenderLayer* compositingAncestor = stackingContext()->enclosingCompositingLayer()) {
             if (compositor()->compositingConsultsOverlap())
                 compositor()->updateCompositingLayers(CompositingUpdateOnScroll, compositingAncestor);
-            else {
-                bool isUpdateRoot = true;
-                compositingAncestor->backing()->updateAfterLayout(RenderLayerBacking::AllDescendants, isUpdateRoot);
-            }
+            else
+                compositingAncestor->backing()->updateAfterLayout(RenderLayerBacking::IsUpdateRoot);
         }
     }
 #endif
index 1cfc9a03ff3e55557e9ecd38ddf217e20c151fc3..5553b5543019cd4748f35948a1495fc86f2e1cdc 100644 (file)
@@ -391,10 +391,11 @@ public:
     bool canRender3DTransforms() const;
 
     enum UpdateLayerPositionsFlag {
-        CheckForRepaint = 1,
-        IsCompositingUpdateRoot = 1 << 1,
-        UpdateCompositingLayers = 1 << 2,
-        UpdatePagination = 1 << 3
+        CheckForRepaint = 1 << 0,
+        NeedsFullRepaintInBacking = 1 << 1,
+        IsCompositingUpdateRoot = 1 << 2,
+        UpdateCompositingLayers = 1 << 3,
+        UpdatePagination = 1 << 4
     };
     typedef unsigned UpdateLayerPositionsFlags;
     static const UpdateLayerPositionsFlags defaultFlags = CheckForRepaint | IsCompositingUpdateRoot | UpdateCompositingLayers;
index 8ec868a95cc023586a009b7152ae4e887f76b94e..569f4226120233a437399e8f080d2aa3d00c8d02 100644 (file)
@@ -416,7 +416,7 @@ void RenderLayerBacking::updateAfterWidgetResize()
     }
 }
 
-void RenderLayerBacking::updateAfterLayout(UpdateDepth updateDepth, bool isUpdateRoot)
+void RenderLayerBacking::updateAfterLayout(UpdateAfterLayoutFlags flags)
 {
     RenderLayerCompositor* layerCompositor = compositor();
     if (!layerCompositor->compositingLayersNeedRebuild()) {
@@ -428,13 +428,16 @@ void RenderLayerBacking::updateAfterLayout(UpdateDepth updateDepth, bool isUpdat
         // The solution is to update compositing children of this layer here,
         // via updateCompositingChildrenGeometry().
         updateCompositedBounds();
-        layerCompositor->updateCompositingDescendantGeometry(m_owningLayer, m_owningLayer, updateDepth);
+        layerCompositor->updateCompositingDescendantGeometry(m_owningLayer, m_owningLayer, flags & CompositingChildrenOnly);
         
-        if (isUpdateRoot) {
+        if (flags & IsUpdateRoot) {
             updateGraphicsLayerGeometry();
             layerCompositor->updateRootLayerPosition();
         }
     }
+    
+    if (flags & NeedsFullRepaint)
+        setContentsNeedDisplay();
 }
 
 bool RenderLayerBacking::updateGraphicsLayerConfiguration()
@@ -1390,8 +1393,7 @@ void RenderLayerBacking::contentChanged(ContentChangeType changeType)
     if ((changeType == MaskImageChanged) && m_maskLayer) {
         // The composited layer bounds relies on box->maskClipRect(), which changes
         // when the mask image becomes available.
-        bool isUpdateRoot = true;
-        updateAfterLayout(CompositingChildren, isUpdateRoot);
+        updateAfterLayout(CompositingChildrenOnly | IsUpdateRoot);
     }
 
 #if ENABLE(WEBGL) || ENABLE(ACCELERATED_2D_CANVAS)
index bfde3b989b7d9b5b22ace494266dbed26293ae30..397fc66f3f8f5ec782f426156a6481eca30ebebf 100644 (file)
@@ -62,8 +62,13 @@ public:
 
     RenderLayer* owningLayer() const { return m_owningLayer; }
 
-    enum UpdateDepth { CompositingChildren, AllDescendants };
-    void updateAfterLayout(UpdateDepth, bool isUpdateRoot);
+    enum UpdateAfterLayoutFlag {
+        CompositingChildrenOnly = 1 << 0,
+        NeedsFullRepaint = 1 << 1,
+        IsUpdateRoot = 1 << 2
+    };
+    typedef unsigned UpdateAfterLayoutFlags;
+    void updateAfterLayout(UpdateAfterLayoutFlags);
     
     // Returns true if layer configuration changed.
     bool updateGraphicsLayerConfiguration();
index 1e44442e2dab2157345b25c1236744b4abfa20f3..1bf7bec49d94a3fad924b45bb133204cebdab6ae 100644 (file)
@@ -1307,7 +1307,7 @@ void RenderLayerCompositor::updateLayerTreeGeometry(RenderLayer* layer, int dept
 }
 
 // Recurs down the RenderLayer tree until its finds the compositing descendants of compositingAncestor and updates their geometry.
-void RenderLayerCompositor::updateCompositingDescendantGeometry(RenderLayer* compositingAncestor, RenderLayer* layer, RenderLayerBacking::UpdateDepth updateDepth)
+void RenderLayerCompositor::updateCompositingDescendantGeometry(RenderLayer* compositingAncestor, RenderLayer* layer, bool compositedChildrenOnly)
 {
     if (layer != compositingAncestor) {
         if (RenderLayerBacking* layerBacking = layer->backing()) {
@@ -1319,13 +1319,13 @@ void RenderLayerCompositor::updateCompositingDescendantGeometry(RenderLayer* com
             }
 
             layerBacking->updateGraphicsLayerGeometry();
-            if (updateDepth == RenderLayerBacking::CompositingChildren)
+            if (compositedChildrenOnly)
                 return;
         }
     }
 
     if (layer->reflectionLayer())
-        updateCompositingDescendantGeometry(compositingAncestor, layer->reflectionLayer(), updateDepth);
+        updateCompositingDescendantGeometry(compositingAncestor, layer->reflectionLayer(), compositedChildrenOnly);
 
     if (!layer->hasCompositingDescendant())
         return;
@@ -1338,21 +1338,21 @@ void RenderLayerCompositor::updateCompositingDescendantGeometry(RenderLayer* com
         if (Vector<RenderLayer*>* negZOrderList = layer->negZOrderList()) {
             size_t listSize = negZOrderList->size();
             for (size_t i = 0; i < listSize; ++i)
-                updateCompositingDescendantGeometry(compositingAncestor, negZOrderList->at(i), updateDepth);
+                updateCompositingDescendantGeometry(compositingAncestor, negZOrderList->at(i), compositedChildrenOnly);
         }
     }
 
     if (Vector<RenderLayer*>* normalFlowList = layer->normalFlowList()) {
         size_t listSize = normalFlowList->size();
         for (size_t i = 0; i < listSize; ++i)
-            updateCompositingDescendantGeometry(compositingAncestor, normalFlowList->at(i), updateDepth);
+            updateCompositingDescendantGeometry(compositingAncestor, normalFlowList->at(i), compositedChildrenOnly);
     }
     
     if (layer->isStackingContext()) {
         if (Vector<RenderLayer*>* posZOrderList = layer->posZOrderList()) {
             size_t listSize = posZOrderList->size();
             for (size_t i = 0; i < listSize; ++i)
-                updateCompositingDescendantGeometry(compositingAncestor, posZOrderList->at(i), updateDepth);
+                updateCompositingDescendantGeometry(compositingAncestor, posZOrderList->at(i), compositedChildrenOnly);
         }
     }
 }
index c4b7e3bcda86498fe2743e754a2cb53eb1256f72..a8b0304b5300f9be0b630fe2a521de1a919d827f 100644 (file)
@@ -28,9 +28,9 @@
 
 #include "ChromeClient.h"
 #include "Frame.h"
+#include "GraphicsLayerClient.h"
 #include "GraphicsLayerUpdater.h"
 #include "RenderLayer.h"
-#include "RenderLayerBacking.h"
 #include <wtf/HashMap.h>
 
 namespace WebCore {
@@ -117,7 +117,7 @@ public:
     bool updateLayerCompositingState(RenderLayer*, CompositingChangeRepaint = CompositingChangeRepaintNow);
 
     // Update the geometry for compositing children of compositingAncestor.
-    void updateCompositingDescendantGeometry(RenderLayer* compositingAncestor, RenderLayer* layer, RenderLayerBacking::UpdateDepth);
+    void updateCompositingDescendantGeometry(RenderLayer* compositingAncestor, RenderLayer*, bool compositedChildrenOnly);
     
     // Whether layer's backing needs a graphics layer to do clipping by an ancestor (non-stacking-context parent with overflow).
     bool clippedByAncestor(RenderLayer*) const;
index ac0f67ddae2ef96a74151e219143205d27499ba8..8e9f7e66261e229aa60e6e676652a330c02d4224 100644 (file)
@@ -52,6 +52,7 @@
 #include "RenderImageResourceStyleImage.h"
 #include "RenderInline.h"
 #include "RenderLayer.h"
+#include "RenderLayerBacking.h"
 #include "RenderListItem.h"
 #include "RenderMultiColumnBlock.h"
 #include "RenderNamedFlowThread.h"
index 49ab8f8bcf9af550c37d7e55e269d311e0708886..e689794a661d756cb22fcf600fed5364a2618997 100644 (file)
@@ -34,6 +34,7 @@
 #include "Page.h"
 #include "RenderGeometryMap.h"
 #include "RenderLayer.h"
+#include "RenderLayerBacking.h"
 #include "RenderNamedFlowThread.h"
 #include "RenderSelectionInfo.h"
 #include "RenderWidget.h"
index d22a699220befae0e09950e068e0c9f3e0912d7a..cc049a7e9a6ad3834464d32dbd98a1deedc5415d 100644 (file)
@@ -1,3 +1,14 @@
+2012-12-15  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix repaint issues when resizing a window with centered content, for platforms with a tile cache
+        https://bugs.webkit.org/show_bug.cgi?id=105073
+
+        Reviewed by Dan Bernstein.
+
+        Include RenderLayerBacking.h, which is no longer included by RenderLayerCompositor.h.
+
+        * tests/ScrollingCoordinatorChromiumTest.cpp:
+
 2012-12-14  Dan Alcantara  <dfalcantara@chromium.org>
 
         WebViewImpl::resetScrollAndScaleState() causes the page to render incorrectly
index 271829de9aa2a354d9b4426819338fcc2eb860a0..4cfbbfc7256046e8e0bcdaa963b86e253644a567 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "CompositorFakeWebGraphicsContext3D.h"
 #include "FrameTestHelpers.h"
+#include "RenderLayerBacking.h"
 #include "RenderLayerCompositor.h"
 #include "RenderView.h"
 #include "URLTestHelpers.h"