Add readback path for CopyTextureCHROMIUM

gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc

Index: gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc b/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
index 338772a23351f630855dd291b39525b7f0656d9b..14ad3ff5f7e4ffb888647bdd5a86b7d192f9014a 100644
--- a/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
+++ b/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
@@ -617,6 +617,190 @@ void DoCopyTexSubImage2D(const gpu::gles2::GLES2Decoder* decoder,
   decoder->RestoreFramebufferBindings();
 }
 
+// Convert RGBA/UNSIGNED_BYTE source to RGB/UNSIGNED_BYTE destination.
+void convertToRGB(const uint8_t* source,
+                  uint8_t* destination,
+                  unsigned length) {
+  for (unsigned i = 0; i < length; ++i) {
+    destination[0] = source[0];
+    destination[1] = source[1];
+    destination[2] = source[2];
+    source += 4;
+    destination += 3;
+  }
+}
+
+// Convert RGBA/UNSIGNED_BYTE source to RGB/FLOAT destination.
+void convertToRGBFloat(const uint8_t* source,
+                       float* destination,
+                       unsigned length) {
+  const float scaleFactor = 1.0f / 255.0f;
+  for (unsigned i = 0; i < length; ++i) {
+    destination[0] = source[0] * scaleFactor;
+    destination[1] = source[1] * scaleFactor;
+    destination[2] = source[2] * scaleFactor;
+    source += 4;
+    destination += 3;
+  }
+}
+
+// Prepare the image data to be uploaded to a texture in pixel unpack buffer.
+void prepareUnpackBuffer(GLuint buffer[2],
+                         bool is_es,
+                         GLenum format,
+                         GLenum type,
+                         GLsizei width,
+                         GLsizei height) {
+  uint32_t pixel_num = width * height;
+
+  // Result of glReadPixels with format == GL_RGB and type == GL_UNSIGNED_BYTE
+  // from read framebuffer in RGBA fromat is not correct on desktop core
+  // profile on both Linux Mesa and Linux NVIDIA. This may be a driver bug.
+  bool is_rgb_unsigned_byte = format == GL_RGB && type == GL_UNSIGNED_BYTE;
+  if ((!is_es && !is_rgb_unsigned_byte) ||
+      (format == GL_RGBA && type == GL_UNSIGNED_BYTE)) {
+    uint32_t bytes_per_group =
+        gpu::gles2::GLES2Util::ComputeImageGroupSize(format, type);
+    glBindBuffer(GL_PIXEL_PACK_BUFFER, buffer[0]);
+    glBufferData(GL_PIXEL_PACK_BUFFER, pixel_num * bytes_per_group, 0,
+                 GL_STATIC_READ);
+    glReadPixels(0, 0, width, height, format, type, 0);
+    glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer[0]);
+    return;
+  }
+
+  uint32_t bytes_per_group =
+      gpu::gles2::GLES2Util::ComputeImageGroupSize(GL_RGBA, GL_UNSIGNED_BYTE);
+  uint32_t buf_size = pixel_num * bytes_per_group;
+
+  if (format == GL_RGB && type == GL_FLOAT) {
+#if defined(OS_ANDROID)
+    // Reading pixels to pbo with glReadPixels will cause random failures of
+    // GLCopyTextureCHROMIUMES3Test.FormatCombinations in gl_tests. This is seen
+    // on Nexus 5 but not Nexus 4. Read pixels to client memory, then upload to
+    // pixel unpack buffer with glBufferData.

[qiankun, 2017/03/29 09:00:10]

I spent a lot of time on investigating this issue on Nexus 5. But I didn't find
a better method to fix it. I used a workaround here. This code path only takes
effect on RGB9_E5 formats. No problem when testing RGB9_E5, but the following
tests for other formats after RGB9_E5 would fail randomly. 

[Zhenyao Mo, 2017/03/29 17:29:34]

This sounds like a memory corruption Qualcomm driver bug to me.  Sorry this cost
your time...

[qiankun, 2017/03/30 07:56:07]

I see. Thanks for pointing it out to me. Per my testing, this workaround gains
simialr performance as below implementation for RGB9_E5 format who will use this
path.

+    std::unique_ptr<uint8_t[]> pixels(new uint8_t[width * height * 4]);
+    glReadPixels(0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE, pixels.get());
+    std::unique_ptr<float[]> data(new float[width * height * 3]);
+    convertToRGBFloat(pixels.get(), data.get(), pixel_num);
+    bytes_per_group =
+        gpu::gles2::GLES2Util::ComputeImageGroupSize(format, type);
+    buf_size = pixel_num * bytes_per_group;
+    glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer[1]);
+    glBufferData(GL_PIXEL_UNPACK_BUFFER, buf_size, data.get(), GL_STATIC_DRAW);
+#else
+    glBindBuffer(GL_PIXEL_PACK_BUFFER, buffer[0]);
+    glBufferData(GL_PIXEL_PACK_BUFFER, buf_size, 0, GL_STATIC_READ);
+    glReadPixels(0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE, 0);
+    uint8_t* pixels = (uint8_t*)glMapBufferRange(GL_PIXEL_PACK_BUFFER, 0,

[Zhenyao Mo, 2017/03/29 17:29:34]

This is against chromium coding style. Use cast.

[qiankun, 2017/03/30 07:56:07]

Done.

+                                                 buf_size, GL_MAP_READ_BIT);
+
+    glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer[1]);
+    bytes_per_group =
+        gpu::gles2::GLES2Util::ComputeImageGroupSize(format, type);
+    buf_size = pixel_num * bytes_per_group;
+    glBufferData(GL_PIXEL_UNPACK_BUFFER, buf_size, 0, GL_STATIC_DRAW);
+    float* data = (float*)glMapBufferRange(GL_PIXEL_UNPACK_BUFFER, 0, buf_size,

[Zhenyao Mo, 2017/03/29 17:29:34]

Same here, use cast.

[qiankun, 2017/03/30 07:56:07]

Done.

+                                           GL_MAP_WRITE_BIT);

