Bug 1483533 - Switch to glClientWaitSync for texture syncing r=jrmuizel a=pascalc
authorDoug Thayer <dothayer@mozilla.com>
Tue, 25 Sep 2018 13:41:25 +0000
changeset 490086 fb0e9934b6cea695d10551d100ffdb2aac527cd0
parent 490085 7b90b8e04295c01a650a7cccbb055eac0c66cf06
child 490087 41f214d66947e7a50012bd558e25a78e8d3aeb11
push id9901
push userebalazs@mozilla.com
push dateThu, 27 Sep 2018 11:50:27 +0000
treeherdermozilla-beta@b404112c2f6c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, pascalc
bugs1483533
milestone63.0
Bug 1483533 - Switch to glClientWaitSync for texture syncing r=jrmuizel a=pascalc glFenceSync/glClientWaitSync just seem to be more well supported on nvidia hardware, and they work fine as well on AMD/intel, so I'm transitioning to that. Depends on D6463 Differential Revision: https://phabricator.services.mozilla.com/D6464
gfx/layers/opengl/CompositorOGL.cpp
gfx/layers/opengl/TextureHostOGL.cpp
gfx/layers/opengl/TextureHostOGL.h
--- a/gfx/layers/opengl/CompositorOGL.cpp
+++ b/gfx/layers/opengl/CompositorOGL.cpp
@@ -1485,16 +1485,17 @@ CompositorOGL::DrawGeometry(const Geomet
         BindMaskForProgram(program, sourceMask, LOCAL_GL_TEXTURE1, maskQuadTransform);
       }
       if (mixBlendBackdrop) {
         BindBackdrop(program, mixBlendBackdrop, LOCAL_GL_TEXTURE2);
       }
 
       BindAndDrawGeometryWithTextureRect(program, aGeometry,
                                          texturedEffect->mTextureCoords, source);
