gpu: Optimize and cleanup code used for CHROMIUM_copy_texture.

gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc

Index: gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc b/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
index bda65ac6a6057d9798b9926a9b0c4b0a0cc41e2d..d91ed7a22bffb368ee1d485bf5d8eaa522a870c7 100644
--- a/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
+++ b/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
@@ -103,6 +103,11 @@ const char* fragment_shader_source[NUM_FRAGMENT_SHADERS] = {
     }),
 };
 
+const GLfloat kDefaultMatrix[16] = {1.0f, 0.0f, 0.0f, 0.0f,
+                                    0.0f, 1.0f, 0.0f, 0.0f,
+                                    0.0f, 0.0f, 1.0f, 0.0f,
+                                    0.0f, 0.0f, 0.0f, 1.0f};
+
 // Returns the correct vertex shader id to evaluate the copy operation for
 // the CHROMIUM_flipy setting.
 VertexShaderId GetVertexShaderId(bool flip_y) {
@@ -167,7 +172,7 @@ FragmentShaderId GetFragmentShaderId(bool premultiply_alpha,
   }
 
   NOTREACHED();
-  return shader_ids[index][SAMPLER_2D];
+  return shader_ids[0][SAMPLER_2D];
 }
 
 void CompileShader(GLuint shader, const char* shader_source) {
@@ -197,13 +202,16 @@ CopyTextureCHROMIUMResourceManager::CopyTextureCHROMIUMResourceManager()
       buffer_id_(0u),
       framebuffer_(0u) {}
 
-CopyTextureCHROMIUMResourceManager::~CopyTextureCHROMIUMResourceManager() {}
+CopyTextureCHROMIUMResourceManager::~CopyTextureCHROMIUMResourceManager() {
+  DCHECK(!buffer_id_ && !framebuffer_);
+}
 
 void CopyTextureCHROMIUMResourceManager::Initialize(
     const gles2::GLES2Decoder* decoder) {
   COMPILE_ASSERT(
       kVertexPositionAttrib == 0u,
       Position_attribs_must_be_0);
+  DCHECK(!buffer_id_ && !framebuffer_ && programs_.empty());
 
   // Initialize all of the GPU resources required to perform the copy.
   glGenBuffersARB(1, &buffer_id_);
@@ -227,6 +235,7 @@ void CopyTextureCHROMIUMResourceManager::Destroy() {
     return;
 
   glDeleteFramebuffersEXT(1, &framebuffer_);
+  framebuffer_ = 0;
 
   std::for_each(vertex_shaders_.begin(), vertex_shaders_.end(), DeleteShader);
   std::for_each(
@@ -239,37 +248,89 @@ void CopyTextureCHROMIUMResourceManager::Destroy() {
   }
 
   glDeleteBuffersARB(1, &buffer_id_);
+  buffer_id_ = 0;
 }
 
 void CopyTextureCHROMIUMResourceManager::DoCopyTexture(
     const gles2::GLES2Decoder* decoder,
     GLenum source_target,
-    GLenum dest_target,
     GLuint source_id,
     GLuint dest_id,
     GLint level,
+    GLenum internal_format,
     GLsizei width,
     GLsizei height,
     bool flip_y,
     bool premultiply_alpha,
     bool unpremultiply_alpha) {
-  // Use default transform matrix if no transform passed in.
-  const static GLfloat default_matrix[16] = {1.0f, 0.0f, 0.0f, 0.0f,
-                                             0.0f, 1.0f, 0.0f, 0.0f,
-                                             0.0f, 0.0f, 1.0f, 0.0f,
-                                             0.0f, 0.0f, 0.0f, 1.0f};
-  DoCopyTextureWithTransform(decoder, source_target, dest_target, source_id,
-      dest_id, level, width, height, flip_y, premultiply_alpha,
-      unpremultiply_alpha, default_matrix);
+  DoCopyTextureWithTransform(decoder, source_target, source_id, dest_id,
+      level, internal_format, width, height, flip_y, premultiply_alpha,
+      unpremultiply_alpha, kDefaultMatrix);
+}
+
+void CopyTextureCHROMIUMResourceManager::DoFastCopyTexture(
+    const gles2::GLES2Decoder* decoder,
+    GLuint source_id,
+    GLuint dest_id,
+    GLint level,
+    GLenum internal_format,
+    GLsizei width,
+    GLsizei height) {

[reveman, 2014/07/09 18:33:28]

Does this need to be limited to source_target=TEXTURE_2D? Seems like
TEXTURE_REXTANGLE_ARB should work fine too.

[dshwang, 2014/07/09 18:53:12]

Do you mean TEXTURE_REXTANGLE_ARB texture can be bound in FBO? as follow;

  glBindTexture(TEXTURE_REXTANGLE_ARB, texture_id);
  // NVidia drivers require texture settings to be a certain way
  // or they won't report FRAMEBUFFER_COMPLETE.
  glTexParameterf(TEXTURE_REXTANGLE_ARB, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
  glTexParameterf(TEXTURE_REXTANGLE_ARB, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
  glTexParameteri(TEXTURE_REXTANGLE_ARB, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
  glTexParameteri(TEXTURE_REXTANGLE_ARB, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
  glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, framebuffer_);
  glFramebufferTexture2DEXT(
      GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, TEXTURE_REXTANGLE_ARB, texture_id,
level);

I did because glFramebufferTexture2D doc doesn't mention about
GL_TEXTURE_REXTANGLE_ARB ;
https://www.khronos.org/opengles/sdk/docs/man/xhtml/glFramebufferTexture2D.xml

[reveman, 2014/07/09 19:45:07]

Hm, maybe this is only supported by standard GL. The FBO spec explicitly lists
TEXTURE_RECTANGLE_ARB as a valid target:
http://www.opengl.org/registry/specs/EXT/framebuffer_object.txt

It's fine with me to restrict this to TEXTURE_2D in the conditional that
determines if this optimization should be used or not for now but I prefer if
::BindFramebufferTexture2D and ::DoFastCopyTexture are not limited to this in
any way.

[dshwang, 2014/07/10 15:19:45]

Yep, I restrict this to TEXTURE_2D, but make ::BindFramebufferTexture2D and
::DoFastCopyTexture be allowed to this.

+  if (BindFramebufferTexture2D(source_id, 0 /* level */)) {
+    glBindTexture(GL_TEXTURE_2D, dest_id);
+    glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glCopyTexImage2D(GL_TEXTURE_2D,
+                     level,
+                     internal_format,
+                     0 /* x */,
+                     0 /* y */,
+                     width,
+                     height,
+                     0 /* border */);
+  }
+
+  decoder->RestoreTextureState(source_id);
+  decoder->RestoreTextureState(dest_id);
+  decoder->RestoreTextureUnitBindings(0);
+  decoder->RestoreActiveTexture();
+  decoder->RestoreFramebufferBindings();
+}
+
+bool CopyTextureCHROMIUMResourceManager::BindFramebufferTexture2D(
+    GLuint texture_id,
+    GLint level) {
+  glActiveTexture(GL_TEXTURE0);
+  glBindTexture(GL_TEXTURE_2D, texture_id);
+  // NVidia drivers require texture settings to be a certain way
+  // or they won't report FRAMEBUFFER_COMPLETE.
+  glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+  glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+  glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, framebuffer_);
+  glFramebufferTexture2DEXT(
+      GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture_id, level);
+
+#ifndef NDEBUG
+  GLenum fb_status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER);
+  if (GL_FRAMEBUFFER_COMPLETE != fb_status) {
+    DLOG(ERROR) << "CopyTextureCHROMIUM: Incomplete framebuffer.";
+    return false;
+  }
+#endif
+  return true;
 }
 
 void CopyTextureCHROMIUMResourceManager::DoCopyTextureWithTransform(
     const gles2::GLES2Decoder* decoder,
     GLenum source_target,
-    GLenum dest_target,
     GLuint source_id,
     GLuint dest_id,
     GLint level,
+    GLenum internal_format,
     GLsizei width,
     GLsizei height,
     bool flip_y,
@@ -286,19 +347,20 @@ void CopyTextureCHROMIUMResourceManager::DoCopyTextureWithTransform(
 
   VertexShaderId vertex_shader_id = GetVertexShaderId(flip_y);
   DCHECK_LT(static_cast<size_t>(vertex_shader_id), vertex_shaders_.size());
-  GLuint* vertex_shader = &vertex_shaders_[vertex_shader_id];
-  if (!*vertex_shader) {
-    *vertex_shader = glCreateShader(GL_VERTEX_SHADER);
-    CompileShader(*vertex_shader, vertex_shader_source[vertex_shader_id]);
-  }
-
   FragmentShaderId fragment_shader_id = GetFragmentShaderId(
       premultiply_alpha, unpremultiply_alpha, source_target);
   DCHECK_LT(static_cast<size_t>(fragment_shader_id), fragment_shaders_.size());
-  GLuint* fragment_shader = &fragment_shaders_[fragment_shader_id];
-  if (!*fragment_shader) {
-    *fragment_shader = glCreateShader(GL_FRAGMENT_SHADER);
-    CompileShader(*fragment_shader, fragment_shader_source[fragment_shader_id]);
+
+  if (vertex_shader_id == VERTEX_SHADER_COPY_TEXTURE &&
+      fragment_shader_id == FRAGMENT_SHADER_COPY_TEXTURE_2D &&

[reveman, 2014/07/09 18:33:28]

I think it's more appropriate to check the parameters. ie. source_target,
premultiply_alpha..

Also, please move this optimization to ::DoCopyTexture instead and require the
client to use that function instead of the "WithTransform" version. It's silly
to pass kDefaultMatrix to this function and then check if it's equal to
kDefaultMatrix..

[dshwang, 2014/07/09 18:53:12]

Alright. I'll update.

+      transform_matrix == kDefaultMatrix) {
+    DoFastCopyTexture(decoder,
+                      source_id,
+                      dest_id,
+                      level,
+                      internal_format,
+                      width,
+                      height);
   }
 
   ProgramMapKey key(vertex_shader_id, fragment_shader_id);
@@ -306,7 +368,18 @@ void CopyTextureCHROMIUMResourceManager::DoCopyTextureWithTransform(
   // Create program if necessary.
   if (!info->program) {
     info->program = glCreateProgram();
+    GLuint* vertex_shader = &vertex_shaders_[vertex_shader_id];
+    if (!*vertex_shader) {
+      *vertex_shader = glCreateShader(GL_VERTEX_SHADER);
+      CompileShader(*vertex_shader, vertex_shader_source[vertex_shader_id]);
+    }
     glAttachShader(info->program, *vertex_shader);
+    GLuint* fragment_shader = &fragment_shaders_[fragment_shader_id];
+    if (!*fragment_shader) {
+      *fragment_shader = glCreateShader(GL_FRAGMENT_SHADER);
+      CompileShader(*fragment_shader,
+                    fragment_shader_source[fragment_shader_id]);
+    }
     glAttachShader(info->program, *fragment_shader);
     glBindAttribLocation(info->program, kVertexPositionAttrib, "a_position");
     glLinkProgram(info->program);
@@ -337,25 +410,8 @@ void CopyTextureCHROMIUMResourceManager::DoCopyTextureWithTransform(
     glUniform2f(info->half_size_handle, width / 2.0f, height / 2.0f);
   else
     glUniform2f(info->half_size_handle, 0.5f, 0.5f);
-  glActiveTexture(GL_TEXTURE0);
-  glBindTexture(GL_TEXTURE_2D, dest_id);
-  // NVidia drivers require texture settings to be a certain way
-  // or they won't report FRAMEBUFFER_COMPLETE.
-  glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
-  glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
-  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
-  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
-  glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, framebuffer_);
-  glFramebufferTexture2DEXT(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, dest_target,
-                            dest_id, level);
 
-#ifndef NDEBUG
-  GLenum fb_status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER);
-  if (GL_FRAMEBUFFER_COMPLETE != fb_status) {
-    DLOG(ERROR) << "CopyTextureCHROMIUM: Incomplete framebuffer.";
-  } else
-#endif
-  {
+  if (BindFramebufferTexture2D(dest_id, level)) {
     decoder->ClearAllAttributes();
     glEnableVertexAttribArray(kVertexPositionAttrib);