gpu: glCopyTextureCHROMIUM() checks dest internal format incorrectly.

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 <stdio.h>
 
 #include <algorithm>
 #include <list>
@@ skipping 10029 matching lines @@
   }
 
   // Clear the source texture if necessary.
   if (!texture_manager()->ClearTextureLevel(
           this, source_texture_ref, source_texture->target(), 0)) {
     LOCAL_SET_GL_ERROR(
         GL_OUT_OF_MEMORY, "glCopyTextureCHROMIUM", "dimensions too big");
     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;
+  // The source format can be GL_BGRA_EXT.
+  bool valid_source_format = (source_internal_format >= GL_ALPHA &&

[no sievers, 2014/08/22 18:07:19]

nit: mind making this comparison more explicit? i.e. fmt == GL_ALPHA || fmt ==
GL_RGB ||... 

[dshwang, 2014/08/22 19:07:54]

Done.

+                              source_internal_format <= GL_LUMINANCE_ALPHA) ||
+                             source_internal_format == GL_BGRA_EXT;
+  if (!valid_source_format || !valid_dest_format) {

[no sievers, 2014/08/22 18:07:19]

Should we make it GL_INVALID_OPERATION if !valid_source_format?

[dshwang, 2014/08/22 19:07:55]

Yes! GL_INVALID_OPERATION is right.

+    LOCAL_SET_GL_ERROR(
+        GL_INVALID_VALUE, "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()) {
     LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glCopyTextureCHROMIUM");
     copy_texture_CHROMIUM_.reset(new CopyTextureCHROMIUMResourceManager());
     copy_texture_CHROMIUM_->Initialize(this);
     RestoreCurrentFramebufferBindings();
     if (LOCAL_PEEK_GL_ERROR("glCopyTextureCHROMIUM") != GL_NO_ERROR)
       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(
       GL_TEXTURE_2D, level, &dest_width, &dest_height);
 
   if (dest_level_defined) {
     dest_texture->GetLevelType(GL_TEXTURE_2D, level, &dest_type_previous,
                                &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 ||
       dest_internal_format != internal_format ||
       dest_type_previous != dest_type) {
     // Ensure that the glTexImage2D succeeds.
     LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glCopyTextureCHROMIUM");
     glBindTexture(GL_TEXTURE_2D, dest_texture->service_id());
     glTexImage2D(
         GL_TEXTURE_2D, level, internal_format, source_width, source_height,
@@ skipping 857 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 "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu