gpu: Change Produce/ConsumeTexture to allow texture sharing

content/common/gpu/texture_image_transport_surface.cc

Index: content/common/gpu/texture_image_transport_surface.cc
diff --git a/content/common/gpu/texture_image_transport_surface.cc b/content/common/gpu/texture_image_transport_surface.cc
index 858c023f700b9aaa2ba35f74f83cf4bdccfe7b27..1e43e95566858f215cdb0138ea93aeb8dffc01ed 100644
--- a/content/common/gpu/texture_image_transport_surface.cc
+++ b/content/common/gpu/texture_image_transport_surface.cc
@@ -18,10 +18,12 @@
 #include "ui/gl/scoped_binders.h"
 
 using gpu::gles2::ContextGroup;
+using gpu::gles2::GLES2Decoder;
 using gpu::gles2::MailboxManager;
 using gpu::gles2::MailboxName;
-using gpu::gles2::TextureDefinition;
+using gpu::gles2::Texture;
 using gpu::gles2::TextureManager;
+using gpu::gles2::TextureRef;
 
 namespace content {
 
@@ -30,7 +32,7 @@ TextureImageTransportSurface::TextureImageTransportSurface(
     GpuCommandBufferStub* stub,
     const gfx::GLSurfaceHandle& handle)
       : fbo_id_(0),
-        backbuffer_(CreateTextureDefinition(gfx::Size(), 0)),
+        current_size_(1, 1),
         stub_destroyed_(false),
         backbuffer_suggested_allocation_(true),
         frontbuffer_suggested_allocation_(true),
@@ -91,41 +93,18 @@ bool TextureImageTransportSurface::DeferDraws() {
   return false;
 }
 
-bool TextureImageTransportSurface::Resize(const gfx::Size&) {
-  return true;
-}
-
 bool TextureImageTransportSurface::IsOffscreen() {
   return true;
 }
 
-bool TextureImageTransportSurface::OnMakeCurrent(gfx::GLContext* context) {
-  if (stub_destroyed_) {
-    // Early-exit so that we don't recreate the fbo. We still want to return
-    // true, so that the context is made current and the GLES2DecoderImpl can
-    // release its own resources.
-    return true;
-  }
-
-  context_ = context;
-
+unsigned int TextureImageTransportSurface::GetBackingFrameBufferObject() {
   if (!fbo_id_) {
     glGenFramebuffersEXT(1, &fbo_id_);
     glBindFramebufferEXT(GL_FRAMEBUFFER, fbo_id_);
-    current_size_ = gfx::Size(1, 1);
     helper_->stub()->AddDestructionObserver(this);
-  }
-
-  // We could be receiving non-deferred GL commands, that is anything that does
-  // not need a framebuffer.
-  if (!backbuffer_->service_id() && !is_swap_buffers_pending_ &&
-      backbuffer_suggested_allocation_) {
     CreateBackTexture();

[piman, 2013/05/23 02:33:01]

note: Construction moved to GetBackingFrameBufferObject because
CreateBackTexture needs the TextureManager to be created (to be able to create
the TextureRef), but the first MakeCurrent happens before that.

   }
-  return true;
-}
 
-unsigned int TextureImageTransportSurface::GetBackingFrameBufferObject() {
   return fbo_id_;
 }
 
@@ -136,7 +115,7 @@ bool TextureImageTransportSurface::SetBackbufferAllocation(bool allocation) {
   backbuffer_suggested_allocation_ = allocation;
 
   if (backbuffer_suggested_allocation_) {
-    DCHECK(!backbuffer_->service_id());
+    DCHECK(!backbuffer_);
     CreateBackTexture();
   } else {
     ReleaseBackTexture();
@@ -150,9 +129,15 @@ void TextureImageTransportSurface::SetFrontbufferAllocation(bool allocation) {
     return;
   frontbuffer_suggested_allocation_ = allocation;
 
-  if (!frontbuffer_suggested_allocation_) {
-    GpuHostMsg_AcceleratedSurfaceRelease_Params params;
-    helper_->SendAcceleratedSurfaceRelease(params);
+  // If a swapbuffers is in flight, wait for the ack before releasing the front
+  // buffer:
+  // - we don't know yet which texture the browser will want to keep
+  // - we want to ensure we don't destroy a texture that is in flight before the
+  // browser got a reference on it.
+  if (!frontbuffer_suggested_allocation_ &&
+      !is_swap_buffers_pending_ &&
+      helper_->MakeCurrent()) {
+    ReleaseFrontTexture();
   }
 }
 
@@ -169,6 +154,8 @@ void* TextureImageTransportSurface::GetConfig() {
 }
 
 void TextureImageTransportSurface::OnResize(gfx::Size size) {
+  DCHECK_GE(size.width(), 1);
+  DCHECK_GE(size.height(), 1);
   current_size_ = size;
   CreateBackTexture();
 }
@@ -176,13 +163,11 @@ void TextureImageTransportSurface::OnResize(gfx::Size size) {
 void TextureImageTransportSurface::OnWillDestroyStub() {
   helper_->stub()->RemoveDestructionObserver(this);
 
-  GpuHostMsg_AcceleratedSurfaceRelease_Params params;
-  helper_->SendAcceleratedSurfaceRelease(params);
-
-  ReleaseBackTexture();
-
   // We are losing the stub owning us, this is our last chance to clean up the
   // resources we allocated in the stub's context.
+  ReleaseBackTexture();
+  ReleaseFrontTexture();
+
   if (fbo_id_) {
     glDeleteFramebuffersEXT(1, &fbo_id_);
     CHECK_GL_ERROR();
@@ -203,7 +188,7 @@ bool TextureImageTransportSurface::SwapBuffers() {
   if (!frontbuffer_suggested_allocation_)
     return true;
 
-  if (!backbuffer_->service_id()) {
+  if (!backbuffer_) {
     LOG(ERROR) << "Swap without valid backing.";
     return true;
   }
@@ -212,14 +197,10 @@ bool TextureImageTransportSurface::SwapBuffers() {
   GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
   params.size = backbuffer_size();
   params.mailbox_name.assign(
-      reinterpret_cast<const char*>(&mailbox_name_), sizeof(mailbox_name_));
+      reinterpret_cast<const char*>(&back_mailbox_name_),
+      sizeof(back_mailbox_name_));
 
   glFlush();

[greggman, 2013/05/23 17:36:19]

This is probably a dumb question since I'm not clear on the entire flow but when
we get here we know this flush is for the correct context? Same on
ReleaseFrontTexture and ReleaseBackTexture

[piman, 2013/05/23 18:02:38]

SwapBuffers and SetBackbufferAllocation are called directly from the decoder, so
the right context is current. That's true of most calls here.

It's not true for a couple of them though: BufferPresentedImpl is called from a
message (or a posted task if we need to wait for the sync point), and
SetFrontbufferAllocation is called from a posted task, so they need to make the
context current.

[piman, 2013/05/24 02:32:22]

Added a bunch of DCHECKs in functions that call GL to make that more explicit.

-  ProduceTexture();
-
-  // Do not allow destruction while we are still waiting for a swap ACK,
-  // so we do not leak a texture in the mailbox.
-  AddRef();
 
   params.latency_info = latency_info_;
   helper_->SendAcceleratedSurfaceBuffersSwapped(params);
@@ -241,7 +222,7 @@ bool TextureImageTransportSurface::PostSubBuffer(
   if (new_damage_rect.IsEmpty())
     return true;
 
-  if (!backbuffer_->service_id()) {
+  if (!backbuffer_) {
     LOG(ERROR) << "Swap without valid backing.";
     return true;
   }
@@ -254,14 +235,10 @@ bool TextureImageTransportSurface::PostSubBuffer(
   params.width = width;
   params.height = height;
   params.mailbox_name.assign(
-      reinterpret_cast<const char*>(&mailbox_name_), sizeof(mailbox_name_));
+      reinterpret_cast<const char*>(&back_mailbox_name_),
+      sizeof(back_mailbox_name_));
 
   glFlush();
-  ProduceTexture();
-
-  // Do not allow destruction while we are still waiting for a swap ACK,
-  // so we do not leak a texture in the mailbox.
-  AddRef();
 
   params.latency_info = latency_info_;
   helper_->SendAcceleratedSurfacePostSubBuffer(params);
@@ -280,11 +257,7 @@ std::string TextureImageTransportSurface::GetExtensions() {
 }
 
 gfx::Size TextureImageTransportSurface::GetSize() {
-  gfx::Size size = current_size_;
-
-  // OSMesa expects a non-zero size.
-  return gfx::Size(size.width() == 0 ? 1 : size.width(),
-                   size.height() == 0 ? 1 : size.height());
+  return current_size_;
 }
 
 void* TextureImageTransportSurface::GetHandle() {
@@ -306,31 +279,22 @@ void TextureImageTransportSurface::OnBufferPresented(
                    this,
                    params.mailbox_name));
   }
-
-  // Careful, we might get deleted now if we were only waiting for
-  // a final swap ACK.
-  Release();
 }
 
 void TextureImageTransportSurface::BufferPresentedImpl(
     const std::string& mailbox_name) {
-  DCHECK(!backbuffer_->service_id());
-  if (!mailbox_name.empty()) {
+  bool swap = true;
+  if (backbuffer_ && !mailbox_name.empty()) {

[no sievers, 2013/05/24 01:31:38]

When would backbuffer_ ever be invalid here?
We can't have deallocated it while the ack was pending.

[piman, 2013/05/24 02:32:22]

I can't remember why I did this originally (I had several passes), but you're
right. I replaced it back to a DCHECK.

     DCHECK(mailbox_name.length() == GL_MAILBOX_SIZE_CHROMIUM);
-    mailbox_name.copy(reinterpret_cast<char *>(&mailbox_name_),
-                      sizeof(MailboxName));
-    ConsumeTexture();
-  }
 
-  if (stub_destroyed_ && backbuffer_->service_id()) {
-    // TODO(sievers): Remove this after changes to the mailbox to take ownership
-    // of the service ids.
-    DCHECK(context_.get() && surface_.get());
-    uint32 service_id = backbuffer_->ReleaseServiceId();
-    if (context_->MakeCurrent(surface_))
-      glDeleteTextures(1, &service_id);
-
-    return;
+    MailboxName name;
+    mailbox_name.copy(reinterpret_cast<char *>(&name), sizeof(MailboxName));
+    if (!memcmp(&name, &back_mailbox_name_, sizeof(name)))

[no sievers, 2013/05/24 01:31:38]

nit: maybe comment that the browser skipped/returned the frame in that case.

[piman, 2013/05/24 02:32:22]

Done. (also simplified a bit, I don't need to copy into a MailboxName to
compare).

+      swap = false;
+  }
+  if (swap) {
+    std::swap(backbuffer_, frontbuffer_);
+    std::swap(back_mailbox_name_, front_mailbox_name_);
   }
 
   DCHECK(is_swap_buffers_pending_);
@@ -341,10 +305,12 @@ void TextureImageTransportSurface::BufferPresentedImpl(
   DCHECK(backbuffer_suggested_allocation_);
 
   // We're relying on the fact that the parent context is
-  // finished with it's context when it inserts the sync point that
+  // finished with its context when it inserts the sync point that
   // triggers this callback.
   if (helper_->MakeCurrent()) {
-    if (backbuffer_size() != current_size_ || !backbuffer_->service_id())
+    if (frontbuffer_ && !frontbuffer_suggested_allocation_)
+      ReleaseFrontTexture();
+    if (!backbuffer_ || backbuffer_size() != current_size_)
       CreateBackTexture();
     else
       AttachBackTextureToFBO();
@@ -363,15 +329,19 @@ void TextureImageTransportSurface::OnResizeViewACK() {
 }
 
 void TextureImageTransportSurface::ReleaseBackTexture() {
-  if (!backbuffer_->service_id())
-    return;
+  backbuffer_ = NULL;
+  back_mailbox_name_ = MailboxName();
+  glFlush();
+  CHECK_GL_ERROR();
+}
 
-  uint32 service_id = backbuffer_->ReleaseServiceId();
-  glDeleteTextures(1, &service_id);
-  backbuffer_.reset(CreateTextureDefinition(gfx::Size(), 0));
-  mailbox_name_ = MailboxName();
+void TextureImageTransportSurface::ReleaseFrontTexture() {
+  frontbuffer_ = NULL;
+  front_mailbox_name_ = MailboxName();
   glFlush();
   CHECK_GL_ERROR();
+  GpuHostMsg_AcceleratedSurfaceRelease_Params params;
+  helper_->SendAcceleratedSurfaceRelease(params);
 }
 
 void TextureImageTransportSurface::CreateBackTexture() {
@@ -379,51 +349,67 @@ void TextureImageTransportSurface::CreateBackTexture() {
   // in the mailbox, so we shouldn't be reallocating it now.
   DCHECK(!is_swap_buffers_pending_);
 
-  if (backbuffer_->service_id() && backbuffer_size() == current_size_)
+  if (backbuffer_ && backbuffer_size() == current_size_)
     return;
 
-  uint32 service_id = backbuffer_->ReleaseServiceId();
-
   VLOG(1) << "Allocating new backbuffer texture";
 
   // On Qualcomm we couldn't resize an FBO texture past a certain
   // size, after we allocated it as 1x1. So here we simply delete
   // the previous texture on resize, to insure we don't 'run out of
   // memory'.
-  if (service_id &&
+  if (backbuffer_ &&
       helper_->stub()
              ->decoder()
              ->GetContextGroup()
              ->feature_info()
              ->workarounds()
              .delete_instead_of_resize_fbo) {
-    glDeleteTextures(1, &service_id);
-    service_id = 0;
-    mailbox_name_ = MailboxName();
+    ReleaseBackTexture();
   }
-
-  if (!service_id) {
-    MailboxName new_mailbox_name;
-    MailboxName& name = mailbox_name_;
-    // This slot should be uninitialized.
-    DCHECK(!memcmp(&name, &new_mailbox_name, sizeof(MailboxName)));
-    mailbox_manager_->GenerateMailboxName(&new_mailbox_name);
-    name = new_mailbox_name;
+  GLES2Decoder* decoder = helper_->stub()->decoder();
+  TextureManager* texture_manager =
+      decoder->GetContextGroup()->texture_manager();
+  if (!backbuffer_) {
+    mailbox_manager_->GenerateMailboxName(&back_mailbox_name_);
+    GLuint service_id;
     glGenTextures(1, &service_id);
+    backbuffer_ = TextureRef::Create(texture_manager, 0, service_id);
+    texture_manager->SetTarget(backbuffer_, GL_TEXTURE_2D);
+    Texture* texture = texture_manager->Produce(backbuffer_);
+    bool success = mailbox_manager_->ProduceTexture(
+        GL_TEXTURE_2D, back_mailbox_name_, texture);
+    DCHECK(success);
   }
 
-  backbuffer_.reset(
-      CreateTextureDefinition(current_size_, service_id));
-
   {
-    gfx::ScopedTextureBinder texture_binder(GL_TEXTURE_2D, service_id);
-    glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
-    glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
-    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
-    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    gfx::ScopedTextureBinder texture_binder(GL_TEXTURE_2D,
+                                            backbuffer_->service_id());
     glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA,
         current_size_.width(), current_size_.height(), 0,
         GL_RGBA, GL_UNSIGNED_BYTE, NULL);
+    gpu::gles2::ErrorState* error_state = decoder->GetErrorState();
+    texture_manager->SetParameter("Backbuffer", error_state, backbuffer_,
+                                  GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    texture_manager->SetParameter("Backbuffer", error_state, backbuffer_,
+                                  GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    texture_manager->SetParameter("Backbuffer", error_state, backbuffer_,
+                                  GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    texture_manager->SetParameter("Backbuffer", error_state, backbuffer_,
+                                  GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    texture_manager->SetLevelInfo(
+        backbuffer_,
+        GL_TEXTURE_2D,
+        0,
+        GL_RGBA,
+        current_size_.width(),
+        current_size_.height(),
+        1,
+        0,
+        GL_RGBA,
+        GL_UNSIGNED_BYTE,
+        true);
+    DCHECK(texture_manager->CanRender(backbuffer_));
     CHECK_GL_ERROR();
   }
 
@@ -448,56 +434,4 @@ void TextureImageTransportSurface::AttachBackTextureToFBO() {
 #endif
 }
 
-TextureDefinition* TextureImageTransportSurface::CreateTextureDefinition(
-    gfx::Size size, int service_id) {
-  TextureDefinition::LevelInfo info(
-      GL_TEXTURE_2D, GL_RGBA, size.width(), size.height(), 1,
-      0, GL_RGBA, GL_UNSIGNED_BYTE, true);
-
-  TextureDefinition::LevelInfos level_infos;
-  level_infos.resize(1);
-  level_infos[0].resize(1);
-  level_infos[0][0] = info;
-  return new TextureDefinition(
-      GL_TEXTURE_2D,
-      service_id,
-      GL_LINEAR,
-      GL_LINEAR,
-      GL_CLAMP_TO_EDGE,
-      GL_CLAMP_TO_EDGE,
-      GL_NONE,
-      true,
-      false,
-      level_infos);
-}
-
-void TextureImageTransportSurface::ConsumeTexture() {
-  DCHECK(!backbuffer_->service_id());
-
-  backbuffer_.reset(mailbox_manager_->ConsumeTexture(
-      GL_TEXTURE_2D, mailbox_name_));
-  if (!backbuffer_) {
-    mailbox_name_ = MailboxName();
-    backbuffer_.reset(CreateTextureDefinition(gfx::Size(), 0));
-  }
-}
-
-void TextureImageTransportSurface::ProduceTexture() {
-  DCHECK(backbuffer_->service_id());
-  DCHECK(!backbuffer_size().IsEmpty());
-
-  // Pass NULL as |owner| here to avoid errors from glConsumeTextureCHROMIUM()
-  // when the renderer context group goes away before the RWHV handles a pending
-  // ACK. We avoid leaking a texture in the mailbox by waiting for the final ACK
-  // at which point we consume the correct texture back.
-  bool success = mailbox_manager_->ProduceTexture(
-      GL_TEXTURE_2D,
-      mailbox_name_,
-      backbuffer_.release(),
-      NULL);
-  DCHECK(success);
-  mailbox_name_ = MailboxName();
-  backbuffer_.reset(CreateTextureDefinition(gfx::Size(), 0));
-}
-
 }  // namespace content