Allow GL_TEXTURE_RECTANGLE_ARB to be the destination target for copyTextureCHROMIUM.

gpu/command_buffer/service/gles2_cmd_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 #include <limits.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
@@ skipping 13064 matching lines @@
   }
 
   Texture* source_texture = source_texture_ref->texture();
   Texture* dest_texture = dest_texture_ref->texture();
   if (source_texture == dest_texture) {
     LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name,
                        "source and destination textures are the same");
     return false;
   }
 
-  if (dest_texture->target() != GL_TEXTURE_2D ||
+  if ((dest_texture->target() != GL_TEXTURE_2D &&
+       dest_texture->target() != GL_TEXTURE_RECTANGLE_ARB) ||

[Zhenyao Mo, 2016/01/04 22:43:54]

nit: maybe use two switch statements here?  Looks nicer and you can have
separate error msgs for src and dst targets.

[erikchen, 2016/01/04 23:28:52]

Done.

       (source_texture->target() != GL_TEXTURE_2D &&
        source_texture->target() != GL_TEXTURE_RECTANGLE_ARB &&
        source_texture->target() != GL_TEXTURE_EXTERNAL_OES)) {
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
                        "invalid texture target binding");
     return false;
   }
 
   GLenum source_type = 0;
   GLenum source_internal_format = 0;
