Bug 1594303 - Code clean up around RenderAndroidSurfaceTextureHostOGL r=jnicol
authorsotaro <sotaro.ikeda.g@gmail.com>
Fri, 08 Nov 2019 12:52:16 +0000
changeset 501297 fc30883fa2df56453b72da1d59112c5af3f01854
parent 501296 dcf27b7319be0e146ada074ac07602767d5cea04
child 501298 682483b64a30714b3adf47feb2586bdacdc1800b
push id114168
push userdluca@mozilla.com
push dateSun, 10 Nov 2019 03:08:55 +0000
treeherdermozilla-inbound@33f64c1ef3e4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjnicol
bugs1594303
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1594303 - Code clean up around RenderAndroidSurfaceTextureHostOGL r=jnicol Removes NofityForUse() functions for simplicity. Ensure that RenderTextureHost::PrepareForUse() is called before RenderTextureHost:: Lock(). When a task of calling RenderTextureHost::PrepareForUse() is simply posted to render thread, there is a case that RenderTextureHost:: Lock() is called before PrepareForUse() . Differential Revision: https://phabricator.services.mozilla.com/D51974
gfx/layers/wr/AsyncImagePipelineManager.cpp
gfx/layers/wr/WebRenderTextureHost.cpp
gfx/layers/wr/WebRenderTextureHost.h
gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp
gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.h
gfx/webrender_bindings/RenderTextureHost.h
gfx/webrender_bindings/RenderThread.cpp
gfx/webrender_bindings/RenderThread.h
--- a/gfx/layers/wr/AsyncImagePipelineManager.cpp
+++ b/gfx/layers/wr/AsyncImagePipelineManager.cpp
@@ -259,18 +259,16 @@ Maybe<TextureHost::ResourceUpdateOp> Asy
   aKeys = aPipeline->mKeys;
 
   auto op = canUpdate ? TextureHost::UPDATE_IMAGE : TextureHost::ADD_IMAGE;
 
   if (!useExternalImage) {
     return UpdateWithoutExternalImage(texture, aKeys[0], op, aMaybeFastTxn);
   }
 
-  wrTexture->MaybeNofityForUse(aMaybeFastTxn);
-
   Range<wr::ImageKey> keys(&aKeys[0], aKeys.Length());
   auto externalImageKey = wrTexture->GetExternalImageKey();
   wrTexture->PushResourceUpdates(aMaybeFastTxn, op, keys, externalImageKey);
 
   return Some(op);
 }
 
 Maybe<TextureHost::ResourceUpdateOp>
--- a/gfx/layers/wr/WebRenderTextureHost.cpp
+++ b/gfx/layers/wr/WebRenderTextureHost.cpp
@@ -13,34 +13,16 @@
 
 #ifdef MOZ_WIDGET_ANDROID
 #  include "mozilla/layers/TextureHostOGL.h"
 #endif
 
 namespace mozilla {
 namespace layers {
 
-class ScheduleNofityForUse : public wr::NotificationHandler {
- public:
-  explicit ScheduleNofityForUse(uint64_t aExternalImageId)
-      : mExternalImageId(aExternalImageId) {}
-
-  virtual void Notify(wr::Checkpoint aCheckpoint) override {
-    if (aCheckpoint == wr::Checkpoint::FrameTexturesUpdated) {
-      MOZ_ASSERT(wr::RenderThread::IsInRenderThread());
-      wr::RenderThread::Get()->NofityForUse(mExternalImageId);
-    } else {
-      MOZ_ASSERT(aCheckpoint == wr::Checkpoint::TransactionDropped);
-    }
-  }
-
- protected:
-  uint64_t mExternalImageId;
-};
-
 WebRenderTextureHost::WebRenderTextureHost(
     const SurfaceDescriptor& aDesc, TextureFlags aFlags, TextureHost* aTexture,
     wr::ExternalImageId& aExternalImageId)
     : TextureHost(aFlags), mExternalImageId(aExternalImageId) {
   // The wrapped textureHost will be used in WebRender, and the WebRender could
   // run at another thread. It's hard to control the life-time when gecko
   // receives PTextureParent destroy message. It's possible that textureHost is
   // still used by WebRender. So, we only accept the textureHost without
@@ -241,22 +223,10 @@ bool WebRenderTextureHost::NeedsYFlip() 
     // But y-flip should not be handled, since
     // SurfaceTexture.getTransformMatrix() is not handled yet.
     // See Bug 1507076.
     yFlip = false;
   }
   return yFlip;
 }
 
-void WebRenderTextureHost::MaybeNofityForUse(wr::TransactionBuilder& aTxn) {
-#if defined(MOZ_WIDGET_ANDROID)
-  if (!mWrappedTextureHost->AsSurfaceTextureHost()) {
-    return;
-  }
-  // SurfaceTexture of video needs NofityForUse() to detect if it is rendered
-  // on WebRender.
-  aTxn.Notify(wr::Checkpoint::FrameTexturesUpdated,
-              MakeUnique<ScheduleNofityForUse>(wr::AsUint64(mExternalImageId)));
-#endif
-}
-
 }  // namespace layers
 }  // namespace mozilla