[Zhenyao Mo, 2017/03/29 17:29:34]

Combine with GL_MAP_INVALIDATE_BUFFER_BIT and see if it's faster.

[qiankun, 2017/03/30 07:56:07]

The GL_MAP_INVALIDATE_BUFFER_BIT didn't have performance benefit per my testing.
But, it will cause poor performance when I click the refresh button for a second
round testing (texImage2D runs/sec: from 47 to 37 for RGB9_E5). So, I didn't use
it here.

[Zhenyao Mo, 2017/03/30 17:08:02]

That's quite the opposite from what we would expect with the
INVALIDATE_BUFFER_BIT... very unfortunate

+    convertToRGBFloat(pixels, data, pixel_num);
+    glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
+    glUnmapBuffer(GL_PIXEL_UNPACK_BUFFER);
+#endif
+    return;
+  }
+
+  if (format == GL_RGB && type == GL_UNSIGNED_BYTE) {
+    glBindBuffer(GL_PIXEL_PACK_BUFFER, buffer[0]);
+    glBufferData(GL_PIXEL_PACK_BUFFER, buf_size, 0, GL_DYNAMIC_DRAW);
+    glReadPixels(0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE, 0);
+    void* pixels = glMapBufferRange(GL_PIXEL_PACK_BUFFER, 0, buf_size,
+                                    GL_MAP_READ_BIT | GL_MAP_WRITE_BIT);
+    void* data = pixels;
+    convertToRGB((uint8_t*)pixels, (uint8_t*)data, pixel_num);
+    glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
+    glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer[0]);
+    return;
+  }
+
+  NOTREACHED();
+}
+
+void DoReadbackAndTexImage(bool is_tex_image,

[Zhenyao Mo, 2017/03/29 17:29:34]

It's better to define enum { kTexImage, kTexSubImage }. The code is much more
readable.

[qiankun, 2017/03/30 07:56:07]

Done.

+                           const gpu::gles2::GLES2Decoder* decoder,
+                           GLenum source_target,
+                           GLuint source_id,
+                           GLint source_level,
+                           GLenum dest_target,
+                           GLuint dest_id,
+                           GLint dest_level,
+                           GLenum dest_internal_format,
+                           GLint xoffset,
+                           GLint yoffset,
+                           GLsizei width,
+                           GLsizei height,
+                           GLuint framebuffer) {
+  DCHECK_EQ(static_cast<GLenum>(GL_TEXTURE_2D), source_target);
+  GLenum dest_binding_target =
+      gpu::gles2::GLES2Util::GLFaceTargetToTextureTarget(dest_target);
+  DCHECK(dest_binding_target == GL_TEXTURE_2D ||
+         dest_binding_target == GL_TEXTURE_CUBE_MAP);
+  DCHECK(source_level == 0 || decoder->GetFeatureInfo()->IsES3Capable());
+  if (BindFramebufferTexture2D(source_target, source_id, source_level,
+                               framebuffer)) {
+    glBindTexture(dest_binding_target, dest_id);
+    glTexParameterf(dest_binding_target, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    glTexParameterf(dest_binding_target, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    glTexParameteri(dest_binding_target, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    glTexParameteri(dest_binding_target, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+
+    GLenum format = GL_RGBA;
+    GLenum type = GL_UNSIGNED_BYTE;
+    switch (dest_internal_format) {
+      case GL_RGB9_E5:
+        format = GL_RGB;
+        type = GL_FLOAT;
+        break;
+      case GL_SRGB_EXT:
+      case GL_SRGB8:
+        format = GL_RGB;
+        break;
+      case GL_RGB5_A1:
+      case GL_SRGB_ALPHA_EXT:
+      case GL_SRGB8_ALPHA8:
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+
+    // TODO(qiankun.miao@intel.com): PIXEL_PACK_BUFFER and PIXEL_UNPACK_BUFFER
+    // are not supported in ES2.
+    bool is_es = decoder->GetFeatureInfo()->gl_version_info().is_es;
+    DCHECK(!is_es || decoder->GetFeatureInfo()->gl_version_info().is_es3);

[Zhenyao Mo, 2017/03/29 17:29:34]

You can just DCHECK(!decoder->GetFeatureInfo()->gl_version_info().is_es2).

[qiankun, 2017/03/30 07:56:07]

Done.

+
+    uint32_t buffer_num = is_es && format == GL_RGB && type == GL_FLOAT ? 2 : 1;
+    GLuint buffer[2];

[Zhenyao Mo, 2017/03/29 17:29:34]

Initialize buffer[0] and buffer[1] to 0.

[qiankun, 2017/03/30 07:56:07]

Done.

+    glGenBuffersARB(buffer_num, buffer);
+    prepareUnpackBuffer(buffer, is_es, format, type, width, height);
+
+    if (is_tex_image) {
+      glTexImage2D(dest_target, dest_level, dest_internal_format, width, height,
+                   0, format, type, 0);
+    } else {
+      glTexSubImage2D(dest_target, dest_level, xoffset, yoffset, width, height,
+                      format, type, 0);
+    }
+    glDeleteBuffersARB(buffer_num, buffer);
+  }
+
+  decoder->RestoreTextureState(source_id);
+  decoder->RestoreTextureState(dest_id);
+  decoder->RestoreTextureUnitBindings(0);
+  decoder->RestoreActiveTexture();
+  decoder->RestoreFramebufferBindings();
+  decoder->RestoreBufferBindings();
+}
+
 }  // namespace
 
 namespace gpu {
@@ -735,9 +919,11 @@ void CopyTextureCHROMIUMResourceManager::DoCopyTexture(
   GLint original_dest_level = dest_level;
   GLenum original_dest_target = dest_target;
   GLenum original_internal_format = dest_internal_format;
-  if (method == DRAW_AND_COPY) {
+  if (method == DRAW_AND_COPY || method == DRAW_AND_READBACK) {
     GLenum adjusted_internal_format =
-        getIntermediateFormat(dest_internal_format);
+        method == DRAW_AND_READBACK
+            ? GL_RGBA
+            : getIntermediateFormat(dest_internal_format);
     dest_target = GL_TEXTURE_2D;
     glGenTextures(1, &intermediate_texture);
     glBindTexture(dest_target, intermediate_texture);
@@ -758,11 +944,18 @@ void CopyTextureCHROMIUMResourceManager::DoCopyTexture(
       dest_target, dest_texture, dest_level, dest_internal_format, width,
       height, flip_y, premultiply_alpha, unpremultiply_alpha, kIdentityMatrix);
 
-  if (method == DRAW_AND_COPY) {
+  if (method == DRAW_AND_COPY || method == DRAW_AND_READBACK) {
     source_level = 0;
-    DoCopyTexImage2D(decoder, dest_target, intermediate_texture, source_level,
-                     original_dest_target, dest_id, original_dest_level,
-                     original_internal_format, width, height, framebuffer_);
+    if (method == DRAW_AND_COPY) {
+      DoCopyTexImage2D(decoder, dest_target, intermediate_texture, source_level,
+                       original_dest_target, dest_id, original_dest_level,
+                       original_internal_format, width, height, framebuffer_);
+    } else if (method == DRAW_AND_READBACK) {
+      DoReadbackAndTexImage(true, decoder, dest_target, intermediate_texture,
+                            source_level, original_dest_target, dest_id,
+                            original_dest_level, original_internal_format, 0, 0,
+                            width, height, framebuffer_);
+    }
     glDeleteTextures(1, &intermediate_texture);
   }
 }
@@ -805,9 +998,12 @@ void CopyTextureCHROMIUMResourceManager::DoCopySubTexture(
   GLint original_dest_level = dest_level;
   GLenum original_dest_target = dest_target;
   GLuint intermediate_texture = 0;
-  if (method == DRAW_AND_COPY) {
+  GLenum original_internal_format = dest_internal_format;
+  if (method == DRAW_AND_COPY || method == DRAW_AND_READBACK) {
     GLenum adjusted_internal_format =
-        getIntermediateFormat(dest_internal_format);
+        method == DRAW_AND_READBACK
+            ? GL_RGBA
+            : getIntermediateFormat(dest_internal_format);
     dest_target = GL_TEXTURE_2D;
     glGenTextures(1, &intermediate_texture);
     glBindTexture(dest_target, intermediate_texture);
@@ -834,12 +1030,19 @@ void CopyTextureCHROMIUMResourceManager::DoCopySubTexture(
       source_height, flip_y, premultiply_alpha, unpremultiply_alpha,
       kIdentityMatrix);
 
-  if (method == DRAW_AND_COPY) {
+  if (method == DRAW_AND_COPY || method == DRAW_AND_READBACK) {
     source_level = 0;
-    DoCopyTexSubImage2D(decoder, dest_target, intermediate_texture,
-                        source_level, original_dest_target, dest_id,
-                        original_dest_level, xoffset, yoffset, 0, 0, width,
-                        height, framebuffer_);
+    if (method == DRAW_AND_COPY) {
+      DoCopyTexSubImage2D(decoder, dest_target, intermediate_texture,
+                          source_level, original_dest_target, dest_id,
+                          original_dest_level, xoffset, yoffset, 0, 0, width,
+                          height, framebuffer_);
+    } else if (method == DRAW_AND_READBACK) {
+      DoReadbackAndTexImage(false, decoder, dest_target, intermediate_texture,
+                            source_level, original_dest_target, dest_id,
+                            original_dest_level, original_internal_format,
+                            xoffset, yoffset, width, height, framebuffer_);
+    }
     glDeleteTextures(1, &intermediate_texture);
   }
 }