Bug 926745 - Don't call ForceRemove manually in compositable code. r=nical,sotaro
authorMatt Woodrow <mwoodrow@mozilla.com>
Mon, 10 Feb 2014 15:24:27 +1300
changeset 167809 0043cc21757b4acb8deb09167037f8d7993f4b00
parent 167808 9009d48cb9c7bd1ed5e742e290979ae2f10667fe
child 167810 09634c58facbc0f3a09d75ad273478f9193a40a4
push id26186
push usercbook@mozilla.com
push dateMon, 10 Feb 2014 11:48:02 +0000
treeherdermozilla-central@063a9e3da435 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical, sotaro
bugs926745
milestone30.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 926745 - Don't call ForceRemove manually in compositable code. r=nical,sotaro
gfx/layers/client/CanvasClient.cpp
gfx/layers/client/ClientLayerManager.cpp
gfx/layers/client/ImageClient.cpp
gfx/layers/client/TextureClient.cpp
gfx/layers/ipc/CompositableForwarder.h
gfx/layers/ipc/ImageBridgeChild.cpp
--- a/gfx/layers/client/CanvasClient.cpp
+++ b/gfx/layers/client/CanvasClient.cpp
@@ -48,17 +48,17 @@ CanvasClient::CreateCanvasClient(CanvasC
   return new CanvasClient2D(aForwarder, aFlags);
 }
 
 void
 CanvasClient2D::Update(gfx::IntSize aSize, ClientCanvasLayer* aLayer)
 {
   if (mBuffer &&
       (mBuffer->IsImmutable() || mBuffer->GetSize() != aSize)) {
-    GetForwarder()->AddForceRemovingTexture(mBuffer);
+    GetForwarder()->HoldUntilTransaction(mBuffer);
     mBuffer = nullptr;
   }
 
   bool bufferCreated = false;
   if (!mBuffer) {
     bool isOpaque = (aLayer->GetContentFlags() & Layer::CONTENT_OPAQUE);
     gfxContentType contentType = isOpaque
                                                 ? gfxContentType::COLOR
--- a/gfx/layers/client/ClientLayerManager.cpp
+++ b/gfx/layers/client/ClientLayerManager.cpp
@@ -383,17 +383,17 @@ ClientLayerManager::ForwardTransaction(b
 
     if (sent) {
       mNeedsComposite = false;
     }
   } else if (HasShadowManager()) {
     NS_WARNING("failed to forward Layers transaction");
   }
 
-  mForwarder->ForceRemoveTexturesIfNecessary();
+  mForwarder->RemoveTexturesIfNecessary();
   mPhase = PHASE_NONE;
 
   // this may result in Layers being deleted, which results in
   // PLayer::Send__delete__() and DeallocShmem()
   mKeepAlive.Clear();
 }
 
 ShadowableLayer*
--- a/gfx/layers/client/ImageClient.cpp
+++ b/gfx/layers/client/ImageClient.cpp
@@ -98,30 +98,30 @@ TextureInfo ImageClientSingle::GetTextur
 {
   return TextureInfo(COMPOSITABLE_IMAGE);
 }
 
 void
 ImageClientSingle::FlushAllImages(bool aExceptFront)
 {
   if (!aExceptFront && mFrontBuffer) {
-    GetForwarder()->AddForceRemovingTexture(mFrontBuffer);
+    GetForwarder()->HoldUntilTransaction(mFrontBuffer);
     mFrontBuffer = nullptr;
   }
 }
 
 void
 ImageClientBuffered::FlushAllImages(bool aExceptFront)
 {
   if (!aExceptFront && mFrontBuffer) {
-    GetForwarder()->AddForceRemovingTexture(mFrontBuffer);
+    GetForwarder()->HoldUntilTransaction(mFrontBuffer);
     mFrontBuffer = nullptr;
   }
   if (mBackBuffer) {
-    GetForwarder()->AddForceRemovingTexture(mBackBuffer);
+    GetForwarder()->HoldUntilTransaction(mBackBuffer);
     mBackBuffer = nullptr;
   }
 }
 
 bool
 ImageClientSingle::UpdateImage(ImageContainer* aContainer,
                                uint32_t aContentFlags)
 {
@@ -135,41 +135,36 @@ ImageClientSingle::UpdateImage(ImageCont
   if (mLastPaintedImageSerial == image->GetSerial()) {
     return true;
   }
 
   if (image->AsSharedImage() && image->AsSharedImage()->GetTextureClient()) {
     // fast path: no need to allocate and/or copy image data
     RefPtr<TextureClient> texture = image->AsSharedImage()->GetTextureClient();
 
-    if (texture->IsSharedWithCompositor()) {
-      // XXX - temporary fix for bug 911941
-      // This will be changed with bug 912907
-      return false;
-    }
 
     if (mFrontBuffer) {
-      GetForwarder()->AddForceRemovingTexture(mFrontBuffer);
+      GetForwarder()->HoldUntilTransaction(mFrontBuffer);
     }
     mFrontBuffer = texture;
     if (!AddTextureClient(texture)) {
       mFrontBuffer = nullptr;
       return false;
     }
     GetForwarder()->UpdatedTexture(this, texture, nullptr);
     GetForwarder()->UseTexture(this, texture);
   } else if (image->GetFormat() == ImageFormat::PLANAR_YCBCR) {
     PlanarYCbCrImage* ycbcr = static_cast<PlanarYCbCrImage*>(image);
     const PlanarYCbCrData* data = ycbcr->GetData();
     if (!data) {
       return false;
     }
 
     if (mFrontBuffer && mFrontBuffer->IsImmutable()) {
-      GetForwarder()->AddForceRemovingTexture(mFrontBuffer);
+      GetForwarder()->HoldUntilTransaction(mFrontBuffer);
       mFrontBuffer = nullptr;
     }
 
     bool bufferCreated = false;
     if (!mFrontBuffer) {
       mFrontBuffer = CreateBufferTextureClient(gfx::SurfaceFormat::YUV, TEXTURE_FLAGS_DEFAULT);
       gfx::IntSize ySize(data->mYSize.width, data->mYSize.height);
       gfx::IntSize cbCrSize(data->mCbCrSize.width, data->mCbCrSize.height);
@@ -202,17 +197,17 @@ ImageClientSingle::UpdateImage(ImageCont
     }
 
   } else if (image->GetFormat() == ImageFormat::SHARED_TEXTURE) {
     SharedTextureImage* sharedImage = static_cast<SharedTextureImage*>(image);
     const SharedTextureImage::Data *data = sharedImage->GetData();
     gfx::IntSize size = gfx::IntSize(image->GetSize().width, image->GetSize().height);
 
     if (mFrontBuffer) {
-      GetForwarder()->AddForceRemovingTexture(mFrontBuffer);
+      GetForwarder()->HoldUntilTransaction(mFrontBuffer);
       mFrontBuffer = nullptr;
     }
 
     RefPtr<SharedTextureClientOGL> buffer = new SharedTextureClientOGL(mTextureFlags);
     buffer->InitWith(data->mHandle, size, data->mShareType, data->mInverted);
     mFrontBuffer = buffer;
     if (!AddTextureClient(mFrontBuffer)) {
       mFrontBuffer = nullptr;
@@ -223,17 +218,17 @@ ImageClientSingle::UpdateImage(ImageCont
   } else {
     nsRefPtr<gfxASurface> surface = image->DeprecatedGetAsSurface();
     MOZ_ASSERT(surface);
 
     gfx::IntSize size = gfx::IntSize(image->GetSize().width, image->GetSize().height);
 
     if (mFrontBuffer &&
         (mFrontBuffer->IsImmutable() || mFrontBuffer->GetSize() != size)) {
-      GetForwarder()->AddForceRemovingTexture(mFrontBuffer);
+      GetForwarder()->HoldUntilTransaction(mFrontBuffer);
       mFrontBuffer = nullptr;
     }
 
     bool bufferCreated = false;
     if (!mFrontBuffer) {
       gfxImageFormat format
         = gfxPlatform::GetPlatform()->OptimalFormatForContent(surface->GetContentType());
       mFrontBuffer = CreateTextureClientForDrawing(gfx::ImageFormatToSurfaceFormat(format),
--- a/gfx/layers/client/TextureClient.cpp
+++ b/gfx/layers/client/TextureClient.cpp
@@ -156,18 +156,21 @@ TextureClient::DestroyIPDLActor(PTexture
 {
   static_cast<TextureChild*>(actor)->ReleaseIPDLReference();
   return true;
 }
 
 bool
 TextureClient::InitIPDLActor(CompositableForwarder* aForwarder)
 {
-  MOZ_ASSERT(!mActor);
   MOZ_ASSERT(aForwarder);
+  if (mActor && mActor->GetForwarder() == aForwarder) {
+    return true;
+  }
+  MOZ_ASSERT(!mActor, "Cannot use a texture on several IPC channels.");
 
   SurfaceDescriptor desc;
   if (!ToSurfaceDescriptor(desc)) {
     return false;
   }
 
   mActor = static_cast<TextureChild*>(aForwarder->CreateTexture(desc, GetFlags()));
   MOZ_ASSERT(mActor);
--- a/gfx/layers/ipc/CompositableForwarder.h
+++ b/gfx/layers/ipc/CompositableForwarder.h
@@ -157,36 +157,33 @@ public:
 
   /**
    * Tell the compositor side to delete the TextureHost corresponding to the
    * TextureClient passed in parameter.
    */
   virtual void RemoveTexture(TextureClient* aTexture) = 0;
 
   /**
-   * Forcibly remove texture data from TextureClient
-   * after a tansaction with Compositor.
+   * Holds a reference to a TextureClient until after the next
+   * compositor transaction, and then drops it.
    */
-  virtual void AddForceRemovingTexture(TextureClient* aClient)
+  virtual void HoldUntilTransaction(TextureClient* aClient)
   {
     if (aClient) {
-      mForceRemovingTextures.AppendElement(aClient);
+      mTexturesToRemove.AppendElement(aClient);
     }
   }
 
   /**
    * Forcibly remove texture data from TextureClient
    * This function needs to be called after a tansaction with Compositor.
    */
-  virtual void ForceRemoveTexturesIfNecessary()
+  virtual void RemoveTexturesIfNecessary()
   {
-    for (uint32_t i = 0; i < mForceRemovingTextures.Length(); i++) {
-       mForceRemovingTextures[i]->ForceRemove();
-    }
-    mForceRemovingTextures.Clear();
+    mTexturesToRemove.Clear();
   }
 
   /**
    * Tell the CompositableHost on the compositor side what texture to use for
    * the next composition.
    */
   virtual void UseTexture(CompositableClient* aCompositable,
                           TextureClient* aClient) = 0;
@@ -239,15 +236,15 @@ public:
   const TextureFactoryIdentifier& GetTextureFactoryIdentifier() const
   {
     return mTextureFactoryIdentifier;
   }
 
 protected:
   TextureFactoryIdentifier mTextureFactoryIdentifier;
   bool mMultiProcess;
-  nsTArray<RefPtr<TextureClient> > mForceRemovingTextures;
+  nsTArray<RefPtr<TextureClient> > mTexturesToRemove;
 };
 
 } // namespace
 } // namespace
 
 #endif
--- a/gfx/layers/ipc/ImageBridgeChild.cpp
+++ b/gfx/layers/ipc/ImageBridgeChild.cpp
@@ -448,37 +448,37 @@ void ImageBridgeChild::FlushAllImagesNow
 
 void
 ImageBridgeChild::BeginTransaction()
 {
   MOZ_ASSERT(mTxn->Finished(), "uncommitted txn?");
   mTxn->Begin();
 }
 
-class MOZ_STACK_CLASS AutoForceRemoveTextures
+class MOZ_STACK_CLASS AutoRemoveTextures
 {
 public:
-  AutoForceRemoveTextures(ImageBridgeChild* aImageBridge)
+  AutoRemoveTextures(ImageBridgeChild* aImageBridge)
     : mImageBridge(aImageBridge) {}
 
-  ~AutoForceRemoveTextures()
+  ~AutoRemoveTextures()
   {
-    mImageBridge->ForceRemoveTexturesIfNecessary();
+    mImageBridge->RemoveTexturesIfNecessary();
   }
 private:
   ImageBridgeChild* mImageBridge;
 };
 
 void
 ImageBridgeChild::EndTransaction()
 {
   MOZ_ASSERT(!mTxn->Finished(), "forgot BeginTransaction?");
 
   AutoEndTransaction _(mTxn);
-  AutoForceRemoveTextures autoForceRemoveTextures(this);
+  AutoRemoveTextures autoRemoveTextures(this);
 
   if (mTxn->IsEmpty()) {
     return;
   }
 
   AutoInfallibleTArray<CompositableOperation, 10> cset;
   cset.SetCapacity(mTxn->mOperations.size());
   if (!mTxn->mOperations.empty()) {