Title: [137811] trunk
Revision
137811
Author
[email protected]
Date
2012-12-15 14:11:27 -0800 (Sat, 15 Dec 2012)

Log Message

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.

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:

Modified Paths

Added Paths

Diff

Modified: trunk/ChangeLog (137810 => 137811)


--- trunk/ChangeLog	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/ChangeLog	2012-12-15 22:11:27 UTC (rev 137811)
@@ -1,3 +1,14 @@
+2012-12-15  Simon Fraser  <[email protected]>
+
+        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  <[email protected]>
 
         Added manual test for canvas setFont speed.

Added: trunk/ManualTests/resize-repaint.html (0 => 137811)


--- trunk/ManualTests/resize-repaint.html	                        (rev 0)
+++ trunk/ManualTests/resize-repaint.html	2012-12-15 22:11:27 UTC (rev 137811)
@@ -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>

Modified: trunk/Source/WebCore/ChangeLog (137810 => 137811)


--- trunk/Source/WebCore/ChangeLog	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/ChangeLog	2012-12-15 22:11:27 UTC (rev 137811)
@@ -1,3 +1,53 @@
+2012-12-15  Simon Fraser  <[email protected]>
+
+        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  <[email protected]>
 
         Fix build.

Modified: trunk/Source/WebCore/page/FrameView.cpp (137810 => 137811)


--- trunk/Source/WebCore/page/FrameView.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/page/FrameView.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -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 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 @@
         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)

Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp (137810 => 137811)


--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -29,6 +29,7 @@
 
 #include "Frame.h"
 #include "FrameView.h"
+#include "GraphicsLayer.h"
 #include "IntRect.h"
 #include "Page.h"
 #include "PlatformWheelEvent.h"

Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (137810 => 137811)


--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm	2012-12-15 22:11:27 UTC (rev 137811)
@@ -29,6 +29,7 @@
 
 #import "ScrollingCoordinatorMac.h"
 
+#include "GraphicsLayer.h"
 #include "Frame.h"
 #include "FrameView.h"
 #include "IntRect.h"

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -416,8 +416,14 @@
         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 @@
         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

Modified: trunk/Source/WebCore/rendering/RenderLayer.h (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderLayer.h	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderLayer.h	2012-12-15 22:11:27 UTC (rev 137811)
@@ -391,10 +391,11 @@
     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;

Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -416,7 +416,7 @@
     }
 }
 
-void RenderLayerBacking::updateAfterLayout(UpdateDepth updateDepth, bool isUpdateRoot)
+void RenderLayerBacking::updateAfterLayout(UpdateAfterLayoutFlags flags)
 {
     RenderLayerCompositor* layerCompositor = compositor();
     if (!layerCompositor->compositingLayersNeedRebuild()) {
@@ -428,13 +428,16 @@
         // 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 @@
     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)

Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.h (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderLayerBacking.h	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.h	2012-12-15 22:11:27 UTC (rev 137811)
@@ -62,8 +62,13 @@
 
     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();

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -1307,7 +1307,7 @@
 }
 
 // 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 @@
             }
 
             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 @@
         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);
         }
     }
 }

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2012-12-15 22:11:27 UTC (rev 137811)
@@ -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 @@
     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;

Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderObject.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -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"

Modified: trunk/Source/WebCore/rendering/RenderView.cpp (137810 => 137811)


--- trunk/Source/WebCore/rendering/RenderView.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebCore/rendering/RenderView.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -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"

Modified: trunk/Source/WebKit/chromium/ChangeLog (137810 => 137811)


--- trunk/Source/WebKit/chromium/ChangeLog	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebKit/chromium/ChangeLog	2012-12-15 22:11:27 UTC (rev 137811)
@@ -1,3 +1,14 @@
+2012-12-15  Simon Fraser  <[email protected]>
+
+        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  <[email protected]>
 
         WebViewImpl::resetScrollAndScaleState() causes the page to render incorrectly

Modified: trunk/Source/WebKit/chromium/tests/ScrollingCoordinatorChromiumTest.cpp (137810 => 137811)


--- trunk/Source/WebKit/chromium/tests/ScrollingCoordinatorChromiumTest.cpp	2012-12-15 22:04:12 UTC (rev 137810)
+++ trunk/Source/WebKit/chromium/tests/ScrollingCoordinatorChromiumTest.cpp	2012-12-15 22:11:27 UTC (rev 137811)
@@ -28,6 +28,7 @@
 
 #include "CompositorFakeWebGraphicsContext3D.h"
 #include "FrameTestHelpers.h"
+#include "RenderLayerBacking.h"
 #include "RenderLayerCompositor.h"
 #include "RenderView.h"
 #include "URLTestHelpers.h"
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to