gpu: Change Produce/ConsumeTexture to allow texture sharing

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 3b6c8aa76fd029f9ffce9d809ec20199fa0e5a06..06afab0ec2b303e3045bc2c99e8a18f8bbe11112 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -52,7 +52,6 @@
 #include "gpu/command_buffer/service/shader_translator_cache.h"
 #include "gpu/command_buffer/service/stream_texture.h"
 #include "gpu/command_buffer/service/stream_texture_manager.h"
-#include "gpu/command_buffer/service/texture_definition.h"
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "gpu/command_buffer/service/vertex_attrib_manager.h"
 #include "gpu/command_buffer/service/vertex_array_manager.h"
@@ -2261,25 +2260,6 @@ bool GLES2DecoderImpl::Initialize(
   if (!attrib_parser.Parse(attribs))
     return false;
 
-  // These are NOT if the back buffer has these proprorties. They are
-  // if we want the command buffer to enforce them regardless of what
-  // the real backbuffer is assuming the real back buffer gives us more than
-  // we ask for. In other words, if we ask for RGB and we get RGBA then we'll
-  // make it appear RGB. If on the other hand we ask for RGBA nd get RGB we
-  // can't do anything about that.
-
-  GLint v = 0;
-  glGetIntegerv(GL_ALPHA_BITS, &v);
-  // This checks if the user requested RGBA and we have RGBA then RGBA. If the
-  // user requested RGB then RGB. If the user did not specify a preference than
-  // use whatever we were given. Same for DEPTH and STENCIL.
-  back_buffer_color_format_ =
-      (attrib_parser.alpha_size_ != 0 && v > 0) ? GL_RGBA : GL_RGB;
-  glGetIntegerv(GL_DEPTH_BITS, &v);
-  back_buffer_has_depth_ = attrib_parser.depth_size_ != 0 && v > 0;
-  glGetIntegerv(GL_STENCIL_BITS, &v);
-  back_buffer_has_stencil_ = attrib_parser.stencil_size_ != 0 && v > 0;
-
   if (offscreen) {
     if (attrib_parser.samples_ > 0 && attrib_parser.sample_buffers_ > 0 &&
         features().chromium_framebuffer_multisample) {
@@ -2408,6 +2388,26 @@ bool GLES2DecoderImpl::Initialize(
     // Bind to the new default frame buffer (the offscreen target frame buffer).
     // This should now be associated with ID zero.
     DoBindFramebuffer(GL_FRAMEBUFFER, 0);
+  } else {
+    glBindFramebufferEXT(GL_FRAMEBUFFER, GetBackbufferServiceId());
+    // These are NOT if the back buffer has these proprorties. They are
+    // if we want the command buffer to enforce them regardless of what
+    // the real backbuffer is assuming the real back buffer gives us more than
+    // we ask for. In other words, if we ask for RGB and we get RGBA then we'll
+    // make it appear RGB. If on the other hand we ask for RGBA nd get RGB we
+    // can't do anything about that.
+
+    GLint v = 0;
+    glGetIntegerv(GL_ALPHA_BITS, &v);
+    // This checks if the user requested RGBA and we have RGBA then RGBA. If the
+    // user requested RGB then RGB. If the user did not specify a preference
+    // than use whatever we were given. Same for DEPTH and STENCIL.
+    back_buffer_color_format_ =
+        (attrib_parser.alpha_size_ != 0 && v > 0) ? GL_RGBA : GL_RGB;
+    glGetIntegerv(GL_DEPTH_BITS, &v);
+    back_buffer_has_depth_ = attrib_parser.depth_size_ != 0 && v > 0;
+    glGetIntegerv(GL_STENCIL_BITS, &v);
+    back_buffer_has_stencil_ = attrib_parser.stencil_size_ != 0 && v > 0;
   }
 
   // OpenGL ES 2.0 implicitly enables the desktop GL capability
@@ -9855,8 +9855,8 @@ void GLES2DecoderImpl::DoProduceTextureCHROMIUM(GLenum target,
     return;
   }
 
-  TextureDefinition* definition = texture_manager()->Save(texture_ref);
-  if (!definition) {
+  Texture* produced = texture_manager()->Produce(texture_ref);
+  if (!produced) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION,
         "glProduceTextureCHROMIUM", "invalid texture");
@@ -9866,18 +9866,12 @@ void GLES2DecoderImpl::DoProduceTextureCHROMIUM(GLenum target,
   if (!group_->mailbox_manager()->ProduceTexture(
       target,
       *reinterpret_cast<const MailboxName*>(mailbox),
-      definition,
-      texture_manager())) {
-    bool success = texture_manager()->Restore(
-        "glProductTextureCHROMIUM", this, texture_ref, definition);
-    DCHECK(success);
+      produced)) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION,
         "glProduceTextureCHROMIUM", "invalid mailbox name");
     return;
   }
-
-  glBindTexture(texture_ref->texture()->target(), texture_ref->service_id());
 }
 
 void GLES2DecoderImpl::DoConsumeTextureCHROMIUM(GLenum target,
@@ -9886,32 +9880,75 @@ void GLES2DecoderImpl::DoConsumeTextureCHROMIUM(GLenum target,
       "context", logger_.GetLogPrefix(),
       "mailbox[0]", static_cast<unsigned char>(mailbox[0]));
 
-  TextureRef* texture_ref = GetTextureInfoForTarget(target);
+  scoped_refptr<TextureRef> texture_ref =
+      GetTextureInfoForTargetUnlessDefault(target);
   if (!texture_ref) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION,
         "glConsumeTextureCHROMIUM", "unknown texture for target");
     return;
   }
-
-  scoped_ptr<TextureDefinition> definition(
+  if (texture_ref->texture()->IsAttachedToFramebuffer()) {
+    LOCAL_SET_GL_ERROR(
+        GL_INVALID_OPERATION,
+        "glConsumeTextureCHROMIUM", "texture attached to framebuffer");
+    return;
+  }
+  GLuint client_id = texture_ref->client_id();
+  if (!client_id) {
+    LOCAL_SET_GL_ERROR(
+        GL_INVALID_OPERATION,
+        "glConsumeTextureCHROMIUM", "unknown texture for target");
+    return;
+  }
+  Texture* texture =
       group_->mailbox_manager()->ConsumeTexture(
       target,
-      *reinterpret_cast<const MailboxName*>(mailbox)));
-  if (!definition.get()) {
+      *reinterpret_cast<const MailboxName*>(mailbox));
+  if (!texture) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION,
         "glConsumeTextureCHROMIUM", "invalid mailbox name");
     return;
   }
-
-  if (!texture_manager()->Restore(
-      "glConsumeTextureCHROMIUM", this, texture_ref, definition.release())) {
+  if (texture->target() != target) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION,
-        "glConsumeTextureCHROMIUM", "invalid texture");
+        "glConsumeTextureCHROMIUM", "invalid target");
     return;
   }
+
+  // TODO(piman): Instead of deleting and recreating the texture, we could
+  // change the Texture inside the TextureRef. We need crbug.com/240504 to be

[greggman, 2013/05/24 17:51:54]

I take it back. This code is fine as is. No need for the TODO. The way it is is
how GL works. Sorry for the confusion.  And, I don't think you need the check
above that it's not attached to an fbo.

Do we need a check that you don't mail something to yourself so you end up with
2 textures with the same service id in one texture manager?

[piman, 2013/05/24 23:03:27]

Done.

+  // fixed first, and we'll need to notify all the decoder in the context group
+  // that they need to rebind the current texture in all units when they're
+  // made current again (since the service id for the TextureRef will have
+  // changed). We currently disallow Consume on a texture bound to an FBO, but
+  // to remove that limitation, we'll need a way to notify them to reattach the
+  // texture when they're made current as well.
+  DeleteTexturesHelper(1, &client_id);
+  texture_ref = texture_manager()->Consume(client_id, texture);
+  glBindTexture(target, texture_ref->service_id());
+
+  TextureUnit& unit = state_.texture_units[state_.active_texture_unit];
+  unit.bind_target = target;
+  switch (target) {
+    case GL_TEXTURE_2D:
+      unit.bound_texture_2d = texture_ref;
+      break;
+    case GL_TEXTURE_CUBE_MAP:
+      unit.bound_texture_cube_map = texture_ref;
+      break;
+    case GL_TEXTURE_EXTERNAL_OES:
+      unit.bound_texture_external_oes = texture_ref;
+      break;
+    case GL_TEXTURE_RECTANGLE_ARB:
+      unit.bound_texture_rectangle_arb = texture_ref;
+      break;
+    default:
+      NOTREACHED();  // Validation should prevent us getting here.
+      break;
+  }
 }
 
 void GLES2DecoderImpl::DoInsertEventMarkerEXT(