--- a/gfx/layers/wr/WebRenderTextureHost.h
+++ b/gfx/layers/wr/WebRenderTextureHost.h
@@ -79,18 +79,16 @@ class WebRenderTextureHost : public Text
 
   void PushDisplayItems(wr::DisplayListBuilder& aBuilder,
                         const wr::LayoutRect& aBounds,
                         const wr::LayoutRect& aClip, wr::ImageRendering aFilter,
                         const Range<wr::ImageKey>& aImageKeys) override;
 
   bool NeedsYFlip() const override;
 
-  void MaybeNofityForUse(wr::TransactionBuilder& aTxn);
-
  protected:
   void CreateRenderTextureHost(const SurfaceDescriptor& aDesc,
                                TextureHost* aTexture);
 
   RefPtr<TextureHost> mWrappedTextureHost;
   wr::ExternalImageId mExternalImageId;
 };
 
--- a/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp
+++ b/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.cpp
@@ -61,34 +61,33 @@ wr::WrExternalImage RenderAndroidSurface
     MOZ_ASSERT_UNREACHABLE("Unexpected GL context");
     return InvalidToWrExternalImage();
   }
 
   if (!mSurfTex || !mGL || !mGL->MakeCurrent()) {
     return InvalidToWrExternalImage();
   }
 
+  MOZ_ASSERT(mAttachedToGLContext);
   if (!mAttachedToGLContext) {
-    // Cache new rendering filter.
-    mCachedRendering = aRendering;
-    if (!EnsureAttachedToGLContext()) {
-      return InvalidToWrExternalImage();
-    }
-  } else if (IsFilterUpdateNecessary(aRendering)) {
+    return InvalidToWrExternalImage();
+  }
+
+  if (IsFilterUpdateNecessary(aRendering)) {
     // Cache new rendering filter.
     mCachedRendering = aRendering;
     ActivateBindAndTexParameteri(mGL, LOCAL_GL_TEXTURE0,
                                  LOCAL_GL_TEXTURE_EXTERNAL_OES,
                                  mSurfTex->GetTexName(), aRendering);
   }
 
   if (mContinuousUpdate) {
     MOZ_ASSERT(!mSurfTex->IsSingleBuffer());
     mSurfTex->UpdateTexImage();
-  } else if (mPrepareStatus == STATUS_PREPARE_NEEDED) {
+  } else if (mPrepareStatus == STATUS_UPDATE_TEX_IMAGE_NEEDED) {
     MOZ_ASSERT(!mSurfTex->IsSingleBuffer());
     // When SurfaceTexture is not single buffer mode, call UpdateTexImage() once
     // just before rendering. During playing video, one SurfaceTexture is used
     // for all RenderAndroidSurfaceTextureHostOGLs of video.
     mSurfTex->UpdateTexImage();
     mPrepareStatus = STATUS_PREPARED;
   }
 
