Implement asynchronous operation for RWHVP::CopyFromCompositingSurface on Mac

content/browser/renderer_host/compositing_iosurface_mac.mm

 // 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/browser/renderer_host/compositing_iosurface_mac.h"
 
 #include <OpenGL/OpenGL.h>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
+#include "base/message_loop.h"
 #include "base/threading/platform_thread.h"
 #include "content/common/content_constants_internal.h"
 #include "content/public/browser/browser_thread.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_switches.h"
 #include "ui/surface/io_surface_support_mac.h"
 
@@ skipping 90 matching lines @@
                              const CVTimeStamp* output_time,
                              CVOptionFlags flags_in,
                              CVOptionFlags* flags_out,
                              void* context) {
   CompositingIOSurfaceMac* surface =
       static_cast<CompositingIOSurfaceMac*>(context);
   surface->DisplayLinkTick(display_link, output_time);
   return kCVReturnSuccess;
 }
 
+CompositingIOSurfaceMac::CopyContext::CopyContext() {
+  Reset();
+}
+
+CompositingIOSurfaceMac::CopyContext::~CopyContext() {
+}
+
 CompositingIOSurfaceMac* CompositingIOSurfaceMac::Create() {
   TRACE_EVENT0("browser", "CompositingIOSurfaceMac::Create");
   IOSurfaceSupport* io_surface_support = IOSurfaceSupport::Initialize();
   if (!io_surface_support) {
     LOG(WARNING) << "No IOSurface support";
     return NULL;
   }
 
   std::vector<NSOpenGLPixelFormatAttribute> attributes;
   attributes.push_back(NSOpenGLPFADoubleBuffer);
@@ skipping 130 matching lines @@
                                                  uint32* interval_numerator,
                                                  uint32* interval_denominator) {
   base::AutoLock lock(lock_);
   *timebase = vsync_timebase_;
   *interval_numerator = vsync_interval_numerator_;
   *interval_denominator = vsync_interval_denominator_;
 }
 
 CompositingIOSurfaceMac::~CompositingIOSurfaceMac() {
   CVDisplayLinkRelease(display_link_);
-  UnrefIOSurface();
+  CGLSetCurrentContext(cglContext_);
+  CleanupResourcesForCopy();
+  UnrefIOSurfaceWithContextCurrent();
+  CGLSetCurrentContext(0);
 }
 
 void CompositingIOSurfaceMac::SetIOSurface(uint64 io_surface_handle,
                                            const gfx::Size& size) {
   pixel_io_surface_size_ = size;
   CGLSetCurrentContext(cglContext_);
   MapIOSurfaceToTexture(io_surface_handle);
   CGLSetCurrentContext(0);
 }
 
 void CompositingIOSurfaceMac::DrawIOSurface(NSView* view, float scale_factor) {
   CGLSetCurrentContext(cglContext_);
-
   bool has_io_surface = MapIOSurfaceToTexture(io_surface_handle_);
+  if (!has_io_surface)

[mazda, 2012/09/20 08:41:11]

The original code allows the case |has_io_surface| is false. Is it OK to change
the behavior?

[Alpha Left Google, 2012/09/21 01:08:32]

Good catch I forgot to remove this bit.

+    return;
 
   TRACE_EVENT1("browser", "CompositingIOSurfaceMac::DrawIOSurface",
                "has_io_surface", has_io_surface);
 
   [glContext_ setView:view];
   gfx::Size window_size(NSSizeToCGSize([view frame].size));
   gfx::Size pixel_window_size = window_size.Scale(scale_factor);
   glViewport(0, 0, pixel_window_size.width(), pixel_window_size.height());
 
   // TODO: After a resolution change, the DPI-ness of the view and the
@@ skipping 81 matching lines @@
   UNSHIPPED_TRACE_EVENT_INSTANT0("test_gpu", "CompositorSwapBuffersComplete");
 
   CGLSetCurrentContext(0);
 
   StartOrContinueDisplayLink();
 
   if (!is_vsync_disabled_)
     RateLimitDraws();
 }
 
