Bug 1464032 Part 7: Take snapshot before return for TextureClients with synchronization. r=mattwoodrow
authorBob Owen <bobowencode@gmail.com>
Sun, 02 Dec 2018 14:14:35 +0000
changeset 477758 a2720ec3086f5c17bee8ddb394cd43ea0674c2f4
parent 477757 258c6c1996568b3e7d3ca442a2d87df3f60a4b32
child 477759 fcc9f5f6dfe1c94e7f76582ee972e960539cb060
push id113373
push userbobowencode@gmail.com
push dateFri, 07 Jun 2019 11:10:59 +0000
treeherdermozilla-inbound@2195b79ea888 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1464032
milestone69.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 1464032 Part 7: Take snapshot before return for TextureClients with synchronization. r=mattwoodrow This is so we don't need to lock the previous back buffer when it might also be locked by the compositor. These locks are generally for copying to the next back buffer or when getting image data from the previous back buffer. This also makes it easier to asynchronously cache the DataSourceSurface in the GPU process, when a page is using getImageData. This is done in a later patch.
gfx/layers/PersistentBufferProvider.cpp
gfx/layers/PersistentBufferProvider.h
gfx/layers/client/TextureClient.cpp
gfx/layers/client/TextureClient.h
--- a/gfx/layers/PersistentBufferProvider.cpp
+++ b/gfx/layers/PersistentBufferProvider.cpp
@@ -343,28 +343,33 @@ PersistentBufferProviderShared::BorrowDr
       }
     }
   }
 
   if (!tex || !tex->Lock(OpenMode::OPEN_READ_WRITE)) {
     return nullptr;
   }
 
+  mDrawTarget = tex->BorrowDrawTarget();
   if (mBack != previousBackBuffer && !aPersistedRect.IsEmpty()) {
-    TextureClient* previous = GetTexture(previousBackBuffer);
-    if (previous && previous->Lock(OpenMode::OPEN_READ)) {
-      DebugOnly<bool> success =
-          previous->CopyToTextureClient(tex, &aPersistedRect, nullptr);
-      MOZ_ASSERT(success);
+    if (mPreviousSnapshot) {
+      mDrawTarget->CopySurface(mPreviousSnapshot, aPersistedRect,
+                               gfx::IntPoint(0, 0));
+    } else {
+      TextureClient* previous = GetTexture(previousBackBuffer);
+      if (previous && previous->Lock(OpenMode::OPEN_READ)) {
+        DebugOnly<bool> success =
+            previous->CopyToTextureClient(tex, &aPersistedRect, nullptr);
+        MOZ_ASSERT(success);
 
-      previous->Unlock();
+        previous->Unlock();
+      }
     }
   }
-
-  mDrawTarget = tex->BorrowDrawTarget();
+  mPreviousSnapshot = nullptr;
 
   if (mDrawTarget) {
     // This is simply to ensure the DrawTarget gets initialized, and will detect
     // a device reset, even if we're on the main thread.
     mDrawTarget->ClearRect(Rect(0, 0, 0, 0));
 
     if (!mDrawTarget->IsValid()) {
       mDrawTarget = nullptr;
@@ -377,22 +382,33 @@ PersistentBufferProviderShared::BorrowDr
 
 bool PersistentBufferProviderShared::ReturnDrawTarget(
     already_AddRefed<gfx::DrawTarget> aDT) {
   RefPtr<gfx::DrawTarget> dt(aDT);
   MOZ_ASSERT(mDrawTarget == dt);
   // Can't change the current front buffer while its snapshot is borrowed!
   MOZ_ASSERT(!mSnapshot);
 
+  TextureClient* back = GetTexture(mBack);
+  MOZ_ASSERT(back);
+
+  // If our TextureClients have internal synchronization then, if locks are
+  // needed for reading and writing, this can cause locking issues with the
+  // compositor. To prevent this we take a snapshot when the DrawTarget is
+  // returned, so this can be used when our own BorrowSnapshot is called and
+  // also for copying to the next TextureClient. Using this snapshot outside of
+  // the locks is safe, because the TextureClient calls DetachAllSnapshots on
+  // its DrawTarget when we Unlock below.
+  if (back->HasSynchronization()) {
+    mPreviousSnapshot = back->BorrowSnapshot();
+  }
+
   mDrawTarget = nullptr;
   dt = nullptr;
 
-  TextureClient* back = GetTexture(mBack);
-  MOZ_ASSERT(back);
-
   if (back) {
     back->Unlock();
     mFront = mBack;
   }
 
   return !!back;
 }
 
@@ -406,47 +422,48 @@ TextureClient* PersistentBufferProviderS
   }
   return texture;
 }
 
 already_AddRefed<gfx::SourceSurface>
 PersistentBufferProviderShared::BorrowSnapshot() {
   MOZ_ASSERT(!mDrawTarget);
 
+  if (mPreviousSnapshot) {
+    mSnapshot = mPreviousSnapshot;
+    return do_AddRef(mSnapshot);
+  }
+
   auto front = GetTexture(mFront);
   if (!front || front->IsLocked()) {
     MOZ_ASSERT(false);
     return nullptr;
   }
 
   if (!front->Lock(OpenMode::OPEN_READ)) {
     return nullptr;
   }
 
-  RefPtr<DrawTarget> dt = front->BorrowDrawTarget();
+  mSnapshot = front->BorrowSnapshot();
 
-  if (!dt) {
-    front->Unlock();
-    return nullptr;
-  }
-
-  mSnapshot = dt->Snapshot();
-
-  RefPtr<SourceSurface> snapshot = mSnapshot;
-  return snapshot.forget();
+  return do_AddRef(mSnapshot);
 }
 
 void PersistentBufferProviderShared::ReturnSnapshot(
     already_AddRefed<gfx::SourceSurface> aSnapshot) {
   RefPtr<SourceSurface> snapshot = aSnapshot;
   MOZ_ASSERT(!snapshot || snapshot == mSnapshot);
 
   mSnapshot = nullptr;
   snapshot = nullptr;
 
+  if (mPreviousSnapshot) {
+    return;
+  }
+
   auto front = GetTexture(mFront);
   if (front) {
     front->Unlock();
   }
 }
 
 void PersistentBufferProviderShared::NotifyInactive() {
   ClearCachedResources();
@@ -472,16 +489,17 @@ void PersistentBufferProviderShared::Cle
     if (mTextures.append(front)) {
       mFront = Some<uint32_t>(mTextures.length() - 1);
     }
   }
 }
 
 void PersistentBufferProviderShared::Destroy() {
   mSnapshot = nullptr;
+  mPreviousSnapshot = nullptr;
   mDrawTarget = nullptr;
 
   for (auto& mTexture : mTextures) {
     TextureClient* texture = mTexture;
     if (texture && texture->IsLocked()) {
       MOZ_ASSERT(false);
       texture->Unlock();
     }
--- a/gfx/layers/PersistentBufferProvider.h
+++ b/gfx/layers/PersistentBufferProvider.h
@@ -175,16 +175,17 @@ class PersistentBufferProviderShared : p
   Vector<RefPtr<TextureClient>, 4> mTextures;
   // Offset of the texture in mTextures that the canvas uses.
   Maybe<uint32_t> mBack;
   // Offset of the texture in mTextures that is presented to the compositor.
   Maybe<uint32_t> mFront;
 
   RefPtr<gfx::DrawTarget> mDrawTarget;
   RefPtr<gfx::SourceSurface> mSnapshot;
+  RefPtr<gfx::SourceSurface> mPreviousSnapshot;
 };
 
 struct AutoReturnSnapshot final {
   PersistentBufferProvider* mBufferProvider;
   RefPtr<gfx::SourceSurface>* mSnapshot;
 
   explicit AutoReturnSnapshot(PersistentBufferProvider* aProvider = nullptr)
       : mBufferProvider(aProvider), mSnapshot(nullptr) {}
--- a/gfx/layers/client/TextureClient.cpp
+++ b/gfx/layers/client/TextureClient.cpp
@@ -831,16 +831,27 @@ gfx::DrawTarget* TextureClient::BorrowDr
 #ifdef DEBUG
     mExpectedDtRefs = mBorrowedDrawTarget ? mBorrowedDrawTarget->refCount() : 0;
 #endif
   }
 
   return mBorrowedDrawTarget;
 }
 
