Fixed loss of rendered output on Mac OS X if Pepper 3D application uses

framebuffer objects. Conditionalized allocation of FBO in
AcceleratedSurface class, and changed SwapBuffers to optionally copy from
the current context's back buffer. Changed GPUProcessor on Mac to always
use PbufferGLContext, and hooked in optional call to AcceleratedSurface's
SwapBufers before calling user's callback. Completely disabled
ViewGLContext on Mac OS X. This causes Pepper 3D applications to use the
GGL default back buffer on Mac, which is the desired behavior.

Ideally the FBO allocation would be factored out of the AcceleratedSurface
class, and ideally the pbuffer setup code would not be duplicated between
this class and PbufferGLContext. However, these cleanups are being deferred
because they require substantial refactorings.

Removed accelerated_surface_stub.cc, which isn't needed any more since
AcceleratedSurface moved to app/.

Tested:
 - Pepper 3D plugin with glBindFramebuffer(GL_FRAMEBUFFER, 0) in place with
   both IOSurface and TransportDIB code paths
 - Unity 3D with IOSurface / Core Animation code path

BUG=41004
TEST=none (ran above tests)

Review URL: http://codereview.chromium.org/1637007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44507 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 116 insertions, 52 deletions

diff --git a/app/surface/accelerated_surface_mac.cc b/app/surface/accelerated_surface_mac.cc
index f348cd8..a8f5a0e 100644
--- a/app/surface/accelerated_surface_mac.cc
+++ b/app/surface/accelerated_surface_mac.cc
@@ -11,14 +11,20 @@
 AcceleratedSurface::AcceleratedSurface()
     : gl_context_(NULL),
       pbuffer_(NULL),
+      allocate_fbo_(false),
       texture_(0),
       fbo_(0),
-      depth_stencil_renderbuffer_(0),
-      bound_fbo_(0),
-      bound_renderbuffer_(0) {
+      depth_stencil_renderbuffer_(0) {
 }
 
-bool AcceleratedSurface::Initialize() {
+bool AcceleratedSurface::Initialize(CGLContextObj share_context,
+                                    bool allocate_fbo) {
+  allocate_fbo_ = allocate_fbo;
+
+  // TODO(kbr): we should reuse the code for PbufferGLContext here instead
+  // of duplicating it. However, in order to do so, we need to move the
+  // GLContext classes out of gpu/ and into gfx/.
+
   // Create a 1x1 pbuffer and associated context to bootstrap things
   static const CGLPixelFormatAttribute attribs[] = {
     (CGLPixelFormatAttribute) kCGLPFAPBuffer,
@@ -36,7 +42,7 @@ bool AcceleratedSurface::Initialize() {
     return false;
   }
   CGLContextObj context;
-  CGLError res = CGLCreateContext(pixel_format, 0, &context);
+  CGLError res = CGLCreateContext(pixel_format, share_context, &context);
   CGLDestroyPixelFormat(pixel_format);
   if (res != kCGLNoError) {
     DLOG(ERROR) << "Error creating context.";
@@ -65,6 +71,11 @@ bool AcceleratedSurface::Initialize() {
 }
 
 void AcceleratedSurface::Destroy() {
+  // The FBO and texture objects will be destroyed when the OpenGL context,
+  // and any other contexts sharing resources with it, is. We don't want to
+  // make the context current one last time here just in order to delete
+  // these objects.
+
   // Release the old TransportDIB in the browser.
   if (dib_free_callback_.get() && transport_dib_.get()) {
     dib_free_callback_->Run(transport_dib_->id());
@@ -79,25 +90,39 @@ void AcceleratedSurface::Destroy() {
 // Call after making changes to the surface which require a visual update.
 // Makes the rendering show up in other processes.
 void AcceleratedSurface::SwapBuffers() {
-  if (bound_fbo_ != fbo_) {
-    glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fbo_);
-  }
   if (io_surface_.get() != NULL) {
-    // Bind and unbind the framebuffer to make changes to the
-    // IOSurface show up in the other process.
-    glFlush();
-    glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0);
-    glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fbo_);
+    if (allocate_fbo_) {
+      // Bind and unbind the framebuffer to make changes to the
+      // IOSurface show up in the other process.
+      glFlush();
+      glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0);
+      glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fbo_);
+    } else {
+      // Copy the current framebuffer's contents into our "live" texture.
+      // Note that the current GL context might not be ours at this point!
+      // This is deliberate, so that surrounding code using GL can produce
+      // rendering results consumed by the AcceleratedSurface.
+      // Need to save and restore OpenGL state around this call.
+      GLint current_texture = 0;
+      GLenum target_binding = GL_TEXTURE_BINDING_RECTANGLE_ARB;
+      GLenum target = GL_TEXTURE_RECTANGLE_ARB;
+      glGetIntegerv(target_binding, &current_texture);
+      glBindTexture(target, texture_);
+      glCopyTexSubImage2D(target, 0,
+                          0, 0,
+                          0, 0,
+                          surface_size_.width(), surface_size_.height());
+      glBindTexture(target, current_texture);
+    }
   } else if (transport_dib_.get() != NULL) {
-    // Pre-Mac OS X 10.6, fetch the rendered image from the FBO and copy it
-    // into the TransportDIB.
+    // Pre-Mac OS X 10.6, fetch the rendered image from the current frame
+    // buffer and copy it into the TransportDIB.
     // TODO(dspringer): There are a couple of options that can speed this up.
     // First is to use async reads into a PBO, second is to use SPI that
     // allows many tasks to access the same CGSSurface.
     void* pixel_memory = transport_dib_->memory();
     if (pixel_memory) {
       // Note that glReadPixels does an implicit glFlush().
-      glReadBuffer(GL_COLOR_ATTACHMENT0_EXT);
       glReadPixels(0,
                    0,
                    surface_size_.width(),
@@ -107,9 +132,6 @@ void AcceleratedSurface::SwapBuffers() {
                    pixel_memory);
     }
   }
-  if (bound_fbo_ != fbo_) {
-    glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, bound_fbo_);
-  }
 }
 
 static void AddBooleanValue(CFMutableDictionaryRef dictionary,
@@ -137,7 +159,6 @@ void AcceleratedSurface::AllocateRenderBuffers(GLenum target,
     // Generate and bind the framebuffer object.
     glGenFramebuffersEXT(1, &fbo_);
     glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fbo_);
-    bound_fbo_ = fbo_;
     // Generate (but don't bind) the depth buffer -- we don't need
     // this bound in order to do offscreen rendering.
     glGenRenderbuffersEXT(1, &depth_stencil_renderbuffer_);
@@ -151,16 +172,14 @@ void AcceleratedSurface::AllocateRenderBuffers(GLenum target,
                            size.height());
 
   // Unbind the renderbuffers.
-  glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, bound_renderbuffer_);
+  glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, 0);
 
   // Make sure that subsequent set-up code affects the render texture.
   glBindTexture(target, texture_);
 }
 
 bool AcceleratedSurface::SetupFrameBufferObject(GLenum target) {
-  if (bound_fbo_ != fbo_) {
-    glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fbo_);
-  }
+  glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fbo_);
   GLenum fbo_status;
   glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT,
                             GL_COLOR_ATTACHMENT0_EXT,