@@ skipping 213 matching lines @@
 
   DoCopyTexImageIfNeeded(source_texture, source_target);
 
   // GL_TEXTURE_EXTERNAL_OES texture requires apply a transform matrix
   // before presenting.
   if (source_target == GL_TEXTURE_EXTERNAL_OES) {
     // TODO(hkuang): get the StreamTexture transform matrix in GPU process
     // instead of using kIdentityMatrix crbug.com/226218.
     copy_texture_CHROMIUM_->DoCopyTextureWithTransform(
         this, source_target, source_texture->service_id(),
-        dest_texture->service_id(), source_width, source_height,
+        dest_target, dest_texture->service_id(), source_width, source_height,
         unpack_flip_y == GL_TRUE,
         unpack_premultiply_alpha == GL_TRUE,
         unpack_unmultiply_alpha == GL_TRUE,
         kIdentityMatrix);
   } else {
     copy_texture_CHROMIUM_->DoCopyTexture(
         this, source_target, source_texture->service_id(),
-        source_internal_format, dest_texture->service_id(), internal_format,
-        source_width, source_height,
+        source_internal_format, dest_target, dest_texture->service_id(),
+        internal_format, source_width, source_height,
         unpack_flip_y == GL_TRUE,
         unpack_premultiply_alpha == GL_TRUE,
         unpack_unmultiply_alpha == GL_TRUE);
   }
 }
 
 void GLES2DecoderImpl::DoCopySubTextureCHROMIUM(
     GLuint source_id,
     GLuint dest_id,
     GLint xoffset,
@@ skipping 142 matching lines @@
                                gfx::Rect(x, y, width, height))) {
       return;
     }
   }
 
   DoCopyTexImageIfNeeded(source_texture, source_target);
 
   // TODO(hkuang): get the StreamTexture transform matrix in GPU process.
   // crbug.com/226218.
   copy_texture_CHROMIUM_->DoCopySubTexture(
-      this, source_target, source_texture->service_id(),
-      source_internal_format, dest_texture->service_id(), dest_internal_format,
+      this, source_target, source_texture->service_id(), source_internal_format,
+      dest_target, dest_texture->service_id(), dest_internal_format,
       xoffset, yoffset, x, y, width, height, dest_width, dest_height,
       source_width, source_height,
       unpack_flip_y == GL_TRUE,
       unpack_premultiply_alpha == GL_TRUE,
       unpack_unmultiply_alpha == GL_TRUE);
 }
 
 void GLES2DecoderImpl::DoCompressedCopyTextureCHROMIUM(GLenum target,
                                                        GLuint source_id,
                                                        GLuint dest_id) {
@@ skipping 153 matching lines @@
       source_height, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE,
       gfx::Rect(source_width, source_height));
 
   // GL_TEXTURE_EXTERNAL_OES texture requires apply a transform matrix
   // before presenting.
   if (source_texture->target() == GL_TEXTURE_EXTERNAL_OES) {
     // TODO(hkuang): get the StreamTexture transform matrix in GPU process
     // instead of using kIdentityMatrix crbug.com/226218.
     copy_texture_CHROMIUM_->DoCopyTextureWithTransform(
         this, source_texture->target(), source_texture->service_id(),
-        dest_texture->service_id(), source_width, source_height,
-        false, false, false, kIdentityMatrix);
+        dest_texture->target(), dest_texture->service_id(), source_width,
+        source_height, false, false, false, kIdentityMatrix);
   } else {
     copy_texture_CHROMIUM_->DoCopyTexture(
         this, source_texture->target(), source_texture->service_id(),
-        source_internal_format, dest_texture->service_id(), GL_RGBA,
-        source_width, source_height, false, false, false);
+        source_internal_format, dest_texture->target(),
+        dest_texture->service_id(), GL_RGBA, source_width, source_height, false,
+        false, false);
   }
 }
 
 void GLES2DecoderImpl::DoCompressedCopySubTextureCHROMIUM(GLenum target,
                                                           GLuint source_id,
                                                           GLuint dest_id,
                                                           GLint xoffset,
                                                           GLint yoffset,
                                                           GLint x,
                                                           GLint y,
@@ skipping 158 matching lines @@
 
   // As a fallback, copy into a non-compressed GL_RGBA texture.
   if (dest_internal_format != GL_RGBA) {
     // To preserve the contents of the original destination texture we must
     // first copy the original destination texture to a temporary storage, then
     // copy it back to the original destination texture.
     GLuint tmp_service_id;
     glGenTextures(1, &tmp_service_id);
     DCHECK_NE(0u, tmp_service_id);
 
     glBindTexture(GL_TEXTURE_2D, tmp_service_id);

[Zhenyao Mo, 2016/01/04 22:43:54]

I think it's clearer if you define a variable of intermediate_target and set it
as TEXTURE_2D, and use it here and below.  Much easier to understand the
semantics that way.

[erikchen, 2016/01/04 23:28:52]

Done. I used the name "tmp_target" to match "tmp_service_id".

 
     LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glCompressedCopyTextureCHROMIUM");
     glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, dest_width, dest_height, 0, GL_RGBA,
                  GL_UNSIGNED_BYTE, NULL);
     GLenum error = LOCAL_PEEK_GL_ERROR("glCompressedCopySubTextureCHROMIUM");
     if (error != GL_NO_ERROR)
       return;
 
     copy_texture_CHROMIUM_->DoCopyTexture(
         this, dest_texture->target(), dest_texture->service_id(),
-        dest_internal_format, tmp_service_id, GL_RGBA,
+        dest_internal_format, GL_TEXTURE_2D, tmp_service_id, GL_RGBA,
         dest_width, dest_height, false, false, false);
 
     // Redefine destination texture to use RGBA.
     glBindTexture(GL_TEXTURE_2D, dest_texture->service_id());

[Zhenyao Mo, 2016/01/04 22:43:55]

Here we have the assumption that dest_texture->target() is GL_TEXTURE_2D, so we
probably should DCHECK right in the beginning? Or actually the tmp_service_id
should be bound to dest_texture->target() instead of GL_TEXTURE_2D?

[erikchen, 2016/01/04 23:28:52]

I added a DCHECK to the beginning. Note that this is part of the implementation
of DoCompressedCopySubTextureCHROMIUM(), which only supports GL_TEXTURE_2D as
the destination target.

     LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glCompressedCopyTextureCHROMIUM");
     glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, dest_width, dest_height, 0, GL_RGBA,
                  GL_UNSIGNED_BYTE, NULL);
     error = LOCAL_PEEK_GL_ERROR("glCompressedCopySubTextureCHROMIUM");
     if (error != GL_NO_ERROR)
       return;
 
     texture_manager()->SetLevelInfo(
         dest_texture_ref, GL_TEXTURE_2D, 0, GL_RGBA, dest_width, dest_height,
         1, 0, GL_RGBA, GL_UNSIGNED_BYTE, gfx::Rect(dest_width, dest_height));
 
     copy_texture_CHROMIUM_->DoCopyTexture(
         this, GL_TEXTURE_2D, tmp_service_id, GL_RGBA,
-        dest_texture->service_id(), GL_RGBA,
+        dest_texture->target(), dest_texture->service_id(), GL_RGBA,
         dest_width, dest_height, false, false, false);
 
     glDeleteTextures(1, &tmp_service_id);
   }
 
   // TODO(hkuang): get the StreamTexture transform matrix in GPU process.
   // crbug.com/226218.
   copy_texture_CHROMIUM_->DoCopySubTexture(
       this, source_texture->target(), source_texture->service_id(),
-      source_internal_format, dest_texture->service_id(), GL_RGBA,
-      xoffset, yoffset, x, y, width, height, dest_width, dest_height,
-      source_width, source_height, false, false, false);
+      source_internal_format, dest_texture->target(),
+      dest_texture->service_id(), GL_RGBA, xoffset, yoffset, x, y, width,
+      height, dest_width, dest_height, source_width, source_height, false,
+      false, false);
 }
 
 void GLES2DecoderImpl::DoTexStorage2DEXT(
     GLenum target,
     GLint levels,
     GLenum internal_format,
     GLsizei width,
     GLsizei height) {
   TRACE_EVENT2("gpu", "GLES2DecoderImpl::DoTexStorage2DEXT",
       "width", width, "height", height);
@@ skipping 1874 matching lines @@
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "base/macros.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu