Support format using enum argument for Async readback

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 <queue>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 109 matching lines @@
                                  uint32 sync_point) {
     return helper_->ConsumeMailboxToTexture(mailbox, sync_point);
   }
 
   void CropScaleReadbackAndCleanTexture(
       GLuint src_texture,
       const gfx::Size& src_size,
       const gfx::Rect& src_subrect,
       const gfx::Size& dst_size,
       unsigned char* out,
-      bool readback_config_rgb565,
+      const SkBitmap::Config config,
       const base::Callback<void(bool)>& callback,
       GLHelper::ScalerQuality quality);
 
   void ReadbackTextureSync(GLuint texture,
                            const gfx::Rect& src_rect,
                            unsigned char* out,
                            SkBitmap::Config format);
 
   // Reads back bytes from the currently bound frame buffer.
   // Note that dst_size is specified in bytes, not pixels.
   void ReadbackAsync(const gfx::Size& dst_size,
                      int32 bytes_per_row,     // generally dst_size.width() * 4
                      int32 row_stride_bytes,  // generally dst_size.width() * 4
                      unsigned char* out,
-                     bool readback_config_rgb565,
+                     const SkBitmap::Config config,
                      const base::Callback<void(bool)>& callback);
 
   void 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);
 
   GLuint CopyAndScaleTexture(GLuint texture,
@@ skipping 125 matching lines @@
 
   // Copies the block of pixels specified with |src_subrect| from |src_texture|,
   // scales it to |dst_size|, writes it into a texture, and returns its ID.
   // |src_size| is the size of |src_texture|.
   GLuint 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,
-                      bool readback_config_rgb565,
+                      SkBitmap::Config config,
                       GLHelper::ScalerQuality quality);
 
   static void nullcallback(bool success) {}
   void ReadbackDone(Request *request, int bytes_per_pixel);
   void FinishRequest(Request* request, bool result);
   void CancelRequests();
 
   static const float kRGBtoYColorWeights[];
   static const float kRGBtoUColorWeights[];
   static const float kRGBtoVColorWeights[];
@@ skipping 25 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,
-    bool readback_config_rgb565,
+    SkBitmap::Config config,
     GLHelper::ScalerQuality quality) {
+
+  bool format_support = ((config == SkBitmap::kRGB_565_Config) ||
+                         (config == SkBitmap::kARGB_8888_Config));
+  if (!format_support) {
+    DCHECK(format_support);
+    LOG(ERROR) << "Format not supported.";

[piman, 2014/01/22 22:10:21]

no need for this LOG. DCHECK is enough.

[sivag, 2014/01/23 12:29:20]

Done.

+    return GL_INVALID_ENUM;

[piman, 2014/01/22 22:10:21]

Maybe return 0 rather than GL_INVALID_ENUM. GL_INVALID_ENUM will look like a
valid texture id.

[sivag, 2014/01/23 12:29:20]

Done.

+  }
   scoped_ptr<ScalerInterface> scaler(
       helper_->CreateScaler(quality,
                             src_size,
                             src_subrect,
                             dst_size,
                             vertically_flip_texture,
                             swizzle));
-
   GLuint dst_texture = 0u;
   gl_->GenTextures(1, &dst_texture);
   {
     ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, dst_texture);
-    GLenum format = readback_config_rgb565 ? GL_RGB : GL_RGBA;
-    GLenum type = readback_config_rgb565 ?
-                  GL_UNSIGNED_SHORT_5_6_5 :
-                  GL_UNSIGNED_BYTE;
+    // Start with ARGB8888 params as any other format which is not
+    // supported is already asserted above.
+    GLenum format = GL_RGBA , type = GL_UNSIGNED_BYTE;
+    switch (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:
+        NOTIMPLEMENTED();

[piman, 2014/01/22 22:10:21]

nit: NOTREACHED() since it's tested above.

[sivag, 2014/01/23 12:29:20]

Done.

+        break;
+    }
     gl_->TexImage2D(GL_TEXTURE_2D,
                     0,
                     format,
                     dst_size.width(),
                     dst_size.height(),
                     0,
                     format,
                     type,
                     NULL);
   }
   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,
-    bool readback_config_rgb565,
+    const SkBitmap::Config 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);
+    LOG(ERROR) << "Format not supported.";

[piman, 2014/01/22 22:10:21]

no need for the LOG.

[sivag, 2014/01/23 12:29:20]

Done.

+    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;
-  int bytes_per_pixel = readback_config_rgb565 ? 2 : 4;
+  // Start with ARGB8888 params as any other format which is not
+  // supported is already asserted above.
+  GLenum format = GL_RGBA , type = GL_UNSIGNED_BYTE;

[piman, 2014/01/22 22:10:21]

nit: no space before ,

[sivag, 2014/01/23 12:29:20]

Done.

[sivag, 2014/01/23 12:29:20]

Done.

+  int bytes_per_pixel = 4;
+
+  switch (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:
+      NOTIMPLEMENTED();

[piman, 2014/01/22 22:10:21]

nit: NOTREACHED()

[sivag, 2014/01/23 12:29:20]

Done.

+      break;
+  }
   gl_->GenBuffers(1, &request->buffer);
   gl_->BindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, request->buffer);
   gl_->BufferData(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
                   bytes_per_pixel * dst_size.GetArea(),
                   NULL,
                   GL_STREAM_READ);
 
   request->query = 0u;
   gl_->GenQueriesEXT(1, &request->query);
   gl_->BeginQueryEXT(GL_ASYNC_PIXEL_PACK_COMPLETED_CHROMIUM, request->query);