+      source->AsSourceOGL()->MaybeFenceTexture();
     }
     break;
   case EffectTypes::YCBCR: {
       EffectYCbCr* effectYCbCr =
         static_cast<EffectYCbCr*>(aEffectChain.mPrimaryEffect.get());
       TextureSource* sourceYCbCr = effectYCbCr->mTexture;
       const int Y = 0, Cb = 1, Cr = 2;
       TextureSourceOGL* sourceY =  sourceYCbCr->GetSubSource(Y)->AsSourceOGL();
@@ -1525,16 +1526,19 @@ CompositorOGL::DrawGeometry(const Geomet
       if (mixBlendBackdrop) {
         BindBackdrop(program, mixBlendBackdrop, LOCAL_GL_TEXTURE4);
       }
       didSetBlendMode = SetBlendMode(gl(), blendMode);
       BindAndDrawGeometryWithTextureRect(program,
                                          aGeometry,
                                          effectYCbCr->mTextureCoords,
                                          sourceYCbCr->GetSubSource(Y));
+      sourceY->MaybeFenceTexture();
+      sourceCb->MaybeFenceTexture();
+      sourceCr->MaybeFenceTexture();
     }
     break;
   case EffectTypes::NV12: {
       EffectNV12* effectNV12 =
         static_cast<EffectNV12*>(aEffectChain.mPrimaryEffect.get());
       TextureSource* sourceNV12 = effectNV12->mTexture;
       const int Y = 0, CbCr = 1;
       TextureSourceOGL* sourceY =  sourceNV12->GetSubSource(Y)->AsSourceOGL();
@@ -1562,16 +1566,18 @@ CompositorOGL::DrawGeometry(const Geomet
       if (mixBlendBackdrop) {
         BindBackdrop(program, mixBlendBackdrop, LOCAL_GL_TEXTURE3);
       }
       didSetBlendMode = SetBlendMode(gl(), blendMode);
       BindAndDrawGeometryWithTextureRect(program,
                                          aGeometry,
                                          effectNV12->mTextureCoords,
                                          sourceNV12->GetSubSource(Y));
+      sourceY->MaybeFenceTexture();
+      sourceCbCr->MaybeFenceTexture();
     }
     break;
   case EffectTypes::RENDER_TARGET: {
       EffectRenderTarget* effectRenderTarget =
         static_cast<EffectRenderTarget*>(aEffectChain.mPrimaryEffect.get());
       RefPtr<CompositingRenderTargetOGL> surface
         = static_cast<CompositingRenderTargetOGL*>(effectRenderTarget->mRenderTarget.get());
 
@@ -1644,16 +1650,19 @@ CompositorOGL::DrawGeometry(const Geomet
       program->SetTexturePass2(true);
       BindAndDrawGeometryWithTextureRect(program,
                                          aGeometry,
                                          effectComponentAlpha->mTextureCoords,
                                          effectComponentAlpha->mOnBlack);
 
       mGLContext->fBlendFuncSeparate(LOCAL_GL_ONE, LOCAL_GL_ONE_MINUS_SRC_ALPHA,
                                      LOCAL_GL_ONE, LOCAL_GL_ONE_MINUS_SRC_ALPHA);
+
+      sourceOnBlack->MaybeFenceTexture();
+      sourceOnWhite->MaybeFenceTexture();
     }
     break;
   default:
     MOZ_ASSERT(false, "Unhandled effect type");
     break;
   }
 
   if (didSetBlendMode) {
--- a/gfx/layers/opengl/TextureHostOGL.cpp
+++ b/gfx/layers/opengl/TextureHostOGL.cpp
@@ -314,53 +314,81 @@ GLTextureSource::IsValid() const
 
 DirectMapTextureSource::DirectMapTextureSource(TextureSourceProvider* aProvider,
                                                gfx::DataSourceSurface* aSurface)
   : GLTextureSource(aProvider,
                     0,
                     LOCAL_GL_TEXTURE_RECTANGLE_ARB,
                     aSurface->GetSize(),
                     aSurface->GetFormat())
+  , mSync(0)
 {
   MOZ_ASSERT(aSurface);
 
   UpdateInternal(aSurface, nullptr, nullptr, true);
 }
 
+DirectMapTextureSource::~DirectMapTextureSource()
+{
+  if (!mSync || !gl() || !gl()->MakeCurrent() || gl()->IsDestroyed()) {
+    return;
+  }
+
+  gl()->fDeleteSync(mSync);
+  mSync = 0;
+}
+
 bool
 DirectMapTextureSource::Update(gfx::DataSourceSurface* aSurface,
                                nsIntRegion* aDestRegion,
                                gfx::IntPoint* aSrcOffset)
 {
   if (!aSurface) {
     return false;
   }
 
   return UpdateInternal(aSurface, aDestRegion, aSrcOffset, false);
 }
 
+void
+DirectMapTextureSource::MaybeFenceTexture()
+{
+  if (!gl() ||
+      !gl()->MakeCurrent() ||
+      gl()->IsDestroyed()) {
+    return;
+  }
+
+  if (mSync) {
+    gl()->fDeleteSync(mSync);
+  }
+  mSync = gl()->fFenceSync(LOCAL_GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
+}
+
+
 bool
 DirectMapTextureSource::Sync(bool aBlocking)
 {
-  if (!gl() || !gl()->MakeCurrent()) {
+  if (!gl() || !gl()->MakeCurrent() || gl()->IsDestroyed()) {
     // We use this function to decide whether we can unlock the texture
     // and clean it up. If we return false here and for whatever reason
     // the context is absent or invalid, the compositor will keep a
     // reference to this texture forever.
     return true;
   }
 
-  if (!gl()->IsDestroyed()) {
-    if (aBlocking) {
-      gl()->fFinishObjectAPPLE(LOCAL_GL_TEXTURE, mTextureHandle);
-    } else {
-      return gl()->fTestObjectAPPLE(LOCAL_GL_TEXTURE, mTextureHandle);
-    }
+  if (!mSync) {
+    return false;
   }
-  return true;
+
+  GLenum waitResult = gl()->fClientWaitSync(mSync,
+                                            LOCAL_GL_SYNC_FLUSH_COMMANDS_BIT,
+                                            aBlocking ? LOCAL_GL_TIMEOUT_IGNORED : 0);
+  return waitResult == LOCAL_GL_ALREADY_SIGNALED ||
+         waitResult == LOCAL_GL_CONDITION_SATISFIED; 
 }
 
 bool
 DirectMapTextureSource::UpdateInternal(gfx::DataSourceSurface* aSurface,
                                        nsIntRegion* aDestRegion,
                                        gfx::IntPoint* aSrcOffset,
                                        bool aInit)
 {
@@ -404,16 +432,21 @@ DirectMapTextureSource::UpdateInternal(g
                                        mTextureHandle,
                                        aSurface->GetSize(),
                                        nullptr,
                                        aInit,
                                        srcPoint,
                                        LOCAL_GL_TEXTURE0,
                                        LOCAL_GL_TEXTURE_RECTANGLE_ARB);
 
+  if (mSync) {
+    gl()->fDeleteSync(mSync);
+    mSync = 0;
+  }
+
   gl()->fPixelStorei(LOCAL_GL_UNPACK_CLIENT_STORAGE_APPLE, LOCAL_GL_FALSE);
   return true;
 }
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 // SurfaceTextureHost
 
--- a/gfx/layers/opengl/TextureHostOGL.h
+++ b/gfx/layers/opengl/TextureHostOGL.h
@@ -87,16 +87,24 @@ public:
     , mHasCachedSamplingFilter(false)
   {}
 
   virtual bool IsValid() const = 0;
 
   virtual void BindTexture(GLenum aTextureUnit,
                            gfx::SamplingFilter aSamplingFilter) = 0;
 
+  // To be overridden in textures that need this. This method will be called
+  // when the compositor has used the texture to draw. This allows us to set
+  // a fence with glFenceSync which we can wait on later to ensure the GPU
+  // is done with the draw calls using that texture. We would like to be able
+  // to simply use glFinishObjectAPPLE, but this returns earlier than
+  // expected with nvidia drivers.
+  virtual void MaybeFenceTexture() {}
+
   virtual gfx::IntSize GetSize() const = 0;
 
   virtual GLenum GetTextureTarget() const { return LOCAL_GL_TEXTURE_2D; }
 
   virtual gfx::SurfaceFormat GetFormat() const = 0;
 
   virtual GLenum GetWrapMode() const = 0;
 
@@ -293,33 +301,38 @@ protected:
 // should be alive until the ~ClientStorageTextureSource(). And if we try to
 // update the surface we mapped before, we need to call Sync() to make sure
 // the surface is not used by compositor.
 class DirectMapTextureSource : public GLTextureSource
 {
 public:
   DirectMapTextureSource(TextureSourceProvider* aProvider,
                          gfx::DataSourceSurface* aSurface);
+  ~DirectMapTextureSource();
 
   virtual bool Update(gfx::DataSourceSurface* aSurface,
                       nsIntRegion* aDestRegion = nullptr,
                       gfx::IntPoint* aSrcOffset = nullptr) override;
 
   virtual bool IsDirectMap() override { return true; }
 
   // If aBlocking is false, check if this texture is no longer being used
   // by the GPU - if aBlocking is true, this will block until the GPU is
   // done with it.
   virtual bool Sync(bool aBlocking) override;
 
+  virtual void MaybeFenceTexture() override;
+
 private:
   bool UpdateInternal(gfx::DataSourceSurface* aSurface,
                       nsIntRegion* aDestRegion,
                       gfx::IntPoint* aSrcOffset,
                       bool aInit);
+
+  GLsync mSync;
 };
 
 class GLTextureHost : public TextureHost
 {
 public:
   GLTextureHost(TextureFlags aFlags,
                 GLuint aTextureHandle,
                 GLenum aTarget,