GLInProcessContext: support async flushes and dedicated GPU thread

gpu/command_buffer/client/gl_in_process_context.cc

Index: gpu/command_buffer/client/gl_in_process_context.cc
diff --git a/gpu/command_buffer/client/gl_in_process_context.cc b/gpu/command_buffer/client/gl_in_process_context.cc
index 52e8d22b27bc8e261e6e17da1531858c84f19420..9fd3af7ed901324829e188858348665e59540bc3 100644
--- a/gpu/command_buffer/client/gl_in_process_context.cc
+++ b/gpu/command_buffer/client/gl_in_process_context.cc
@@ -16,35 +16,23 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/callback.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
-#include "base/synchronization/lock.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "gpu/command_buffer/client/gpu_memory_buffer.h"
 #include "gpu/command_buffer/client/image_factory.h"
 #include "gpu/command_buffer/client/transfer_buffer.h"
+#include "gpu/command_buffer/common/command_buffer.h"
 #include "gpu/command_buffer/common/constants.h"
-#include "gpu/command_buffer/common/id_allocator.h"
-#include "gpu/command_buffer/service/command_buffer_service.h"
-#include "gpu/command_buffer/service/context_group.h"
-#include "gpu/command_buffer/service/gl_context_virtual.h"
-#include "gpu/command_buffer/service/gpu_scheduler.h"
-#include "gpu/command_buffer/service/image_manager.h"
-#include "gpu/command_buffer/service/transfer_buffer_manager.h"
+#include "gpu/command_buffer/service/in_process_command_buffer.h"
 #include "ui/gfx/size.h"
-#include "ui/gl/gl_context.h"
 #include "ui/gl/gl_image.h"
