Add RGB565 Texture readback support in gl_helper

content/common/gpu/client/gl_helper.cc

Index: content/common/gpu/client/gl_helper.cc
diff --git a/content/common/gpu/client/gl_helper.cc b/content/common/gpu/client/gl_helper.cc
index 3ea2557a69dbb970af8e6b656a62001b30e82d50..0562b00cfd33b722c1b7e5765fbfaa238c5646ad 100644
--- a/content/common/gpu/client/gl_helper.cc
+++ b/content/common/gpu/client/gl_helper.cc
@@ -139,9 +139,12 @@ class GLHelper::CopyTextureToImpl :
       const gfx::Rect& src_subrect,
       const gfx::Size& dst_size,
       unsigned char* out,
+      bool readback_config_rgb565,
       const base::Callback<void(bool)>& callback,
       GLHelper::ScalerQuality quality);
 
+  WebGLId CopyTo565Texture(const gfx::Size& dst_size);
+
   void ReadbackTextureSync(WebGLId texture,
                            const gfx::Rect& src_rect,
                            unsigned char* out);
@@ -153,6 +156,8 @@ class GLHelper::CopyTextureToImpl :
       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,
+      int readback_factor,
       const base::Callback<void(bool)>& callback);
 
   void ReadbackPlane(TextureFrameBufferPair* source,
@@ -308,7 +313,7 @@ class GLHelper::CopyTextureToImpl :
                        GLHelper::ScalerQuality quality);
 
   static void nullcallback(bool success) {}
-  void ReadbackDone(Request *request);
+  void ReadbackDone(Request *request,int readback_factor);
   void FinishRequest(Request* request, bool result);
   void CancelRequests();
 
@@ -372,7 +377,7 @@ WebGLId GLHelper::CopyTextureToImpl::ScaleTexture(
                          GL_RGBA,
                          GL_UNSIGNED_BYTE,
                          NULL);
-  }
+}
   scaler->Scale(src_texture, dst_texture);
   return dst_texture;
 }
@@ -382,6 +387,8 @@ void GLHelper::CopyTextureToImpl::ReadbackAsync(
     int32 bytes_per_row,
     int32 row_stride_bytes,
     unsigned char* out,
+    bool readback_config_rgb565,
+    int readback_factor,

[hubbe, 2013/12/26 21:41:49]

readback_factor is redundant and not well named.
Just do something like:
int bytes_per_pixel = readback_config_rgb565 ? 2 : 4;
inside the function instead.

     const base::Callback<void(bool)>& callback) {
   Request* request = new Request(dst_size,
                                  bytes_per_row,
@@ -393,29 +400,57 @@ void GLHelper::CopyTextureToImpl::ReadbackAsync(
   context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
                        request->buffer);
   context_->bufferData(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
-                       4 * dst_size.GetArea(),
+                       readback_factor * dst_size.GetArea(),
                        NULL,
                        GL_STREAM_READ);
 
   request->query = context_->createQueryEXT();
   context_->beginQueryEXT(GL_ASYNC_PIXEL_PACK_COMPLETED_CHROMIUM,
                           request->query);
-  context_->readPixels(0, 0, dst_size.width(), dst_size.height(),
+  if(readback_config_rgb565){
+    context_->readPixels(0, 0, dst_size.width(), dst_size.height(),
+                       GL_RGB, GL_UNSIGNED_SHORT_5_6_5, NULL);
+  }else{
+    context_->readPixels(0, 0, dst_size.width(), dst_size.height(),
                        GL_RGBA, GL_UNSIGNED_BYTE, NULL);
+  }
   context_->endQueryEXT(GL_ASYNC_PIXEL_PACK_COMPLETED_CHROMIUM);
   context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, 0);
   context_support_->SignalQuery(
       request->query,
-      base::Bind(&CopyTextureToImpl::ReadbackDone, AsWeakPtr(), request));
+      base::Bind(&CopyTextureToImpl::ReadbackDone, AsWeakPtr(),
+                 request, readback_factor));
+}
+WebGLId GLHelper::CopyTextureToImpl::CopyTo565Texture(const gfx::Size& dst_size)
+{
+  //Using copy to texture

[hubbe, 2013/12/26 21:41:49]

Nit: Space after //

+  WebGLId dst_rgb_565_texture = context_->createTexture();
+  {
+    ScopedTextureBinder<GL_TEXTURE_2D> rgb_texture_binder(context_,
+                                                          dst_rgb_565_texture);
+    context_->texImage2D(GL_TEXTURE_2D,
+                         0,
+                         GL_RGB,
+                         dst_size.width(),
+                         dst_size.height(),
+                         0,
+                         GL_RGB,
+                         GL_UNSIGNED_SHORT_5_6_5,
+                         NULL);
+    context_->copyTexSubImage2D(GL_TEXTURE_2D, 0,
+                                0, 0,
+                                0, 0,
+                                dst_size.width(), dst_size.height());
+  }
+  return dst_rgb_565_texture;
 }
