Reland of "webgl: optimize webgl.texSubImage2D(video) path."

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 680c81a0f273a4736d99ac85c63acbed6902893d..1322aa837a0901e193ee8842ad5307c4736c0c51 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -13203,6 +13203,13 @@ void GLES2DecoderImpl::DoCopySubTextureCHROMIUM(
       return;
     }
 
+    if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
+                                         width, height, 1)) {
+      LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTextureCHROMIUM",
+                         "source texture bad dimensions.");
+      return;
+    }
+

[dshwang, 2015/10/29 09:18:09]

As kbr reviewed, I fold https://codereview.chromium.org/1417913004/ into here.
It's fixed as part of cube map CL in
https://codereview.chromium.org/1275773003/diff/80001/gpu/command_buffer/serv...
,
but the CL was reverted.

zmo, piman, could you review?

[piman, 2015/10/30 00:16:30]

Can you explain? It seems that we would want to make sure that the source
contains the rectangle we're trying to copy, even for the GLImage case. Why are
we removing the check?

What would happen if x/y/width/height were out of range?

[dshwang, 2015/10/30 11:06:28]

That's good concern. Currently GLES2DecoderImpl cannot validation bad dimensions
as well as valid target, because some methods (e.g.
GetLevelSize/ValidForTexture/ValidForTarget) in Texture class are not valid for
GLImage. Let me solve it in
https://code.google.com/p/chromium/issues/detail?id=549531 and then remove this
unfortunate branch together.

     // Check that this type of texture is allowed.
     if (!texture_manager()->ValidForTarget(source_texture->target(), 0,
                                            source_width, source_height, 1)) {
@@ -13216,12 +13223,6 @@ void GLES2DecoderImpl::DoCopySubTextureCHROMIUM(
   GLenum source_internal_format = 0;
   source_texture->GetLevelType(source_texture->target(), 0, &source_type,
                                &source_internal_format);
-  if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
-                                       width, height, 1)) {
-    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTextureCHROMIUM",
-                       "source texture bad dimensions.");
-    return;
-  }
 
   GLenum dest_type = 0;
   GLenum dest_internal_format = 0;
@@ -13525,6 +13526,13 @@ void GLES2DecoderImpl::DoCompressedCopySubTextureCHROMIUM(GLenum target,
       return;
     }
 
+    if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
+                                         width, height, 1)) {
+      LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCompressedCopySubTextureCHROMIUM",
+                         "source texture bad dimensions.");
+      return;
+    }
+
     // Check that this type of texture is allowed.
     if (!texture_manager()->ValidForTarget(source_texture->target(), 0,
                                            source_width, source_height, 1)) {
@@ -13538,12 +13546,6 @@ void GLES2DecoderImpl::DoCompressedCopySubTextureCHROMIUM(GLenum target,
   GLenum source_internal_format = 0;
   source_texture->GetLevelType(source_texture->target(), 0, &source_type,
                                &source_internal_format);
-  if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
-                                       width, height, 1)) {
-    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCompressedCopySubTextureCHROMIUM",
-                       "source texture bad dimensions.");
-    return;
-  }
 
   GLenum dest_type = 0;
   GLenum dest_internal_format = 0;