@@ -134,105 +133,62 @@ bool RenderAndroidSurfaceTextureHostOGL:
       return false;
     }
   }
 
   mAttachedToGLContext = true;
   return true;
 }
 
-bool RenderAndroidSurfaceTextureHostOGL::CheckIfAttachedToGLContext() {
-  if (mAttachedToGLContext) {
-    return true;
-  }
-
-  if (!mGL) {
-    mGL = RenderThread::Get()->SharedGL();
-  }
-
-  if (!mSurfTex || !mGL || !mGL->MakeCurrent()) {
-    return false;
-  }
-
-  if (!mSurfTex->IsAttachedToGLContext((int64_t)mGL.get())) {
-    return false;
-  }
-
-  mAttachedToGLContext = true;
-  return true;
-}
-
 void RenderAndroidSurfaceTextureHostOGL::PrepareForUse() {
   // When SurfaceTexture is single buffer mode, UpdateTexImage needs to be
   // called only once for each publish. If UpdateTexImage is called more
   // than once, it causes hang on puglish side. And UpdateTexImage needs to
   // be called on render thread, since the SurfaceTexture is consumed on render
   // thread.
   MOZ_ASSERT(RenderThread::IsInRenderThread());
   MOZ_ASSERT(mPrepareStatus == STATUS_NONE);
 
+  if (!EnsureAttachedToGLContext()) {
+    return;
+  }
+
   if (mContinuousUpdate) {
     return;
   }
 
-  mPrepareStatus = STATUS_MIGHT_BE_USED;
+  MOZ_ASSERT(mSurfTex);
+  mPrepareStatus = STATUS_UPDATE_TEX_IMAGE_NEEDED;
 
-  if (mSurfTex && mSurfTex->IsSingleBuffer()) {
+  if (mSurfTex->IsSingleBuffer()) {
     // When SurfaceTexture is single buffer mode, it is OK to call
     // UpdateTexImage() here.
-    EnsureAttachedToGLContext();
     mSurfTex->UpdateTexImage();
     mPrepareStatus = STATUS_PREPARED;
-  } else {
-    // If SurfaceTexture is already bound to gl texture, it is already used for
-    // rendering to WebRender.
-    // During video playback, there is a case that rendering is skipped. In this
-    // case, NofityForUse() is not called. But we want to call UpdateTexImage()
-    // for adjusting SurfaceTexture's buffer status.
-    if (CheckIfAttachedToGLContext()) {
-      mPrepareStatus = STATUS_PREPARE_NEEDED;
-    }
-  }
-}
-
-void RenderAndroidSurfaceTextureHostOGL::NofityForUse() {
-  MOZ_ASSERT(RenderThread::IsInRenderThread());
-
-  if (mPrepareStatus == STATUS_MIGHT_BE_USED) {
-    // This happens when SurfaceTexture of video is rendered on WebRender and
-    // SurfaceTexture is not bounded to gl context yet.
-    // It is necessary to handle a case that SurfaceTexture is not rendered on
-    // WebRender, instead rendered to WebGL.
-    // It is not a good way. But it is same to Compositor rendering.
-    MOZ_ASSERT(!mSurfTex || !mSurfTex->IsSingleBuffer());
-    if (!EnsureAttachedToGLContext()) {
-      return;
-    }
-    mPrepareStatus = STATUS_PREPARE_NEEDED;
   }
 }
 
 void RenderAndroidSurfaceTextureHostOGL::NotifyNotUsed() {
   MOZ_ASSERT(RenderThread::IsInRenderThread());
 
-  if (mSurfTex && mSurfTex->IsSingleBuffer() &&
-      mPrepareStatus == STATUS_PREPARED) {
-    if (!EnsureAttachedToGLContext()) {
-      return;
-    }
+  if (!mSurfTex) {
+    MOZ_ASSERT(mPrepareStatus == STATUS_NONE);
+    return;
+  }
+
+  if (mSurfTex->IsSingleBuffer()) {
+    MOZ_ASSERT(mPrepareStatus == STATUS_PREPARED);
+    MOZ_ASSERT(mAttachedToGLContext);
     // Release SurfaceTexture's buffer to client side.
     mGL->MakeCurrent();
     mSurfTex->ReleaseTexImage();
-  } else if (mSurfTex && mPrepareStatus == STATUS_PREPARE_NEEDED) {
+  } else if (mPrepareStatus == STATUS_UPDATE_TEX_IMAGE_NEEDED) {
+    MOZ_ASSERT(mAttachedToGLContext);
     // This could happen when video frame was skipped. UpdateTexImage() neeeds
     // to be called for adjusting SurfaceTexture's buffer status.
-    MOZ_ASSERT(!mSurfTex->IsSingleBuffer());
-    if (!EnsureAttachedToGLContext()) {
-      return;
-    }
     mSurfTex->UpdateTexImage();
   }
 
   mPrepareStatus = STATUS_NONE;
 }
 
 }  // namespace wr
 }  // namespace mozilla