-bool CompositingIOSurfaceMac::CopyTo(
+void CompositingIOSurfaceMac::CopyTo(
       const gfx::Rect& src_pixel_subrect,
       const gfx::Size& dst_pixel_size,
-      void* out) {
-  if (!MapIOSurfaceToTexture(io_surface_handle_))
-    return false;
+      void* out,
+      const base::Callback<void(bool)>& callback) {
+  if (copy_context_.started) {

[mazda, 2012/09/20 08:41:11]

This will make successive calls of CompositingIOSurfaceMac::CopyTo in a short
period of time likely to fail. If this function is used from more than one
clients frequently, the probability of success is affected by the clients.
(e.g. While tab is captured using this function, taking thumbnails is likely to
fail). I think it's better to avoid it.
(queueing |copy_context_| or using PostDelayedTask?)

[Alpha Left Google, 2012/09/21 01:08:32]

I'm not sure we should allow multiple copies at the same time. If we queue more
CopyContext it is easy to leak resources I think we should make sure the user is
throttling the call to CopyFromCompositingSurface correctly, meaning no more
than one outstanding call.

+    callback.Run(false);
+    return;
+  }
+
+  TRACE_EVENT0("browser", "CompositingIOSurfaceMac::CopyTo()");

[mazda, 2012/09/20 08:41:11]

Is it better to put this below the check of MapIOSurfaceToTexture so that
TRACE_EVENT0 does not include the case of returning immediately?

[Alpha Left Google, 2012/09/21 01:08:32]

Sure will do.

 
   CGLSetCurrentContext(cglContext_);
-  GLuint target = GL_TEXTURE_RECTANGLE_ARB;
+  if (!MapIOSurfaceToTexture(io_surface_handle_)) {

[mazda, 2012/09/20 08:41:11]

Is there any reason why this is moved after CGLSetCurrentContext(cglContext_)?

[Alpha Left Google, 2012/09/21 01:08:32]

I'm following the strcuture in DrawIOSurface, I'm surprised that it maps
correctly before setting the context.

+    CGLSetCurrentContext(0);
+    callback.Run(false);
+    return;
+  }
 
-  GLuint dst_texture = 0;
-  glGenTextures(1, &dst_texture); CHECK_GL_ERROR();
-  glBindTexture(target, dst_texture); CHECK_GL_ERROR();
+  static bool initialized_use_fence = false;
+  static bool use_fence = false;
 
-  GLuint dst_framebuffer = 0;
-  glGenFramebuffersEXT(1, &dst_framebuffer); CHECK_GL_ERROR();
-  glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, dst_framebuffer); CHECK_GL_ERROR();
+  if (!initialized_use_fence) {
+    use_fence =

[mazda, 2012/09/20 08:41:11]

Would it be better to extract this into a function?

[Alpha Left Google, 2012/09/21 01:08:32]

Could do.

+        strstr(reinterpret_cast<const char*>(glGetString(GL_EXTENSIONS)),
+               "GL_APPLE_fence") != NULL;
+    initialized_use_fence = true;
+  }
 
-  glTexImage2D(target,
+  copy_context_.started = true;
+  copy_context_.src_rect = src_pixel_subrect;
+  copy_context_.dest_size = dst_pixel_size;
+  copy_context_.out_buf = out;
+  copy_context_.callback = callback;
+
+  if (use_fence) {
+    glGenFencesAPPLE(1, &copy_context_.fence); CHECK_GL_ERROR();
+    copy_context_.use_fence = true;
+    copy_context_.cycles_elapsed = 0;
+  }
+
+  // Create an offscreen framebuffer.
+  // This is used to render and scale a subrect of IOSurface.
+  const GLenum kTarget = GL_TEXTURE_RECTANGLE_ARB;
+  const int kDestWidth = copy_context_.dest_size.width();

[mazda, 2012/09/20 08:41:11]

s/kDestWidth/dest_width/

Because this is not a compile-time constant.

[Alpha Left Google, 2012/09/21 01:08:32]

Done.

+  const int kDestHeight = copy_context_.dest_size.height();

[mazda, 2012/09/20 08:41:11]

Ditto.

[Alpha Left Google, 2012/09/21 01:08:32]

Done.

+
+  glGenTextures(1, &copy_context_.frame_buffer_texture); CHECK_GL_ERROR();
+  glBindTexture(kTarget, copy_context_.frame_buffer_texture); CHECK_GL_ERROR();
+  glGenFramebuffersEXT(1, &copy_context_.frame_buffer); CHECK_GL_ERROR();
+  glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, copy_context_.frame_buffer);
+  CHECK_GL_ERROR();
+
+  glTexImage2D(kTarget,
                0,
                GL_RGBA,
-               dst_pixel_size.width(),
-               dst_pixel_size.height(),
+               kDestWidth,
+               kDestHeight,
                0,
                GL_BGRA,
                GL_UNSIGNED_INT_8_8_8_8_REV,
                NULL); CHECK_GL_ERROR();
   glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT,
                             GL_COLOR_ATTACHMENT0_EXT,
-                            target,
-                            dst_texture,
+                            kTarget,
+                            copy_context_.frame_buffer_texture,
                             0); CHECK_GL_ERROR();
-  glBindTexture(target, 0); CHECK_GL_ERROR();
 
-  glViewport(0, 0, dst_pixel_size.width(), dst_pixel_size.height());
+  glViewport(0, 0, kDestWidth, kDestHeight); CHECK_GL_ERROR();
+  glMatrixMode(GL_PROJECTION); CHECK_GL_ERROR();
+  glLoadIdentity(); CHECK_GL_ERROR();
+  glOrtho(0, kDestWidth, 0, kDestHeight, -1, 1); CHECK_GL_ERROR();
+  glMatrixMode(GL_MODELVIEW); CHECK_GL_ERROR();
+  glLoadIdentity(); CHECK_GL_ERROR();
 
-  glMatrixMode(GL_PROJECTION);
-  glLoadIdentity();
-  glOrtho(0, dst_pixel_size.width(), 0, dst_pixel_size.height(), -1, 1);
-  glMatrixMode(GL_MODELVIEW);
-  glLoadIdentity();
+  glDisable(GL_DEPTH_TEST); CHECK_GL_ERROR();
+  glDisable(GL_BLEND); CHECK_GL_ERROR();
 
-  glDisable(GL_DEPTH_TEST);
-  glDisable(GL_BLEND);
+  glUseProgram(shader_program_blit_rgb_); CHECK_GL_ERROR();
 
-  glUseProgram(shader_program_blit_rgb_);
-
-  int texture_unit = 0;
-  glUniform1i(blit_rgb_sampler_location_, texture_unit);
-  glActiveTexture(GL_TEXTURE0 + texture_unit);
-  glBindTexture(GL_TEXTURE_RECTANGLE_ARB, texture_);
+  const int kTextureUnit = 0;
+  glUniform1i(blit_rgb_sampler_location_, kTextureUnit); CHECK_GL_ERROR();
+  glActiveTexture(GL_TEXTURE0 + kTextureUnit); CHECK_GL_ERROR();
+  glBindTexture(kTarget, texture_); CHECK_GL_ERROR();
+  glTexParameterf(kTarget, GL_TEXTURE_MIN_FILTER, GL_LINEAR); CHECK_GL_ERROR();
+  glTexParameterf(kTarget, GL_TEXTURE_MAG_FILTER, GL_NEAREST); CHECK_GL_ERROR();
 
   SurfaceQuad quad;
-  quad.set_rect(0.0f, 0.0f, dst_pixel_size.width(), dst_pixel_size.height());
-  quad.set_texcoord_rect(src_pixel_subrect.x(), src_pixel_subrect.y(),
-                         src_pixel_subrect.right(), src_pixel_subrect.bottom());
+  quad.set_rect(0.0f, 0.0f, kDestWidth, kDestHeight); CHECK_GL_ERROR();
+  quad.set_texcoord_rect(
+      copy_context_.src_rect.x(), copy_context_.src_rect.y(),
+      copy_context_.src_rect.right(), copy_context_.src_rect.bottom());
   DrawQuad(quad);
 
-  glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0); CHECK_GL_ERROR();
-  glUseProgram(0);
+  glBindTexture(kTarget, 0); CHECK_GL_ERROR();
+  glUseProgram(0); CHECK_GL_ERROR();
 
-  CGLFlushDrawable(cglContext_);
+  // Copy the offscreen framebuffer to a PBO.
+  glGenBuffersARB(1, &copy_context_.pixel_buffer); CHECK_GL_ERROR();
+  glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, copy_context_.pixel_buffer);
+  CHECK_GL_ERROR();
+  glBufferDataARB(GL_PIXEL_PACK_BUFFER_ARB,
+                  kDestWidth * kDestHeight * 4,
+                  NULL, GL_STREAM_READ_ARB); CHECK_GL_ERROR();
+  glReadPixels(0, 0, kDestWidth, kDestHeight, GL_BGRA,
+               GL_UNSIGNED_INT_8_8_8_8_REV, 0); CHECK_GL_ERROR();
 
