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

10 files changed, 179 insertions, 76 deletions

diff --git a/gpu/command_buffer/service/gl_context.h b/gpu/command_buffer/service/gl_context.h
index 832624db9..e4d033e 100644
--- a/gpu/command_buffer/service/gl_context.h
+++ b/gpu/command_buffer/service/gl_context.h
@@ -8,13 +8,12 @@
 #include <build/build_config.h>
 
 #include "base/basictypes.h"
+#include "base/logging.h"
 #include "gfx/native_widget_types.h"
 #include "gfx/size.h"
 #include "gpu/command_buffer/common/logging.h"
 #include "gpu/command_buffer/service/gl_utils.h"
 
-class AcceleratedSurface;
-
 namespace gpu {
 
 #if defined(UNIT_TEST)
@@ -47,6 +46,9 @@ class GLContext {
   // Makes the GL context current on the current thread.
   virtual bool MakeCurrent() = 0;
 
+  // Returns true if this context is current.
+  virtual bool IsCurrent() = 0;
+
   // Returns true if this context is offscreen.
   virtual bool IsOffscreen() = 0;
 
@@ -84,8 +86,8 @@ class ViewGLContext : public GLContext {
     DCHECK(window);
   }
 #elif defined(OS_MACOSX)
-  explicit ViewGLContext(AcceleratedSurface* surface) : surface_(surface) {
-    DCHECK(surface);
+  ViewGLContext() {
+    NOTIMPLEMENTED() << "ViewGLContext not supported on Mac platform.";
   }
 #endif
 
@@ -94,6 +96,7 @@ class ViewGLContext : public GLContext {
 
   virtual void Destroy();
   virtual bool MakeCurrent();
+  virtual bool IsCurrent();
   virtual bool IsOffscreen();
   virtual void SwapBuffers();
   virtual gfx::Size GetSize();
@@ -109,7 +112,7 @@ class ViewGLContext : public GLContext {
   gfx::PluginWindowHandle window_;
   GLContextHandle context_;
 #elif defined(OS_MACOSX)
-  AcceleratedSurface* surface_;
+  // This context isn't implemented on Mac OS X.
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(ViewGLContext);
@@ -142,9 +145,11 @@ class PbufferGLContext : public GLContext {
 
   // Initializes the GL context.
   bool Initialize(GLContext* shared_context);
+  bool Initialize(GLContextHandle shared_handle);
 
   virtual void Destroy();
   virtual bool MakeCurrent();
+  virtual bool IsCurrent();
   virtual bool IsOffscreen();
   virtual void SwapBuffers();
   virtual gfx::Size GetSize();
diff --git a/gpu/command_buffer/service/gl_context_linux.cc b/gpu/command_buffer/service/gl_context_linux.cc
index 0acbb02..9b56bb3 100644
--- a/gpu/command_buffer/service/gl_context_linux.cc
+++ b/gpu/command_buffer/service/gl_context_linux.cc
@@ -126,8 +126,7 @@ Bool result = glXMakeCurrent(display_, 0, 0);
 
 bool ViewGLContext::MakeCurrent() {
 #if !defined(UNIT_TEST)
-  if (glXGetCurrentDrawable() == window_ &&
-      glXGetCurrentContext() == context_) {
+  if (IsCurrent()) {
     return true;
   }
   if (glXMakeCurrent(display_, window_, context_) != True) {
@@ -141,6 +140,15 @@ bool ViewGLContext::MakeCurrent() {
   return true;
 }
 
+bool ViewGLContext::IsCurrent() {
+#if !defined(UNIT_TEST)
+  return glXGetCurrentDrawable() == window_ &&
+      glXGetCurrentContext() == context_;
+#else
+  return true;
+#endif
+}
+
 bool ViewGLContext::IsOffscreen() {
   return false;
 }
@@ -170,6 +178,10 @@ GLContextHandle ViewGLContext::GetHandle() {
 }
 
 bool PbufferGLContext::Initialize(GLContext* shared_context) {
+  return Initialize(shared_context ? shared_context->GetHandle() : NULL);
+}
+
+bool PbufferGLContext::Initialize(GLContextHandle shared_handle) {
 #if !defined(UNIT_TEST)
   if (!InitializeGLXEW(display_))
     return false;
@@ -182,11 +194,6 @@ bool PbufferGLContext::Initialize(GLContext* shared_context) {
     return false;
   }
 
-  // Get the shared context handle.
-  GLContextHandle shared_handle = NULL;
-  if (shared_context)
-    shared_handle = shared_context->GetHandle();
-
   static const int config_attributes[] = {
     GLX_DRAWABLE_TYPE,
     GLX_PBUFFER_BIT,
@@ -269,8 +276,7 @@ void PbufferGLContext::Destroy() {
 
 bool PbufferGLContext::MakeCurrent() {
 #if !defined(UNIT_TEST)
-  if (glXGetCurrentDrawable() == pbuffer_ &&
-      glXGetCurrentContext() == context_) {
+  if (IsCurrent()) {
     return true;
   }
   if (glXMakeCurrent(display_, pbuffer_, context_) != True) {
@@ -284,6 +290,15 @@ bool PbufferGLContext::MakeCurrent() {
   return true;
 }
 
+bool PbufferGLContext::IsCurrent() {
+#if !defined(UNIT_TEST)
+  return glXGetCurrentDrawable() == pbuffer_ &&
+      glXGetCurrentContext() == context_;
+#else
+  return true;
+#endif
+}
+
 bool PbufferGLContext::IsOffscreen() {
   return true;
 }
diff --git a/gpu/command_buffer/service/gl_context_mac.cc b/gpu/command_buffer/service/gl_context_mac.cc
index 9e79a69..6d2f726 100644
--- a/gpu/command_buffer/service/gl_context_mac.cc
+++ b/gpu/command_buffer/service/gl_context_mac.cc
@@ -13,29 +13,13 @@
 
 namespace gpu {
 
+static const char* error_message =
+    "ViewGLContext not supported on Mac platform.";
+
 bool ViewGLContext::Initialize(bool multisampled) {
 #if !defined(UNIT_TEST)
-  if (multisampled) {
-    DLOG(WARNING) << "Multisampling not implemented.";
-  }
-
-  if (!surface_->Initialize()) {
-    DLOG(ERROR) << "Error initializing accelerated surface.";
-    return false;
-  }
-
-  if (!MakeCurrent()) {
-    Destroy();
-    DLOG(ERROR) << "Couldn't make context current for initialization.";
-    return false;
-  }
-
-  if (!InitializeGLEW()) {
-    Destroy();
-    return false;
-  }
-
-  return true;
+  NOTIMPLEMENTED() << error_message;
+  return false;
 #else
   return true;
 #endif  // UNIT_TEST
@@ -43,31 +27,43 @@ bool ViewGLContext::Initialize(bool multisampled) {
 
 void ViewGLContext::Destroy() {
 #if !defined(UNIT_TEST)
-  surface_->Destroy();
+  NOTIMPLEMENTED() << error_message;
 #endif  // UNIT_TEST
 }
 
 bool ViewGLContext::MakeCurrent() {
 #if !defined(UNIT_TEST)
-  return surface_->MakeCurrent();
+  NOTIMPLEMENTED() << error_message;
+  return false;
+#else
+  return true;
+#endif
+}
+
+bool ViewGLContext::IsCurrent() {
+#if !defined(UNIT_TEST)
+  NOTIMPLEMENTED() << error_message;
+  return false;
 #else
   return true;
 #endif
 }
 
 bool ViewGLContext::IsOffscreen() {
+  NOTIMPLEMENTED() << error_message;
   return false;
 }
 
 void ViewGLContext::SwapBuffers() {
 #if !defined(UNIT_TEST)
-  surface_->SwapBuffers();
+  NOTIMPLEMENTED() << error_message;
 #endif  // UNIT_TEST
 }
 
 gfx::Size ViewGLContext::GetSize() {
 #if !defined(UNIT_TEST)
-  return surface_->GetSize();
+  NOTIMPLEMENTED() << error_message;
+  return gfx::Size();
 #else
   return gfx::Size();
 #endif  // UNIT_TEST
@@ -75,19 +71,17 @@ gfx::Size ViewGLContext::GetSize() {
 
 GLContextHandle ViewGLContext::GetHandle() {
 #if !defined(UNIT_TEST)
-  return surface_->context();
-#else
-  return NULL;
+  NOTIMPLEMENTED() << error_message;
 #endif  // UNIT_TEST
+  return NULL;
 }
 
 bool PbufferGLContext::Initialize(GLContext* shared_context) {
-#if !defined(UNIT_TEST)
-  // Get the shared context handle.
-  GLContextHandle shared_handle = NULL;
-  if (shared_context)
-    shared_handle = shared_context->GetHandle();
+  return Initialize(shared_context ? shared_context->GetHandle() : NULL);
+}
 
+bool PbufferGLContext::Initialize(GLContextHandle shared_handle) {
+#if !defined(UNIT_TEST)
   // Create a 1x1 pbuffer and associated context to bootstrap things.
   static const CGLPixelFormatAttribute attribs[] = {
     (CGLPixelFormatAttribute) kCGLPFAPBuffer,
@@ -156,7 +150,7 @@ void PbufferGLContext::Destroy() {
 
 bool PbufferGLContext::MakeCurrent() {
 #if !defined(UNIT_TEST)
-  if (CGLGetCurrentContext() != context_) {
+  if (!IsCurrent()) {
     if (CGLSetCurrentContext(context_) != kCGLNoError) {
       DLOG(ERROR) << "Unable to make gl context current.";
       return false;
@@ -167,6 +161,14 @@ bool PbufferGLContext::MakeCurrent() {
   return true;
 }
 
+bool PbufferGLContext::IsCurrent() {
+#if !defined(UNIT_TEST)
+  return CGLGetCurrentContext() == context_;
+#else
+  return true;
+#endif
+}
+
 bool PbufferGLContext::IsOffscreen() {
   return true;
 }
diff --git a/gpu/command_buffer/service/gl_context_win.cc b/gpu/command_buffer/service/gl_context_win.cc
index 3be106a..6b5896c 100644
--- a/gpu/command_buffer/service/gl_context_win.cc
+++ b/gpu/command_buffer/service/gl_context_win.cc
@@ -228,8 +228,7 @@ void ViewGLContext::Destroy() {
 
 bool ViewGLContext::MakeCurrent() {
 #if !defined(UNIT_TEST)
-  if (wglGetCurrentDC() == device_context_ &&
-      wglGetCurrentContext() == context_) {
+  if (IsCurrent()) {
     return true;
   }
   if (!wglMakeCurrent(device_context_, context_)) {
@@ -241,6 +240,15 @@ bool ViewGLContext::MakeCurrent() {
   return true;
 }
 
+bool ViewGLContext::IsCurrent() {
+#if !defined(UNIT_TEST)
+  return wglGetCurrentDC() == device_context_ &&
+      wglGetCurrentContext() == context_;
+#else
+  return true;
+#endif
+}
+
 bool ViewGLContext::IsOffscreen() {
   return false;
 }
@@ -271,6 +279,10 @@ GLContextHandle ViewGLContext::GetHandle() {
 }
 
 bool PbufferGLContext::Initialize(GLContext* shared_context) {
+  return Initialize(shared_context ? shared_context->GetHandle() : NULL);
+}
+
+bool PbufferGLContext::Initialize(GLContextHandle shared_handle) {
 #if !defined(UNIT_TEST)
   InitializeOneOff();
 
@@ -306,8 +318,8 @@ bool PbufferGLContext::Initialize(GLContext* shared_context) {
     return false;
   }
 
-  if (shared_context) {
-    if (!wglShareLists(shared_context->GetHandle(), context_)) {
+  if (shared_handle) {
+    if (!wglShareLists(shared_handle, context_)) {
       DLOG(ERROR) << "Could not share GL contexts.";
       Destroy();
       return false;
@@ -339,8 +351,7 @@ void PbufferGLContext::Destroy() {
 
 bool PbufferGLContext::MakeCurrent() {
 #if !defined(UNIT_TEST)
-  if (wglGetCurrentDC() == device_context_ &&
-      wglGetCurrentContext() == context_) {
+  if (IsCurrent()) {
     return true;
   }
   if (!wglMakeCurrent(device_context_, context_)) {
@@ -352,6 +363,15 @@ bool PbufferGLContext::MakeCurrent() {
   return true;
 }
 
+bool PbufferGLContext::IsCurrent() {
+#if !defined(UNIT_TEST)
+  return wglGetCurrentDC() == device_context_ &&
+      wglGetCurrentContext() == context_;
+#else
+  return true;
+#endif
+}
+
 bool PbufferGLContext::IsOffscreen() {
   return true;
 }
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 911c19a..ce4f44d 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -3405,6 +3405,14 @@ error::Error GLES2DecoderImpl::HandleSwapBuffers(
 #endif
   }
 
+  // TODO(kbr): when the back buffer is multisampled, then at least on Mac
+  // OS X (and probably on all platforms, for best semantics), we will need
+  // to perform the resolve step and bind the offscreen_saved_color_texture_
+  // as the color attachment before calling the swap buffers callback, which
+  // expects a normal (non-multisampled) frame buffer for glCopyTexImage2D /
+  // glReadPixels. After the callback runs, the multisampled frame buffer
+  // needs to be bound again.
+
   if (swap_buffers_callback_.get()) {
     swap_buffers_callback_->Run();
   }
diff --git a/gpu/command_buffer/service/gpu_processor.cc b/gpu/command_buffer/service/gpu_processor.cc
index 4514211..ab3230b 100644
--- a/gpu/command_buffer/service/gpu_processor.cc
+++ b/gpu/command_buffer/service/gpu_processor.cc
@@ -69,7 +69,7 @@ bool GPUProcessor::InitializeCommon(const gfx::Size& size,
   return true;
 }
 
-void GPUProcessor::Destroy() {
+void GPUProcessor::DestroyCommon() {
   if (decoder_.get()) {
     decoder_->Destroy();
     decoder_.reset();
@@ -141,7 +141,10 @@ void GPUProcessor::ResizeOffscreenFrameBuffer(const gfx::Size& size) {
 
 void GPUProcessor::SetSwapBuffersCallback(
     Callback0::Type* callback) {
-  decoder_->SetSwapBuffersCallback(callback);
+  wrapped_swap_buffers_callback_.reset(callback);
+  decoder_->SetSwapBuffersCallback(
+      NewCallback(this,
+                  &GPUProcessor::WillSwapBuffers));
 }
 
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/gpu_processor.h b/gpu/command_buffer/service/gpu_processor.h
index c4c0ea5..eb2d52b 100644
--- a/gpu/command_buffer/service/gpu_processor.h
+++ b/gpu/command_buffer/service/gpu_processor.h
@@ -51,6 +51,7 @@ class GPUProcessor : public CommandBufferEngine {
                         uint32 parent_texture_id);
 
   void Destroy();
+  void DestroyCommon();
 
   virtual void ProcessCommands();
 
@@ -84,6 +85,10 @@ class GPUProcessor : public CommandBufferEngine {
   virtual void SetSwapBuffersCallback(Callback0::Type* callback);
 
  private:
+  // Called via a callback just before we are supposed to call the
+  // user's swap buffers callback.
+  virtual void WillSwapBuffers();
+
   // The GPUProcessor holds a weak reference to the CommandBuffer. The
   // CommandBuffer owns the GPUProcessor and holds a strong reference to it
   // through the ProcessCommands callback.
@@ -97,10 +102,11 @@ class GPUProcessor : public CommandBufferEngine {
   scoped_ptr<GLContext> context_;
 
 #if defined(OS_MACOSX) && !defined(UNIT_TEST)
-  AcceleratedSurface surface_;
+  scoped_ptr<AcceleratedSurface> surface_;
 #endif
 
   ScopedRunnableMethodFactory<GPUProcessor> method_factory_;
+  scoped_ptr<Callback0::Type> wrapped_swap_buffers_callback_;
 };
 
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/gpu_processor_linux.cc b/gpu/command_buffer/service/gpu_processor_linux.cc
index 10f1a87..4217b72 100644
--- a/gpu/command_buffer/service/gpu_processor_linux.cc
+++ b/gpu/command_buffer/service/gpu_processor_linux.cc
@@ -57,4 +57,14 @@ bool GPUProcessor::Initialize(gfx::PluginWindowHandle window,
   return true;
 }
 
+void GPUProcessor::Destroy() {
+  DestroyCommon();
+}
+
+void GPUProcessor::WillSwapBuffers() {
+  if (wrapped_swap_buffers_callback_.get()) {
+    wrapped_swap_buffers_callback_->Run();
+  }
+}
+
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/gpu_processor_mac.cc b/gpu/command_buffer/service/gpu_processor_mac.cc
index 55b6816..4893c89 100644
--- a/gpu/command_buffer/service/gpu_processor_mac.cc
+++ b/gpu/command_buffer/service/gpu_processor_mac.cc
@@ -28,27 +28,25 @@ bool GPUProcessor::Initialize(gfx::PluginWindowHandle window,
     DCHECK(parent_context);
   }
 
-  // Create either a view or pbuffer based GLContext.
+  scoped_ptr<PbufferGLContext> context(new PbufferGLContext);
+  if (!context->Initialize(parent_context)) {
+    Destroy();
+    return false;
+  }
+  context_.reset(context.release());
+  // On Mac OS X since we can not render on-screen we don't even
+  // attempt to create a view based GLContext. The only difference
+  // between "on-screen" and "off-screen" rendering on this platform
+  // is whether we allocate an AcceleratedSurface, which transmits the
+  // rendering results back to the browser.
   if (window) {
 #if !defined(UNIT_TEST)
-    AcceleratedSurface* surface_ptr = &surface_;
-#else
-    AcceleratedSurface* surface_ptr = NULL;
-#endif
-    scoped_ptr<ViewGLContext> context(new ViewGLContext(surface_ptr));
-    // TODO(apatrick): support multisampling.
-    if (!context->Initialize(false)) {
-      Destroy();
-      return false;
-    }
-    context_.reset(context.release());
-  } else {
-    scoped_ptr<PbufferGLContext> context(new PbufferGLContext);
-    if (!context->Initialize(parent_context)) {
+    surface_.reset(new AcceleratedSurface());
+    if (!surface_->Initialize(context_->GetHandle(), false)) {
       Destroy();
       return false;
     }
-    context_.reset(context.release());
+#endif
   }
 
   return InitializeCommon(size, parent_decoder, parent_texture_id);
@@ -56,9 +54,20 @@ bool GPUProcessor::Initialize(gfx::PluginWindowHandle window,
   return true;
 }
 
+void GPUProcessor::Destroy() {
+#if !defined(UNIT_TEST)
+  if (surface_.get()) {
+    surface_->Destroy();
+  }
+  surface_.reset();
+#endif
+  DestroyCommon();
+}
+
 uint64 GPUProcessor::SetWindowSizeForIOSurface(const gfx::Size& size) {
 #if !defined(UNIT_TEST)
-  return surface_.SetSurfaceSize(size);
+  ResizeOffscreenFrameBuffer(size);
+  return surface_->SetSurfaceSize(size);
 #else
   return 0;
 #endif
@@ -67,7 +76,8 @@ uint64 GPUProcessor::SetWindowSizeForIOSurface(const gfx::Size& size) {
 TransportDIB::Handle GPUProcessor::SetWindowSizeForTransportDIB(
     const gfx::Size& size) {
 #if !defined(UNIT_TEST)
-  return surface_.SetTransportDIBSize(size);
+  ResizeOffscreenFrameBuffer(size);
+  return surface_->SetTransportDIBSize(size);
 #else
   return TransportDIB::DefaultHandleValue();
 #endif
@@ -77,8 +87,21 @@ void GPUProcessor::SetTransportDIBAllocAndFree(
       Callback2<size_t, TransportDIB::Handle*>::Type* allocator,
       Callback1<TransportDIB::Id>::Type* deallocator) {
 #if !defined(UNIT_TEST)
-  surface_.SetTransportDIBAllocAndFree(allocator, deallocator);
+  surface_->SetTransportDIBAllocAndFree(allocator, deallocator);
+#endif
+}
+
+void GPUProcessor::WillSwapBuffers() {
+  DCHECK(context_->IsCurrent());
+#if !defined(UNIT_TEST)
+  if (surface_.get()) {
+    surface_->SwapBuffers();
+  }
 #endif
+
+  if (wrapped_swap_buffers_callback_.get()) {
+    wrapped_swap_buffers_callback_->Run();
+  }
 }
 
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/gpu_processor_win.cc b/gpu/command_buffer/service/gpu_processor_win.cc
index 143d5a6..77523be 100644
--- a/gpu/command_buffer/service/gpu_processor_win.cc
+++ b/gpu/command_buffer/service/gpu_processor_win.cc
@@ -52,4 +52,15 @@ bool GPUProcessor::Initialize(gfx::PluginWindowHandle window,
 
   return true;
 }
+
+void GPUProcessor::Destroy() {
+  DestroyCommon();
+}
+
+void GPUProcessor::WillSwapBuffers() {
+  if (wrapped_swap_buffers_callback_.get()) {
+    wrapped_swap_buffers_callback_->Run();
+  }
+}
+
 }  // namespace gpu