@@ -183,9 +202,6 @@ bool AcceleratedSurface::SetupFrameBufferObject(GLenum target) {
                                  depth_stencil_renderbuffer_);
     fbo_status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
   }
-  if (bound_fbo_ != fbo_) {
-    glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, bound_fbo_);
-  }
   return fbo_status == GL_FRAMEBUFFER_COMPLETE_EXT;
 }
 
@@ -225,7 +241,15 @@ uint64 AcceleratedSurface::SetSurfaceSize(const gfx::Size& size) {
   // GL_TEXTURE_RECTANGLE_ARB is the best supported render target on
   // Mac OS X and is required for IOSurface interoperability.
   GLenum target = GL_TEXTURE_RECTANGLE_ARB;
-  AllocateRenderBuffers(target, size);
+  if (allocate_fbo_) {
+    AllocateRenderBuffers(target, size);
+  } else if (!texture_) {
+    // Generate the texture object.
+    glGenTextures(1, &texture_);
+    glBindTexture(target, texture_);
+    glTexParameteri(target, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(target, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  }
 
   // Allocate a new IOSurface, which is the GPU resource that can be
   // shared across processes.
@@ -258,8 +282,10 @@ uint64 AcceleratedSurface::SetSurfaceSize(const gfx::Size& size) {
                                              GL_UNSIGNED_INT_8_8_8_8_REV,
                                              io_surface_.get(),
                                              plane);
-  // Set up the frame buffer object.
-  SetupFrameBufferObject(target);
+  if (allocate_fbo_) {
+    // Set up the frame buffer object.
+    SetupFrameBufferObject(target);
+  }
   surface_size_ = size;
 
   // Now send back an identifier for the IOSurface. We originally
@@ -305,20 +331,22 @@ TransportDIB::Handle AcceleratedSurface::SetTransportDIBSize(
     return TransportDIB::DefaultHandleValue();
   }
 
-  // Set up the render buffers and reserve enough space on the card for the
-  // framebuffer texture.
-  GLenum target = GL_TEXTURE_RECTANGLE_ARB;
-  AllocateRenderBuffers(target, size);
-  glTexImage2D(target,
-               0,  // mipmap level 0
-               GL_RGBA8,  // internal pixel format
-               size.width(),
-               size.height(),
-               0,  // 0 border
-               GL_BGRA,  // Used for consistency
-               GL_UNSIGNED_INT_8_8_8_8_REV,
-               NULL);  // No data, just reserve room on the card.
-  SetupFrameBufferObject(target);
+  if (allocate_fbo_) {
+    // Set up the render buffers and reserve enough space on the card for the
+    // framebuffer texture.
+    GLenum target = GL_TEXTURE_RECTANGLE_ARB;
+    AllocateRenderBuffers(target, size);
+    glTexImage2D(target,
+                 0,  // mipmap level 0
+                 GL_RGBA8,  // internal pixel format
+                 size.width(),
+                 size.height(),
+                 0,  // 0 border
+                 GL_BGRA,  // Used for consistency
+                 GL_UNSIGNED_INT_8_8_8_8_REV,
+                 NULL);  // No data, just reserve room on the card.
+    SetupFrameBufferObject(target);
+  }
   return transport_dib_->handle();
 }
 
diff --git a/app/surface/accelerated_surface_mac.h b/app/surface/accelerated_surface_mac.h
index 3808dcd..336d0bf 100644
--- a/app/surface/accelerated_surface_mac.h
+++ b/app/surface/accelerated_surface_mac.h
@@ -29,8 +29,17 @@ class AcceleratedSurface {
   AcceleratedSurface();
   virtual ~AcceleratedSurface() { }
 
-  // Set up internal buffers. Returns false upon failure.
-  bool Initialize();
+  // Set up internal buffers. |share_context|, if non-NULL, is a context
+  // with which the internally created OpenGL context shares textures and
+  // other resources. |allocate_fbo| indicates whether or not this surface
+  // should allocate an offscreen frame buffer object (FBO) internally. If
+  // not, then the user is expected to allocate one. NOTE that allocating
+  // an FBO internally does NOT work properly with client code which uses
+  // OpenGL (i.e., via GLES2 command buffers), because the GLES2
+  // implementation does not know to bind the accelerated surface's
+  // internal FBO when the default FBO is bound. Returns false upon
+  // failure.
+  bool Initialize(CGLContextObj share_context, bool allocate_fbo);
   // Tear down. Must be called before destructor to prevent leaks.
   void Destroy();
 
@@ -50,7 +59,25 @@ class AcceleratedSurface {
   void Clear(const gfx::Rect& rect);
   // Call after making changes to the surface which require a visual update.
   // Makes the rendering show up in other processes.
+  //
+  // If this AcceleratedSurface is configured with its own FBO, then
+  // this call causes the color buffer to be transmitted. Otherwise,
+  // it causes the frame buffer of the current GL context to be copied
+  // either into an internal texture via glCopyTexSubImage2D or into a
+  // TransportDIB via glReadPixels.
+  //
+  // The size of the rectangle copied is the size last specified via
+  // SetSurfaceSize.  If another GL context than the one this
+  // AcceleratedSurface contains is responsible for the production of
+  // the pixels, then when this entry point is called, the color
+  // buffer must be in a state where a glCopyTexSubImage2D or
+  // glReadPixels is legal. (For example, if using multisampled FBOs,
+  // the FBO must have been resolved into a non-multisampled color
+  // texture.) Additionally, in this situation, the contexts must
+  // share server-side GL objects, so that this AcceleratedSurface's
+  // texture is a legal name in the namespace of the current context.
   void SwapBuffers();
+
   CGLContextObj context() { return gl_context_; }
 
   // These methods are only used when there is a transport DIB.
@@ -81,6 +108,11 @@ class AcceleratedSurface {
   // object is valid.
   bool SetupFrameBufferObject(GLenum target);
 
+  // The OpenGL context, and pbuffer drawable, used to transfer data
+  // to the shared region (IOSurface or TransportDIB). Strictly
+  // speaking, we do not need to allocate a GL context all of the
+  // time. We only need one if (a) we are using the IOSurface code
+  // path, or (b) if we are allocating an FBO internally.
   CGLContextObj gl_context_;
   CGLPBufferObj pbuffer_;
   // Either |io_surface_| or |transport_dib_| is a valid pointer, but not both.
@@ -95,15 +127,19 @@ class AcceleratedSurface {
   // make this work (or even compile).
   scoped_ptr<TransportDIB> transport_dib_;
   gfx::Size surface_size_;
+  // TODO(kbr): the FBO management should not be in this class at all.
+  // However, if it is factored out, care needs to be taken to not
+  // introduce another copy of the color data on the GPU; the direct
+  // binding of the internal texture to the IOSurface saves a copy.
+  bool allocate_fbo_;
+  // If the IOSurface code path is being used, then this texture
+  // object is always allocated. Otherwise, it is only allocated if
+  // the user requests an FBO be allocated.
   GLuint texture_;
+  // The FBO and renderbuffer are only allocated if allocate_fbo_ is
+  // true.
   GLuint fbo_;
   GLuint depth_stencil_renderbuffer_;
-  // For tracking whether the default framebuffer / renderbuffer or
-  // ones created by the end user are currently bound
-  // TODO(kbr): Need to property hook up and track the OpenGL state and hook
-  // up the notion of the currently bound FBO.
-  GLuint bound_fbo_;
-  GLuint bound_renderbuffer_;
   // Allocate a TransportDIB in the renderer.
   scoped_ptr<Callback2<size_t, TransportDIB::Handle*>::Type>
       dib_alloc_callback_;