Fix ReadPixels implementation specific read format/type on desktop GL.

gpu/command_buffer/common/gles2_cmd_utils.cc

Index: gpu/command_buffer/common/gles2_cmd_utils.cc
diff --git a/gpu/command_buffer/common/gles2_cmd_utils.cc b/gpu/command_buffer/common/gles2_cmd_utils.cc
index 94692c10b63a1cf7c5683d4e2b4b8b29b02b0035..bdad9ffe7fc8211bdfda2bce0b5a32d80a46d699 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils.cc
+++ b/gpu/command_buffer/common/gles2_cmd_utils.cc
@@ -811,12 +811,43 @@ size_t GLES2Util::GLTargetToFaceIndex(uint32 target) {
 
 uint32 GLES2Util::GetPreferredGLReadPixelsFormat(uint32 internal_format) {
   switch (internal_format) {
-    case GL_RGB16F_EXT:
-    case GL_RGB32F_EXT:
+    case GL_R8:
+    case GL_R16F:
+    case GL_R32F:
+      return GL_RED;
+    case GL_R8UI:
+    case GL_R8I:
+    case GL_R16UI:
+    case GL_R16I:
+    case GL_R32UI:
+    case GL_R32I:
+      return GL_RED_INTEGER;
+    case GL_RG8:
+    case GL_RG16F:
+    case GL_RG32F:
+      return GL_RG;
+    case GL_RG8UI:
+    case GL_RG8I:
+    case GL_RG16UI:
+    case GL_RG16I:
+    case GL_RG32UI:
+    case GL_RG32I:
+      return GL_RG_INTEGER;
+    case GL_RGB:
+    case GL_RGB8:
+    case GL_RGB565:
+    case GL_RGB16F:
+    case GL_RGB32F:
+    case GL_RGBA8:

[Ken Russell (switch to Gerrit), 2015/08/26 01:00:44]

Is the GL_RGBA8 intended? Looks like an accident.

[Zhenyao Mo, 2015/08/26 01:11:05]

It is intended to allow RGB readback for RGBA8 as an alternative.

[Ken Russell (switch to Gerrit), 2015/08/26 01:53:32]

OK. In that case "GetPreferredGLReadPixelsFormat" is a little confusing since
it's really the preferred *implementation* read format.

[Zhenyao Mo, 2015/08/26 21:21:25]

Switched to a different name.

       return GL_RGB;
-    case GL_RGBA16F_EXT:
-    case GL_RGBA32F_EXT:
-      return GL_RGBA;
+    case GL_RGBA8UI:
+    case GL_RGBA8I:
+    case GL_RGB10_A2UI:
+    case GL_RGBA16UI:
+    case GL_RGBA16I:
+    case GL_RGBA32UI:
+    case GL_RGBA32I:
+      return GL_RGB_INTEGER;
     default:
       return GL_RGBA;
   }
@@ -825,11 +856,38 @@ uint32 GLES2Util::GetPreferredGLReadPixelsFormat(uint32 internal_format) {
 uint32 GLES2Util::GetPreferredGLReadPixelsType(
     uint32 internal_format, uint32 texture_type) {
   switch (internal_format) {
-    case GL_RGBA32F_EXT:
-    case GL_RGB32F_EXT:
+    case GL_R16UI:
+    case GL_RG16UI:
+    case GL_RGBA16UI:
+    case GL_RGB10_A2:
+    case GL_RGB10_A2UI:
+      return GL_UNSIGNED_SHORT;
+    case GL_R32UI:
+    case GL_RG32UI:
+    case GL_RGBA32UI:
+      return GL_UNSIGNED_INT;
+    case GL_R8I:
+    case GL_RG8I:
+    case GL_RGBA8I:
+      return GL_BYTE;
+    case GL_R16I:
+    case GL_RG16I:
+    case GL_RGBA16I:
+      return GL_SHORT;
+    case GL_R32I:
+    case GL_RG32I:
+    case GL_RGBA32I:
+      return GL_INT;
+    case GL_R32F:
+    case GL_RG32F:
+    case GL_RGB32F:
+    case GL_RGBA32F:
       return GL_FLOAT;
-    case GL_RGBA16F_EXT:
-    case GL_RGB16F_EXT:
+    case GL_R16F:
+    case GL_RG16F:
+    case GL_RGB16F:
+    case GL_RGBA16F:
+      // TODO(zmo): Return GL_HALF_FLOAT on ES3.

[Ken Russell (switch to Gerrit), 2015/08/26 01:00:44]

Additionally -- I think that EXT_color_buffer_float on ES3 mandates that
half-float color buffers actually be read back as type FLOAT, rather than
HALF_FLOAT. Not sure whether the implementation color read type could be
HALF_FLOAT for these.

[Zhenyao Mo, 2015/08/26 01:11:05]

Then the original code is a bug?

[Ken Russell (switch to Gerrit), 2015/08/26 01:53:33]

For ES 3.0, yes -- I think this is something that will need to be fixed when we
expose EXT_color_buffer_float to WebGL 2.0 contexts. Worth expanding the TODO?

[Zhenyao Mo, 2015/08/26 21:21:24]

OK, I think I will return GL_FLOAT so the renderbuffer case is consistent with
the texture case in WebGL 1 below. Add a TODO to consider return HALF_FLOAT in
WebGL 2.  I checked the spec and it is a possibility.

       return GL_HALF_FLOAT_OES;
     case GL_RGBA:
     case GL_RGB: