cc: Make async readback path use async glRreadPixels.

cc/output/gl_renderer.cc

Index: cc/output/gl_renderer.cc
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index 4a0c7ba1fcf3d132ce09bcb04e7b269e748b19bc..54016b47d8363442a1fe655d73173a415dde9d1d 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -30,6 +30,7 @@
 #include "cc/resources/layer_quad.h"
 #include "cc/resources/priority_calculator.h"
 #include "cc/resources/scoped_resource.h"
+#include "cc/resources/sync_point_helper.h"
 #include "cc/trees/damage_tracker.h"
 #include "cc/trees/proxy.h"
 #include "cc/trees/single_thread_proxy.h"
@@ -87,6 +88,17 @@ const float kAntiAliasingEpsilon = 1.0f / 1024.0f;
 
 }  // anonymous namespace
 
+struct GLRenderer::PendingAsyncReadPixels {
+  PendingAsyncReadPixels() : sync_point(0) {}
+
+  CopyRenderPassCallback copy_callback;
+  base::CancelableClosure finished_read_pixels_callback;
+  unsigned sync_point;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PendingAsyncReadPixels);
+};
+
 scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient* client,
                                           OutputSurface* output_surface,
                                           ResourceProvider* resource_provider,
@@ -192,6 +204,13 @@ bool GLRenderer::Initialize() {
 }
 
 GLRenderer::~GLRenderer() {
+  while (!pending_async_read_pixels_.empty()) {
+    pending_async_read_pixels_.back()->finished_read_pixels_callback.Cancel();
+    pending_async_read_pixels_.back()->copy_callback.Run(
+        scoped_ptr<SkBitmap>());
+    pending_async_read_pixels_.pop_back();
+  }
+
   context_->setMemoryAllocationChangedCallbackCHROMIUM(NULL);
   CleanupSharedObjects();
 }
@@ -1776,17 +1795,10 @@ void GLRenderer::EnsureScissorTestDisabled() {
   is_scissor_enabled_ = false;
 }
 
