[chromium] Ensure framebuffer exists at the start of beginDrawingFrame.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Mar 2012 20:36:02 +0000 (20:36 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Mar 2012 20:36:02 +0000 (20:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=82569

Patch by Michal Mocny <mmocny@google.com> on 2012-03-29
Reviewed by James Robinson.

Source/WebCore:

Updated LayerRendererChromiumTest unittests.

* platform/graphics/chromium/LayerRendererChromium.cpp:
(WebCore::LayerRendererChromium::setVisible):
(WebCore::LayerRendererChromium::beginDrawingFrame):
* platform/graphics/chromium/LayerRendererChromium.h:
* platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:
(WebCore::CCSingleThreadProxy::compositeAndReadback):
* platform/graphics/chromium/cc/CCThreadProxy.cpp:
(WebCore::CCThreadProxy::compositeAndReadback):
(WebCore::CCThreadProxy::requestReadbackOnImplThread):

Source/WebKit/chromium:

* tests/LayerRendererChromiumTest.cpp:
(FakeLayerRendererChromiumClient::FakeLayerRendererChromiumClient):
(FakeLayerRendererChromiumClient::rootLayer):
(FakeLayerRendererChromiumClient):
(TEST_F):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h
Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp
Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/LayerRendererChromiumTest.cpp

index 6c1876f..0ee5e53 100644 (file)
@@ -1,3 +1,22 @@
+2012-03-29  Michal Mocny  <mmocny@google.com>
+
+        [chromium] Ensure framebuffer exists at the start of beginDrawingFrame.
+        https://bugs.webkit.org/show_bug.cgi?id=82569
+
+        Reviewed by James Robinson.
+
+        Updated LayerRendererChromiumTest unittests.
+
+        * platform/graphics/chromium/LayerRendererChromium.cpp:
+        (WebCore::LayerRendererChromium::setVisible):
+        (WebCore::LayerRendererChromium::beginDrawingFrame):
+        * platform/graphics/chromium/LayerRendererChromium.h:
+        * platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:
+        (WebCore::CCSingleThreadProxy::compositeAndReadback):
+        * platform/graphics/chromium/cc/CCThreadProxy.cpp:
+        (WebCore::CCThreadProxy::compositeAndReadback):
+        (WebCore::CCThreadProxy::requestReadbackOnImplThread):
+
 2012-03-29  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add a compile assert for the size of RenderBlock
index 4656934..ccb1dd8 100644 (file)
@@ -350,10 +350,6 @@ void LayerRendererChromium::setVisible(bool visible)
     if (!visible)
         releaseRenderSurfaceTextures();
 
-    // FIXME: Remove this once framebuffer is automatically recreated on first use
-    if (visible)
-        ensureFramebuffer();
-
     // TODO: Replace setVisibilityCHROMIUM with an extension to explicitly manage front/backbuffers
     // crbug.com/116049
     if (m_capabilities.usingSetVisibility) {
@@ -406,6 +402,10 @@ void LayerRendererChromium::clearRenderSurface(CCRenderSurface* renderSurface, C
 void LayerRendererChromium::beginDrawingFrame()
 {
     ASSERT(rootLayer());
+
+    // FIXME: Remove this once framebuffer is automatically recreated on first use
+    ensureFramebuffer();
+
     m_defaultRenderSurface = rootLayer()->renderSurface();
 
     // FIXME: use the frame begin time from the overall compositor scheduler.
index 59b8ff8..15a62f6 100644 (file)
@@ -61,6 +61,7 @@ class CCTextureDrawQuad;
 class GeometryBinding;
 class GraphicsContext3D;
 class LayerRendererSwapBuffersCompleteCallbackAdapter;
+class LayerRendererGpuMemoryAllocationChangedCallbackAdapter;
 class ScopedEnsureFramebufferAllocation;
 
 class LayerRendererChromiumClient {
@@ -163,11 +164,12 @@ public:
                           float width, float height, float opacity, const FloatQuad&,
                           int matrixLocation, int alphaLocation, int quadLocation);
 
+protected:
+    friend class LayerRendererGpuMemoryAllocationChangedCallbackAdapter;
     void discardFramebuffer();
     void ensureFramebuffer();
     bool isFramebufferDiscarded() const { return m_isFramebufferDiscarded; }
 
-protected:
     LayerRendererChromium(LayerRendererChromiumClient*, PassRefPtr<GraphicsContext3D>);
     bool initialize();
 
@@ -266,33 +268,6 @@ private:
     bool m_isFramebufferDiscarded;
 };
 
-// The purpose of this helper is twofold:
-// 1. To ensure that a framebuffer is available for scope lifetime, and
-// 2. To reset framebuffer allocation to previous state on scope exit.
-// If the framebuffer is recreated, its contents are undefined.
-// FIXME: Prevent/delay discarding framebuffer via any means while any
-// instance of this is alive. At the moment, this isn't an issue.
-class ScopedEnsureFramebufferAllocation {
-public:
-    explicit ScopedEnsureFramebufferAllocation(LayerRendererChromium* layerRenderer)
-        : m_layerRenderer(layerRenderer)
-        , m_framebufferWasInitiallyDiscarded(layerRenderer->isFramebufferDiscarded())
-    {
-        if (m_framebufferWasInitiallyDiscarded)
-            m_layerRenderer->ensureFramebuffer();
-    }
-
-    ~ScopedEnsureFramebufferAllocation()
-    {
-        if (m_framebufferWasInitiallyDiscarded)
-            m_layerRenderer->discardFramebuffer();
-    }
-
-private:
-    LayerRendererChromium* m_layerRenderer;
-    bool m_framebufferWasInitiallyDiscarded;
-};
-
 
 // Setting DEBUG_GL_CALLS to 1 will call glGetError() after almost every GL
 // call made by the compositor. Useful for debugging rendering issues but
index 1b41f7e..ec8428d 100644 (file)
@@ -72,8 +72,6 @@ bool CCSingleThreadProxy::compositeAndReadback(void *pixels, const IntRect& rect
     TRACE_EVENT("CCSingleThreadProxy::compositeAndReadback", this, 0);
     ASSERT(CCProxy::isMainThread());
 
-    ScopedEnsureFramebufferAllocation ensureFramebuffer(m_layerTreeHostImpl->layerRenderer());
-
     if (!commitIfNeeded())
         return false;
 
index 055ca08..c11f464 100644 (file)
@@ -93,8 +93,6 @@ bool CCThreadProxy::compositeAndReadback(void *pixels, const IntRect& rect)
     ASSERT(isMainThread());
     ASSERT(m_layerTreeHost);
 
-    ScopedEnsureFramebufferAllocation ensureFramebuffer(m_layerTreeHostImpl->layerRenderer());
-
     if (!m_layerRendererInitialized) {
         TRACE_EVENT("compositeAndReadback_EarlyOut_LR_Uninitialized", this, 0);
         return false;
@@ -125,6 +123,7 @@ void CCThreadProxy::requestReadbackOnImplThread(ReadbackRequest* request)
         request->completion.signal();
         return;
     }
+
     m_readbackRequestOnImplThread = request;
     m_schedulerOnImplThread->setNeedsRedraw();
     m_schedulerOnImplThread->setNeedsForcedRedraw();
index 1249af2..9aa7fe6 100644 (file)
@@ -1,3 +1,16 @@
+2012-03-29  Michal Mocny  <mmocny@google.com>
+
+        [chromium] Ensure framebuffer exists at the start of beginDrawingFrame.
+        https://bugs.webkit.org/show_bug.cgi?id=82569
+
+        Reviewed by James Robinson.
+
+        * tests/LayerRendererChromiumTest.cpp:
+        (FakeLayerRendererChromiumClient::FakeLayerRendererChromiumClient):
+        (FakeLayerRendererChromiumClient::rootLayer):
+        (FakeLayerRendererChromiumClient):
+        (TEST_F):
+
 2012-03-29  Adam Barth  <abarth@webkit.org>
 
         Move createURLLoader() into Platform
index 451acea..49a3618 100644 (file)
@@ -63,13 +63,17 @@ private:
 
 class FakeLayerRendererChromiumClient : public LayerRendererChromiumClient {
 public:
-    FakeLayerRendererChromiumClient() : m_setFullRootLayerDamageCount(0) { }
+    FakeLayerRendererChromiumClient()
+        : m_setFullRootLayerDamageCount(0)
+        , m_rootLayer(CCLayerImpl::create(1))
+    {
+    }
 
     // LayerRendererChromiumClient methods.
     virtual const IntSize& viewportSize() const { static IntSize fakeSize; return fakeSize; }
     virtual const CCSettings& settings() const { static CCSettings fakeSettings; return fakeSettings; }
-    virtual CCLayerImpl* rootLayer() { return 0; }
-    virtual const CCLayerImpl* rootLayer() const { return 0; }
+    virtual CCLayerImpl* rootLayer() { return m_rootLayer.get(); }
+    virtual const CCLayerImpl* rootLayer() const { return m_rootLayer.get(); }
     virtual void didLoseContext() { }
     virtual void onSwapBuffersComplete() { }
     virtual void setFullRootLayerDamage() { m_setFullRootLayerDamageCount++; }
@@ -80,6 +84,7 @@ public:
 private:
     int m_setFullRootLayerDamageCount;
     DebugScopedSetImplThread m_implThread;
+    OwnPtr<CCLayerImpl> m_rootLayer;
 };
 
 class FakeLayerRendererChromium : public LayerRendererChromium {
@@ -90,6 +95,7 @@ public:
 
     // Changing visibility to public.
     using LayerRendererChromium::initialize;
+    using LayerRendererChromium::isFramebufferDiscarded;
 };
 
 class LayerRendererChromiumTest : public testing::Test {
@@ -178,51 +184,17 @@ TEST_F(LayerRendererChromiumTest, SwapBuffersWhileBackbufferDiscardedShouldIgnor
 }
 
 // Test LayerRendererChromium discardFramebuffer functionality:
-// Discard framebuffer, then set visibility to true.
-// Expected: recreates the framebuffer.
-TEST_F(LayerRendererChromiumTest, SetVisibilityTrueShouldRecreateBackbuffer)
-{
-    m_mockContext.setMemoryAllocation(m_suggestHaveBackbufferNo);
-    EXPECT_TRUE(m_layerRendererChromium.isFramebufferDiscarded());
-
-    m_layerRendererChromium.setVisible(true);
-    EXPECT_FALSE(m_layerRendererChromium.isFramebufferDiscarded());
-}
-
-// Test LayerRendererChromium discardFramebuffer functionality:
-// Create a ScopedEnsureFramebufferAllocation while a framebuffer was discarded and then swapBuffers.
-// Expected: will recreate framebuffer scope duration, during which swaps will work fine, and discard on scope exit.
+// Begin drawing a frame while a framebuffer is discarded.
+// Expected: will recreate framebuffer.
 TEST_F(LayerRendererChromiumTest, DiscardedBackbufferIsRecreatredForScopeDuration)
 {
     m_mockContext.setMemoryAllocation(m_suggestHaveBackbufferNo);
     EXPECT_TRUE(m_layerRendererChromium.isFramebufferDiscarded());
     EXPECT_EQ(1, m_mockClient.setFullRootLayerDamageCount());
-    {
-        ScopedEnsureFramebufferAllocation ensureFramebuffer(&m_layerRendererChromium);
-        EXPECT_FALSE(m_layerRendererChromium.isFramebufferDiscarded());
 
-        swapBuffers();
-        EXPECT_EQ(1, m_mockContext.frameCount());
-    }
-    EXPECT_TRUE(m_layerRendererChromium.isFramebufferDiscarded());
-    EXPECT_EQ(2, m_mockClient.setFullRootLayerDamageCount());
-}
-
-// Test LayerRendererChromium discardFramebuffer functionality:
-// Create a ScopedEnsureFramebufferAllocation while a framebuffer was not discarded and then swapBuffers.
-// Expected: will have no effect.
-TEST_F(LayerRendererChromiumTest, EnsuringBackbufferForScopeDurationDoesNothingIfAlreadyExists)
-{
+    m_layerRendererChromium.beginDrawingFrame();
     EXPECT_FALSE(m_layerRendererChromium.isFramebufferDiscarded());
-    EXPECT_EQ(0, m_mockClient.setFullRootLayerDamageCount());
-    {
-        ScopedEnsureFramebufferAllocation ensureFramebuffer(&m_layerRendererChromium);
-        EXPECT_FALSE(m_layerRendererChromium.isFramebufferDiscarded());
-        EXPECT_EQ(0, m_mockClient.setFullRootLayerDamageCount());
 
-        swapBuffers();
-        EXPECT_EQ(1, m_mockContext.frameCount());
-    }
-    EXPECT_FALSE(m_layerRendererChromium.isFramebufferDiscarded());
-    EXPECT_EQ(0, m_mockClient.setFullRootLayerDamageCount());
+    swapBuffers();
+    EXPECT_EQ(1, m_mockContext.frameCount());
 }