--- a/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.h
+++ b/gfx/webrender_bindings/RenderAndroidSurfaceTextureHostOGL.h
@@ -24,29 +24,26 @@ class RenderAndroidSurfaceTextureHostOGL
   wr::WrExternalImage Lock(uint8_t aChannelIndex, gl::GLContext* aGL,
                            wr::ImageRendering aRendering) override;
   void Unlock() override;
 
   gfx::IntSize GetSize(uint8_t aChannelIndex) const override;
   GLuint GetGLHandle(uint8_t aChannelIndex) const override;
 
   virtual void PrepareForUse() override;
-  virtual void NofityForUse() override;
   virtual void NotifyNotUsed() override;
 
  private:
   virtual ~RenderAndroidSurfaceTextureHostOGL();
   void DeleteTextureHandle();
   bool EnsureAttachedToGLContext();
-  bool CheckIfAttachedToGLContext();
 
   enum PrepareStatus {
     STATUS_NONE,
-    STATUS_MIGHT_BE_USED,
-    STATUS_PREPARE_NEEDED,
+    STATUS_UPDATE_TEX_IMAGE_NEEDED,
     STATUS_PREPARED
   };
 
   const mozilla::java::GeckoSurfaceTexture::GlobalRef mSurfTex;
   const gfx::IntSize mSize;
   // mContinuousUpdate was used for rendering video in the past.
   // It is not used on current gecko.
   const bool mContinuousUpdate;
