gpu: Allow fences to check whether a flush has occurred

This skips waiting on a fence that was created but never committed.

BUG=352419

Review URL: https://codereview.chromium.org/197563003

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

9 files changed, 149 insertions, 27 deletions

diff --git a/gpu/command_buffer/service/mailbox_manager_unittest.cc b/gpu/command_buffer/service/mailbox_manager_unittest.cc
index ae871f9..13005e6 100644
--- a/gpu/command_buffer/service/mailbox_manager_unittest.cc
+++ b/gpu/command_buffer/service/mailbox_manager_unittest.cc
@@ -8,7 +8,9 @@
 #include "gpu/command_buffer/service/mailbox_synchronizer.h"
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gl/gl_context_stub.h"
 #include "ui/gl/gl_mock.h"
+#include "ui/gl/gl_surface_stub.h"
 
 namespace gpu {
 namespace gles2 {
@@ -188,6 +190,9 @@ class MailboxManagerSyncTest : public MailboxManagerTest {
     manager2_ = new MailboxManager;
     gl_.reset(new ::testing::StrictMock< ::gfx::MockGLInterface>());
     ::gfx::MockGLInterface::SetGLInterface(gl_.get());
+    context_ = new gfx::GLContextStub();
+    surface_ = new gfx::GLSurfaceStub();
+    context_->MakeCurrent(surface_);
   }
 
   Texture* DefineTexture() {
@@ -249,12 +254,15 @@ class MailboxManagerSyncTest : public MailboxManagerTest {
   virtual void TearDown() {
     MailboxManagerTest::TearDown();
     MailboxSynchronizer::Terminate();
+    context_->ReleaseCurrent(NULL);
     ::gfx::MockGLInterface::SetGLInterface(NULL);
     gl_.reset();
   }
 
   scoped_ptr< ::testing::StrictMock< ::gfx::MockGLInterface> > gl_;
   scoped_refptr<MailboxManager> manager2_;
+  scoped_refptr<gfx::GLContext> context_;
+  scoped_refptr<gfx::GLSurface> surface_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MailboxManagerSyncTest);
@@ -283,7 +291,6 @@ TEST_F(MailboxManagerSyncTest, ProduceSyncDestroy) {
   EXPECT_EQ(texture, manager_->ConsumeTexture(GL_TEXTURE_2D, name));
 
   // Synchronize
-  EXPECT_CALL(*gl_, Flush()).Times(1);
   manager_->PushTextureUpdates();
   manager2_->PullTextureUpdates();
 
@@ -305,7 +312,6 @@ TEST_F(MailboxManagerSyncTest, ProduceConsumeResize) {
   EXPECT_EQ(texture, manager_->ConsumeTexture(GL_TEXTURE_2D, name));
 
   // Synchronize
-  EXPECT_CALL(*gl_, Flush()).Times(1);
   manager_->PushTextureUpdates();
   manager2_->PullTextureUpdates();
 
@@ -334,7 +340,6 @@ TEST_F(MailboxManagerSyncTest, ProduceConsumeResize) {
   EXPECT_TRUE(texture->GetLevelImage(GL_TEXTURE_2D, 0) == NULL);
 
   // Synchronize again
-  EXPECT_CALL(*gl_, Flush()).Times(1);
   manager_->PushTextureUpdates();
   SetupUpdateTexParamExpectations(
       kNewTextureId, GL_LINEAR, GL_LINEAR, GL_REPEAT, GL_REPEAT);
@@ -396,7 +401,6 @@ TEST_F(MailboxManagerSyncTest, ProduceConsumeBidirectional) {
   manager2_->ProduceTexture(GL_TEXTURE_2D, name2, texture2);
 
   // Make visible.
-  EXPECT_CALL(*gl_, Flush()).Times(2);
   manager_->PushTextureUpdates();
   manager2_->PushTextureUpdates();
 
@@ -435,7 +439,6 @@ TEST_F(MailboxManagerSyncTest, ProduceConsumeBidirectional) {
   Mock::VerifyAndClearExpectations(gl_.get());
 
   // Synchronize in both directions
-  EXPECT_CALL(*gl_, Flush()).Times(2);
   manager_->PushTextureUpdates();
   manager2_->PushTextureUpdates();
   // manager1 should see the change to texture2 mag_filter being applied.
diff --git a/gpu/command_buffer/service/mailbox_synchronizer.cc b/gpu/command_buffer/service/mailbox_synchronizer.cc
index 0503fb1..d25368a 100644
--- a/gpu/command_buffer/service/mailbox_synchronizer.cc
+++ b/gpu/command_buffer/service/mailbox_synchronizer.cc
@@ -174,8 +174,6 @@ void MailboxSynchronizer::PushTextureUpdates(MailboxManager* manager) {
       textures_.insert(std::make_pair(texture, TextureVersion(group)));
     }
   }
-  // Make sure all write fences are flushed.
-  glFlush();
 }
 
 void MailboxSynchronizer::UpdateTextureLocked(Texture* texture,
diff --git a/gpu/command_buffer/service/texture_definition.cc b/gpu/command_buffer/service/texture_definition.cc
index 462131f..9b31dac 100644
--- a/gpu/command_buffer/service/texture_definition.cc
+++ b/gpu/command_buffer/service/texture_definition.cc
@@ -278,12 +278,8 @@ void NativeImageBuffer::DidRead(gfx::GLImage* client) {
 
 void NativeImageBuffer::DidWrite(gfx::GLImage* client) {
   base::AutoLock lock(lock_);
-  // TODO(sievers): crbug.com/352419
-  // This is super-risky. We need to somehow find out about when the current
-  // context gets flushed, so that we will only ever wait on the write fence
-  // (esp. from another context) if it was flushed and is guaranteed to clear.
-  // On the other hand, proactively flushing here is not feasible in terms
-  // of perf when there are multiple draw calls per frame.
+  // Sharing semantics require the client to flush in order to make changes
+  // visible to other clients.
   write_fence_.reset(gfx::GLFence::CreateWithoutFlush());
   write_client_ = client;
   for (std::list<ClientInfo>::iterator it = client_infos_.begin();
diff --git a/ui/gl/gl_context.cc b/ui/gl/gl_context.cc
index cd9a28c..d2deb61 100644
--- a/ui/gl/gl_context.cc
+++ b/ui/gl/gl_context.cc
@@ -26,6 +26,20 @@ base::LazyInstance<base::ThreadLocalPointer<GLContext> >::Leaky
     current_real_context_ = LAZY_INSTANCE_INITIALIZER;
 }  // namespace
 
+GLContext::FlushEvent::FlushEvent() {
+}
+
+GLContext::FlushEvent::~FlushEvent() {
+}
+
+void GLContext::FlushEvent::Signal() {
+  flag_.Set();
+}
+
+bool GLContext::FlushEvent::IsSignaled() {
+  return flag_.IsSet();
+}
+
 GLContext::GLContext(GLShareGroup* share_group) : share_group_(share_group) {
   if (!share_group_.get())
     share_group_ = new GLShareGroup;
@@ -40,6 +54,13 @@ GLContext::~GLContext() {
   }
 }
 
+scoped_refptr<GLContext::FlushEvent> GLContext::SignalFlush() {
+  DCHECK(IsCurrent(NULL));
+  scoped_refptr<FlushEvent> flush_event = new FlushEvent();
+  flush_events_.push_back(flush_event);
+  return flush_event;
+}
+
 bool GLContext::GetTotalGpuMemory(size_t* bytes) {
   DCHECK(bytes);
   *bytes = 0;
@@ -172,6 +193,12 @@ void GLContext::SetRealGLApi() {
   SetGLToRealGLApi();
 }
 
+void GLContext::OnFlush() {
+  for (size_t n = 0; n < flush_events_.size(); n++)
+    flush_events_[n]->Signal();
+  flush_events_.clear();
+}
+
 GLContextReal::GLContextReal(GLShareGroup* share_group)
     : GLContext(share_group) {}
 
diff --git a/ui/gl/gl_context.h b/ui/gl/gl_context.h
index f40b1bb..b4a7a16 100644
--- a/ui/gl/gl_context.h
+++ b/ui/gl/gl_context.h
@@ -6,10 +6,12 @@
 #define UI_GL_GL_CONTEXT_H_
 
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/synchronization/cancellation_flag.h"
 #include "ui/gl/gl_share_group.h"
 #include "ui/gl/gl_state_restorer.h"
 #include "ui/gl/gpu_preference.h"
@@ -32,6 +34,25 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
   virtual bool Initialize(
       GLSurface* compatible_surface, GpuPreference gpu_preference) = 0;
 
+  class FlushEvent : public base::RefCountedThreadSafe<FlushEvent> {
+    public:
+      bool IsSignaled();
+
+    private:
+      friend class base::RefCountedThreadSafe<FlushEvent>;
+      friend class GLContext;
+      FlushEvent();
+      virtual ~FlushEvent();
+      void Signal();
+
+      base::CancellationFlag flag_;
+  };
+
+  // Needs to be called with this context current. It will return a FlushEvent
+  // that is initially unsignaled, but will transition to signaled after the
+  // next glFlush() or glFinish() occurs in this context.
+  scoped_refptr<FlushEvent> SignalFlush();
+
   // Destroys the GL context.
   virtual void Destroy() = 0;
 
@@ -114,6 +135,9 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
   // Returns the GL renderer string. The context must be current.
   virtual std::string GetGLRenderer();
 
+  // Called when glFlush()/glFinish() is called with this context current.
+  void OnFlush();
+
  protected:
   virtual ~GLContext();
 
@@ -140,6 +164,8 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
   scoped_ptr<GLStateRestorer> state_restorer_;
   scoped_ptr<GLVersionInfo> version_info_;
 
+  std::vector<scoped_refptr<FlushEvent> > flush_events_;
+
   DISALLOW_COPY_AND_ASSIGN(GLContext);
 };
 
diff --git a/ui/gl/gl_fence.cc b/ui/gl/gl_fence.cc
index f44570e..262de48 100644
--- a/ui/gl/gl_fence.cc
+++ b/ui/gl/gl_fence.cc
@@ -25,8 +25,11 @@ class GLFenceNVFence: public gfx::GLFence {
     //     We will arbitrarily return TRUE for consistency.
     glGenFencesNV(1, &fence_);
     glSetFenceNV(fence_, GL_ALL_COMPLETED_NV);
-    if (flush)
+    if (flush) {
       glFlush();
+    } else {
+      flush_event_ = gfx::GLContext::GetCurrent()->SignalFlush();
+    }
   }
 
   virtual bool HasCompleted() OVERRIDE {
@@ -34,11 +37,15 @@ class GLFenceNVFence: public gfx::GLFence {
   }
 
   virtual void ClientWait() OVERRIDE {
-    glFinishFenceNV(fence_);
+    if (!flush_event_ || flush_event_->IsSignaled()) {
+      glFinishFenceNV(fence_);
+    } else {
+      LOG(ERROR) << "Trying to wait for uncommitted fence. Skipping...";
+    }
   }
 
   virtual void ServerWait() OVERRIDE {
-    glFinishFenceNV(fence_);
+    ClientWait();
   }
 
  private:
@@ -47,14 +54,18 @@ class GLFenceNVFence: public gfx::GLFence {
   }
 
   GLuint fence_;
+  scoped_refptr<gfx::GLContext::FlushEvent> flush_event_;
 };
 
 class GLFenceARBSync: public gfx::GLFence {
  public:
   GLFenceARBSync(bool flush) {
     sync_ = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
-    if (flush)
+    if (flush) {
       glFlush();
+    } else {
+      flush_event_ = gfx::GLContext::GetCurrent()->SignalFlush();
+    }
   }
 
   virtual bool HasCompleted() OVERRIDE {
@@ -69,11 +80,19 @@ class GLFenceARBSync: public gfx::GLFence {
   }
 
   virtual void ClientWait() OVERRIDE {
-    glClientWaitSync(sync_, GL_SYNC_FLUSH_COMMANDS_BIT, GL_TIMEOUT_IGNORED);
+    if (!flush_event_ || flush_event_->IsSignaled()) {
+      glClientWaitSync(sync_, GL_SYNC_FLUSH_COMMANDS_BIT, GL_TIMEOUT_IGNORED);
+    } else {
+      LOG(ERROR) << "Trying to wait for uncommitted fence. Skipping...";
+    }
   }
 
   virtual void ServerWait() OVERRIDE {
-    glWaitSync(sync_, 0, GL_TIMEOUT_IGNORED);
+    if (!flush_event_ || flush_event_->IsSignaled()) {
+      glWaitSync(sync_, 0, GL_TIMEOUT_IGNORED);
+    } else {
+      LOG(ERROR) << "Trying to wait for uncommitted fence. Skipping...";
+    }
   }
 
  private:
@@ -82,6 +101,7 @@ class GLFenceARBSync: public gfx::GLFence {
   }
 
   GLsync sync_;
+  scoped_refptr<gfx::GLContext::FlushEvent> flush_event_;
 };
 
 #if !defined(OS_MACOSX)
@@ -90,8 +110,11 @@ class EGLFenceSync : public gfx::GLFence {
   EGLFenceSync(bool flush) {
     display_ = eglGetCurrentDisplay();
     sync_ = eglCreateSyncKHR(display_, EGL_SYNC_FENCE_KHR, NULL);
-    if (flush)
+    if (flush) {
       glFlush();
+    } else {
+      flush_event_ = gfx::GLContext::GetCurrent()->SignalFlush();
+    }
   }
 
   virtual bool HasCompleted() OVERRIDE {
@@ -102,14 +125,22 @@ class EGLFenceSync : public gfx::GLFence {
   }
 
   virtual void ClientWait() OVERRIDE {
-    EGLint flags = 0;
-    EGLTimeKHR time = EGL_FOREVER_KHR;
-    eglClientWaitSyncKHR(display_, sync_, flags, time);
+    if (!flush_event_ || flush_event_->IsSignaled()) {
+      EGLint flags = 0;
+      EGLTimeKHR time = EGL_FOREVER_KHR;
+      eglClientWaitSyncKHR(display_, sync_, flags, time);
+    } else {
+      LOG(ERROR) << "Trying to wait for uncommitted fence. Skipping...";
+    }
   }
 
   virtual void ServerWait() OVERRIDE {
-    EGLint flags = 0;
-    eglWaitSyncKHR(display_, sync_, flags);
+    if (!flush_event_ || flush_event_->IsSignaled()) {
+      EGLint flags = 0;
+      eglWaitSyncKHR(display_, sync_, flags);
+    } else {
+      LOG(ERROR) << "Trying to wait for uncommitted fence. Skipping...";
+    }
   }
 
 
@@ -120,11 +151,15 @@ class EGLFenceSync : public gfx::GLFence {
 
   EGLSyncKHR sync_;
   EGLDisplay display_;
+  scoped_refptr<gfx::GLContext::FlushEvent> flush_event_;
 };
 #endif // !OS_MACOSX
 
 // static
 gfx::GLFence* CreateFence(bool flush) {
+  DCHECK(gfx::GLContext::GetCurrent())
+      << "Trying to create fence with no context";
+
 #if !defined(OS_MACOSX)
   if (gfx::g_driver_egl.ext.b_EGL_KHR_fence_sync)
     return new EGLFenceSync(flush);
diff --git a/ui/gl/gl_fence.h b/ui/gl/gl_fence.h
index c1967ec..021f345 100644
--- a/ui/gl/gl_fence.h
+++ b/ui/gl/gl_fence.h
@@ -16,11 +16,16 @@ class GL_EXPORT GLFence {
   virtual ~GLFence();
 
   static GLFence* Create();
+
   // Creates a fence that is not guaranteed to signal until the current context
-  // is flushed. Use with caution.
+  // is flushed. It is illegal to call Client/ServerWait() on a fence without
+  // having explicitly called glFlush() or glFinish() in the originating
+  // context.
   static GLFence* CreateWithoutFlush();
+
   virtual bool HasCompleted() = 0;
   virtual void ClientWait() = 0;
+
   // Will block the server if supported, but might fall back to blocking the
   // client.
   virtual void ServerWait() = 0;
diff --git a/ui/gl/gl_gl_api_implementation.cc b/ui/gl/gl_gl_api_implementation.cc
index f925f64..0965d91 100644
--- a/ui/gl/gl_gl_api_implementation.cc
+++ b/ui/gl/gl_gl_api_implementation.cc
@@ -343,6 +343,11 @@ void GLApiBase::InitializeBase(DriverGL* driver) {
   driver_ = driver;
 }
 
+void GLApiBase::SignalFlush() {
+  DCHECK(GLContext::GetCurrent());
+  GLContext::GetCurrent()->OnFlush();
+}
+
 RealGLApi::RealGLApi() {
 }
 
@@ -353,6 +358,16 @@ void RealGLApi::Initialize(DriverGL* driver) {
   InitializeBase(driver);
 }
 
+void RealGLApi::glFlushFn() {
+  GLApiBase::glFlushFn();
+  GLApiBase::SignalFlush();
+}
+
+void RealGLApi::glFinishFn() {
+  GLApiBase::glFinishFn();
+  GLApiBase::SignalFlush();
+}
+
 TraceGLApi::~TraceGLApi() {
 }
 
@@ -445,4 +460,14 @@ const GLubyte* VirtualGLApi::glGetStringFn(GLenum name) {
   }
 }
 
+void VirtualGLApi::glFlushFn() {
+  GLApiBase::glFlushFn();
+  GLApiBase::SignalFlush();
+}
+
+void VirtualGLApi::glFinishFn() {
+  GLApiBase::glFinishFn();
+  GLApiBase::SignalFlush();
+}
+
 }  // namespace gfx
diff --git a/ui/gl/gl_gl_api_implementation.h b/ui/gl/gl_gl_api_implementation.h
index c87dbd1..e764ec3 100644
--- a/ui/gl/gl_gl_api_implementation.h
+++ b/ui/gl/gl_gl_api_implementation.h
@@ -41,6 +41,7 @@ class GL_EXPORT GLApiBase : public GLApi {
   GLApiBase();
   virtual ~GLApiBase();
   void InitializeBase(DriverGL* driver);
+  void SignalFlush();
 
   DriverGL* driver_;
 };
@@ -51,6 +52,10 @@ class GL_EXPORT RealGLApi : public GLApiBase {
   RealGLApi();
   virtual ~RealGLApi();
   void Initialize(DriverGL* driver);
+
+ private:
+  virtual void glFinishFn() OVERRIDE;
+  virtual void glFlushFn() OVERRIDE;
 };
 
 // Inserts a TRACE for every GL call.
@@ -82,10 +87,12 @@ class GL_EXPORT VirtualGLApi : public GLApiBase {
 
   void OnReleaseVirtuallyCurrent(GLContext* virtual_context);
 
+private:
   // Overridden functions from GLApiBase
   virtual const GLubyte* glGetStringFn(GLenum name) OVERRIDE;
+  virtual void glFinishFn() OVERRIDE;
+  virtual void glFlushFn() OVERRIDE;
 
- private:
   // The real context we're running on.
   GLContext* real_context_;