From e893bc882afe24f4a168e54ef984bb32d140c8a1 Mon Sep 17 00:00:00 2001 From: "sievers@chromium.org" Date: Wed, 19 Mar 2014 22:09:44 +0000 Subject: [PATCH] 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 --- .../service/mailbox_manager_unittest.cc | 13 ++-- .../service/mailbox_synchronizer.cc | 2 - .../service/texture_definition.cc | 8 +-- ui/gl/gl_context.cc | 27 +++++++++ ui/gl/gl_context.h | 26 ++++++++ ui/gl/gl_fence.cc | 59 +++++++++++++++---- ui/gl/gl_fence.h | 7 ++- ui/gl/gl_gl_api_implementation.cc | 25 ++++++++ ui/gl/gl_gl_api_implementation.h | 9 ++- 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 ae871f97a3b586..13005e6c35e2da 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 manager2_; + scoped_refptr context_; + scoped_refptr 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 0503fb12bdaba7..d25368ab09642c 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 462131fdc12e4d..9b31dac460dfc6 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::iterator it = client_infos_.begin(); diff --git a/ui/gl/gl_context.cc b/ui/gl/gl_context.cc index cd9a28c387dcfa..d2deb6141e4b6a 100644 --- a/ui/gl/gl_context.cc +++ b/ui/gl/gl_context.cc @@ -26,6 +26,20 @@ base::LazyInstance >::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::SignalFlush() { + DCHECK(IsCurrent(NULL)); + scoped_refptr 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 f40b1bba78647a..b4a7a169c1525e 100644 --- a/ui/gl/gl_context.h +++ b/ui/gl/gl_context.h @@ -6,10 +6,12 @@ #define UI_GL_GL_CONTEXT_H_ #include +#include #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 { virtual bool Initialize( GLSurface* compatible_surface, GpuPreference gpu_preference) = 0; + class FlushEvent : public base::RefCountedThreadSafe { + public: + bool IsSignaled(); + + private: + friend class base::RefCountedThreadSafe; + 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 SignalFlush(); + // Destroys the GL context. virtual void Destroy() = 0; @@ -114,6 +135,9 @@ class GL_EXPORT GLContext : public base::RefCounted { // 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 { scoped_ptr state_restorer_; scoped_ptr version_info_; + std::vector > flush_events_; + DISALLOW_COPY_AND_ASSIGN(GLContext); }; diff --git a/ui/gl/gl_fence.cc b/ui/gl/gl_fence.cc index f44570e2b60429..262de48af63870 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 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 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 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 c1967ec9c43df4..021f3456f132c8 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 f925f64ffa88af..0965d91b45dab5 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 c87dbd1de088e0..e764ec3ce3d4af 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_;