diff --git a/gfx/layers/Compositor.h b/gfx/layers/Compositor.h index 8bdccb372a7b..a924b1ae1c8b 100644 --- a/gfx/layers/Compositor.h +++ b/gfx/layers/Compositor.h @@ -533,6 +533,12 @@ public: return mParent; } + /// Most compositor backends operate asynchronously under the hood. This + /// means that when a layer stops using a texture it is often desirable to + /// wait for the end of the next composition before releasing the texture's + /// ReadLock. + /// This function provides a convenient way to do this delayed unlocking, if + /// the texture itself requires it. void UnlockAfterComposition(already_AddRefed aLock) { mUnlockAfterComposition.AppendElement(aLock); diff --git a/gfx/layers/client/TextureClient.cpp b/gfx/layers/client/TextureClient.cpp index 552d72694135..c0ee1f493a1e 100644 --- a/gfx/layers/client/TextureClient.cpp +++ b/gfx/layers/client/TextureClient.cpp @@ -452,6 +452,12 @@ TextureClient::UnlockActor() const bool TextureClient::IsReadLocked() const { + // mPendingReadUnlock is true when the texture has been written into (and as + // a result the read-count already incremented on the behalf of the compositor), + // but we haven't sent the notification for the compositor to use the texture, + // so it is still OK to access the texture data on this side. + // If we didn't take mPendingReadUnlock into account here, we would not be able + // to Lock and Unlock a texture twice before sending it. return mReadLock && mReadLock->GetReadCount() > 1 && !mPendingReadUnlock; } diff --git a/gfx/layers/composite/ImageHost.h b/gfx/layers/composite/ImageHost.h index 339d2b517d7b..e8ca547383e0 100644 --- a/gfx/layers/composite/ImageHost.h +++ b/gfx/layers/composite/ImageHost.h @@ -127,6 +127,9 @@ protected: int32_t mInputFrameID; }; + // Use a simple RefPtr because the same texture is already held by a + // a CompositableTextureHostRef in the array of TimedImage. + // See the comment in CompositableTextureRef for more details. RefPtr mCurrentTextureHost; CompositableTextureSourceRef mCurrentTextureSource; // When doing texture uploads it's best to alternate between two (or three) diff --git a/gfx/layers/composite/TextureHost.cpp b/gfx/layers/composite/TextureHost.cpp index dff4a19bd55b..f21b185a32b7 100644 --- a/gfx/layers/composite/TextureHost.cpp +++ b/gfx/layers/composite/TextureHost.cpp @@ -312,6 +312,10 @@ TextureHost::TextureHost(TextureFlags aFlags) TextureHost::~TextureHost() { + // If we still have a ReadLock, unlock it. At this point we don't care about + // the texture client being written into on the other side since it should be + // destroyed by now. But we will hit assertions if we don't ReadUnlock before + // destroying the lock itself. ReadUnlock(); MOZ_COUNT_DTOR(TextureHost); } @@ -693,6 +697,8 @@ BufferTextureHost::MaybeUpload(nsIntRegion *aRegion) } if (mHasIntermediateBuffer) { + // We just did the texture upload, the content side can now freely write + // into the shared buffer. ReadUnlock(); } diff --git a/gfx/layers/composite/TextureHost.h b/gfx/layers/composite/TextureHost.h index 7ad5691a0183..8c5285aa504e 100644 --- a/gfx/layers/composite/TextureHost.h +++ b/gfx/layers/composite/TextureHost.h @@ -168,10 +168,33 @@ protected: int mCompositableCount; }; -/** - * equivalent of a RefPtr, that calls AddCompositableRef and - * ReleaseCompositableRef in addition to the usual AddRef and Release. - */ +/// Equivalent of a RefPtr, that calls AddCompositableRef and +/// ReleaseCompositableRef in addition to the usual AddRef and Release. +/// +/// The semantoics of these CompositableTextureRefs are important because they +/// are used both as a synchronization/safety mechanism, and as an optimization +/// mechanism. They are also tricky and subtle because we use them in a very +/// implicit way (assigning to a CompositableTextureRef is less visible than +/// explicitly calling a method or whatnot). +/// It is Therefore important to be careful about the way we use this tool. +/// +/// CompositableTextureRef is a mechanism that lets us count how many compositables +/// are using a given texture (for TextureSource and TextureHost). +/// We use it to run specific code when a texture is not used anymore, and also +/// we trigger fast paths on some operations when we can see that the texture's +/// CompositableTextureRef counter is equal to 1 (the texture is not shared +/// between compositables). +/// This means that it is important to observe the following rules: +/// * CompositableHosts that receive UseTexture and similar messages *must* store +/// all of the TextureHosts they receive in CompositableTextureRef slots for as +/// long as they may be using them. +/// * CompositableHosts must store each texture in a *single* CompositableTextureRef +/// slot to ensure that the counter properly reflects how many compositables are +/// using the texture. +/// If a compositable needs to hold two references to a given texture (for example +/// to have a pointer to the current texture in a list of textures that may be +/// used), it can hold its extra references with RefPtr or whichever pointer type +/// makes sense. template class CompositableTextureRef { public: @@ -581,11 +604,11 @@ public: TextureReadLock* GetReadLock() { return mReadLock; } - void ReadUnlock(); - virtual Compositor* GetCompositor() = 0; protected: + void ReadUnlock(); + FenceHandle mReleaseFenceHandle; FenceHandle mAcquireFenceHandle; diff --git a/gfx/layers/composite/TiledContentHost.h b/gfx/layers/composite/TiledContentHost.h index 056b80d91ad9..6e1e65e45cc9 100644 --- a/gfx/layers/composite/TiledContentHost.h +++ b/gfx/layers/composite/TiledContentHost.h @@ -93,12 +93,6 @@ public: bool IsPlaceholderTile() const { return mTextureHost == nullptr; } - void ReadUnlock() { - if (mTextureHost) { - mTextureHost->ReadUnlock(); - } - } - void Dump(std::stringstream& aStream) { aStream << "TileHost(...)"; // fill in as needed }