-
-
 void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(
     WebGLId 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 base::Callback<void(bool)>& callback,
     GLHelper::ScalerQuality quality) {
   WebGLId texture = ScaleTexture(src_texture,
@@ -438,12 +473,45 @@ void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(
                                  GL_TEXTURE_2D,
                                  texture,
                                  0);
+  int readback_factor = 0;

[hubbe, 2013/12/26 21:41:49]

readback_factor -> bytes_per_pixel

+  if(readback_config_rgb565){
+    readback_factor = 2;
+  }else{
+    readback_factor = 4;
+  }
+  WebGLId dst_rgb_565_texture = 0;
+  if(readback_config_rgb565){
+    dst_rgb_565_texture = CopyTo565Texture(dst_size);

[hubbe, 2013/12/26 21:41:49]

It would (probably) be faster to have the scaler write into a 565 texture
directly instead of scaling and then copying.

+    ScopedTextureBinder<GL_TEXTURE_2D> rgb_texture_binder(context_,
+                                                          dst_rgb_565_texture);
+    context_->framebufferTexture2D(GL_FRAMEBUFFER,
+                                   GL_COLOR_ATTACHMENT0,
+                                   GL_TEXTURE_2D,
+                                   dst_rgb_565_texture,
+                                   0);
+    //Check format of read after binding with fbo.

[hubbe, 2013/12/26 21:41:49]

Space after //

+    int ext_format, ext_type;
+    context_->getIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT, &ext_format);
+    context_->getIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE, &ext_type);
+    if((ext_format != GL_RGB) || (ext_type != GL_UNSIGNED_SHORT_5_6_5)){

[hubbe, 2013/12/26 21:41:49]

This is the wrong place to detect this.
This detection should happen long before this function is ever called.
Also, we need to make sure that we can render into/copy to 565 textures before
calling CopyTo565Texure, right?

I suggest that we add a function like: CanUseRgb565Readback() that does the
testing, so that the caller can check if 565 readback is supporting before
decinging to use the "readback_config_rgb565" parameter.

PS: There is an alternate way to do this that is *always* supported on GLES2:
You create a shader that outputs RGBA pixels, but in fact puts one 565 pixel
into the "RG" components and another 565 pixel into the "BA" components. Then
you read out RGBA as usual, but what you get is actually 565. Because RGBA
rendering and readback is guaranteed by GLES2, this should work everywhere.
However, it will fail if the precision of the shaders is less than 8 bits per
channel.

This is essentially how the YUV readback works.

+      LOG(ERROR)<<"Readbackformat rgb565  not supported";

[hubbe, 2013/12/26 21:41:49]

NIT: space around <<

+      context_->deleteTexture(texture);
+      context_->deleteTexture(dst_rgb_565_texture);
+      callback.Run(false);
+      return;
+    }
+  }
   ReadbackAsync(dst_size,
-                dst_size.width() * 4,
-                dst_size.width() * 4,
+                dst_size.width() * readback_factor,
+                dst_size.width() * readback_factor,
                 out,
+                readback_config_rgb565,
+                readback_factor,
                 callback);
   context_->deleteTexture(texture);
+  if(readback_config_rgb565){
+    context_->deleteTexture(dst_rgb_565_texture);
+  }
 }
 
 void GLHelper::CopyTextureToImpl::ReadbackTextureSync(
@@ -483,7 +551,8 @@ blink::WebGLId GLHelper::CopyTextureToImpl::CopyAndScaleTexture(
                       quality);
 }
 
-void GLHelper::CopyTextureToImpl::ReadbackDone(Request* finished_request) {
+void GLHelper::CopyTextureToImpl::ReadbackDone(Request* finished_request,
+                                               int read_factor) {
   TRACE_EVENT0("mirror",
                "GLHelper::CopyTextureToImpl::CheckReadbackFramebufferComplete");
   finished_request->done = true;
@@ -505,15 +574,15 @@ void GLHelper::CopyTextureToImpl::ReadbackDone(Request* finished_request) {
               GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, GL_READ_ONLY));
       if (data) {
         result = true;
-        if (request->bytes_per_row == request->size.width() * 4 &&
+        if (request->bytes_per_row == request->size.width() * read_factor &&
             request->bytes_per_row == request->row_stride_bytes) {
-          memcpy(request->pixels, data, request->size.GetArea() * 4);
+          memcpy(request->pixels, data, request->size.GetArea() * read_factor);
         } else {
           unsigned char* out = request->pixels;
           for (int y = 0; y < request->size.height(); y++) {
             memcpy(out, data, request->bytes_per_row);
             out += request->row_stride_bytes;
-            data += request->size.width() * 4;
+            data += request->size.width() * read_factor;
           }
         }
         context_->unmapBufferCHROMIUM(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM);
@@ -565,6 +634,7 @@ void GLHelper::CropScaleReadbackAndCleanTexture(
     const gfx::Rect& src_subrect,
     const gfx::Size& dst_size,
     unsigned char* out,
+    bool readback_config_rgb565,
     const base::Callback<void(bool)>& callback) {
   InitCopyTextToImpl();
   copy_texture_to_impl_->CropScaleReadbackAndCleanTexture(
@@ -573,6 +643,7 @@ void GLHelper::CropScaleReadbackAndCleanTexture(
       src_subrect,
       dst_size,
       out,
+      readback_config_rgb565,
       callback,
       GLHelper::SCALER_QUALITY_FAST);
 }
@@ -584,10 +655,13 @@ void GLHelper::CropScaleReadbackAndCleanMailbox(
     const gfx::Rect& src_subrect,
     const gfx::Size& dst_size,
     unsigned char* out,
+    bool readback_config_rgb565,
     const base::Callback<void(bool)>& callback) {
   WebGLId mailbox_texture = ConsumeMailboxToTexture(src_mailbox, sync_point);
   CropScaleReadbackAndCleanTexture(
-      mailbox_texture, src_size, src_subrect, dst_size, out, callback);
+      mailbox_texture, src_size, src_subrect, dst_size, out,
+      readback_config_rgb565,
+      callback);
   context_->deleteTexture(mailbox_texture);
 }
 
@@ -773,6 +847,8 @@ void GLHelper::CopyTextureToImpl::ReadbackPlane(
       dst_subrect.width() >> size_shift,
       target->stride(plane),
       target->data(plane) + offset,
+      false,
+      4,
       callback);
 }