Mailbox support for texture layers.

cc/texture_layer_impl.cc

Index: cc/texture_layer_impl.cc
diff --git a/cc/texture_layer_impl.cc b/cc/texture_layer_impl.cc
index 433ce34256e7260b5bdf10b9c573d56420e729d2..57d87aafdbd21d61ada84fd9bba94a9050e32406 100644
--- a/cc/texture_layer_impl.cc
+++ b/cc/texture_layer_impl.cc
@@ -5,19 +5,22 @@
 #include "cc/texture_layer_impl.h"
 
 #include "base/stringprintf.h"
+#include "cc/layer_tree_impl.h"
 #include "cc/quad_sink.h"
 #include "cc/renderer.h"
 #include "cc/texture_draw_quad.h"
 
 namespace cc {
 
-TextureLayerImpl::TextureLayerImpl(LayerTreeImpl* treeImpl, int id)
+TextureLayerImpl::TextureLayerImpl(LayerTreeImpl* treeImpl, int id, bool mailboxLayer)
     : LayerImpl(treeImpl, id)
     , m_textureId(0)
     , m_externalTextureResource(0)
     , m_premultipliedAlpha(true)
     , m_flipped(true)
     , m_uvRect(0, 0, 1, 1)
+    , m_newMailboxPending(false)
+    , m_mailboxLayer(mailboxLayer)
 {
   m_vertexOpacity[0] = 1.0f;
   m_vertexOpacity[1] = 1.0f;
@@ -27,10 +30,31 @@ TextureLayerImpl::TextureLayerImpl(LayerTreeImpl* treeImpl, int id)
 
 TextureLayerImpl::~TextureLayerImpl()
 {
+    if (m_externalTextureResource) {
+        ResourceProvider* provider = layerTreeImpl()->resource_provider();

[danakj, 2013/01/02 16:17:22]

DCHECK() that we're a mailbox layer here?

+        DCHECK(!provider->inUseByConsumer(m_externalTextureResource));

[danakj, 2013/01/02 16:17:22]

This will work for now, but puts us in the same position overall for ubercomp.
The texture in the impl layer may indeed be in use by the consumer when the
layer disappears. What then?

[alexst (slow to review), 2013/01/02 19:31:35]

Since callback is stored on the resource, it could be executed by the consumer
when it's done. This would become an if statement instead once that usage case
is active.

On 2013/01/02 16:17:22, danakj wrote:

[danakj, 2013/01/02 19:43:28]

Will that require the consumer to know that this layer is gone? Or are you
thinking that all resources are freed by the browser process once it's done with
them?

I think that's going to be problematic because we ship contents textures to the
browser as well. And, while it could differentiate, I'd rather they came back to
the renderer and it freed them. (Also keeps it in the same process.)

So, I think that the freeing responsibilities here should move to something else
that lives on the LTHImpl and would outlive this layer.

+        provider->deleteResource(m_externalTextureResource);
+    }
+    if (m_newMailboxPending && m_mailboxName.size())
+        m_mailboxReleaseCallback.Run(0);
+}
+
+void TextureLayerImpl::setTextureMailbox(const std::string& mailboxName, const base::Callback<void(unsigned)>& releaseCallback)
+{
+    // Same mailbox name was commited, nothing to do
+    if (m_mailboxName.compare(mailboxName) == 0)
+        return;
+    m_mailboxName = mailboxName;
+    m_newMailboxPending = true;
+    m_mailboxReleaseCallback = releaseCallback;

[danakj, 2013/01/02 16:17:22]

can we prefix all these things with something? like
m_pendingMailboxReleaseCallback, m_pendingMailboxName, m_havePendingMailbox?

[alexst (slow to review), 2013/01/02 19:31:35]

Done.

 }
 
 void TextureLayerImpl::willDraw(ResourceProvider* resourceProvider)
 {
+    if (m_mailboxLayer) {
+        handleMailboxResources(resourceProvider);

[danakj, 2013/01/02 16:17:22]

nit: prefer to keep all the logic in here as there's only one caller for
handleMailboxResources.

Since that's the path forward, maybe consider making the current (pretty short)
code an early out if (!m_usesMailbox) {} and then you don't need to nest all the
new code at all.

[alexst (slow to review), 2013/01/02 19:31:35]

Done.

+        return;
+    }
     if (!m_textureId)
         return;
     DCHECK(!m_externalTextureResource);
@@ -59,6 +83,8 @@ void TextureLayerImpl::appendQuads(QuadSink& quadSink, AppendQuadsData& appendQu
 
 void TextureLayerImpl::didDraw(ResourceProvider* resourceProvider)
 {
+    if (m_mailboxLayer)
+        return;
     if (!m_externalTextureResource)
         return;
     // FIXME: the following assert will not be true when sending resources to a
@@ -94,4 +120,19 @@ const char* TextureLayerImpl::layerTypeAsString() const
     return "TextureLayer";
 }
 
+void TextureLayerImpl::handleMailboxResources(ResourceProvider* resourceProvider)
+{
+    if (!m_newMailboxPending)
+        return;
+
+    if (m_externalTextureResource) {
+        DCHECK(!resourceProvider->inUseByConsumer(m_externalTextureResource));
+        resourceProvider->deleteResource(m_externalTextureResource);
+        m_externalTextureResource = 0;
+    }
+    if (m_mailboxName.size())
+        m_externalTextureResource = resourceProvider->createResourceFromTextureMailbox(m_mailboxName, m_mailboxReleaseCallback);
+    m_newMailboxPending = false;
+}
+
 }  // namespace cc