Change CHROMIUM_image declarations to support multi planar input.

gpu/command_buffer/client/gles2_implementation.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.
 
 // A class to emulate GLES2 over command buffers.
 
 #include "gpu/command_buffer/client/gles2_implementation.h"
 
 #include <GLES2/gl2ext.h>
 #include <GLES2/gl2extchromium.h>
@@ skipping 4695 matching lines @@
     case GL_MAP_CHROMIUM:
     case GL_SCANOUT_CHROMIUM:
       return true;
     default:
       return false;
   }
 }
 
 }  // namespace
 
-GLuint GLES2Implementation::CreateImageCHROMIUMHelper(ClientBuffer buffer,
+GLuint GLES2Implementation::CreateImageCHROMIUMHelper(ClientBuffer* buffers,
                                                       GLsizei width,
                                                       GLsizei height,
                                                       GLenum internalformat) {
+  if (buffers == nullptr) {

[reveman, 2015/03/02 20:09:55]

I don't think this check makes sense. We used to crash if buffer was null. I
think we can keep crashing if buffers is not valid. After all, there's no good
way to check if this is valid value when multiple planes are used without
crashing anyhow...

[emircan, 2015/03/03 02:02:18]

I agree there is no way to check each of the multiple planes. However, since we
are passing a pointer along rather than a reference, a nullptr would get past a
lot of calls and fail much later down the line, and would be confusing to debug.
I thought it would be good to intercept it early, and return a meaningful
message. 

[reveman, 2015/03/03 05:57:09]

In general, I don't think it makes sense to check (even with a DCHECK) that a
function argument is non-null unless there's some configuration of arguments
that makes a null argument a valid way of calling the function. I think that
defining a GL error for this type of usage is inconsistent with the GL API.
There are a lot of GL API functions that are passed pointers but none of them
will generate an error if the pointer is null.

[emircan, 2015/03/04 03:03:15]

Changed it as suggested.

+    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "invalid |buffers|");
+    return 0;
+  }
+
   if (width <= 0) {
-    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "width <= 0");
+    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "|width| <= 0");

[reveman, 2015/03/02 20:09:56]

| is typically used when referring to variables in code comments. Not sure we
should be using that in log output.

[reveman, 2015/03/02 20:09:56]

| is typically used when referring to variables in code comments. Not sure we
should be using that in log output.

[emircan, 2015/03/03 02:02:19]

Done.

     return 0;
   }
 
   if (height <= 0) {
-    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "height <= 0");
+    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "|height| <= 0");

[reveman, 2015/03/02 20:09:56]

ditto

[emircan, 2015/03/03 02:02:18]

Done.

     return 0;
   }
 
   if (!ValidImageFormat(internalformat)) {
-    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "invalid format");
+    SetGLError(GL_INVALID_ENUM, "glCreateImageCHROMIUM", "invalid format");
     return 0;
   }
 
+  // TODO(emircan): See http://crbug.com/439520; support passing multiple
+  // buffers when new multi-planar formats are added.
   int32_t image_id =
-      gpu_control_->CreateImage(buffer, width, height, internalformat);
+      gpu_control_->CreateImage(buffers[0], width, height, internalformat);

[reveman, 2015/03/02 20:09:55]

I think we should update the GpuControl interface in this patch too.

   if (image_id < 0) {
     SetGLError(GL_OUT_OF_MEMORY, "glCreateImageCHROMIUM", "image_id < 0");
     return 0;
   }
   return image_id;
 }
 
-GLuint GLES2Implementation::CreateImageCHROMIUM(ClientBuffer buffer,
+GLuint GLES2Implementation::CreateImageCHROMIUM(ClientBuffer* buffers,
                                                 GLsizei width,
                                                 GLsizei height,
                                                 GLenum internalformat) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glCreateImageCHROMIUM(" << width
                      << ", " << height << ", "
                      << GLES2Util::GetStringImageInternalFormat(internalformat)
                      << ")");
   GLuint image_id =
-      CreateImageCHROMIUMHelper(buffer, width, height, internalformat);
+      CreateImageCHROMIUMHelper(buffers, width, height, internalformat);
   CheckGLError();
   return image_id;
 }
 
 void GLES2Implementation::DestroyImageCHROMIUMHelper(GLuint image_id) {
   // Flush the command stream to make sure all pending commands
   // that may refer to the image_id are executed on the service side.
   helper_->CommandBufferHelper::Flush();
   gpu_control_->DestroyImage(image_id);
 }
@@ skipping 226 matching lines @@
   CheckGLError();
 }
 
 // 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/client/gles2_implementation_impl_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu