gpu: Change Produce/ConsumeTexture to allow texture sharing

gpu/command_buffer/service/texture_manager.cc

Index: gpu/command_buffer/service/texture_manager.cc
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index e2fac59a44cbcdbfc78b68ccf04e01ea89e91da3..95f1146b915a2b0ee6dfc7931fa1523f01f68dcc 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -12,7 +12,6 @@
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "gpu/command_buffer/service/mailbox_manager.h"
 #include "gpu/command_buffer/service/memory_tracking.h"
-#include "gpu/command_buffer/service/texture_definition.h"
 
 namespace gpu {
 namespace gles2 {
@@ -89,7 +88,8 @@ void TextureManager::Destroy(bool have_context) {
 }
 
 Texture::Texture(GLuint service_id)
-    : memory_tracking_ref_(NULL),
+    : mailbox_manager_(NULL),
+      memory_tracking_ref_(NULL),
       service_id_(service_id),
       cleared_(true),
       num_uncleared_mips_(0),
@@ -114,6 +114,8 @@ Texture::Texture(GLuint service_id)
 }
 
 Texture::~Texture() {
+  if (mailbox_manager_)
+    mailbox_manager_->TextureDeleted(this);
 }
 
 void Texture::AddTextureRef(TextureRef* ref) {
@@ -266,6 +268,11 @@ void Texture::AddToSignature(
       usage_);
 }
 
+void Texture::SetMailboxManager(MailboxManager* mailbox_manager) {
+  DCHECK(!mailbox_manager_ || mailbox_manager_ == mailbox_manager);
+  mailbox_manager_ = mailbox_manager;
+}
+
 bool Texture::MarkMipmapsGenerated(
     const FeatureInfo* feature_info) {
   if (!CanGenerateMipmaps(feature_info)) {
@@ -794,9 +801,12 @@ gfx::GLImage* Texture::GetLevelImage(
 }
 
 
-TextureRef::TextureRef(TextureManager* manager, Texture* texture)
+TextureRef::TextureRef(TextureManager* manager,
+                       GLuint client_id,
+                       Texture* texture)
     : manager_(manager),
-      texture_(texture) {
+      texture_(texture),
+      client_id_(client_id) {
   DCHECK(manager_);
   DCHECK(texture_);
   texture_->AddTextureRef(this);
@@ -804,8 +814,9 @@ TextureRef::TextureRef(TextureManager* manager, Texture* texture)
 }
 
 scoped_refptr<TextureRef> TextureRef::Create(TextureManager* manager,
+                                             GLuint client_id,
                                              GLuint service_id) {
-  return new TextureRef(manager, new Texture(service_id));
+  return new TextureRef(manager, client_id, new Texture(service_id));
 }
 
 TextureRef::~TextureRef() {
@@ -814,7 +825,6 @@ TextureRef::~TextureRef() {
   manager_ = NULL;
 }
 
-
 TextureManager::TextureManager(
     MemoryTracker* memory_tracker,
     FeatureInfo* feature_info,
@@ -897,7 +907,8 @@ scoped_refptr<TextureRef>
   }
   glBindTexture(target, 0);
 
-  scoped_refptr<TextureRef> default_texture(TextureRef::Create(this, ids[1]));
+  scoped_refptr<TextureRef> default_texture(
+      TextureRef::Create(this, 0, ids[1]));
   SetTarget(default_texture, target);
   if (needs_faces) {
     for (int ii = 0; ii < GLES2Util::kNumFaces; ++ii) {
@@ -999,140 +1010,22 @@ void TextureManager::SetLevelInfo(
   texture->GetMemTracker()->TrackMemAlloc(texture->estimated_size());
 }
 
-TextureDefinition* TextureManager::Save(TextureRef* ref) {
+Texture* TextureManager::Produce(TextureRef* ref) {
   DCHECK(ref);
   Texture* texture = ref->texture();
-  DCHECK(texture->owned_);
-
-  if (texture->IsAttachedToFramebuffer())
+  if (!texture->owned_)
     return NULL;
+  return texture;
+}
 
-  TextureDefinition::LevelInfos level_infos(texture->level_infos_.size());
-  for (size_t face = 0; face < level_infos.size(); ++face) {
-    GLenum target =
-        texture->target() == GL_TEXTURE_CUBE_MAP ? FaceIndexToGLTarget(face)
-                                                 : texture->target();
-    for (GLint level = 0; level <= texture->max_level_set_; ++level) {
-      const Texture::LevelInfo& level_info =
-          texture->level_infos_[face][level];
-
-      level_infos[face].push_back(
-          TextureDefinition::LevelInfo(target,
-                                       level_info.internal_format,
-                                       level_info.width,
-                                       level_info.height,
-                                       level_info.depth,
-                                       level_info.border,
-                                       level_info.format,
-                                       level_info.type,
-                                       level_info.cleared));
-
-      SetLevelInfo(ref,
-                   target,
-                   level,
-                   GL_RGBA,
-                   0,
-                   0,
-                   0,
-                   0,
-                   GL_RGBA,
-                   GL_UNSIGNED_BYTE,
-                   true);
-    }
-  }
-
-  GLuint old_service_id = texture->service_id();
-  bool immutable = texture->IsImmutable();
-  bool stream_texture = texture->IsStreamTexture();
-
-  GLuint new_service_id = 0;
-  glGenTextures(1, &new_service_id);
-  texture->SetServiceId(new_service_id);
-  texture->SetImmutable(false);
-  texture->SetStreamTexture(false);
-
-  return new TextureDefinition(texture->target(),
-                               old_service_id,
-                               texture->min_filter(),
-                               texture->mag_filter(),
-                               texture->wrap_s(),
-                               texture->wrap_t(),
-                               texture->usage(),
-                               immutable,
-                               stream_texture,
-                               level_infos);
-}
-
-bool TextureManager::Restore(
-    const char* function_name,
-    GLES2Decoder* decoder,
-    TextureRef* ref,
-    TextureDefinition* definition) {
-  DCHECK(ref);
-  Texture* texture = ref->texture();
-  DCHECK(texture->owned_);
-
-  scoped_ptr<TextureDefinition> scoped_definition(definition);
-
-  if (texture->IsAttachedToFramebuffer())
-    return false;
-
-  if (texture->target() != definition->target())
-    return false;
-
-  if (texture->level_infos_.size() < definition->level_infos().size())
-    return false;
-
-  if (texture->level_infos_[0].size() < definition->level_infos()[0].size())
-    return false;
-
-  for (size_t face = 0; face < definition->level_infos().size(); ++face) {
-    GLenum target =
-        texture->target() == GL_TEXTURE_CUBE_MAP ? FaceIndexToGLTarget(face)
-                                                 : texture->target();
-    GLint new_max_level = definition->level_infos()[face].size() - 1;
-    for (GLint level = 0;
-         level <= std::max(texture->max_level_set_, new_max_level);
-         ++level) {
-      const TextureDefinition::LevelInfo& level_info =
-          level <= new_max_level ? definition->level_infos()[face][level]
-                                 : TextureDefinition::LevelInfo();
-      SetLevelInfo(ref,
-                   target,
-                   level,
-                   level_info.internal_format,
-                   level_info.width,
-                   level_info.height,
-                   level_info.depth,
-                   level_info.border,
-                   level_info.format,
-                   level_info.type,
-                   level_info.cleared);
-    }
-  }
-
-  GLuint old_service_id = texture->service_id();
-  glDeleteTextures(1, &old_service_id);
-  texture->SetServiceId(definition->ReleaseServiceId());
-  glBindTexture(texture->target(), texture->service_id());
-  texture->SetImmutable(definition->immutable());
-  texture->SetStreamTexture(definition->stream_texture());
-  ErrorState* error_state = decoder->GetErrorState();
-  SetParameter(function_name, error_state, ref, GL_TEXTURE_MIN_FILTER,
-               definition->min_filter());
-  SetParameter(function_name, error_state, ref, GL_TEXTURE_MAG_FILTER,
-               definition->mag_filter());
-  SetParameter(function_name, error_state, ref, GL_TEXTURE_WRAP_S,
-               definition->wrap_s());
-  SetParameter(function_name, error_state, ref, GL_TEXTURE_WRAP_T,
-               definition->wrap_t());
-  if (feature_info_->validators()->texture_parameter.IsValid(
-      GL_TEXTURE_USAGE_ANGLE)) {
-    SetParameter(function_name, error_state, ref, GL_TEXTURE_USAGE_ANGLE,
-                 definition->usage());
-  }
-
-  return true;
+TextureRef* TextureManager::Consume(
+    GLuint client_id,
+    Texture* texture) {
+  DCHECK(client_id);
+  scoped_refptr<TextureRef> ref(new TextureRef(this, client_id, texture));
+  bool result = textures_.insert(std::make_pair(client_id, ref)).second;
+  DCHECK(result);

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

Seems like there needs to be some bookkeeping calls right? If you're inserting a
new texture and it's unrenderable the unrenderable count needs to be updated
etc...

I'm wondering it Consume would be better reusing the same ref? Something like

   
   Textures::Iter it = textures_.find(client_id);
   if (it != end()) {
        TextureRef ref = it->second
        ref->set_texture(texture);
   }

Then all the refs in this context are updated.

This would also auto delete the old texture is calling ref->set_texture release
the last reference

Right now it seems like this would get out of sync

  ctx2->genTex(1, &t);
  ctx2->bindTex(..., t);
  ctx2->framebufferTexture2D(,,,,t);
  ctx2->consume(...)

Because the framebuffer will have a scoped_refptr to the old TextureRef and so
will any texture_units 

Of course sadly, even if we do it that way the real FBO on the driver still has
the old texture bound and so do the texture units. Not sure what to do about
that. For the texture units we could set a flag to call
RestoreAllTextureUnitBindings before the next draw

For FBOs, we could make it an error to consume if the texture being replaced is
attached to anything.

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

This is done when we create the TextureRef - it calls StartTracking in the
constructor.
The problem is state in other contexts and FBO attachments. The service id for a
texture ref changes, so if we want to do that, we need to keep track everywhere
that we need to do something in real GL terms - e.g. when we switch to a new
context, we need to rebind all texture units (or keep track of which one refers
to this texture ref), and reattach all FBOs.
Right, my thinking is to make this explicit, and responsibility to the client:
consume deletes the current texture and creates a new one. It removes
implementation headaches, at the cost of what could be seen as a small quirk on
the client side. Delete has very well-defined semantics (resets units in current
context and attachments in current fbo, other ones keep a reference).

In practice, the quirk is not a problem: in our usage patterns, clients would
consume once, on a newly created texture. So I'm not sure it's worth it to add
implementation complexity for something that doesn't matter on the client side.

[greggman, 2013/05/23 18:38:26]

I'm confused. TextureRefs are per context so changing a TextureRef in this
context to point to a new texture doesn't affect other contexts right?

It seems like making this change would delete about 20 lines of code from this
CL making it simpler. In GLES2DecoderImpl::::DoConsumeTextureCHROMIUM the call
to DeleteTextureHelper before calling texture_manager_->Consume and all the code
after to reset texture refs disappears as does the bind. It just becomes


if (!current_texture->IsAttachedToFramebuffer()) {
   texture-manager->Consume(client_id, texture_being_consumed);
   RestoreAllTextureUnitBindings();  
}

The code here just becomes

  TextureRef* ref = GetTexture(client_id);
  DCHECK(ref);
  ref->SetTexture(this, texture_to_consume);

And SetTexture is

TextureRef::SetTexture(TextureManager* manager, Texture* texture) {
  manager->StopTracking(this);
  texture_->RemoveTextureRef(this);  
  texture_ = texture;
  texture_->AddTexturRef(this);
  manager_->StartTracking(this);
}

And suddenly there's no path for the client to have weird situations where units
are pointing to the wrong stuff.

Does that make any sense or am I missing something?

+  return ref.get();
 }
 
 void TextureManager::SetParameter(
@@ -1171,7 +1064,8 @@ bool TextureManager::MarkMipmapsGenerated(TextureRef* ref) {
 TextureRef* TextureManager::CreateTexture(
     GLuint client_id, GLuint service_id) {
   DCHECK_NE(0u, service_id);
-  scoped_refptr<TextureRef> ref(TextureRef::Create(this, service_id));
+  scoped_refptr<TextureRef> ref(TextureRef::Create(
+      this, client_id, service_id));
   std::pair<TextureMap::iterator, bool> result =
       textures_.insert(std::make_pair(client_id, ref));
   DCHECK(result.second);
@@ -1187,6 +1081,7 @@ TextureRef* TextureManager::GetTexture(
 void TextureManager::RemoveTexture(GLuint client_id) {
   TextureMap::iterator it = textures_.find(client_id);
   if (it != textures_.end()) {
+    it->second->reset_client_id();
     textures_.erase(it);
   }
 }