-void GLRenderer::CopyCurrentRenderPassToBitmap(DrawingFrame* frame,
-                                               SkBitmap* bitmap) {
-  gfx::Size render_pass_size = frame->current_render_pass->output_rect.size();
-  bitmap->setConfig(SkBitmap::kARGB_8888_Config,
-                    render_pass_size.width(),
-                    render_pass_size.height());
-  if (bitmap->allocPixels()) {
-    bitmap->lockPixels();
-    GetFramebufferPixels(bitmap->getPixels(), gfx::Rect(render_pass_size));
-    bitmap->unlockPixels();
-  }
+void GLRenderer::CopyCurrentRenderPassToBitmap(
+    DrawingFrame* frame,
+    const CopyRenderPassCallback& callback) {
+  GetFramebufferPixels(frame->current_render_pass->output_rect, callback);
 }
 
 void GLRenderer::ToGLMatrix(float* gl_matrix, const gfx::Transform& transform) {
@@ -1988,18 +2000,68 @@ void GLRenderer::EnsureBackbuffer() {
 }
 
 void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
+  if (!pixels || rect.IsEmpty())
+    return;
+
+  scoped_ptr<PendingAsyncReadPixels> pending_read(new PendingAsyncReadPixels);
+  pending_async_read_pixels_.insert(pending_async_read_pixels_.begin(),
+                                    pending_read.Pass());
+
+  // This is a syncronous call since the callback is null.
+  DoGetFramebufferPixels(static_cast<uint8*>(pixels),
+                         rect,
+                         AsyncGetFramebufferPixelsCleanupCallback());
+}
+
+void GLRenderer::GetFramebufferPixels(gfx::Rect rect,
+                                      CopyRenderPassCallback callback) {
+  if (callback.is_null())
+    return;
+  if (rect.IsEmpty()) {
+    callback.Run(scoped_ptr<SkBitmap>());
+    return;
+  }
+
+  scoped_ptr<SkBitmap> bitmap(new SkBitmap);
+  bitmap->setConfig(SkBitmap::kARGB_8888_Config, rect.width(), rect.height());
+  bitmap->allocPixels();
+
+  scoped_ptr<SkAutoLockPixels> lock(new SkAutoLockPixels(*bitmap));
+
+  // Save a pointer to the pixels, the bitmap is owned by the cleanup_callback.
+  uint8* pixels = static_cast<uint8*>(bitmap->getPixels());
+
+  AsyncGetFramebufferPixelsCleanupCallback cleanup_callback = base::Bind(
+      &GLRenderer::PassOnSkBitmap,
+      base::Unretained(this),
+      base::Passed(&bitmap),
+      base::Passed(&lock),
+      callback);
+
+  scoped_ptr<PendingAsyncReadPixels> pending_read(new PendingAsyncReadPixels);
+  pending_read->copy_callback = callback;
+  pending_async_read_pixels_.insert(pending_async_read_pixels_.begin(),
+                                    pending_read.Pass());
+
+  // This is an asyncronous call since the callback is not null.
+  DoGetFramebufferPixels(pixels, rect, cleanup_callback);
+}
+
+void GLRenderer::DoGetFramebufferPixels(
+    uint8* dest_pixels,
+    gfx::Rect rect,
+    const AsyncGetFramebufferPixelsCleanupCallback& cleanup_callback) {
   DCHECK(rect.right() <= ViewportWidth());
   DCHECK(rect.bottom() <= ViewportHeight());
 
-  if (!pixels)
-    return;
+  bool is_async = !cleanup_callback.is_null();
 
   MakeContextCurrent();
 
   bool do_workaround = NeedsIOSurfaceReadbackWorkaround();
 
-  GLuint temporary_texture = 0;
-  GLuint temporary_fbo = 0;
+  unsigned temporary_texture = 0;
+  unsigned temporary_fbo = 0;
 
   if (do_workaround) {
     // On Mac OS X, calling glReadPixels() against an FBO whose color attachment
@@ -2047,8 +2109,21 @@ void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
            GL_FRAMEBUFFER_COMPLETE);
   }
 
-  scoped_ptr<uint8_t[]> src_pixels(
-      new uint8_t[rect.width() * rect.height() * 4]);
+  unsigned buffer = 0;
+  scoped_ptr<uint8[]> sync_pixels;
+
+  if (is_async) {

[piman, 2013/04/27 00:42:25]

I think you can make your life a bit easier if:
- you always use the buffer path
- you always make the finished_callback
- when sync, you finish, then call the finished_callback. when async you insert
the sync point and SignalSyncPoint on the finished_callback

[danakj, 2013/04/29 14:59:05]

Thanks! This is what I was trying to do at first, then I thought "but I can't
make finish() guarantee the callback is run" but of course I don't need to
signal/wait for the callback at all if i finish() first..

So, will do!

+    buffer = context_->createBuffer();
+    GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                       buffer));
+    GLC(context_, context_->bufferData(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                       4 * rect.size().GetArea(),
+                                       NULL,
+                                       GL_STREAM_READ));
+  } else {
+    sync_pixels.reset(new uint8[4 * rect.size().GetArea()]);
+  }
+
   GLC(context_,
       context_->readPixels(rect.x(),
                            ViewportSize().height() - rect.bottom(),
@@ -2056,22 +2131,11 @@ void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
                            rect.height(),
                            GL_RGBA,
                            GL_UNSIGNED_BYTE,
-                           src_pixels.get()));
-
-  uint8_t* dest_pixels = static_cast<uint8_t*>(pixels);
-  size_t row_bytes = rect.width() * 4;
-  int num_rows = rect.height();
-  size_t total_bytes = num_rows * row_bytes;
-  for (size_t dest_y = 0; dest_y < total_bytes; dest_y += row_bytes) {
-    // Flip Y axis.
-    size_t src_y = total_bytes - dest_y - row_bytes;
-    // Swizzle BGRA -> RGBA.
-    for (size_t x = 0; x < row_bytes; x += 4) {
-      dest_pixels[dest_y + (x + 0)] = src_pixels.get()[src_y + (x + 2)];
-      dest_pixels[dest_y + (x + 1)] = src_pixels.get()[src_y + (x + 1)];
-      dest_pixels[dest_y + (x + 2)] = src_pixels.get()[src_y + (x + 0)];
-      dest_pixels[dest_y + (x + 3)] = src_pixels.get()[src_y + (x + 3)];
-    }
+                           sync_pixels.get()));
+
+  if (is_async) {
+    GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                       0));
   }
 
   if (do_workaround) {
@@ -2082,9 +2146,109 @@ void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
     GLC(context_, context_->deleteTexture(temporary_texture));
   }
 
