Bug 1739908 p2: Only use PersistentBufferProviderShared::mPermanentBackBuffer when first needed. r=lsalzman

This removes some of the changes that meant we started using
mPermanentBackBuffer straight away and we now wait until we actually try and
lock a read locked texture.
While this might still give a very small risk of contention, it gives
improvements in the following two circumstances.
* If a canvas texture is never forwarded and never read locked, it means we will
  only use one texture with no copies.
* If a canvas is always fully overwritten at the start of the frame (and a
  snapshot is not taken between frames), then we avoid a copy on each frame.

This also adds back in code so that on an OPEN_READ_WRITE lock we cache the data
surface if required, because that texture will be the new front buffer and we
won't be using mPermanentBackBuffer at that point.

Depends on D132601

Differential Revision: https://phabricator.services.mozilla.com/D132602

gfx/layers/PersistentBufferProvider.cpp

@@ -111,6 +111,15 @@ ClientWebGLContext* PersistentBufferProviderAccelerated::AsWebgl() {
       NativeSurfaceType::WEBGL_CONTEXT);
 }
 
+static already_AddRefed<TextureClient> CreateTexture(
+    KnowsCompositor* aKnowsCompositor, gfx::SurfaceFormat aFormat,
+    gfx::IntSize aSize) {
+  return TextureClient::CreateForDrawing(
+      aKnowsCompositor, aFormat, aSize, BackendSelector::Canvas,
+      TextureFlags::DEFAULT | TextureFlags::NON_BLOCKING_READ_LOCK,
+      TextureAllocationFlags::ALLOC_DEFAULT);
+}
+
 // static
 already_AddRefed<PersistentBufferProviderShared>
 PersistentBufferProviderShared::Create(gfx::IntSize aSize,
@@ -135,11 +144,8 @@ PersistentBufferProviderShared::Create(gfx::IntSize aSize,
   }
 #endif
 
-  RefPtr<TextureClient> texture = TextureClient::CreateForDrawing(
-      aKnowsCompositor, aFormat, aSize, BackendSelector::Canvas,
-      TextureFlags::DEFAULT | TextureFlags::NON_BLOCKING_READ_LOCK,
-      TextureAllocationFlags::ALLOC_DEFAULT);
-
+  RefPtr<TextureClient> texture =
+      CreateTexture(aKnowsCompositor, aFormat, aSize);
   if (!texture) {
     return nullptr;
   }
@@ -159,17 +165,8 @@ PersistentBufferProviderShared::PersistentBufferProviderShared(
       mKnowsCompositor(aKnowsCompositor),
       mFront(Nothing()) {
   MOZ_ASSERT(aKnowsCompositor);
-
-  // If our textures have synchronization use a separate permanent back buffer.
-  if (aTexture->HasSynchronization()) {
-    mPermanentBackBuffer = aTexture;
-    if (!mPermanentBackBuffer->Lock(OpenMode::OPEN_READ_WRITE)) {
-      mPermanentBackBuffer = nullptr;
-    }
-  } else {
-    if (mTextures.append(aTexture)) {
-      mBack = Some<uint32_t>(0);
-    }
+  if (mTextures.append(aTexture)) {
+    mBack = Some<uint32_t>(0);
   }
 
   // XXX KnowsCompositor could be used for mMaxAllowedTextures
@@ -221,10 +218,8 @@ bool PersistentBufferProviderShared::SetKnowsCompositor(
     Destroy();
 
     if (prevTexture) {
-      RefPtr<TextureClient> newTexture = TextureClient::CreateForDrawing(
-          aKnowsCompositor, mFormat, mSize, BackendSelector::Canvas,
-          TextureFlags::DEFAULT | TextureFlags::NON_BLOCKING_READ_LOCK,
-          TextureAllocationFlags::ALLOC_DEFAULT);
+      RefPtr<TextureClient> newTexture =
+          CreateTexture(aKnowsCompositor, mFormat, mSize);
 
       MOZ_ASSERT(newTexture);
       if (!newTexture) {
@@ -236,24 +231,6 @@ bool PersistentBufferProviderShared::SetKnowsCompositor(
       // Destroy. Not ideal but at least we won't try to use it with a
       // an incompatible ipc channel.
 
-      // If our new texture has internal synchronization then create a new
-      // permanent back buffer as well.
-      if (newTexture->HasSynchronization()) {
-        RefPtr<TextureClient> mPermanentBackBuffer =
-            TextureClient::CreateForDrawing(
-                aKnowsCompositor, mFormat, mSize, BackendSelector::Canvas,
-                TextureFlags::DEFAULT | TextureFlags::NON_BLOCKING_READ_LOCK,
-                TextureAllocationFlags::ALLOC_DEFAULT);
-        if (!mPermanentBackBuffer) {
-          return false;
-        }
-
-        if (!mPermanentBackBuffer->Lock(OpenMode::OPEN_READ_WRITE)) {
-          mPermanentBackBuffer = nullptr;
-          return false;
-        }
-      }
-
       if (!newTexture->Lock(OpenMode::OPEN_WRITE)) {
         return false;
       }
@@ -266,11 +243,6 @@ bool PersistentBufferProviderShared::SetKnowsCompositor(
       bool success =
           prevTexture->CopyToTextureClient(newTexture, nullptr, nullptr);
 
-      if (success && mPermanentBackBuffer) {
-        success = prevTexture->CopyToTextureClient(mPermanentBackBuffer,
-                                                   nullptr, nullptr);
-      }
-
       prevTexture->Unlock();
       newTexture->Unlock();
 
@@ -376,10 +348,8 @@ PersistentBufferProviderShared::BorrowDrawTarget(
       }
     }
 
-    RefPtr<TextureClient> newTexture = TextureClient::CreateForDrawing(
-        mKnowsCompositor, mFormat, mSize, BackendSelector::Canvas,
-        TextureFlags::DEFAULT | TextureFlags::NON_BLOCKING_READ_LOCK,
-        TextureAllocationFlags::ALLOC_DEFAULT);
+    RefPtr<TextureClient> newTexture =
+        CreateTexture(mKnowsCompositor, mFormat, mSize);
 
     MOZ_ASSERT(newTexture);
     if (newTexture) {
@@ -408,6 +378,20 @@ PersistentBufferProviderShared::BorrowDrawTarget(
     Maybe<TextureClientAutoLock> autoReadLock;
     TextureClient* previous = nullptr;
     if (mBack != previousBackBuffer && !aPersistedRect.IsEmpty()) {
+      if (tex->HasSynchronization()) {
+        // We are about to read lock a texture that is in use by the compositor
+        // and has synchronization. To prevent possible future contention we
+        // switch to using a permanent back buffer.
+        mPermanentBackBuffer = CreateTexture(mKnowsCompositor, mFormat, mSize);
+        if (!mPermanentBackBuffer) {
+          return nullptr;
+        }
+        if (!tex->Lock(OpenMode::OPEN_WRITE)) {
+          return nullptr;
+        }
+        tex = mPermanentBackBuffer;
+      }
+
       previous = GetTexture(previousBackBuffer);
       if (previous) {
         autoReadLock.emplace(previous, OpenMode::OPEN_READ);
@@ -531,6 +515,28 @@ PersistentBufferProviderShared::BorrowSnapshot() {
     return nullptr;
   }
 
+  if (front->IsReadLocked() && front->HasSynchronization()) {
+    // We are about to read lock a texture that is in use by the compositor and
+    // has synchronization. To prevent possible future contention we switch to
+    // using a permanent back buffer.
+    mPermanentBackBuffer = CreateTexture(mKnowsCompositor, mFormat, mSize);
+    if (!mPermanentBackBuffer ||
+        !mPermanentBackBuffer->Lock(OpenMode::OPEN_READ_WRITE)) {
+      return nullptr;
+    }
+
+    if (!front->Lock(OpenMode::OPEN_READ)) {
+      return nullptr;
+    }
+
+    DebugOnly<bool> success =
+        front->CopyToTextureClient(mPermanentBackBuffer, nullptr, nullptr);
+    MOZ_ASSERT(success);
+    front->Unlock();
+    mSnapshot = mPermanentBackBuffer->BorrowSnapshot();
+    return do_AddRef(mSnapshot);
+  }
+
   if (!front->Lock(OpenMode::OPEN_READ)) {
     return nullptr;
   }
@@ -568,6 +574,7 @@ void PersistentBufferProviderShared::ClearCachedResources() {
 
   // Clear all textures (except the front and back ones that we just kept).
   mTextures.clear();
+  mPermanentBackBuffer = nullptr;
 
   if (back) {
     if (mTextures.append(back)) {

gfx/layers/client/TextureRecorded.cpp

@@ -66,6 +66,13 @@ bool RecordedTextureData::Lock(OpenMode aMode) {
 }
 
 void RecordedTextureData::Unlock() {
+  if ((mLockedMode == OpenMode::OPEN_READ_WRITE) &&
+      mCanvasChild->ShouldCacheDataSurface()) {
+    mSnapshot = mDT->Snapshot();
+    mDT->DetachAllSnapshots();
+    mCanvasChild->RecordEvent(RecordedCacheDataSurface(mSnapshot.get()));
+  }
+
   mCanvasChild->RecordEvent(RecordedTextureUnlock(mTextureId));
   mLockedMode = OpenMode::OPEN_NONE;
 }
@@ -77,7 +84,7 @@ already_AddRefed<gfx::DrawTarget> RecordedTextureData::BorrowDrawTarget() {
 
 void RecordedTextureData::EndDraw() {
   MOZ_ASSERT(mDT->hasOneRef());
-  MOZ_ASSERT(mLockedMode & OpenMode::OPEN_READ_WRITE);
+  MOZ_ASSERT(mLockedMode == OpenMode::OPEN_READ_WRITE);
 
   if (mCanvasChild->ShouldCacheDataSurface()) {
     mSnapshot = mDT->Snapshot();