-  glReadPixels(0, 0, dst_pixel_size.width(), dst_pixel_size.height(),
-               GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, out);
-
+  glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, 0); CHECK_GL_ERROR();
   glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0); CHECK_GL_ERROR();
 
-  glDeleteFramebuffersEXT(1, &dst_framebuffer);
-  glDeleteTextures(1, &dst_texture);
+  if (use_fence) {
+    glSetFenceAPPLE(copy_context_.fence); CHECK_GL_ERROR();
+  }
+  glFlush(); CHECK_GL_ERROR();
+  CGLSetCurrentContext(0);
 
-  CGLSetCurrentContext(0);
-  return true;
+  // 20ms is an estimate assuming most hardware can complete asynchronous
+  // readback within this time limit. The timer will keep running until
+  // operation is completed.
+  const int kIntervalMilliseconds = 20;
+  copy_timer_.Start(FROM_HERE,
+                    base::TimeDelta::FromMilliseconds(kIntervalMilliseconds),
+                    this, &CompositingIOSurfaceMac::FinishCopy);
 }
 
 bool CompositingIOSurfaceMac::MapIOSurfaceToTexture(
     uint64 io_surface_handle) {
   if (io_surface_.get() && io_surface_handle == io_surface_handle_)
     return true;
 
   TRACE_EVENT0("browser", "CompositingIOSurfaceMac::MapIOSurfaceToTexture");
   UnrefIOSurfaceWithContextCurrent();
 
@@ skipping 139 matching lines @@
     CVDisplayLinkStart(display_link_);
   }
   display_link_stop_timer_.Reset();
 }
 
 void CompositingIOSurfaceMac::StopDisplayLink() {
   if (CVDisplayLinkIsRunning(display_link_))
     CVDisplayLinkStop(display_link_);
 }
 
+void CompositingIOSurfaceMac::FinishCopy() {
+  CHECK(copy_context_.started);
+  TRACE_EVENT0("browser", "CompositingIOSurfaceMac::FinishCopy()");
+
+  CGLSetCurrentContext(cglContext_);
+
+  if (copy_context_.use_fence) {
+    bool copy_completed = glTestFenceAPPLE(copy_context_.fence);
+    CHECK_GL_ERROR();
+
+    // Allow 1s for the operation to complete.
+    const int kRetryCycles = 50;
+
+    if (!copy_completed && copy_context_.cycles_elapsed < kRetryCycles) {
+      ++copy_context_.cycles_elapsed;
+      CGLSetCurrentContext(0);
+      return;
+    }
+  }
+  copy_timer_.Stop();
+
+  const int kDestWidth = copy_context_.dest_size.width();
+  const int kDestHeight = copy_context_.dest_size.height();
+
+  glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, copy_context_.pixel_buffer);
+  CHECK_GL_ERROR();
+
+  void* buf = glMapBuffer(GL_PIXEL_PACK_BUFFER_ARB, GL_READ_ONLY_ARB);
+  CHECK_GL_ERROR();
+
+  if (buf) {
+    memcpy(copy_context_.out_buf, buf, kDestWidth * kDestHeight * 4);

[mazda, 2012/09/20 08:41:11]

s/kDestWidth * kDestHeight/copy_context_.dest_size.GetArea()/

Then kDestWidth and kDestHeight can be removed.

[Alpha Left Google, 2012/09/21 01:08:32]

Done.

+    glUnmapBufferARB(GL_PIXEL_PACK_BUFFER_ARB); CHECK_GL_ERROR();
+  }
+  glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, 0); CHECK_GL_ERROR();
+
+  base::Callback<void(bool)> callback = copy_context_.callback;
+  CleanupResourcesForCopy();
+  CGLSetCurrentContext(0);
+
+  callback.Run(buf != NULL);
+}
+
+void CompositingIOSurfaceMac::CleanupResourcesForCopy() {
+  if (!copy_context_.started)
+    return;
+
+  glDeleteFramebuffersEXT(1, &copy_context_.frame_buffer); CHECK_GL_ERROR();
+  glDeleteTextures(1, &copy_context_.frame_buffer_texture); CHECK_GL_ERROR();
+  glDeleteBuffers(1, &copy_context_.pixel_buffer); CHECK_GL_ERROR();
+  if (copy_context_.use_fence) {
+    glDeleteFencesAPPLE(1, &copy_context_.fence); CHECK_GL_ERROR();
+  }
+  copy_context_.Reset();
+}
+
 }  // namespace content