Changeset 86208 in webkit

Timestamp:

May 10, 2011 7:28:52 PM (13 years ago)

Author:

jamesr@google.com

Message:

2011-05-10 James Robinson <​jamesr@chromium.org>

Reviewed by Kenneth Russell.

[chromium] Clean up setLayerRenderer() calls in LayerRendererChromium
​https://bugs.webkit.org/show_bug.cgi?id=60524

This refactors the way LayerRendererChromium calls setLayerRenderer on its layers in a few ways to clean things
up:

*) Move the pre-paint call to setLayerRenderer from updatePropertiesAndRenderSurfaces() to paintLayerContents().

updatePropertiesAndRenderSurfaces() does too much and the layer renderer pointer only has to be up to date to do
the actual paint. Longer term we won't need to call this at all in order to call paint and this call will just
go away.

*) Tweaks updateCompositorResourcesRecursive() to be slightly shorter and less error-prone.
*) Explicitly set the LayerRendererChromium for each layer in the tree when transfering the root layer (used

during lost context). LayerChromium::setLayerRenderer checks if the new LayerRendererChromium is different
by comparing the pointer value to the old one. This check will be incorrect if there are multiple lost
contexts between a setLayerRenderer() call and a new LayerRendererChromium happens to be allocated at the
same address as the old one. An explicit call to setLayerRenderer() for every layer in the tree fixes this
since when this function is called the new and old LayerRendererChromium are still live and so they cannot
live at the same address.

The first two changes are purely refactors and do not change behavior. The third issue can't be tested via a
layout test directly as we don't have a way to induce a lost context on the compositor in a layout test.

• platform/graphics/chromium/LayerRendererChromium.cpp: (WebCore::LayerRendererChromium::paintLayerContents): (WebCore::LayerRendererChromium::clearLayerRendererRecursive): (WebCore::LayerRendererChromium::transferRootLayer): (WebCore::LayerRendererChromium::updatePropertiesAndRenderSurfaces): (WebCore::LayerRendererChromium::updateCompositorResourcesRecursive):
• platform/graphics/chromium/LayerRendererChromium.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/chromium/LayerRendererChromium.cpp (modified) (4 diffs)
• platform/graphics/chromium/LayerRendererChromium.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-05-10  James Robinson  <jamesr@chromium.org>
+
+        Reviewed by Kenneth Russell.
+
+        [chromium] Clean up setLayerRenderer() calls in LayerRendererChromium
+        https://bugs.webkit.org/show_bug.cgi?id=60524
+
+        This refactors the way LayerRendererChromium calls setLayerRenderer on its layers in a few ways to clean things
+        up:
+
+        *) Move the pre-paint call to setLayerRenderer from updatePropertiesAndRenderSurfaces() to paintLayerContents().
+            updatePropertiesAndRenderSurfaces() does too much and the layer renderer pointer only has to be up to date to do
+            the actual paint. Longer term we won't need to call this at all in order to call paint and this call will just
+            go away.
+        *) Tweaks updateCompositorResourcesRecursive() to be slightly shorter and less error-prone.
+        *) Explicitly set the LayerRendererChromium for each layer in the tree when transfering the root layer (used
+            during lost context).  LayerChromium::setLayerRenderer checks if the new LayerRendererChromium is different
+            by comparing the pointer value to the old one.  This check will be incorrect if there are multiple lost
+            contexts between a setLayerRenderer() call and a new LayerRendererChromium happens to be allocated at the
+            same address as the old one.  An explicit call to setLayerRenderer() for every layer in the tree fixes this
+            since when this function is called the new and old LayerRendererChromium are still live and so they cannot
+            live at the same address.
+
+        The first two changes are purely refactors and do not change behavior.  The third issue can't be tested via a
+        layout test directly as we don't have a way to induce a lost context on the compositor in a layout test.
+
+        * platform/graphics/chromium/LayerRendererChromium.cpp:
+        (WebCore::LayerRendererChromium::paintLayerContents):
+        (WebCore::LayerRendererChromium::clearLayerRendererRecursive):
+        (WebCore::LayerRendererChromium::transferRootLayer):
+        (WebCore::LayerRendererChromium::updatePropertiesAndRenderSurfaces):
+        (WebCore::LayerRendererChromium::updateCompositorResourcesRecursive):
+        * platform/graphics/chromium/LayerRendererChromium.h:
+
 2011-05-10  Anton D'Auria  <adauria@apple.com>
 

trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp

@@ -366 +366 @@
                 continue;
 
+            layer->setLayerRenderer(this);
+            if (layer->maskLayer())
+                layer->maskLayer()->setLayerRenderer(this);
+            if (layer->replicaLayer()) {
+                layer->replicaLayer()->setLayerRenderer(this);
+                if (layer->replicaLayer()->maskLayer())
+                    layer->replicaLayer()->maskLayer()->setLayerRenderer(this);
+            }
+
             IntRect targetSurfaceRect = ccLayerImpl->targetRenderSurface() ? ccLayerImpl->targetRenderSurface()->contentRect() : m_defaultRenderSurface->contentRect();
             IntRect scissorRect = layer->ccLayerImpl()->scissorRect();
@@ -484 +493 @@
         m_rootLayer->setLayerRenderer(this);
     m_rootLayerContentTiler->invalidateEntireLayer();
+}
+
+void LayerRendererChromium::setLayerRendererRecursive(LayerChromium* layer)
+{
+    const Vector<RefPtr<LayerChromium> >& children = layer->children();
+    for (size_t i = 0; i < children.size(); ++i)
+        setLayerRendererRecursive(children[i].get());
+
+    if (layer->maskLayer())
+        setLayerRendererRecursive(layer->maskLayer());
+    if (layer->replicaLayer())
+        setLayerRendererRecursive(layer->replicaLayer());
+
+    layer->setLayerRenderer(this);
+}
+
+void LayerRendererChromium::transferRootLayer(LayerRendererChromium* other)
+{
+    other->setLayerRendererRecursive(m_rootLayer.get());
+    other->m_rootLayer = m_rootLayer.release();
 }
 
@@ -545 +574 @@
     // Make sure we have CCLayerImpls for this subtree.
     layer->createCCLayerImplIfNeeded();
-    layer->setLayerRenderer(this);
-    if (layer->maskLayer()) {
+    if (layer->maskLayer())
         layer->maskLayer()->createCCLayerImplIfNeeded();
-        layer->maskLayer()->setLayerRenderer(this);
-    }
-    if (layer->replicaLayer()) {
+    if (layer->replicaLayer())
         layer->replicaLayer()->createCCLayerImplIfNeeded();
-        layer->replicaLayer()->setLayerRenderer(this);
-    }
-    if (layer->replicaLayer() && layer->replicaLayer()->maskLayer()) {
+    if (layer->replicaLayer() && layer->replicaLayer()->maskLayer())
         layer->replicaLayer()->maskLayer()->createCCLayerImplIfNeeded();
-        layer->replicaLayer()->maskLayer()->setLayerRenderer(this);
-    }
 
     CCLayerImpl* drawLayer = layer->ccLayerImpl();
@@ -813 +835 @@
         return;
 
+    if (layer->maskLayer())
+        updateCompositorResourcesRecursive(layer->maskLayer());
+    if (layer->replicaLayer())
+        updateCompositorResourcesRecursive(layer->replicaLayer());
+
     CCLayerImpl* drawLayer = layer->ccLayerImpl();
 
     if (drawLayer->drawsContent())
         drawLayer->updateCompositorResources();
-    if (drawLayer->maskLayer() && drawLayer->maskLayer()->drawsContent())
-        drawLayer->maskLayer()->updateCompositorResources();
-    if (drawLayer->replicaLayer() && drawLayer->replicaLayer()->drawsContent())
-        drawLayer->replicaLayer()->updateCompositorResources();
-    if (drawLayer->replicaLayer() && drawLayer->replicaLayer()->maskLayer() && drawLayer->replicaLayer()->maskLayer()->drawsContent())
-        drawLayer->replicaLayer()->maskLayer()->updateCompositorResources();
 
     layer->pushPropertiesTo(drawLayer);

trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h

@@ -92 +92 @@
     void setRootLayer(PassRefPtr<LayerChromium>);
     LayerChromium* rootLayer() { return m_rootLayer.get(); }
-    void transferRootLayer(LayerRendererChromium* other) { other->m_rootLayer = m_rootLayer.release(); }
+    void transferRootLayer(LayerRendererChromium* other);
 
     bool hardwareCompositing() const { return m_hardwareCompositing; }
@@ -173 +173 @@
     bool initializeSharedObjects();
     void cleanupSharedObjects();
+
+    void setLayerRendererRecursive(LayerChromium*);
 
     IntRect m_viewportVisibleRect;