-  GLenum format = readback_config_rgb565 ? GL_RGB : GL_RGBA;
-  GLenum type = readback_config_rgb565 ?
-                GL_UNSIGNED_SHORT_5_6_5 :
-                GL_UNSIGNED_BYTE;
   gl_->ReadPixels(0,
                   0,
                   dst_size.width(),
                   dst_size.height(),
                   format,
                   type,
                   NULL);
   gl_->EndQueryEXT(GL_ASYNC_PIXEL_PACK_COMPLETED_CHROMIUM);
   gl_->BindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, 0);
   context_support_->SignalQuery(
       request->query,
       base::Bind(&CopyTextureToImpl::ReadbackDone, AsWeakPtr(),
                  request, bytes_per_pixel));
 }
 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,
-    bool readback_config_rgb565,
+    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);
+    LOG(ERROR) << "Format not supported.";

[piman, 2014/01/22 22:10:21]

nit: remove.

[sivag, 2014/01/23 12:29:20]

Done.

+    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,
 #else
                                 false,
 #endif
-                                readback_config_rgb565,
+                                bitmap_config,
                                 quality);
+  if(texture == GL_INVALID_ENUM) {

[piman, 2014/01/22 22:10:21]

nit: you can just DCHECK(texture), since you tested above.

[sivag, 2014/01/23 12:29:20]

Done.

+    callback.Run(false);
+    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 = readback_config_rgb565 ? 2 : 4;
+  int bytes_per_pixel = 4;
+  switch (bitmap_config) {
+    case SkBitmap::kARGB_8888_Config:
+      // Do nothing params already set.
+      break;
+    case SkBitmap::kRGB_565_Config:
+      bytes_per_pixel = 2;
+      break;
+    default:
+      NOTIMPLEMENTED();

[piman, 2014/01/22 22:10:21]

NOTREACHED()

[sivag, 2014/01/23 12:29:20]

Done.

+      break;
+  }
   ReadbackAsync(dst_size,
                 dst_size.width() * bytes_per_pixel,
                 dst_size.width() * bytes_per_pixel,
                 out,
-                readback_config_rgb565,
+                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));
   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);
-  DCHECK((config == SkBitmap::kRGB_565_Config) ||
-         (config == SkBitmap::kARGB_8888_Config));
   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);
 }
 
 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,
-                      false,
+                      SkBitmap::kARGB_8888_Config,
                       quality);
 }
 
 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
@@ skipping 62 matching lines @@
       support_565_format_(false) {}
 
 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,
-    bool readback_config_rgb565,
+    const SkBitmap::Config config,
     const base::Callback<void(bool)>& callback) {
   InitCopyTextToImpl();
   copy_texture_to_impl_->CropScaleReadbackAndCleanTexture(
       src_texture,
       src_size,
       src_subrect,
       dst_size,
       out,
-      readback_config_rgb565,
+      config,
       callback,
       GLHelper::SCALER_QUALITY_FAST);
 }
 
 void GLHelper::CropScaleReadbackAndCleanMailbox(
     const gpu::Mailbox& src_mailbox,
     uint32 sync_point,
     const gfx::Size& src_size,
     const gfx::Rect& src_subrect,
     const gfx::Size& dst_size,
     unsigned char* out,
-    bool readback_config_rgb565,
+    const SkBitmap::Config bitmap_config,
     const base::Callback<void(bool)>& callback) {
   GLuint mailbox_texture = ConsumeMailboxToTexture(src_mailbox, sync_point);
   CropScaleReadbackAndCleanTexture(
       mailbox_texture, src_size, src_subrect, dst_size, out,
-      readback_config_rgb565,
+      bitmap_config,
       callback);
   gl_->DeleteTextures(1, &mailbox_texture);
 }
 
 void GLHelper::ReadbackTextureSync(GLuint texture,
                                    const gfx::Rect& src_rect,
                                    unsigned char* out,
                                    SkBitmap::Config format) {
   InitCopyTextToImpl();
   copy_texture_to_impl_->ReadbackTextureSync(texture, src_rect, out, format);
@@ skipping 211 matching lines @@
     int size_shift,
     const gfx::Rect& dst_subrect,
     const base::Callback<void(bool)>& callback) {
   gl_->BindFramebuffer(GL_FRAMEBUFFER, source->framebuffer());
   size_t offset = target->stride(plane) * (dst_subrect.y() >> size_shift) +
       (dst_subrect.x() >> size_shift);
   ReadbackAsync(source->size(),
                 dst_subrect.width() >> size_shift,
                 target->stride(plane),
                 target->data(plane) + offset,
-                false,
+                SkBitmap::kARGB_8888_Config,
                 callback);
 }
 
 const float GLHelper::CopyTextureToImpl::kRGBtoYColorWeights[] = {
     0.257f, 0.504f, 0.098f, 0.0625f};
 const float GLHelper::CopyTextureToImpl::kRGBtoUColorWeights[] = {
     -0.148f, -0.291f, 0.439f, 0.5f};
 const float GLHelper::CopyTextureToImpl::kRGBtoVColorWeights[] = {
     0.439f, -0.368f, -0.071f, 0.5f};
 
@@ skipping 290 matching lines @@
   return copy_texture_to_impl_->CreateReadbackPipelineYUV(quality,
                                                           src_size,
                                                           src_subrect,
                                                           dst_size,
                                                           dst_subrect,
                                                           flip_vertically,
                                                           use_mrt);
 }
 
 }  // namespace content