Bug 1289380 - Simplify PersistentBufferProviderShared. r=edwin

dom/canvas/CanvasRenderingContext2D.cpp

@@ -905,7 +905,7 @@ public:
     if (!context || !context->mTarget)
       return;
 
-    context->ReturnTarget();
+    context->OnStableState();
   }
 
   static void DidTransactionCallback(void* aData)
@@ -1504,6 +1504,10 @@ CanvasRenderingContext2D::ScheduleStableStateCallback()
 void
 CanvasRenderingContext2D::OnStableState()
 {
+  if (!mHasPendingStableStateCallback) {
+    return;
+  }
+
   ReturnTarget();
 
   mHasPendingStableStateCallback = false;
@@ -1529,6 +1533,8 @@ CanvasRenderingContext2D::EnsureTarget(const gfx::Rect* aCoveredRect,
     return mRenderingMode;
   }
 
+  ScheduleStableStateCallback();
+
   if (mBufferProvider && mode == mRenderingMode) {
     gfx::Rect rect(0, 0, mWidth, mHeight);
     if (aCoveredRect && CurrentState().transform.TransformBounds(*aCoveredRect).Contains(rect)) {
@@ -1537,8 +1543,6 @@ CanvasRenderingContext2D::EnsureTarget(const gfx::Rect* aCoveredRect,
       mTarget = mBufferProvider->BorrowDrawTarget(IntRect(0, 0, mWidth, mHeight));
     }
 
-    ScheduleStableStateCallback();
-
     if (mTarget) {
       // Restore clip and transform.
       for (uint32_t i = 0; i < mStyleStack.Length(); i++) {
@@ -1731,7 +1735,7 @@ CanvasRenderingContext2D::ClearTarget()
 void
 CanvasRenderingContext2D::ReturnTarget()
 {
-  if (mTarget && mBufferProvider) {
+  if (mTarget && mBufferProvider && mTarget != sErrorTarget) {
     CurrentState().transform = mTarget->GetTransform();
     for (uint32_t i = 0; i < mStyleStack.Length(); i++) {
       for (uint32_t c = 0; c < mStyleStack[i].clipsPushed.Length(); c++) {
@@ -5693,7 +5697,7 @@ CanvasRenderingContext2D::GetBufferProvider(LayerManager* aManager)
     return mBufferProvider;
   }
 
-  if (aManager) {
+  if (aManager && !mIsSkiaGL) {
     mBufferProvider = aManager->CreatePersistentBufferProvider(gfx::IntSize(mWidth, mHeight),
                                                                GetSurfaceFormat());
   }

gfx/layers/PersistentBufferProvider.cpp

@@ -103,8 +103,6 @@ PersistentBufferProviderShared::Create(gfx::IntSize aSize,
     return nullptr;
   }
 
-  texture->EnableReadLock();
-
   RefPtr<PersistentBufferProviderShared> provider =
     new PersistentBufferProviderShared(aSize, aFormat, aFwd, texture);
   return provider.forget();
@@ -118,8 +116,11 @@ PersistentBufferProviderShared::PersistentBufferProviderShared(gfx::IntSize aSiz
 : mSize(aSize)
 , mFormat(aFormat)
 , mFwd(aFwd)
-, mBack(aTexture.forget())
+, mFront(Nothing())
 {
+  if (mTextures.append(aTexture)) {
+    mBack = Some<uint32_t>(0);
+  }
   MOZ_COUNT_CTOR(PersistentBufferProviderShared);
 }
 
@@ -134,6 +135,15 @@ PersistentBufferProviderShared::~PersistentBufferProviderShared()
   Destroy();
 }
 
+TextureClient*
+PersistentBufferProviderShared::GetTexture(Maybe<uint32_t> aIndex)
+{
+  if (aIndex.isNothing() || !CheckIndex(aIndex.value())) {
+    return nullptr;
+  }
+  return mTextures[aIndex.value()];
+}
+
 already_AddRefed<gfx::DrawTarget>
 PersistentBufferProviderShared::BorrowDrawTarget(const gfx::IntRect& aPersistedRect)
 {
@@ -149,51 +159,76 @@ PersistentBufferProviderShared::BorrowDrawTarget(const gfx::IntRect& aPersistedR
     mFwd->GetActiveResourceTracker().AddObject(this);
   }
 
-  if (!mDrawTarget) {
+  if (mDrawTarget) {
-    bool changedBackBuffer = false;
+    RefPtr<gfx::DrawTarget> dt(mDrawTarget);
-    if (!mBack || mBack->IsReadLocked()) {
+    return dt.forget();
-      if (mBuffer && !mBuffer->IsReadLocked()) {
+  }
-        mBack.swap(mBuffer);
+
-      } else {
+  mFront = Nothing();
-        mBack = TextureClient::CreateForDrawing(
+
-          mFwd, mFormat, mSize,
+  auto previousBackBuffer = mBack;
-          BackendSelector::Canvas,
+
-          TextureFlags::DEFAULT,
+  TextureClient* tex = GetTexture(mBack);
-          TextureAllocationFlags::ALLOC_DEFAULT
+
-        );
+  // First try to reuse the current back buffer. If we can do that it means
-        if (mBack) {
+  // we can skip copying its content to the new back buffer.
-          mBack->EnableReadLock();
+  if (tex && tex->IsReadLocked()) {
-        }
+    // The back buffer is currently used by the compositor, we can't draw
+    // into it.
+    tex = nullptr;
+  }
+
+  if (!tex) {
+    // Try to grab an already allocated texture if any is available.
+    for (uint32_t i = 0; i < mTextures.length(); ++i) {
+      if (!mTextures[i]->IsReadLocked()) {
+        mBack = Some(i);
+        tex = mTextures[i];
+        break;
       }
-      changedBackBuffer = true;
-    } else {
-      // Fast path, our front buffer is already writable because the texture upload
-      // has completed on the compositor side.
-      if (mBack->HasIntermediateBuffer()) {
-        // No need to keep an extra buffer around
-        mBuffer = nullptr;
-      }
-    }
-
-    if (!mBack || !mBack->Lock(OpenMode::OPEN_READ_WRITE)) {
-      return nullptr;
-    }
-
-    if (changedBackBuffer && !aPersistedRect.IsEmpty()
-        && mFront && mFront->Lock(OpenMode::OPEN_READ)) {
-
-      DebugOnly<bool> success = mFront->CopyToTextureClient(mBack, &aPersistedRect, nullptr);
-      MOZ_ASSERT(success);
-
-      mFront->Unlock();
-    }
-
-    mDrawTarget = mBack->BorrowDrawTarget();
-    if (!mDrawTarget) {
-      return nullptr;
     }
   }
 
+  if (!tex) {
+    // We have to allocate a new texture.
+    if (mTextures.length() >= 4) {
+      // We should never need to buffer that many textures, something's wrong.
+      MOZ_ASSERT(false);
+      return nullptr;
+    }
+
+    RefPtr<TextureClient> newTexture = TextureClient::CreateForDrawing(
+      mFwd, mFormat, mSize,
+      BackendSelector::Canvas,
+      TextureFlags::DEFAULT,
+      TextureAllocationFlags::ALLOC_DEFAULT
+    );
+
+    MOZ_ASSERT(newTexture);
+    if (newTexture) {
+      if (mTextures.append(newTexture)) {
+        tex = newTexture;
+        mBack = Some<uint32_t>(mTextures.length() - 1);
+      }
+    }
+  }
+
+  if (!tex || !tex->Lock(OpenMode::OPEN_READ_WRITE)) {
+    return nullptr;
+  }
+
+  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);
+
+      previous->Unlock();
+    }
+  }
+
+  mDrawTarget = tex->BorrowDrawTarget();
+
   RefPtr<gfx::DrawTarget> dt(mDrawTarget);
   return dt.forget();
 }
@@ -203,45 +238,60 @@ PersistentBufferProviderShared::ReturnDrawTarget(already_AddRefed<gfx::DrawTarge
 {
   RefPtr<gfx::DrawTarget> dt(aDT);
   MOZ_ASSERT(mDrawTarget == dt);
+  // Can't change the current front buffer while its snapshot is borrowed!
   MOZ_ASSERT(!mSnapshot);
 
   mDrawTarget = nullptr;
   dt = nullptr;
 
-  mBack->Unlock();
+  TextureClient* back = GetTexture(mBack);
+  MOZ_ASSERT(back);
 
-  if (!mBuffer && mFront && !mFront->IsLocked()) {
+  if (back) {
-    mBuffer.swap(mFront);
+    back->Unlock();
+    mFront = mBack;
   }
 
-  mFront = mBack;
+  return !!back;
+}
 
-  return true;
+TextureClient*
+PersistentBufferProviderShared::GetTextureClient()
+{
+  // Can't access the front buffer while drawing.
+  MOZ_ASSERT(!mDrawTarget);
+  TextureClient* texture = GetTexture(mFront);
+  if (texture) {
+    texture->EnableReadLock();
+  } else {
+    gfxCriticalNote << "PersistentBufferProviderShared: front buffer unavailable";
+  }
+  return texture;
 }
 
 already_AddRefed<gfx::SourceSurface>
 PersistentBufferProviderShared::BorrowSnapshot()
 {
-  // TODO[nical] currently we can't snapshot while drawing, looks like it does
-  // the job but I am not sure whether we want to be able to do that.
   MOZ_ASSERT(!mDrawTarget);
 
-  if (!mFront || mFront->IsLocked()) {
+  auto front = GetTexture(mFront);
+  if (!front || front->IsLocked()) {
     MOZ_ASSERT(false);
     return nullptr;
   }
 
-  if (!mFront->Lock(OpenMode::OPEN_READ)) {
+  if (!front->Lock(OpenMode::OPEN_READ)) {
     return nullptr;
   }
 
-  mDrawTarget = mFront->BorrowDrawTarget();
+  RefPtr<DrawTarget> dt = front->BorrowDrawTarget();
 
-  if (!mDrawTarget) {
+  if (!dt) {
-    mFront->Unlock();
+    front->Unlock();
+    return nullptr;
   }
 
-  mSnapshot = mDrawTarget->Snapshot();
+  mSnapshot = dt->Snapshot();
 
   RefPtr<SourceSurface> snapshot = mSnapshot;
   return snapshot.forget();
@@ -256,20 +306,35 @@ PersistentBufferProviderShared::ReturnSnapshot(already_AddRefed<gfx::SourceSurfa
   mSnapshot = nullptr;
   snapshot = nullptr;
 
-  mDrawTarget = nullptr;
+  auto front = GetTexture(mFront);
-
+  if (front) {
-  mFront->Unlock();
+    front->Unlock();
+  }
 }
 
 void
 PersistentBufferProviderShared::NotifyInactive()
 {
-  if (mBuffer && mBuffer->IsLocked()) {
+  RefPtr<TextureClient> front = GetTexture(mFront);
-    // mBuffer should never be locked
+  RefPtr<TextureClient> back = GetTexture(mBack);
-    MOZ_ASSERT(false);
+
-    mBuffer->Unlock();
+  // Clear all textures (except the front and back ones that we just kept).
+  mTextures.clear();
+
+  if (back) {
+    if (mTextures.append(back)) {
+      mBack = Some<uint32_t>(0);
+    }
+    if (front == back) {
+      mFront = mBack;
+    }
+  }
+
+  if (front && front != back) {
+    if (mTextures.append(front)) {
+      mFront = Some<uint32_t>(mTextures.length() - 1);
+    }
   }
-  mBuffer = nullptr;
 }
 
 void
@@ -278,21 +343,15 @@ PersistentBufferProviderShared::Destroy()
   mSnapshot = nullptr;
   mDrawTarget = nullptr;
 
-  if (mFront && mFront->IsLocked()) {
+  for (uint32_t i = 0; i < mTextures.length(); ++i) {
-    mFront->Unlock();
+    TextureClient* texture = mTextures[i];
+    if (texture && texture->IsLocked()) {
+      MOZ_ASSERT(false);
+      texture->Unlock();
+    }
   }
 
-  if (mBack && mBack->IsLocked()) {
+  mTextures.clear();
-    mBack->Unlock();
-  }
-
-  if (mBuffer && mBuffer->IsLocked()) {
-    mBuffer->Unlock();
-  }
-
-  mFront = nullptr;
-  mBack = nullptr;
-  mBuffer = nullptr;
 }
 
 } // namespace layers

gfx/layers/PersistentBufferProvider.h

@@ -11,6 +11,7 @@
 #include "mozilla/layers/LayersTypes.h"
 #include "mozilla/layers/CompositableForwarder.h"
 #include "mozilla/gfx/Types.h"
+#include "mozilla/Vector.h"
 
 namespace mozilla {
 
@@ -117,9 +118,7 @@ public:
 
   virtual void ReturnSnapshot(already_AddRefed<gfx::SourceSurface> aSnapshot) override;
 
-  TextureClient* GetTextureClient() override {
+  virtual TextureClient* GetTextureClient() override;
-    return mFront;
-  }
 
   virtual void NotifyInactive() override;
 
@@ -132,17 +131,20 @@ protected:
 
   ~PersistentBufferProviderShared();
 
+  TextureClient* GetTexture(Maybe<uint32_t> aIndex);
+  bool CheckIndex(uint32_t aIndex) { return aIndex < mTextures.length(); }
+
   void Destroy();
 
   gfx::IntSize mSize;
   gfx::SurfaceFormat mFormat;
   RefPtr<CompositableForwarder> mFwd;
-  // The texture presented to the compositor.
+  Vector<RefPtr<TextureClient>, 4> mTextures;
-  RefPtr<TextureClient> mFront;
+  // Offset of the texture in mTextures that the canvas uses.
-  // The texture that the canvas uses.
+  Maybe<uint32_t> mBack;
-  RefPtr<TextureClient> mBack;
+  // Offset of the texture in mTextures that is presented to the compositor.
-  // An extra texture we keep around temporarily to avoid allocating.
+  Maybe<uint32_t> mFront;
-  RefPtr<TextureClient> mBuffer;
+
   RefPtr<gfx::DrawTarget> mDrawTarget;
   RefPtr<gfx::SourceSurface > mSnapshot;
 };

gfx/layers/client/CanvasClient.cpp

@@ -77,11 +77,13 @@ CanvasClient2D::UpdateFromTexture(TextureClient* aTexture)
     }
   }
 
-  mBackBuffer = aTexture;
+  mBackBuffer = nullptr;
+  mFrontBuffer = nullptr;
+  mBufferProviderTexture = aTexture;
 
   AutoTArray<CompositableForwarder::TimedTextureClient,1> textures;
   CompositableForwarder::TimedTextureClient* t = textures.AppendElement();
-  t->mTextureClient = mBackBuffer;
+  t->mTextureClient = aTexture;
   t->mPictureRect = nsIntRect(nsIntPoint(0, 0), aTexture->GetSize());
   t->mFrameID = mFrameID;
   t->mInputFrameID = VRManagerChild::Get()->GetInputFrameID();
@@ -93,6 +95,8 @@ CanvasClient2D::UpdateFromTexture(TextureClient* aTexture)
 void
 CanvasClient2D::Update(gfx::IntSize aSize, ClientCanvasLayer* aLayer)
 {
+  mBufferProviderTexture = nullptr;
+
   AutoRemoveTexture autoRemove(this);
   if (mBackBuffer &&
       (mBackBuffer->IsImmutable() || mBackBuffer->GetSize() != aSize)) {

gfx/layers/client/CanvasClient.h

@@ -128,6 +128,12 @@ private:
 
   RefPtr<TextureClient> mBackBuffer;
   RefPtr<TextureClient> mFrontBuffer;
+  // We store this texture separately to make sure it is not written into
+  // in Update() if for some silly reason we end up alternating between
+  // UpdateFromTexture and Update.
+  // This code is begging for a cleanup. The situation described above should
+  // not be made possible.
+  RefPtr<TextureClient> mBufferProviderTexture;
 };
 
 // Used for GL canvases where we don't need to do any readback, i.e., with a