Implement GLHelperReadbackSupport for GLHelper usage.

content/common/gpu/client/gl_helper.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 "content/common/gpu/client/gl_helper.h"
 
+#include <algorithm>
 #include <queue>
 #include <string>
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_util.h"
@@ skipping 154 matching lines @@
       const gfx::Rect& src_subrect,
       const gfx::Size& dst_size,
       const gfx::Rect& dst_subrect,
       bool flip_vertically,
       bool use_mrt);
 
   // Returns the maximum number of draw buffers available,
   // 0 if GL_EXT_draw_buffers is not available.
   GLint MaxDrawBuffers() const { return max_draw_buffers_; }
 
+  bool IsReadBackConfigSupported(SkBitmap::Config bitmap_config);
+
  private:
   // A single request to CropScaleReadbackAndCleanTexture.
   // The main thread can cancel the request, before it's handled by the helper
   // thread, by resetting the texture and pixels fields. Alternatively, the
   // thread marks that it handles the request by resetting the pixels field
   // (meaning it guarantees that the callback with be called).
   // In either case, the callback must be called exactly once, and the texture
   // must be deleted by the main thread gl.
   struct Request {
     Request(const gfx::Size& size_,
@@ skipping 143 matching lines @@
                                     swizzle);
 }
 
 GLuint GLHelper::CopyTextureToImpl::ScaleTexture(
     GLuint src_texture,
     const gfx::Size& src_size,
     const gfx::Rect& src_subrect,
     const gfx::Size& dst_size,
     bool vertically_flip_texture,
     bool swizzle,
-    SkBitmap::Config config,
+    SkBitmap::Config bitmap_config,
     GLHelper::ScalerQuality quality) {
 
-  bool format_support = ((config == SkBitmap::kRGB_565_Config) ||
-                         (config == SkBitmap::kARGB_8888_Config));
-  if (!format_support) {
-    DCHECK(format_support);
+  if (!IsReadBackConfigSupported(bitmap_config))
     return 0;
-  }
+
   scoped_ptr<ScalerInterface> scaler(
       helper_->CreateScaler(quality,
                             src_size,
                             src_subrect,
                             dst_size,
                             vertically_flip_texture,
                             swizzle));
   GLuint dst_texture = 0u;
   // Start with ARGB8888 params as any other format which is not
   // supported is already asserted above.
   GLenum format = GL_RGBA , type = GL_UNSIGNED_BYTE;
   gl_->GenTextures(1, &dst_texture);
   {
     ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, dst_texture);
-    switch (config) {
+    switch (bitmap_config) {
       case SkBitmap::kARGB_8888_Config:
         // Do nothing params already set.
         break;
       case SkBitmap::kRGB_565_Config:
         format = GL_RGB;
         type = GL_UNSIGNED_SHORT_5_6_5;
         break;
       default:
         NOTREACHED();
         break;
@@ skipping 10 matching lines @@
   }
   scaler->Scale(src_texture, dst_texture);
   return dst_texture;
 }
 
 void GLHelper::CopyTextureToImpl::ReadbackAsync(
     const gfx::Size& dst_size,
     int32 bytes_per_row,
     int32 row_stride_bytes,
     unsigned char* out,
-    const SkBitmap::Config config,
+    const SkBitmap::Config bitmap_config,
     const base::Callback<void(bool)>& callback) {
-  bool format_support = ((config == SkBitmap::kRGB_565_Config) ||
-                         (config == SkBitmap::kARGB_8888_Config));
-  if (!format_support) {
-    DCHECK(format_support);
+
+   if (!IsReadBackConfigSupported(bitmap_config)) {

[no sievers, 2014/02/18 19:35:44]

See comment in ReadbackTextureAsync().

[sivag, 2014/02/19 15:49:04]

Replied at ReadbackTextureAsync.

     callback.Run(false);
     return;
   }
   Request* request =
       new Request(dst_size, bytes_per_row, row_stride_bytes, out, callback);
   request_queue_.push(request);
   request->buffer = 0u;
   // Start with ARGB8888 params as any other format which is not
   // supported is already asserted above.
   GLenum format = GL_RGBA, type = GL_UNSIGNED_BYTE;
   int bytes_per_pixel = 4;
 
-  switch (config) {
+  switch (bitmap_config) {
     case SkBitmap::kARGB_8888_Config:
       // Do nothing params already set.
       break;
     case SkBitmap::kRGB_565_Config:
       format = GL_RGB;
       type = GL_UNSIGNED_SHORT_5_6_5;
       bytes_per_pixel = 2;
       break;
     default:
       NOTREACHED();
@@ skipping 25 matching lines @@
 }
 void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(
     GLuint src_texture,
     const gfx::Size& src_size,
     const gfx::Rect& src_subrect,
     const gfx::Size& dst_size,
     unsigned char* out,
     const SkBitmap::Config bitmap_config,
     const base::Callback<void(bool)>& callback,
     GLHelper::ScalerQuality quality) {
-  bool format_support = ((bitmap_config == SkBitmap::kRGB_565_Config) ||
-                         (bitmap_config == SkBitmap::kARGB_8888_Config));
-  if (!format_support) {
-    DCHECK(format_support);
+
+  if (!IsReadBackConfigSupported(bitmap_config)) {
     callback.Run(false);
     return;
   }
   GLuint texture = ScaleTexture(src_texture,
                                 src_size,
                                 src_subrect,
                                 dst_size,
                                 true,
 #if (SK_R32_SHIFT == 16) && !SK_B32_SHIFT
                                 true,
@@ skipping 31 matching lines @@
                 bitmap_config,
                 callback);
   gl_->DeleteTextures(1, &texture);
 }
 
 void GLHelper::CopyTextureToImpl::ReadbackTextureSync(GLuint texture,
                                                       const gfx::Rect& src_rect,
                                                       unsigned char* out,
                                                       SkBitmap::Config config) {
   DCHECK((config == SkBitmap::kRGB_565_Config) ||
          (config == SkBitmap::kARGB_8888_Config));

[no sievers, 2014/02/18 19:35:44]

See comment below. The check here should be consistent with
ReadbackTextureAsync().

[sivag, 2014/02/19 15:49:04]

Done.

   ScopedFramebuffer dst_framebuffer(gl_);
   ScopedFramebufferBinder<GL_FRAMEBUFFER> framebuffer_binder(gl_,
                                                              dst_framebuffer);
   ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, texture);
   gl_->FramebufferTexture2D(
       GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
   GLenum format = (config == SkBitmap::kRGB_565_Config) ?
                   GL_RGB :
                   GL_RGBA;
   GLenum type = (config == SkBitmap::kRGB_565_Config) ?
                 GL_UNSIGNED_SHORT_5_6_5 :
                 GL_UNSIGNED_BYTE;
   gl_->ReadPixels(src_rect.x(),
                   src_rect.y(),
                   src_rect.width(),
                   src_rect.height(),
                   format,
                   type,
                   out);
 }
 
 void GLHelper::CopyTextureToImpl::ReadbackTextureAsync(
     GLuint texture,
     const gfx::Size& dst_size,
     unsigned char* out,
-    SkBitmap::Config config,
+    SkBitmap::Config bitmap_config,
     const base::Callback<void(bool)>& callback) {
-  // Only ARGB888 and RGB565 supported as of now.
-  bool format_support = ((config == SkBitmap::kRGB_565_Config) ||
-                         (config == SkBitmap::kARGB_8888_Config));
-  if (!format_support) {
-    DCHECK(format_support);
+
+  if (!IsReadBackConfigSupported(bitmap_config))

[no sievers, 2014/02/18 19:35:44]

This check is actually not sufficient.
IsReadBackConfigSupported() tells you whether we can read back a 565 texture
1:1. However, what if |texture| here is RGBA8?

Maybe we should just remove the checks in ReadbackTexture(A)Sync, and leave it
to the client to make sure readbacks are not requested with implied format
conversion?

For the internal call sites where we create an intermediate texture, we already
have the checks and create the texture in the destination format.

[sivag, 2014/02/19 15:49:04]

I think then we should actually validate the texture also using
GetTexParameteriv for the GL_TEXTURE_INTERNAL_FORMAT , and match that with the
SkBitmap::Config passed.

Or 

we should remove the config parameter itself , in both the Readbacktexturesync
and async and validate just by using the texture by getting the interformat
details using GetTexParameteriv.

[no sievers, 2014/02/19 19:09:38]

Sounds good. Feel free to try as a follow-up since there's no need to change it
functionally in this patch. But you want to do it as a DCHECK() only since it's
synchronous. Also, is that check with GL_TEXTURE_INTERNAL_FORMAT even supported
in OpenGL ES?

[sivag, 2014/02/21 11:40:50]

When i checked , there is no such support in the api list.
I will confirm this.

     return;
-  }
+
   ScopedFramebuffer dst_framebuffer(gl_);
   ScopedFramebufferBinder<GL_FRAMEBUFFER> framebuffer_binder(gl_,
                                                              dst_framebuffer);
   ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, texture);
   gl_->FramebufferTexture2D(GL_FRAMEBUFFER,
                             GL_COLOR_ATTACHMENT0,
                             GL_TEXTURE_2D,
                             texture,
                             0);
-  int bytes_per_pixel = (config == SkBitmap::kRGB_565_Config) ? 2 : 4;
+  int bytes_per_pixel = (bitmap_config == SkBitmap::kRGB_565_Config) ? 2 : 4;
   ReadbackAsync(dst_size,
                 dst_size.width() * bytes_per_pixel,
                 dst_size.width() * bytes_per_pixel,
                 out,
-                config,
+                bitmap_config,
                 callback);
 }
 
 GLuint GLHelper::CopyTextureToImpl::CopyAndScaleTexture(
     GLuint src_texture,
     const gfx::Size& src_size,
     const gfx::Size& dst_size,
     bool vertically_flip_texture,
     GLHelper::ScalerQuality quality) {
   return ScaleTexture(src_texture,
                       src_size,
                       gfx::Rect(src_size),
                       dst_size,
                       vertically_flip_texture,
                       false,
                       SkBitmap::kARGB_8888_Config,
                       quality);
 }
 
+bool GLHelper::CopyTextureToImpl::IsReadBackConfigSupported(
+    SkBitmap::Config bitmap_config) {
+  if(!helper_){

[piman, 2014/02/18 22:23:15]

nit: spaces

Try git cl format.

[sivag, 2014/02/19 15:49:04]

Done.

+    NOTREACHED();
+    return false;
+  }
+  return helper_->IsReadBackConfigSupported(bitmap_config);
+}
+
 void GLHelper::CopyTextureToImpl::ReadbackDone(Request* finished_request,
                                                int bytes_per_pixel) {
   TRACE_EVENT0("mirror",
                "GLHelper::CopyTextureToImpl::CheckReadbackFramebufferComplete");
   finished_request->done = true;
 
   // We process transfer requests in the order they were received, regardless
   // of the order we get the callbacks in.
   while (!request_queue_.empty()) {
     Request* request = request_queue_.front();
@@ skipping 47 matching lines @@
 
 void GLHelper::CopyTextureToImpl::CancelRequests() {
   while (!request_queue_.empty()) {
     Request* request = request_queue_.front();
     FinishRequest(request, false);
   }
 }
 
 GLHelper::GLHelper(GLES2Interface* gl, gpu::ContextSupport* context_support)
     : gl_(gl),
-      context_support_(context_support),
-      initialized_565_format_check_(false),
-      support_565_format_(false) {}
+      context_support_(context_support) {}
 
 GLHelper::~GLHelper() {}
 
 void GLHelper::CropScaleReadbackAndCleanTexture(
     GLuint src_texture,
     const gfx::Size& src_size,
     const gfx::Rect& src_subrect,
     const gfx::Size& dst_size,
     unsigned char* out,
     const SkBitmap::Config config,
@@ skipping 202 matching lines @@
                          rect.width(),
                          rect.height());
 }
 
 void GLHelper::CopyTextureFullImage(GLuint texture, const gfx::Size& size) {
   content::ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, texture);
   gl_->CopyTexImage2D(
       GL_TEXTURE_2D, 0, GL_RGB, 0, 0, size.width(), size.height(), 0);
 }
 
-bool GLHelper::CanUseRgb565Readback() {
-  if(initialized_565_format_check_){
-    return support_565_format_;
-  }
+bool GLHelper::SupportsFormat(GLint format, GLint type) {
   const int kTestSize = 64;
   GLuint dst_texture = 0u;
+  bool supports_format = false;
   gl_->GenTextures(1, &dst_texture);
   ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, dst_texture);
   gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
   gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
   gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
   gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
   gl_->TexImage2D(GL_TEXTURE_2D,
                   0,
-                  GL_RGB,
+                  format,
                   kTestSize,
                   kTestSize,
                   0,
-                  GL_RGB,
-                  GL_UNSIGNED_SHORT_5_6_5,
+                  format,
+                  type,
                   NULL);
   ScopedFramebuffer dst_framebuffer(gl_);
   ScopedFramebufferBinder<GL_FRAMEBUFFER> framebuffer_binder(gl_,
                                                              dst_framebuffer);
   gl_->FramebufferTexture2D(GL_FRAMEBUFFER,
                             GL_COLOR_ATTACHMENT0,
                             GL_TEXTURE_2D,
                             dst_texture,
                             0);
   GLint  ext_format = 0, ext_type = 0;
   gl_->GetIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT, &ext_format);
   gl_->GetIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE, &ext_type);
   gl_->DeleteTextures(1, &dst_texture);
-  if ((ext_format == GL_RGB) && (ext_type == GL_UNSIGNED_SHORT_5_6_5)) {
-    support_565_format_ = true;
+  if ((ext_format == format) && (ext_type == type)) {
+    supports_format = true;
   }
-  initialized_565_format_check_ = true;
-  return support_565_format_;
+  return supports_format;
+}
+
+bool GLHelper::IsReadBackConfigSupported(
+  const SkBitmap::Config& texture_format) {

[no sievers, 2014/02/18 19:35:44]

nit: bitmap_format

[sivag, 2014/02/19 15:49:04]

Done.

+
+  std::map<SkBitmap::Config, bool>::iterator find_format =
+                                          config_map_.find(texture_format);
+  if (find_format != config_map_.end())
+    return find_format->second;
+
+  switch (texture_format) {
+    case SkBitmap::kRGB_565_Config:
+      return config_map_[texture_format] = SupportsFormat(GL_RGB,
+                                                     GL_UNSIGNED_SHORT_5_6_5);

[no sievers, 2014/02/18 19:35:44]

nit: indent

[sivag, 2014/02/19 15:49:04]

Done.

+
+    case SkBitmap::kARGB_8888_Config:
+      return config_map_[texture_format] = SupportsFormat(GL_RGBA,
+                                                         GL_UNSIGNED_BYTE);
+    case SkBitmap::kA8_Config:
+    case SkBitmap::kARGB_4444_Config:
+    default:
+      NOTREACHED();
+      return config_map_[texture_format] = false;
+  }
 }
 
 void GLHelper::CopyTextureToImpl::ReadbackPlane(
     TextureFrameBufferPair* source,
     const scoped_refptr<media::VideoFrame>& target,
     int plane,
     int size_shift,
     const gfx::Rect& dst_subrect,
     const base::Callback<void(bool)>& callback) {
   gl_->BindFramebuffer(GL_FRAMEBUFFER, source->framebuffer());
@@ skipping 307 matching lines @@
   return copy_texture_to_impl_->CreateReadbackPipelineYUV(quality,
                                                           src_size,
                                                           src_subrect,
                                                           dst_size,
                                                           dst_subrect,
                                                           flip_vertically,
                                                           use_mrt);
 }
 
 }  // namespace content