-#include "ui/gl/gl_share_group.h"
-#include "ui/gl/gl_surface.h"
 
 namespace gpu {
 
-using gles2::ImageManager;
-
 namespace {
 
 const int32 kCommandBufferSize = 1024 * 1024;
@@ -54,51 +42,19 @@ const size_t kStartTransferBufferSize = 4 * 1024 * 1024;
 const size_t kMinTransferBufferSize = 1 * 256 * 1024;
 const size_t kMaxTransferBufferSize = 16 * 1024 * 1024;
 
-// In the normal command buffer implementation, all commands are passed over IPC
-// to the gpu process where they are fed to the GLES2Decoder from a single
-// thread. In layout tests, any thread could call this function. GLES2Decoder,
-// and in particular the GL implementations behind it, are not generally
-// threadsafe, so we guard entry points with a mutex.
-static base::LazyInstance<base::Lock> g_decoder_lock =
-    LAZY_INSTANCE_INITIALIZER;
-
-class GLInProcessContextImpl;
-
-static base::LazyInstance<
-    std::set<GLInProcessContextImpl*> >
-        g_all_shared_contexts = LAZY_INSTANCE_INITIALIZER;
-
-static bool g_use_virtualized_gl_context = false;
-
 static GLInProcessContext::GpuMemoryBufferCreator* g_gpu_memory_buffer_creator =
     NULL;
 
-// Also calls DetachFromThreadHack on all GLES2Decoders before the lock is
-// released to maintain the invariant that all decoders are unbound while the
-// lock is not held. This is to workaround DumpRenderTree using WGC3DIPCBI with
-// shared resources on different threads.
-// Remove this as part of crbug.com/234964.
-class AutoLockAndDecoderDetachThread {
- public:
-  AutoLockAndDecoderDetachThread(
-      base::Lock& lock,
-      const std::set<GLInProcessContextImpl*>& contexts);
-  ~AutoLockAndDecoderDetachThread();
-
- private:
-  base::AutoLock auto_lock_;
-  const std::set<GLInProcessContextImpl*>& contexts_;
-};
-
 class GLInProcessContextImpl
     : public GLInProcessContext,
       public gles2::ImageFactory,
       public base::SupportsWeakPtr<GLInProcessContextImpl> {
  public:
-  explicit GLInProcessContextImpl(bool share_resources);
+  explicit GLInProcessContextImpl();
   virtual ~GLInProcessContextImpl();
 
   bool Initialize(bool is_offscreen,
+                  bool share_resources,
                   gfx::AcceleratedWidget window,
                   const gfx::Size& size,
                   const char* allowed_extensions,
@@ -119,67 +75,24 @@ class GLInProcessContextImpl
       unsigned* image_id) OVERRIDE;
   virtual void DeleteGpuMemoryBuffer(unsigned image_id) OVERRIDE;
 
-  // Other methods:
-  gles2::GLES2Decoder* GetDecoder();
-  bool GetBufferChanged(int32 transfer_buffer_id);
-  void PumpCommands();
-  void OnResizeView(gfx::Size size, float scale_factor);
-  void OnContextLost();
-
  private:
   void Destroy();
-  bool IsCommandBufferContextLost();
   void PollQueryCallbacks();
   void CallQueryCallback(size_t index);
-  bool MakeCurrent();
-
-  gles2::ImageManager* GetImageManager();
 
-  base::Closure context_lost_callback_;
-  scoped_ptr<TransferBufferManagerInterface> transfer_buffer_manager_;
-  scoped_ptr<CommandBuffer> command_buffer_;
-  scoped_ptr<GpuScheduler> gpu_scheduler_;
-  scoped_ptr<gles2::GLES2Decoder> decoder_;
-  scoped_refptr<gfx::GLContext> context_;
-  scoped_refptr<gfx::GLSurface> surface_;
   scoped_ptr<gles2::GLES2CmdHelper> gles2_helper_;
   scoped_ptr<TransferBuffer> transfer_buffer_;
   scoped_ptr<gles2::GLES2Implementation> gles2_implementation_;
-  bool share_resources_;
-  bool context_lost_;
+  scoped_ptr<InProcessCommandBuffer> command_buffer_;
 
   typedef std::pair<unsigned, base::Closure> QueryCallback;
   std::vector<QueryCallback> query_callbacks_;
 
-  std::vector<base::Closure> signal_sync_point_callbacks_;
-
   DISALLOW_COPY_AND_ASSIGN(GLInProcessContextImpl);
 };
 
-AutoLockAndDecoderDetachThread::AutoLockAndDecoderDetachThread(
-    base::Lock& lock,
-    const std::set<GLInProcessContextImpl*>& contexts)
-    : auto_lock_(lock),
-      contexts_(contexts) {
-}
-
-void DetachThread(GLInProcessContextImpl* context) {
-  if (context->GetDecoder())
-    context->GetDecoder()->DetachFromThreadHack();
-}
-
-AutoLockAndDecoderDetachThread::~AutoLockAndDecoderDetachThread() {
-  std::for_each(contexts_.begin(),
-                contexts_.end(),
-                &DetachThread);
-}
-
 scoped_ptr<GpuMemoryBuffer> GLInProcessContextImpl::CreateGpuMemoryBuffer(
     int width, int height, GLenum internalformat, unsigned int* image_id) {
-  // We're taking the lock here because we're accessing the ContextGroup's
-  // shared IdManager.
-  AutoLockAndDecoderDetachThread lock(g_decoder_lock.Get(),
-                                      g_all_shared_contexts.Get());
   // For Android WebView we assume the |internalformat| will always be
   // GL_RGBA8_OES.
   DCHECK_EQ(static_cast<GLenum>(GL_RGBA8_OES), internalformat);
@@ -192,107 +105,45 @@ scoped_ptr<GpuMemoryBuffer> GLInProcessContextImpl::CreateGpuMemoryBuffer(
   scoped_refptr<gfx::GLImage> gl_image =
       gfx::GLImage::CreateGLImageForGpuMemoryBuffer(buffer->GetNativeBuffer(),
                                                     gfx::Size(width, height));
-  *image_id = decoder_->GetContextGroup()
-      ->GetIdAllocator(gles2::id_namespaces::kImages)->AllocateID();
-  GetImageManager()->AddImage(gl_image.get(), *image_id);
+  *image_id = command_buffer_->AddImage(gl_image);
   return buffer.Pass();
 }
 
 void GLInProcessContextImpl::DeleteGpuMemoryBuffer(unsigned int image_id) {
-  // We're taking the lock here because we're accessing the ContextGroup's
-  // shared ImageManager.
-  AutoLockAndDecoderDetachThread lock(g_decoder_lock.Get(),
-                                      g_all_shared_contexts.Get());
-  GetImageManager()->RemoveImage(image_id);
-  decoder_->GetContextGroup()->GetIdAllocator(gles2::id_namespaces::kImages)
-      ->FreeID(image_id);
+  command_buffer_->RemoveImage(image_id);
 }
 
-GLInProcessContextImpl::GLInProcessContextImpl(bool share_resources)
-    : share_resources_(share_resources),
-      context_lost_(false) {
-}
+GLInProcessContextImpl::GLInProcessContextImpl() {}
 
 GLInProcessContextImpl::~GLInProcessContextImpl() {
   Destroy();
 }
 
-bool GLInProcessContextImpl::MakeCurrent() {
-  if (decoder_->MakeCurrent())
-    return true;
-  DLOG(ERROR) << "Context lost because MakeCurrent failed.";
-  command_buffer_->SetContextLostReason(decoder_->GetContextLostReason());
-  command_buffer_->SetParseError(gpu::error::kLostContext);
-  return false;
-}
-
-void GLInProcessContextImpl::PumpCommands() {
-  {
-    AutoLockAndDecoderDetachThread lock(g_decoder_lock.Get(),
-                                        g_all_shared_contexts.Get());
-    if (!MakeCurrent())
-      return;
-    gpu_scheduler_->PutChanged();
-    CommandBuffer::State state = command_buffer_->GetState();
-    DCHECK((!error::IsError(state.error) && !context_lost_) ||
-           (error::IsError(state.error) && context_lost_));
-  }
-
-  if (!context_lost_ && signal_sync_point_callbacks_.size()) {
-    for (size_t n = 0; n < signal_sync_point_callbacks_.size(); n++) {
-      base::MessageLoop::current()->PostTask(FROM_HERE,
-                                             signal_sync_point_callbacks_[n]);
-    }
-  }
-  signal_sync_point_callbacks_.clear();
-}
-
-bool GLInProcessContextImpl::GetBufferChanged(int32 transfer_buffer_id) {
-  return gpu_scheduler_->SetGetBuffer(transfer_buffer_id);
-}
-
 void GLInProcessContextImpl::SignalSyncPoint(unsigned sync_point,
                                              const base::Closure& callback) {
   DCHECK(!callback.is_null());
-  signal_sync_point_callbacks_.push_back(callback);
-}
-
-bool GLInProcessContextImpl::IsCommandBufferContextLost() {
-  if (context_lost_ || !command_buffer_) {
-    return true;
-  }
-  CommandBuffer::State state = command_buffer_->GetState();
-  return error::IsError(state.error);
-}
-
-gles2::GLES2Decoder* GLInProcessContextImpl::GetDecoder() {
-  return decoder_.get();
-}
-
-void GLInProcessContextImpl::OnResizeView(gfx::Size size, float scale_factor) {
-  DCHECK(!surface_->IsOffscreen());
-  surface_->Resize(size);
+  command_buffer_->SignalSyncPoint(sync_point, callback);
 }
 
 gles2::GLES2Implementation* GLInProcessContextImpl::GetImplementation() {
   return gles2_implementation_.get();
 }
 
-gles2::ImageManager* GLInProcessContextImpl::GetImageManager() {
-  return decoder_->GetContextGroup()->image_manager();
-}
-
 bool GLInProcessContextImpl::Initialize(
     bool is_offscreen,
+    bool share_resources,
     gfx::AcceleratedWidget window,
     const gfx::Size& size,
     const char* allowed_extensions,
     const int32* attrib_list,
     gfx::GpuPreference gpu_preference,
     const base::Closure& context_lost_callback) {
-  // Use one share group for all contexts.
-  CR_DEFINE_STATIC_LOCAL(scoped_refptr<gfx::GLShareGroup>, share_group,
-                         (new gfx::GLShareGroup));
+  struct ShareGroupInstance {
+    ShareGroupInstance() : share_group(new gles2::ShareGroup(true, false)) {}
+    scoped_refptr<gles2::ShareGroup> share_group;
+  };

[piman, 2013/07/23 01:57:14]

nit: can you move the struct declaration out of the function (possibly in the
anonymous namespace)? I think templates + local types is a disaster area in
compilers.

[no sievers, 2013/07/25 00:41:23]

Done.

+  static base::LazyInstance<ShareGroupInstance> share_group =
+      LAZY_INSTANCE_INITIALIZER;

[piman, 2013/07/23 01:57:14]

So, the previous code had logic around lost contexts to make sure you're not
trying to insert yourself into a lost share group during recovery. Also, it made
sure to signal all contexts to be lost together.
It looks like that logic is gone - well it's still there for the service-side
ContextGroup, but not for the client-side ShareGroup any more, and I think a
mismatch between those isn't meant to happen, and you'll cause
texture/buffer/program ids to be confused (which can lead to command buffer
errors and lost contexts).

[no sievers, 2013/07/25 00:41:23]

I've solved this in a cheesy way by letting the Initialize() function return the
share group where it's created on the GPU thread.

Am still trying to understand a few things.... for example in the command buffer
implementation how is it not racy with context lost handling being async on the
client side and the stub being removed on the service side as a posted task? For
example you create a new context from the client side and pass in the share stub
but the context is already lost on the service side.

 
   DCHECK(size.width() >= 0 && size.height() >= 0);
 
@@ -323,128 +174,20 @@ bool GLInProcessContextImpl::Initialize(
     }
   }
 
-  {
-    TransferBufferManager* manager = new TransferBufferManager();
-    transfer_buffer_manager_.reset(manager);
-    manager->Initialize();
-  }
-
-  scoped_ptr<CommandBufferService> command_buffer(
-      new CommandBufferService(transfer_buffer_manager_.get()));
-  command_buffer->SetPutOffsetChangeCallback(base::Bind(
-      &GLInProcessContextImpl::PumpCommands, base::Unretained(this)));
-  command_buffer->SetGetBufferChangeCallback(base::Bind(
-      &GLInProcessContextImpl::GetBufferChanged, base::Unretained(this)));
-  command_buffer->SetParseErrorCallback(base::Bind(
-      &GLInProcessContextImpl::OnContextLost, base::Unretained(this)));
-
-  command_buffer_ = command_buffer.Pass();
-  if (!command_buffer_->Initialize()) {
-    LOG(ERROR) << "Could not initialize command buffer.";
-    Destroy();
-    return false;
-  }
-
-  GLInProcessContextImpl* context_group = NULL;
-
-  {
-    AutoLockAndDecoderDetachThread lock(g_decoder_lock.Get(),
-                                        g_all_shared_contexts.Get());
-    if (share_resources_ && !g_all_shared_contexts.Get().empty()) {
-      for (std::set<GLInProcessContextImpl*>::iterator it =
-               g_all_shared_contexts.Get().begin();
-           it != g_all_shared_contexts.Get().end();
-           ++it) {
-        if (!(*it)->IsCommandBufferContextLost()) {
-          context_group = *it;
-          break;
-        }
-      }
-      if (!context_group)
-        share_group = new gfx::GLShareGroup;
-    }
-
-    // TODO(gman): This needs to be true if this is Pepper.
-    bool bind_generates_resource = false;
-    decoder_.reset(gles2::GLES2Decoder::Create(
-        context_group ? context_group->decoder_->GetContextGroup()
-                      : new gles2::ContextGroup(
-                            NULL, NULL, NULL, NULL, bind_generates_resource)));
-
-    gpu_scheduler_.reset(new GpuScheduler(command_buffer_.get(),
-                                          decoder_.get(),
-                                          decoder_.get()));
-
-    decoder_->set_engine(gpu_scheduler_.get());
-
-    if (is_offscreen)
-      surface_ = gfx::GLSurface::CreateOffscreenGLSurface(size);
-    else
-      surface_ = gfx::GLSurface::CreateViewGLSurface(window);
-
-    if (!surface_.get()) {
-      LOG(ERROR) << "Could not create GLSurface.";
-      Destroy();
-      return false;
-    }
-
-    if (g_use_virtualized_gl_context) {
-      context_ = share_group->GetSharedContext();
-      if (!context_.get()) {
-        context_ = gfx::GLContext::CreateGLContext(
-            share_group.get(), surface_.get(), gpu_preference);
-        share_group->SetSharedContext(context_.get());
-      }
-
-      context_ = new GLContextVirtual(
-          share_group.get(), context_.get(), decoder_->AsWeakPtr());
-      if (context_->Initialize(surface_.get(), gpu_preference)) {
-        VLOG(1) << "Created virtual GL context.";
-      } else {
-        context_ = NULL;
-      }
-    } else {
-      context_ = gfx::GLContext::CreateGLContext(share_group.get(),
-                                                 surface_.get(),
-                                                 gpu_preference);
-    }
-
-    if (!context_.get()) {
-      LOG(ERROR) << "Could not create GLContext.";
-      Destroy();
-      return false;
-    }
-
-    if (!context_->MakeCurrent(surface_.get())) {
-      LOG(ERROR) << "Could not make context current.";
-      Destroy();
-      return false;
-    }
-
-    gles2::DisallowedFeatures disallowed_features;
-    disallowed_features.swap_buffer_complete_callback = true;
-    disallowed_features.gpu_memory_manager = true;
-    if (!decoder_->Initialize(surface_,
-                              context_,
-                              is_offscreen,
-                              size,
-                              disallowed_features,
-                              allowed_extensions,
-                              attribs)) {
-      LOG(ERROR) << "Could not initialize decoder.";
-      Destroy();
-      return false;
-    }
-
-    if (!is_offscreen) {
-      decoder_->SetResizeCallback(base::Bind(
-          &GLInProcessContextImpl::OnResizeView, base::Unretained(this)));
-    }
-  }
+  command_buffer_.reset(new InProcessCommandBuffer());
+  command_buffer_->Initialize(is_offscreen,
+                  share_resources,

[piman, 2013/07/23 01:57:14]

nit: indentation

[no sievers, 2013/07/25 00:41:23]

Done.

+                  window,
+                  size,
+                  allowed_extensions,
+                  attribs,
+                  gpu_preference,
+                  context_lost_callback);
 
   // Create the GLES2 helper, which writes the command buffer protocol.
   gles2_helper_.reset(new gles2::GLES2CmdHelper(command_buffer_.get()));
   if (!gles2_helper_->Initialize(kCommandBufferSize)) {
+    LOG(INFO) << "fail";

[piman, 2013/07/23 01:57:14]

nit: more descriptive log?

[no sievers, 2013/07/25 00:41:23]

Done.

     Destroy();
     return false;
   }
@@ -455,7 +198,7 @@ bool GLInProcessContextImpl::Initialize(
   // Create the object exposing the OpenGL API.
   gles2_implementation_.reset(new gles2::GLES2Implementation(
       gles2_helper_.get(),
-      context_group ? context_group->GetImplementation()->share_group() : NULL,
+      share_resources ? share_group.Get().share_group : NULL,
       transfer_buffer_.get(),
       true,
       false,
@@ -468,13 +211,6 @@ bool GLInProcessContextImpl::Initialize(
     return false;
   }
 
-  if (share_resources_) {
-    AutoLockAndDecoderDetachThread lock(g_decoder_lock.Get(),
-                                        g_all_shared_contexts.Get());
-    g_all_shared_contexts.Pointer()->insert(this);
-  }
-
-  context_lost_callback_ = context_lost_callback;
   return true;
 }
 
@@ -483,8 +219,6 @@ void GLInProcessContextImpl::Destroy() {
     CallQueryCallback(0);
   }
 
-  bool context_lost = IsCommandBufferContextLost();
-
   if (gles2_implementation_) {
     // First flush the context to ensure that any pending frees of resources
     // are completed. Otherwise, if this context is part of a share group,
@@ -499,28 +233,6 @@ void GLInProcessContextImpl::Destroy() {
   transfer_buffer_.reset();
   gles2_helper_.reset();
   command_buffer_.reset();
-
-  AutoLockAndDecoderDetachThread lock(g_decoder_lock.Get(),
-                                      g_all_shared_contexts.Get());
-  if (decoder_) {
-    decoder_->Destroy(!context_lost);
-  }
-
-  g_all_shared_contexts.Pointer()->erase(this);
-}
-
-void GLInProcessContextImpl::OnContextLost() {
-  if (!context_lost_callback_.is_null())
-    context_lost_callback_.Run();
-
-  context_lost_ = true;
-  if (share_resources_) {
-      for (std::set<GLInProcessContextImpl*>::iterator it =
-               g_all_shared_contexts.Get().begin();
-           it != g_all_shared_contexts.Get().end();
-           ++it)
-        (*it)->context_lost_ = true;
-  }
 }
 
 void GLInProcessContextImpl::CallQueryCallback(size_t index) {
@@ -531,6 +243,7 @@ void GLInProcessContextImpl::CallQueryCallback(size_t index) {
   query_callback.second.Run();
 }
 
+// TODO(sievers): Move this to the service side
 void GLInProcessContextImpl::PollQueryCallbacks() {
   for (size_t i = 0; i < query_callbacks_.size();) {
     unsigned query = query_callbacks_[i].first;
@@ -579,9 +292,10 @@ GLInProcessContext* GLInProcessContext::CreateContext(
     gfx::GpuPreference gpu_preference,
     const base::Closure& callback) {
   scoped_ptr<GLInProcessContextImpl> context(
-      new GLInProcessContextImpl(share_resources));
+      new GLInProcessContextImpl());
   if (!context->Initialize(
       is_offscreen,
+      share_resources,
       window,
       size,
       allowed_extensions,
@@ -599,16 +313,4 @@ void GLInProcessContext::SetGpuMemoryBufferCreator(
   g_gpu_memory_buffer_creator = creator;
 }
 
-// static
-void GLInProcessContext::EnableVirtualizedContext() {
-#if !defined(NDEBUG)
-  {
-    AutoLockAndDecoderDetachThread lock(g_decoder_lock.Get(),
-                                        g_all_shared_contexts.Get());
-    DCHECK(g_all_shared_contexts.Get().empty());
-  }
-#endif  // !defined(NDEBUG)
-  g_use_virtualized_gl_context = true;
-}
-
 }  // namespace gpu