--- a/gfx/webrender_bindings/RenderTextureHost.h
+++ b/gfx/webrender_bindings/RenderTextureHost.h
@@ -36,18 +36,23 @@ class RenderTextureHost {
  public:
   RenderTextureHost();
 
   virtual wr::WrExternalImage Lock(uint8_t aChannelIndex, gl::GLContext* aGL,
                                    wr::ImageRendering aRendering) = 0;
   virtual void Unlock() = 0;
   virtual void ClearCachedResources() {}
 
+  // Called asynchronouly when corresponding TextureHost's mCompositableCount
+  // becomes from 0 to 1. For now, it is used only for
+  // SurfaceTextureHost/RenderAndroidSurfaceTextureHostOGL.
   virtual void PrepareForUse() {}
-  virtual void NofityForUse() {}
+  // Called asynchronouly when corresponding TextureHost's mCompositableCount
+  // becomes 0. For now, it is used only for
+  // SurfaceTextureHost/RenderAndroidSurfaceTextureHostOGL.
   virtual void NotifyNotUsed() {}
 
   virtual RenderDXGITextureHostOGL* AsRenderDXGITextureHostOGL() {
     return nullptr;
   }
 
  protected:
   virtual ~RenderTextureHost();
--- a/gfx/webrender_bindings/RenderThread.cpp
+++ b/gfx/webrender_bindings/RenderThread.cpp
@@ -338,16 +338,20 @@ void RenderThread::HandleFrameOneDoc(wr:
 
     MOZ_ASSERT(frameInfo.mDocFramesSeen == frameInfo.mDocFramesTotal);
     render = frameInfo.mFrameNeedsRender;
 
     frame = frameInfo;
     hadSlowFrame = info->mHadSlowFrame;
   }
 
+  // It is for ensuring that PrepareForUse() is called before
+  // RenderTextureHost::Lock().
+  HandlePrepareForUse();
+
   UpdateAndRender(aWindowId, frame.mStartId, frame.mStartTime, render,
                   /* aReadbackSize */ Nothing(),
                   /* aReadbackFormat */ Nothing(),
                   /* aReadbackBuffer */ Nothing(), hadSlowFrame);
 
   {  // scope lock
     auto windows = mWindowInfos.Lock();
     auto it = windows->find(AsUint64(aWindowId));
@@ -652,87 +656,75 @@ void RenderThread::UnregisterExternalIma
         "RenderThread::DeferredRenderTextureHostDestroy", this,
         &RenderThread::DeferredRenderTextureHostDestroy));
   } else {
     mRenderTextures.erase(it);
   }
 }
 
 void RenderThread::PrepareForUse(uint64_t aExternalImageId) {
-  if (!IsInRenderThread()) {
-    Loop()->PostTask(NewRunnableMethod<uint64_t>(
-        "RenderThread::PrepareForUse", this, &RenderThread::PrepareForUse,
-        aExternalImageId));
-    return;
-  }
+  MOZ_ASSERT(!IsInRenderThread());
 
   MutexAutoLock lock(mRenderTextureMapLock);
   if (mHasShutdown) {
     return;
   }
 
   auto it = mRenderTextures.find(aExternalImageId);
   MOZ_ASSERT(it != mRenderTextures.end());
   if (it == mRenderTextures.end()) {
     return;
   }
 
   RefPtr<RenderTextureHost> texture = it->second;
-  texture->PrepareForUse();
+  mRenderTexturesPrepareForUse.emplace_back(std::move(texture));
+  Loop()->PostTask(NewRunnableMethod("RenderThread::HandlePrepareForUse", this,
+                                     &RenderThread::HandlePrepareForUse));
 }
 
 void RenderThread::NotifyNotUsed(uint64_t aExternalImageId) {
-  if (!IsInRenderThread()) {
-    Loop()->PostTask(NewRunnableMethod<uint64_t>(
-        "RenderThread::NotifyNotUsed", this, &RenderThread::NotifyNotUsed,
-        aExternalImageId));
-    return;
-  }
+  MOZ_ASSERT(!IsInRenderThread());
 
   MutexAutoLock lock(mRenderTextureMapLock);
   if (mHasShutdown) {
     return;
   }
 
   auto it = mRenderTextures.find(aExternalImageId);
-#ifndef MOZ_WIDGET_ANDROID
-  // This assert fails on GeckoView intermittently. Bug 1559958 tracks it.
   MOZ_ASSERT(it != mRenderTextures.end());
-#endif
   if (it == mRenderTextures.end()) {
     return;
   }
-
   RefPtr<RenderTextureHost> texture = it->second;
-  texture->NotifyNotUsed();
-}
-
-void RenderThread::NofityForUse(uint64_t aExternalImageId) {
-  MOZ_ASSERT(RenderThread::IsInRenderThread());
-
-  MutexAutoLock lock(mRenderTextureMapLock);
-  if (mHasShutdown) {
-    return;
-  }
-  auto it = mRenderTextures.find(aExternalImageId);
-  if (it == mRenderTextures.end()) {
-    return;
-  }
-  it->second->NofityForUse();
+  RefPtr<Runnable> task =
+      NS_NewRunnableFunction("RenderThread::DoNotifyNotUsed",
+                             [renderTexture = std::move(texture)]() -> void {
+                               renderTexture->NotifyNotUsed();
+                             });
+  Loop()->PostTask(task.forget());
 }
 
 void RenderThread::UnregisterExternalImageDuringShutdown(
     uint64_t aExternalImageId) {
   MOZ_ASSERT(IsInRenderThread());
   MutexAutoLock lock(mRenderTextureMapLock);
   MOZ_ASSERT(mHasShutdown);
   MOZ_ASSERT(mRenderTextures.find(aExternalImageId) != mRenderTextures.end());
   mRenderTextures.erase(aExternalImageId);
 }
 