+already_AddRefed<gfx::SourceSurface> TextureClient::BorrowSnapshot() {
+  MOZ_ASSERT(mIsLocked);
+
+  RefPtr<gfx::SourceSurface> surface = mData->BorrowSnapshot();
+  if (!surface) {
+    surface = BorrowDrawTarget()->Snapshot();
+  }
+
+  return surface.forget();
+}
+
 bool TextureClient::BorrowMappedData(MappedTextureData& aMap) {
   MOZ_ASSERT(IsValid());
 
   // TODO - SharedRGBImage just accesses the buffer without properly locking
   // the texture. It's bad.
   // MOZ_ASSERT(mIsLocked);
   // if (!mIsLocked) {
   //  return nullptr;
--- a/gfx/layers/client/TextureClient.h
+++ b/gfx/layers/client/TextureClient.h
@@ -263,16 +263,20 @@ class TextureData {
   virtual bool Lock(OpenMode aMode) = 0;
 
   virtual void Unlock() = 0;
 
   virtual already_AddRefed<gfx::DrawTarget> BorrowDrawTarget() {
     return nullptr;
   }
 
+  virtual already_AddRefed<gfx::SourceSurface> BorrowSnapshot() {
+    return nullptr;
+  }
+
   virtual bool BorrowMappedData(MappedTextureData&) { return false; }
 
   virtual bool BorrowMappedYCbCrData(MappedYCbCrTextureData&) { return false; }
 
   virtual void Deallocate(LayersIPCChannel* aAllocator) = 0;
 
   /// Depending on the texture's flags either Deallocate or Forget is called.
   virtual void Forget(LayersIPCChannel* aAllocator) {}
@@ -437,16 +441,18 @@ class TextureClient : public AtomicRefCo
    *   DrawTarget* dt = texture->BorrowDrawTarget();
    *   // use the draw target ...
    * }
    * texture->Unlock();
    *
    */
   gfx::DrawTarget* BorrowDrawTarget();
 
+  already_AddRefed<gfx::SourceSurface> BorrowSnapshot();
+
   /**
    * Similar to BorrowDrawTarget but provides direct access to the texture's
    * bits instead of a DrawTarget.
    */
   bool BorrowMappedData(MappedTextureData&);
   bool BorrowMappedYCbCrData(MappedYCbCrTextureData&);
 
   /**