gpu: glCopyTextureCHROMIUM() checks dest internal format incorrectly.

|internal_format| which a client passes as argument and dest internal format
which is the format of current destination texture can be different. If
different, glCopyTextureCHROMIUM redefines the destination texture. So we
should check dest internal format using |internal_format| argument.

In addition, some platforms reports error when |internal_format| is GL_ALPHA,
GL_LUMINANCE or GL_LUMINANCE_ALPHA, because those formats can not attached as
color attachment of FBO on some platforms. So we restrict |internal_format| to
GL_RGB and GL_RGBA.

TEST=GLCopyTextureCHROMIUMTest.InternalFormat,
     GLCopyTextureCHROMIUMTest.RedefineDestinationTexture

Review URL: https://codereview.chromium.org/481913005

Cr-Commit-Position: refs/heads/master@{#291528}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291528 0039d316-1c4b-4281-b951-d872f2087c98

gpu/GLES2/extensions/CHROMIUM/CHROMIUM_copy_texture.txt

@@ -48,7 +48,10 @@ New Procedures and Functions
 
     The internal format of the destination texture is converted to that
     specified by <internal_format>. Must be one of the following symbolic
-    constants: GL_ALPHA, GL_LUMINANCE, GL_LUMINANCE_ALPHA, GL_RGB, GL_RGBA
+    constants: GL_RGB, GL_RGBA
+    The internal format of <source_id> texture must be one of the following
+    symbolic constants: GL_ALPHA, GL_LUMINANCE, GL_LUMINANCE_ALPHA, GL_RGB,
+    GL_RGBA, GL_BGRA_EXT
     When <source_id> texture doens't contain a superset of the component
     required by <internal_format>, fill the components by following rules.
 
@@ -59,12 +62,19 @@ New Procedures and Functions
                    GL_LUMINANCE_ALPHA     (L, L, L, A)
                    GL_RGB                 (R, G, B, 1)
                    GL_RGBA                (R, G, B, A)
+                   GL_BGRA_EXT            (R, G, B, A)
 
     The format type of the destination texture is converted to that specified
     by <dest_type>.
 
     <target> uses the same parameters as TexImage2D.
 
+    INVALID_OPERATION is generated if <internal_format> is not one of the valid formats
+    described above.
+
+    INVALID_OPERATION is generated if the internal format of <source_id> is not one of
+    formats from the table above.
+
     INVALID_VALUE is generated if <target> is not GL_TEXTURE_2D.
 
     INVALID_VALUE is generated if <source_id> or <dest_id> are not valid texture

gpu/command_buffer/service/gles2_cmd_decoder.cc

@@ -10047,6 +10047,29 @@ void GLES2DecoderImpl::DoCopyTextureCHROMIUM(
     return;
   }
 
+  GLenum source_type = 0;
+  GLenum source_internal_format = 0;
+  source_texture->GetLevelType(
+      source_texture->target(), 0, &source_type, &source_internal_format);
+
+  // The destination format should be GL_RGB, or GL_RGBA. GL_ALPHA,
+  // GL_LUMINANCE, and GL_LUMINANCE_ALPHA are not supported because they are not
+  // renderable on some platforms.
+  bool valid_dest_format =
+      internal_format == GL_RGB || internal_format == GL_RGBA;
+  bool valid_source_format = source_internal_format == GL_ALPHA ||
+                             source_internal_format == GL_RGB ||
+                             source_internal_format == GL_RGBA ||
+                             source_internal_format == GL_LUMINANCE ||
+                             source_internal_format == GL_LUMINANCE_ALPHA ||
+                             source_internal_format == GL_BGRA_EXT;
+  if (!valid_source_format || !valid_dest_format) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION,
+                       "glCopyTextureCHROMIUM",
+                       "invalid internal format");
+    return;
+  }
+
   // Defer initializing the CopyTextureCHROMIUMResourceManager until it is
   // needed because it takes 10s of milliseconds to initialize.
   if (!copy_texture_CHROMIUM_.get()) {
@@ -10058,11 +10081,6 @@ void GLES2DecoderImpl::DoCopyTextureCHROMIUM(
       return;
   }
 
-  GLenum source_type = 0;
-  GLenum source_internal_format = 0;
-  source_texture->GetLevelType(
-      source_texture->target(), 0, &source_type, &source_internal_format);
-
   GLenum dest_type_previous = dest_type;
   GLenum dest_internal_format = internal_format;
   bool dest_level_defined = dest_texture->GetLevelSize(
@@ -10073,20 +10091,6 @@ void GLES2DecoderImpl::DoCopyTextureCHROMIUM(
                                &dest_internal_format);
   }
 
-  // The destination format should be GL_ALPHA, GL_RGB, GL_RGBA, GL_LUMINANCE,
-  // or GL_LUMINANCE_ALPHA.
-  bool valid_dest_format = dest_internal_format >= GL_ALPHA &&
-                           dest_internal_format <= GL_LUMINANCE_ALPHA;
-  // The source format can be GL_BGRA_EXT.
-  bool valid_source_format = (source_internal_format >= GL_ALPHA &&
-                              source_internal_format <= GL_LUMINANCE_ALPHA) ||
-                             source_internal_format == GL_BGRA_EXT;
-  if (!valid_source_format || !valid_dest_format) {
-    LOCAL_SET_GL_ERROR(
-        GL_INVALID_VALUE, "glCopyTextureCHROMIUM", "invalid internal format");
-    return;
-  }
-
   // Resize the destination texture to the dimensions of the source texture.
   if (!dest_level_defined || dest_width != source_width ||
       dest_height != source_height ||

gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc

@@ -76,6 +76,118 @@ TEST_F(GLCopyTextureCHROMIUMTest, Basic) {
   EXPECT_TRUE(GL_NO_ERROR == glGetError());
 }
 
+TEST_F(GLCopyTextureCHROMIUMTest, InternalFormat) {
+  GLint src_formats[] = {GL_ALPHA,     GL_RGB,             GL_RGBA,
+                         GL_LUMINANCE, GL_LUMINANCE_ALPHA, GL_BGRA_EXT};
+  GLint dest_formats[] = {GL_RGB, GL_RGBA};
+
+  for (size_t src_index = 0; src_index < arraysize(src_formats); src_index++) {
+    for (size_t dest_index = 0; dest_index < arraysize(dest_formats);
+         dest_index++) {
+      glBindTexture(GL_TEXTURE_2D, textures_[0]);
+      glTexImage2D(GL_TEXTURE_2D,
+                   0,
+                   src_formats[src_index],
+                   1,
+                   1,
+                   0,
+                   src_formats[src_index],
+                   GL_UNSIGNED_BYTE,
+                   NULL);
+      EXPECT_TRUE(GL_NO_ERROR == glGetError());
+
+      glCopyTextureCHROMIUM(GL_TEXTURE_2D,
+                            textures_[0],
+                            textures_[1],
+                            0,
+                            dest_formats[dest_index],
+                            GL_UNSIGNED_BYTE);
+      EXPECT_TRUE(GL_NO_ERROR == glGetError()) << "src_index:" << src_index
+                                               << " dest_index:" << dest_index;
+    }
+  }
+}
+
+TEST_F(GLCopyTextureCHROMIUMTest, InternalFormatNotSupported) {
+  glBindTexture(GL_TEXTURE_2D, textures_[0]);
+  glTexImage2D(
+      GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, NULL);
+  EXPECT_TRUE(GL_NO_ERROR == glGetError());
+
+  // Check unsupported format reports error.
+  GLint unsupported_dest_formats[] = {GL_ALPHA, GL_LUMINANCE,
+                                      GL_LUMINANCE_ALPHA, GL_BGRA_EXT};
+  for (size_t dest_index = 0; dest_index < arraysize(unsupported_dest_formats);
+       dest_index++) {
+    glCopyTextureCHROMIUM(GL_TEXTURE_2D,
+                          textures_[0],
+                          textures_[1],
+                          0,
+                          unsupported_dest_formats[dest_index],
+                          GL_UNSIGNED_BYTE);
+    EXPECT_TRUE(GL_INVALID_OPERATION == glGetError())
+        << "dest_index:" << dest_index;
+  }
+}
+
+// Test to ensure that the destination texture is redefined if the properties
+// are different.
+TEST_F(GLCopyTextureCHROMIUMTest, RedefineDestinationTexture) {
+  uint8 pixels[4 * 4] = {255u, 0u, 0u, 255u, 255u, 0u, 0u, 255u,
+                         255u, 0u, 0u, 255u, 255u, 0u, 0u, 255u};
+
+  glBindTexture(GL_TEXTURE_2D, textures_[0]);
+  glTexImage2D(
+      GL_TEXTURE_2D, 0, GL_RGBA, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+
+  glBindTexture(GL_TEXTURE_2D, textures_[1]);
+  glTexImage2D(GL_TEXTURE_2D,
+               0,
+               GL_BGRA_EXT,
+               1,
+               1,
+               0,
+               GL_BGRA_EXT,
+               GL_UNSIGNED_BYTE,
+               pixels);
+  EXPECT_TRUE(GL_NO_ERROR == glGetError());
+
+  // GL_INVALID_OPERATION due to "intrinsic format" != "internal format".
+  glTexSubImage2D(
+      GL_TEXTURE_2D, 0, 0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+  EXPECT_TRUE(GL_INVALID_OPERATION == glGetError());
+  // GL_INVALID_VALUE due to bad dimensions.
+  glTexSubImage2D(
+      GL_TEXTURE_2D, 0, 1, 1, 1, 1, GL_BGRA_EXT, GL_UNSIGNED_BYTE, pixels);
+  EXPECT_TRUE(GL_INVALID_VALUE == glGetError());
+
+  // If the dest texture has different properties, glCopyTextureCHROMIUM()
+  // redefines them.
+  glCopyTextureCHROMIUM(
+      GL_TEXTURE_2D, textures_[0], textures_[1], 0, GL_RGBA, GL_UNSIGNED_BYTE);
+  EXPECT_TRUE(GL_NO_ERROR == glGetError());
+
+  // glTexSubImage2D() succeeds because textures_[1] is redefined into 2x2
+  // dimension and GL_RGBA format.
+  glBindTexture(GL_TEXTURE_2D, textures_[1]);
+  glTexSubImage2D(
+      GL_TEXTURE_2D, 0, 1, 1, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+  EXPECT_TRUE(GL_NO_ERROR == glGetError());
+
+  // Check the FB is still bound.
+  GLint value = 0;
+  glGetIntegerv(GL_FRAMEBUFFER_BINDING, &value);
+  GLuint fb_id = value;
+  EXPECT_EQ(framebuffer_id_, fb_id);
+
+  // Check that FB is complete.
+  EXPECT_EQ(static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE),
+            glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+  GLTestHelper::CheckPixels(1, 1, 1, 1, 0, &pixels[12]);
+  EXPECT_TRUE(GL_NO_ERROR == glGetError());
+}
+
 // Test that the extension respects the flip-y pixel storage setting.
 TEST_F(GLCopyTextureCHROMIUMTest, FlipY) {
   uint8 pixels[2][2][4];