Don't tell browser to swap until swap actually executed.

By deferring to fence, we're ensuring that the GL commands are executed GPU side before sending a message to the browser to act on them. This is lighter weight than a glFinish because the other commands buffers on the channel (e.g. WebGL) can continue executing while we're waiting for the fence.

TEST=by hand building use_aura running http://webglsamples.googlecode.com/hg/aquarium/aquarium.html

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

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

10 files changed, 236 insertions, 67 deletions

diff --git a/content/common/gpu/gpu_channel.cc b/content/common/gpu/gpu_channel.cc
index 52fab1a..7622e30 100644
--- a/content/common/gpu/gpu_channel.cc
+++ b/content/common/gpu/gpu_channel.cc
@@ -263,24 +263,17 @@ void GpuChannel::HandleMessage() {
       // If the channel becomes unscheduled as a result of handling the message
       // or has more work to do, synthesize an IPC message to flush the command
       // buffer that became unscheduled.
-      bool has_more_work = false;
-      for (StubMap::Iterator<GpuCommandBufferStub> it(&stubs_);
-           !it.IsAtEnd();
-           it.Advance()) {
-        GpuCommandBufferStub* stub = it.GetCurrentValue();
-
-        if (!stub->IsScheduled() || stub->HasMoreWork()) {
-          has_more_work = true;
-          deferred_messages_.push_front(new GpuCommandBufferMsg_Rescheduled(
-              stub->route_id()));
-        }
+      GpuCommandBufferStub* stub = stubs_.Lookup(message->routing_id());
+      if (!stub->IsScheduled() || stub->HasMoreWork()) {
+        deferred_messages_.push_front(new GpuCommandBufferMsg_Rescheduled(
+            stub->route_id()));
       }
-
-      if (has_more_work) {
+      if (stub->HasMoreWork() && !handle_messages_scheduled_) {
         MessageLoop::current()->PostDelayedTask(
             FROM_HERE,
             base::Bind(&GpuChannel::HandleMessage, weak_factory_.GetWeakPtr()),
             kHandleMoreWorkPeriod);
+        handle_messages_scheduled_ = true;
       }
     }
   }
diff --git a/content/common/gpu/image_transport_surface_linux.cc b/content/common/gpu/image_transport_surface_linux.cc
index 87d5648..c4b789d 100644
--- a/content/common/gpu/image_transport_surface_linux.cc
+++ b/content/common/gpu/image_transport_surface_linux.cc
@@ -13,7 +13,8 @@
 #include <X11/Xlib.h>
 #include <X11/extensions/Xcomposite.h>
 
-#include "base/callback.h"
+#include "base/bind.h"
+#include "base/memory/weak_ptr.h"
 #include "base/debug/trace_event.h"
 #include "content/common/gpu/gpu_channel.h"
 #include "content/common/gpu/gpu_channel_manager.h"
@@ -35,7 +36,7 @@ namespace {
 // an instance is created or destroyed.
 class EGLAcceleratedSurface : public base::RefCounted<EGLAcceleratedSurface> {
  public:
-  EGLAcceleratedSurface(const gfx::Size& size);
+  explicit EGLAcceleratedSurface(const gfx::Size& size);
   const gfx::Size& size() const { return size_; }
   uint32 pixmap() const { return pixmap_; }
   uint32 texture() const { return texture_; }
@@ -54,8 +55,10 @@ class EGLAcceleratedSurface : public base::RefCounted<EGLAcceleratedSurface> {
 
 // We are backed by an Pbuffer offscreen surface for the purposes of creating a
 // context, but use FBOs to render to X Pixmap backed EGLImages.
-class EGLImageTransportSurface : public ImageTransportSurface,
-                                 public gfx::PbufferGLSurfaceEGL {
+class EGLImageTransportSurface
+    : public ImageTransportSurface,
+      public gfx::PbufferGLSurfaceEGL,
+      public base::SupportsWeakPtr<EGLImageTransportSurface> {
  public:
   EGLImageTransportSurface(GpuChannelManager* manager,
                            int32 render_view_id,
@@ -86,6 +89,7 @@ class EGLImageTransportSurface : public ImageTransportSurface,
  private:
   virtual ~EGLImageTransportSurface() OVERRIDE;
   void ReleaseSurface(scoped_refptr<EGLAcceleratedSurface>* surface);
+  void SendBuffersSwapped();
 
   uint32 fbo_id_;
 
@@ -102,8 +106,10 @@ class EGLImageTransportSurface : public ImageTransportSurface,
 
 // We render to an off-screen (but mapped) window that the browser process will
 // read from via XComposite
-class GLXImageTransportSurface : public ImageTransportSurface,
-                                 public gfx::NativeViewGLSurfaceGLX {
+class GLXImageTransportSurface
+    : public ImageTransportSurface,
+      public gfx::NativeViewGLSurfaceGLX,
+      public base::SupportsWeakPtr<GLXImageTransportSurface> {
  public:
   GLXImageTransportSurface(GpuChannelManager* manager,
                            int32 render_view_id,
@@ -135,6 +141,9 @@ class GLXImageTransportSurface : public ImageTransportSurface,
   // Tell the browser to release the surface.
   void ReleaseSurface();
 
+  void SendBuffersSwapped();
+  void SendPostSubBuffer(int x, int y, int width, int height);
+
   XID dummy_parent_;
   gfx::Size size_;
 
@@ -352,11 +361,9 @@ void EGLImageTransportSurface::OnResize(gfx::Size size) {
 bool EGLImageTransportSurface::SwapBuffers() {
   front_surface_.swap(back_surface_);
   DCHECK_NE(front_surface_.get(), static_cast<EGLAcceleratedSurface*>(NULL));
-  glFlush();
-
-  GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
-  params.surface_id = front_surface_->pixmap();
-  helper_->SendAcceleratedSurfaceBuffersSwapped(params);
+  helper_->DeferToFence(base::Bind(
+      &EGLImageTransportSurface::SendBuffersSwapped,
+      AsWeakPtr()));
 
   gfx::Size expected_size = front_surface_->size();
   if (!back_surface_.get() || back_surface_->size() != expected_size) {
@@ -368,10 +375,16 @@ bool EGLImageTransportSurface::SwapBuffers() {
                               back_surface_->texture(),
                               0);
   }
-  helper_->SetScheduled(false);
   return true;
 }
 
+void EGLImageTransportSurface::SendBuffersSwapped() {
+  GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
+  params.surface_id = front_surface_->pixmap();
+  helper_->SendAcceleratedSurfaceBuffersSwapped(params);
+  helper_->SetScheduled(false);
+}
+
 bool EGLImageTransportSurface::PostSubBuffer(
     int x, int y, int width, int height) {
   NOTREACHED();
@@ -523,7 +536,9 @@ void GLXImageTransportSurface::OnResize(gfx::Size size) {
 
 bool GLXImageTransportSurface::SwapBuffers() {
   gfx::NativeViewGLSurfaceGLX::SwapBuffers();
-  glFlush();
+  helper_->DeferToFence(base::Bind(
+      &GLXImageTransportSurface::SendBuffersSwapped,
+      AsWeakPtr()));
 
   if (needs_resize_) {
     GpuHostMsg_AcceleratedSurfaceNew_Params params;
@@ -534,19 +549,22 @@ bool GLXImageTransportSurface::SwapBuffers() {
     bound_ = true;
     needs_resize_ = false;
   }
+  return true;
+}
 
+void GLXImageTransportSurface::SendBuffersSwapped() {
   GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
   params.surface_id = window_;
   helper_->SendAcceleratedSurfaceBuffersSwapped(params);
-
   helper_->SetScheduled(false);
-  return true;
 }
 
 bool GLXImageTransportSurface::PostSubBuffer(
     int x, int y, int width, int height) {
   gfx::NativeViewGLSurfaceGLX::PostSubBuffer(x, y, width, height);
-  glFlush();
+  helper_->DeferToFence(base::Bind(
+      &GLXImageTransportSurface::SendPostSubBuffer,
+      AsWeakPtr(), x, y, width, height));
 
   if (needs_resize_) {
     GpuHostMsg_AcceleratedSurfaceNew_Params params;
@@ -557,7 +575,11 @@ bool GLXImageTransportSurface::PostSubBuffer(
     bound_ = true;
     needs_resize_ = false;
   }
+  return true;
+}
 
+void GLXImageTransportSurface::SendPostSubBuffer(
+    int x, int y, int width, int height) {
   GpuHostMsg_AcceleratedSurfacePostSubBuffer_Params params;
   params.surface_id = window_;
   params.x = x;
@@ -566,9 +588,7 @@ bool GLXImageTransportSurface::PostSubBuffer(
   params.height = height;
 
   helper_->SendAcceleratedSurfacePostSubBuffer(params);
-
   helper_->SetScheduled(false);
-  return true;
 }
 
 std::string GLXImageTransportSurface::GetExtensions() {
diff --git a/gpu/command_buffer/common/gl_mock.h b/gpu/command_buffer/common/gl_mock.h
index ae2259d..8ebb4c3 100644
--- a/gpu/command_buffer/common/gl_mock.h
+++ b/gpu/command_buffer/common/gl_mock.h
@@ -472,6 +472,14 @@ class MockGLInterface : public GLInterface {
 
   MOCK_METHOD3(GetFenceivNV, void(GLuint fence, GLenum pname, GLint *params));
 
+  MOCK_METHOD2(FenceSync, GLsync(GLenum condition, GLbitfield flags));
+
+  MOCK_METHOD1(DeleteSync, void(GLsync sync));
+
+  MOCK_METHOD5(GetSynciv, void(
+      GLsync sync, GLenum pname, GLsizei bufSize,
+      GLsizei* length, GLint* values));
+
   MOCK_METHOD1(SetSurfaceCHROMIUM, void(GLuint));
 
   MOCK_METHOD0(GetGraphicsResetStatusARB, GLenum());
diff --git a/gpu/command_buffer/service/gpu_scheduler.cc b/gpu/command_buffer/service/gpu_scheduler.cc
index e294dcf..cff93f2 100644
--- a/gpu/command_buffer/service/gpu_scheduler.cc
+++ b/gpu/command_buffer/service/gpu_scheduler.cc
@@ -11,6 +11,7 @@
 #include "base/message_loop.h"
 #include "base/time.h"
 #include "ui/gfx/gl/gl_bindings.h"
+#include "ui/gfx/gl/gl_fence.h"
 #include "ui/gfx/gl/gl_switches.h"
 
 using ::base::SharedMemory;
@@ -154,48 +155,28 @@ void GpuScheduler::SetCommandProcessedCallback(
 }
 
 void GpuScheduler::DeferToFence(base::Closure task) {
-  UnscheduleFence fence;
-
-  // What if either of these GL calls fails? TestFenceNV will return true and
-  // PutChanged will treat the fence as having been crossed and thereby not
-  // poll indefinately. See spec:
-  // http://www.opengl.org/registry/specs/NV/fence.txt
-  //
-  // What should happen if TestFenceNV is called for a name before SetFenceNV
-  // is called?
-  //     We generate an INVALID_OPERATION error, and return TRUE.
-  //     This follows the semantics for texture object names before
-  //     they are bound, in that they acquire their state upon binding.
-  //     We will arbitrarily return TRUE for consistency.
-  if (gfx::g_GL_NV_fence) {
-    glGenFencesNV(1, &fence.fence);
-    glSetFenceNV(fence.fence, GL_ALL_COMPLETED_NV);
-  }
-
-  glFlush();
-
-  fence.task = task;
-
-  unschedule_fences_.push(fence);
+  unschedule_fences_.push(make_linked_ptr(
+       new UnscheduleFence(gfx::GLFence::Create(), task)));
 }
 
 bool GpuScheduler::PollUnscheduleFences() {
-  if (gfx::g_GL_NV_fence) {
+  if (unschedule_fences_.empty())
+    return true;
+
+  if (unschedule_fences_.front()->fence.get()) {
     while (!unschedule_fences_.empty()) {
-      if (glTestFenceNV(unschedule_fences_.front().fence)) {
-        glDeleteFencesNV(1, &unschedule_fences_.front().fence);
-        unschedule_fences_.front().task.Run();
+      if (unschedule_fences_.front()->fence->HasCompleted()) {
+        unschedule_fences_.front()->task.Run();
         unschedule_fences_.pop();
       } else {
         return false;
       }
     }
   } else {
-    if (!unschedule_fences_.empty())
-      glFinish();
+    glFinish();
 
     while (!unschedule_fences_.empty()) {
-      unschedule_fences_.front().task.Run();
+      unschedule_fences_.front()->task.Run();
       unschedule_fences_.pop();
     }
   }
@@ -203,7 +184,8 @@ bool GpuScheduler::PollUnscheduleFences() {
   return true;
 }
 
-GpuScheduler::UnscheduleFence::UnscheduleFence() : fence(0) {
+GpuScheduler::UnscheduleFence::UnscheduleFence(
+    gfx::GLFence* fence_, base::Closure task_): fence(fence_), task(task_) {
 }
 
 GpuScheduler::UnscheduleFence::~UnscheduleFence() {
diff --git a/gpu/command_buffer/service/gpu_scheduler.h b/gpu/command_buffer/service/gpu_scheduler.h
index 3a33da4..46afc60 100644
--- a/gpu/command_buffer/service/gpu_scheduler.h
+++ b/gpu/command_buffer/service/gpu_scheduler.h
@@ -8,8 +8,8 @@
 #include <queue>
 
 #include "base/callback.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/linked_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/shared_memory.h"
 #include "gpu/command_buffer/common/command_buffer.h"
@@ -17,6 +17,10 @@
 #include "gpu/command_buffer/service/cmd_parser.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
+namespace gfx {
+class GLFence;
+}
+
 namespace gpu {
 
 // This class schedules commands that have been flushed. They are received via
@@ -95,13 +99,13 @@ class GpuScheduler
   // The GpuScheduler will unschedule itself in the event that further GL calls
   // are issued to it before all these fences have been crossed by the GPU.
   struct UnscheduleFence {
-    UnscheduleFence();
+    UnscheduleFence(gfx::GLFence* fence, base::Closure task);
     ~UnscheduleFence();
 
-    uint32 fence;
+    scoped_ptr<gfx::GLFence> fence;
     base::Closure task;
   };
-  std::queue<UnscheduleFence> unschedule_fences_;
+  std::queue<linked_ptr<UnscheduleFence> > unschedule_fences_;
 
   base::Closure scheduled_callback_;
   base::Closure command_processed_callback_;
diff --git a/ui/gfx/gl/generate_bindings.py b/ui/gfx/gl/generate_bindings.py
index 4742438..f255401 100755
--- a/ui/gfx/gl/generate_bindings.py
+++ b/ui/gfx/gl/generate_bindings.py
@@ -620,7 +620,18 @@ GL_FUNCTIONS = [
   'arguments': 'GLuint fence', },
 { 'return_type': 'void',
   'names': ['glGetFenceivNV'],
-  'arguments': 'GLuint fence, GLenum pname, GLint* params', }
+  'arguments': 'GLuint fence, GLenum pname, GLint* params', },
+{ 'return_type': 'GLsync',
+  'names': ['glFenceSync'],
+  'arguments': 'GLenum condition, GLbitfield flags', },
+{ 'return_type': 'void',
+  'names': ['glDeleteSync'],
+  'arguments': 'GLsync sync', },
+{ 'return_type': 'void',
+  'names': ['glGetSynciv'],
+  'arguments':
+    'GLsync sync, GLenum pname, GLsizei bufSize, GLsizei* length,'
+    'GLint* values', },
 ]
 
 OSMESA_FUNCTIONS = [
diff --git a/ui/gfx/gl/gl.gyp b/ui/gfx/gl/gl.gyp
index 7c1be85..9096dcd 100644
--- a/ui/gfx/gl/gl.gyp
+++ b/ui/gfx/gl/gl.gyp
@@ -50,6 +50,8 @@
         'gl_context_stub.h',
         'gl_context_win.cc',
         'gl_export.h',
+        'gl_fence.cc',
+        'gl_fence.h',
         'gl_implementation.cc',
         'gl_implementation.h',
         'gl_implementation_android.cc',
diff --git a/ui/gfx/gl/gl_fence.cc b/ui/gfx/gl/gl_fence.cc
new file mode 100644
index 0000000..ae09115
--- /dev/null
+++ b/ui/gfx/gl/gl_fence.cc
@@ -0,0 +1,108 @@
+// Copyright (c) 2011 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 "ui/gfx/gl/gl_fence.h"
+
+#include "base/compiler_specific.h"
+#include "ui/gfx/gl/gl_bindings.h"
+#include "ui/gfx/gl/gl_context.h"
+
+namespace {
+
+class GLFenceNVFence: public gfx::GLFence {
+ public:
+  GLFenceNVFence() {
+    // What if either of these GL calls fails? TestFenceNV will return true.
+    // See spec:
+    // http://www.opengl.org/registry/specs/NV/fence.txt
+    //
+    // What should happen if TestFenceNV is called for a name before SetFenceNV
+    // is called?
+    //     We generate an INVALID_OPERATION error, and return TRUE.
+    //     This follows the semantics for texture object names before
+    //     they are bound, in that they acquire their state upon binding.
+    //     We will arbitrarily return TRUE for consistency.
+    glGenFencesNV(1, &fence_);
+    glSetFenceNV(fence_, GL_ALL_COMPLETED_NV);
+    glFlush();
+  }
+
+  virtual bool HasCompleted() OVERRIDE {
+    return IsContextLost() || glTestFenceNV(fence_);
+  }
+
+ private:
+  ~GLFenceNVFence() {
+    glDeleteFencesNV(1, &fence_);
+  }
+
+  GLuint fence_;
+};
+
+class GLFenceARBSync: public gfx::GLFence {
+ public:
+  GLFenceARBSync() {
+    sync_ = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
+    glFlush();
+  }
+
+  virtual bool HasCompleted() OVERRIDE {
+    // Handle the case where FenceSync failed.
+    if (!sync_ || IsContextLost())
+      return true;
+
+    GLsizei length = 0;
+    GLsizei value = 0;
+    glGetSynciv(sync_,
+                GL_SYNC_STATUS,
+                1,  // bufSize
+                &length,
+                &value);
+    return length == 1 && value == GL_SIGNALED;
+  }
+
+ private:
+  ~GLFenceARBSync() {
+    glDeleteSync(sync_);
+  }
+
+  GLsync sync_;
+};
+
+}  // namespace
+
+namespace gfx {
+
+GLFence::GLFence() {
+}
+
+GLFence::~GLFence() {
+}
+
+// static
+GLFence* GLFence::Create() {
+  if (gfx::g_GL_NV_fence) {
+    return new GLFenceNVFence();
+  } else if (gfx::g_GL_ARB_sync) {
+    return new GLFenceARBSync();
+  } else {
+    return NULL;
+  }
+}
+
+// static
+bool GLFence::IsContextLost() {
+  if (!gfx::g_GL_ARB_robustness)
+    return false;
+
+  if (!gfx::GLContext::GetCurrent() ||
+      !gfx::GLContext::GetCurrent()->
+          WasAllocatedUsingARBRobustness())
+    return false;
+
+  GLenum status = glGetGraphicsResetStatusARB();
+  return status != GL_NO_ERROR;
+}
+
+}  // namespace gfx
diff --git a/ui/gfx/gl/gl_fence.h b/ui/gfx/gl/gl_fence.h
new file mode 100644
index 0000000..c901707
--- /dev/null
+++ b/ui/gfx/gl/gl_fence.h
@@ -0,0 +1,31 @@
+// Copyright (c) 2011 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.
+
+#ifndef UI_GFX_GL_GL_FENCE_H_
+#define UI_GFX_GL_GL_FENCE_H_
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "ui/gfx/gl/gl_export.h"
+
+namespace gfx {
+
+class GL_EXPORT GLFence {
+ public:
+  GLFence();
+  virtual ~GLFence();
+
+  static GLFence* Create();
+  virtual bool HasCompleted() = 0;
+
+ protected:
+  static bool IsContextLost();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(GLFence);
+};
+
+}  // namespace gfx
+
+#endif  // UI_GFX_GL_GL_FENCE_H_
diff --git a/ui/gfx/gl/gl_interface.h b/ui/gfx/gl/gl_interface.h
index fba90f9..a6e5aca 100644
--- a/ui/gfx/gl/gl_interface.h
+++ b/ui/gfx/gl/gl_interface.h
@@ -590,6 +590,16 @@ class GL_EXPORT GLInterface {
 
   virtual GLenum GetGraphicsResetStatusARB() = 0;
 
+  virtual GLsync FenceSync(GLenum condition, GLbitfield flags) = 0;
+
+  virtual void DeleteSync(GLsync sync) = 0;
+
+  virtual void GetSynciv(GLsync sync,
+                         GLenum pname,
+                         GLsizei bufSize,
+                         GLsizei* length,
+                         GLint* values) = 0;
+
  private:
   static GLInterface* interface_;
 };