+void RenderThread::HandlePrepareForUse() {
+  MOZ_ASSERT(IsInRenderThread());
+  MutexAutoLock lock(mRenderTextureMapLock);
+  for (auto& texture : mRenderTexturesPrepareForUse) {
+    texture->PrepareForUse();
+  }
+  mRenderTexturesPrepareForUse.clear();
+}
+
 void RenderThread::DeferredRenderTextureHostDestroy() {
   MutexAutoLock lock(mRenderTextureMapLock);
   mRenderTexturesDeferred.clear();
 }
 
 RenderTextureHost* RenderThread::GetRenderTexture(
     wr::ExternalImageId aExternalImageId) {
   MOZ_ASSERT(IsInRenderThread());
--- a/gfx/webrender_bindings/RenderThread.h
+++ b/gfx/webrender_bindings/RenderThread.h
@@ -203,19 +203,16 @@ class RenderThread final {
 
   /// Can be called from any thread.
   void PrepareForUse(uint64_t aExternalImageId);
 
   /// Can be called from any thread.
   void NotifyNotUsed(uint64_t aExternalImageId);
 
   /// Can only be called from the render thread.
-  void NofityForUse(uint64_t aExternalImageId);
-
-  /// Can only be called from the render thread.
   void UnregisterExternalImageDuringShutdown(uint64_t aExternalImageId);
 
   /// Can only be called from the render thread.
   RenderTextureHost* GetRenderTexture(ExternalImageId aExternalImageId);
 
   /// Can be called from any thread.
   bool IsDestroyed(wr::WindowId aWindowId);
   /// Can be called from any thread.
@@ -273,16 +270,17 @@ class RenderThread final {
   void WriteCollectedFramesForWindow(wr::WindowId aWindowId);
 
   Maybe<layers::CollectedFrames> GetCollectedFramesForWindow(
       wr::WindowId aWindowId);
 
  private:
   explicit RenderThread(base::Thread* aThread);
 
+  void HandlePrepareForUse();
   void DeferredRenderTextureHostDestroy();
   void ShutDownTask(layers::SynchronousTask* aTask);
   void InitDeviceTask();
 
   void DoAccumulateMemoryReport(MemoryReport,
                                 const RefPtr<MemoryReportPromise::Private>&);
 
   ~RenderThread();
@@ -319,16 +317,20 @@ class RenderThread final {
     bool mIsDestroyed = false;
     bool mHadSlowFrame = false;
   };
 
   DataMutex<std::unordered_map<uint64_t, WindowInfo*>> mWindowInfos;
 
   Mutex mRenderTextureMapLock;
   std::unordered_map<uint64_t, RefPtr<RenderTextureHost>> mRenderTextures;
+  // Hold RenderTextureHosts that are waiting for handling PrepareForUse().
+  // It is for ensuring that PrepareForUse() is called before
+  // RenderTextureHost::Lock().
+  std::list<RefPtr<RenderTextureHost>> mRenderTexturesPrepareForUse;
   // Used to remove all RenderTextureHost that are going to be removed by
   // a deferred callback and remove them right away without waiting for the
   // callback. On device reset we have to remove all GL related resources right
   // away.
   std::list<RefPtr<RenderTextureHost>> mRenderTexturesDeferred;
   bool mHasShutdown;
 
   bool mHandlingDeviceReset;