+  unsigned sync_point = 0;
+  if (is_async)
+    sync_point = context_->insertSyncPoint();
+  pending_async_read_pixels_.back()->sync_point = sync_point;
+
+  base::Closure finished_callback =
+      base::Bind(&GLRenderer::FinishedReadback,
+                 base::Unretained(this),
+                 cleanup_callback,
+                 buffer,
+                 base::Passed(&sync_pixels),
+                 dest_pixels,
+                 rect.size(),
+                 sync_point);

[piman, 2013/04/27 00:42:25]

nit: Given the above, and given that you only pass the sync_point for a DCHECK,
maybe you can save yourself some 'if' madness by instead storing the buffer in
pending_async_read_pixels_.back(), and DCHECK that instead, since you already
pass it to FinishedReadback. Saves a param too.

[danakj, 2013/04/29 14:59:05]

Oh, good idea.

+  // Save the finished_callback so it can be cancelled.
+  pending_async_read_pixels_.back()->finished_read_pixels_callback.Reset(
+      finished_callback);
+
+  if (is_async) {
+    SyncPointHelper::SignalSyncPoint(
+        context_,
+        sync_point,
+        finished_callback);
+  } else {
+    finished_callback.Run();
+  }
+
   EnforceMemoryPolicy();
 }
 
+void GLRenderer::FinishedReadback(
+    const AsyncGetFramebufferPixelsCleanupCallback& cleanup_callback,
+    unsigned source_buffer,
+    scoped_ptr<uint8[]> sync_pixels,
+    uint8* dest_pixels,
+    gfx::Size size,
+    unsigned sync_point) {
+  DCHECK(!pending_async_read_pixels_.empty());
+  DCHECK_EQ(sync_point, pending_async_read_pixels_.back()->sync_point);
+
+  uint8* src_pixels = NULL;
+  bool success = false;
+
+  if (sync_pixels) {
+    src_pixels = sync_pixels.get();
+    success = true;
+  } else if (source_buffer != 0) {
+    GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                       source_buffer));
+    src_pixels = static_cast<uint8*>(
+        context_->mapBufferCHROMIUM(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                    GL_READ_ONLY));
+
+    if (src_pixels) {
+      memcpy(dest_pixels, src_pixels, 4 * size.GetArea());
+      GLC(context_, context_->unmapBufferCHROMIUM(
+          GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM));
+      success = true;
+    }
+    GLC(context_, context_->bindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,
+                                       0));
+    GLC(context_, context_->deleteBuffer(source_buffer));
+  }
+
+  if (src_pixels) {
+    size_t row_bytes = size.width() * 4;
+    int num_rows = size.height();
+    size_t total_bytes = num_rows * row_bytes;
+    for (size_t dest_y = 0; dest_y < total_bytes; dest_y += row_bytes) {
+      // Flip Y axis.
+      size_t src_y = total_bytes - dest_y - row_bytes;
+      // Swizzle BGRA -> RGBA.
+      for (size_t x = 0; x < row_bytes; x += 4) {
+        dest_pixels[dest_y + (x + 0)] = src_pixels[src_y + (x + 2)];
+        dest_pixels[dest_y + (x + 1)] = src_pixels[src_y + (x + 1)];
+        dest_pixels[dest_y + (x + 2)] = src_pixels[src_y + (x + 0)];
+        dest_pixels[dest_y + (x + 3)] = src_pixels[src_y + (x + 3)];
+      }
+    }
+  }
+
+  // TODO(danakj): This can go away when synchronous readback is no more and its
+  // contents can just move here.
+  if (!cleanup_callback.is_null())
+    cleanup_callback.Run(success);
+
+  pending_async_read_pixels_.pop_back();
+}
+
+void GLRenderer::PassOnSkBitmap(
+    scoped_ptr<SkBitmap> bitmap,
+    scoped_ptr<SkAutoLockPixels> lock,
+    const CopyRenderPassCallback& callback,
+    bool success) {
+  DCHECK(callback.Equals(pending_async_read_pixels_.back()->copy_callback));
+
+  lock.reset();
+  if (success)
+    callback.Run(bitmap.Pass());
+  else
+    callback.Run(scoped_ptr<SkBitmap>());
+}
+
 bool GLRenderer::GetFramebufferTexture(ScopedResource* texture,
                                        gfx::Rect device_rect) {
   DCHECK(!texture->id() || (texture->size() == device_rect.size() &&