diff --git a/gfx/layers/Compositor.cpp b/gfx/layers/Compositor.cpp index 1166febc58f6..37f281b9e18c 100644 --- a/gfx/layers/Compositor.cpp +++ b/gfx/layers/Compositor.cpp @@ -7,6 +7,7 @@ #include "base/message_loop.h" // for MessageLoop #include "mozilla/layers/CompositorBridgeParent.h" // for CompositorBridgeParent #include "mozilla/layers/Effects.h" // for Effect, EffectChain, etc +#include "mozilla/layers/TextureClient.h" #include "mozilla/layers/CompositorThread.h" #include "mozilla/mozalloc.h" // for operator delete, etc #include "gfx2DGlue.h" @@ -23,6 +24,35 @@ namespace mozilla { namespace layers { +Compositor::Compositor(widget::CompositorWidgetProxy* aWidget, + CompositorBridgeParent* aParent) + : mCompositorID(0) + , mDiagnosticTypes(DiagnosticTypes::NO_DIAGNOSTIC) + , mParent(aParent) + , mPixelsPerFrame(0) + , mPixelsFilled(0) + , mScreenRotation(ROTATION_0) + , mWidget(aWidget) +{ +} + +Compositor::~Compositor() +{ + for (auto& lock : mUnlockAfterComposition) { + lock->ReadUnlock(); + } + mUnlockAfterComposition.Clear(); +} + +void +Compositor::EndFrame() +{ + for (auto& lock : mUnlockAfterComposition) { + lock->ReadUnlock(); + } + mUnlockAfterComposition.Clear(); +} + /* static */ void Compositor::AssertOnCompositorThread() { diff --git a/gfx/layers/Compositor.h b/gfx/layers/Compositor.h index c3cb533f5a18..8bdccb372a7b 100644 --- a/gfx/layers/Compositor.h +++ b/gfx/layers/Compositor.h @@ -133,6 +133,7 @@ class CompositorOGL; class CompositorD3D9; class CompositorD3D11; class BasicCompositor; +class TextureReadLock; enum SurfaceInitMode { @@ -185,22 +186,13 @@ enum SurfaceInitMode class Compositor { protected: - virtual ~Compositor() {} + virtual ~Compositor(); public: NS_INLINE_DECL_REFCOUNTING(Compositor) explicit Compositor(widget::CompositorWidgetProxy* aWidget, - CompositorBridgeParent* aParent = nullptr) - : mCompositorID(0) - , mDiagnosticTypes(DiagnosticTypes::NO_DIAGNOSTIC) - , mParent(aParent) - , mPixelsPerFrame(0) - , mPixelsFilled(0) - , mScreenRotation(ROTATION_0) - , mWidget(aWidget) - { - } + CompositorBridgeParent* aParent = nullptr); virtual already_AddRefed CreateDataTextureSource(TextureFlags aFlags = TextureFlags::NO_FLAGS) = 0; @@ -388,8 +380,10 @@ public: /** * Flush the current frame to the screen and tidy up. + * + * Derived class overriding this should call Compositor::EndFrame. */ - virtual void EndFrame() = 0; + virtual void EndFrame(); virtual void SetDispAcquireFence(Layer* aLayer); @@ -539,6 +533,11 @@ public: return mParent; } + void UnlockAfterComposition(already_AddRefed aLock) + { + mUnlockAfterComposition.AppendElement(aLock); + } + protected: void DrawDiagnosticsInternal(DiagnosticFlags aFlags, const gfx::Rect& aVisibleRect, @@ -563,6 +562,11 @@ protected: gfx::Matrix4x4* aOutTransform, gfx::Rect* aOutLayerQuad = nullptr); + /** + * An array of locks that will need to be unlocked after the next composition. + */ + nsTArray> mUnlockAfterComposition; + /** * Render time for the current composition. */ diff --git a/gfx/layers/TextureDIB.cpp b/gfx/layers/TextureDIB.cpp index 4366db27e5ff..a021154e1408 100644 --- a/gfx/layers/TextureDIB.cpp +++ b/gfx/layers/TextureDIB.cpp @@ -436,6 +436,8 @@ DIBTextureHost::UpdatedInternal(const nsIntRegion* aRegion) if (!mTextureSource->Update(surf, const_cast(aRegion))) { mTextureSource = nullptr; } + + ReadUnlock(); } TextureHostFileMapping::TextureHostFileMapping(TextureFlags aFlags, @@ -484,6 +486,8 @@ TextureHostFileMapping::UpdatedInternal(const nsIntRegion* aRegion) } else { mTextureSource = nullptr; } + + ReadUnlock(); } } diff --git a/gfx/layers/TextureDIB.h b/gfx/layers/TextureDIB.h index 52d310f17454..e9cf3acedabf 100644 --- a/gfx/layers/TextureDIB.h +++ b/gfx/layers/TextureDIB.h @@ -66,6 +66,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual gfx::SurfaceFormat GetFormat() const override { return mFormat; } virtual gfx::IntSize GetSize() const override { return mSize; } diff --git a/gfx/layers/basic/BasicCompositor.cpp b/gfx/layers/basic/BasicCompositor.cpp index 81d52290ddb7..f9d66d2d334f 100644 --- a/gfx/layers/basic/BasicCompositor.cpp +++ b/gfx/layers/basic/BasicCompositor.cpp @@ -650,6 +650,8 @@ BasicCompositor::BeginFrame(const nsIntRegion& aInvalidRegion, void BasicCompositor::EndFrame() { + Compositor::EndFrame(); + // Pop aClipRectIn/bounds rect mRenderTarget->mDrawTarget->PopClip(); diff --git a/gfx/layers/basic/GrallocTextureHostBasic.cpp b/gfx/layers/basic/GrallocTextureHostBasic.cpp index fa950896ece4..c68490fd9e1c 100644 --- a/gfx/layers/basic/GrallocTextureHostBasic.cpp +++ b/gfx/layers/basic/GrallocTextureHostBasic.cpp @@ -162,6 +162,7 @@ GrallocTextureHostBasic::BindTextureSource(CompositableTextureSourceRef& aTextur void GrallocTextureHostBasic::UnbindTextureSource() { + TextureHost::UnbindTextureSource(); ClearTextureSource(); } diff --git a/gfx/layers/basic/GrallocTextureHostBasic.h b/gfx/layers/basic/GrallocTextureHostBasic.h index d72f67e86a3b..abae76f014b2 100644 --- a/gfx/layers/basic/GrallocTextureHostBasic.h +++ b/gfx/layers/basic/GrallocTextureHostBasic.h @@ -28,6 +28,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override; + virtual bool Lock() override; virtual gfx::SurfaceFormat GetFormat() const override; diff --git a/gfx/layers/basic/MacIOSurfaceTextureHostBasic.h b/gfx/layers/basic/MacIOSurfaceTextureHostBasic.h index 3dd6afb468f7..7949aecfca1b 100644 --- a/gfx/layers/basic/MacIOSurfaceTextureHostBasic.h +++ b/gfx/layers/basic/MacIOSurfaceTextureHostBasic.h @@ -62,6 +62,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual bool Lock() override; virtual gfx::SurfaceFormat GetFormat() const override; diff --git a/gfx/layers/client/SingleTiledContentClient.cpp b/gfx/layers/client/SingleTiledContentClient.cpp index d5b7a7525872..b7e5090f016c 100644 --- a/gfx/layers/client/SingleTiledContentClient.cpp +++ b/gfx/layers/client/SingleTiledContentClient.cpp @@ -30,11 +30,6 @@ SingleTiledContentClient::ClearCachedResources() void SingleTiledContentClient::UpdatedBuffer(TiledBufferType aType) { - // Take a ReadLock on behalf of the TiledContentHost. This - // reference will be adopted when the descriptor is opened in - // TiledLayerBufferComposite. - mTiledBuffer->ReadLock(); - mForwarder->UseTiledLayerBuffer(this, mTiledBuffer->GetSurfaceDescriptorTiles()); mTiledBuffer->ClearPaintedRegion(); } @@ -56,13 +51,6 @@ ClientSingleTiledLayerBuffer::ClientSingleTiledLayerBuffer(ClientTiledPaintedLay { } -void -ClientSingleTiledLayerBuffer::ReadLock() { - if (!mTile.IsPlaceholderTile()) { - mTile.ReadLock(); - } -} - void ClientSingleTiledLayerBuffer::ReleaseTiles() { diff --git a/gfx/layers/client/SingleTiledContentClient.h b/gfx/layers/client/SingleTiledContentClient.h index 42b5359a6319..0302d7d884af 100644 --- a/gfx/layers/client/SingleTiledContentClient.h +++ b/gfx/layers/client/SingleTiledContentClient.h @@ -70,8 +70,6 @@ public: return false; } - void ReadLock(); - void ReleaseTiles(); void DiscardBuffers(); diff --git a/gfx/layers/client/TextureClient.cpp b/gfx/layers/client/TextureClient.cpp index 4dfc33cf407a..552d72694135 100644 --- a/gfx/layers/client/TextureClient.cpp +++ b/gfx/layers/client/TextureClient.cpp @@ -452,7 +452,7 @@ TextureClient::UnlockActor() const bool TextureClient::IsReadLocked() const { - return mReadLock && mReadLock->GetReadCount() > 1; + return mReadLock && mReadLock->GetReadCount() > 1 && !mPendingReadUnlock; } bool @@ -528,6 +528,15 @@ TextureClient::Unlock() mBorrowedDrawTarget = nullptr; } + if (mOpenMode & OpenMode::OPEN_WRITE) { + if (mReadLock && !mPendingReadUnlock) { + // Take a read lock on behalf of the TextureHost. The latter will unlock + // after the shared data is available again for drawing. + mReadLock->ReadLock(); + } + mPendingReadUnlock = true; + } + mData->Unlock(); mIsLocked = false; mOpenMode = OpenMode::OPEN_NONE; @@ -535,8 +544,43 @@ TextureClient::Unlock() UnlockActor(); } +void +TextureClient::EnableReadLock() +{ + if (!mReadLock) { + mReadLock = TextureReadLock::Create(mAllocator); + if (mPendingReadUnlock) { + // We would have done this during TextureClient::Unlock if the ReadLock + // had been there, need to account for this here. + mReadLock->ReadLock(); + } + } +} + +void +TextureClient::SetReadLock(TextureReadLock* aLock) +{ + MOZ_ASSERT(!mReadLock); + mReadLock = aLock; +} + +void +TextureClient::SerializeReadLock(ReadLockDescriptor& aDescriptor) +{ + if (mReadLock && mPendingReadUnlock) { + mReadLock->Serialize(aDescriptor); + mPendingReadUnlock = false; + } else { + aDescriptor = null_t(); + } +} + TextureClient::~TextureClient() { + if (mPendingReadUnlock && mReadLock) { + mReadLock->ReadUnlock(); + mReadLock = nullptr; + } Destroy(false); } @@ -1044,6 +1088,7 @@ TextureClient::TextureClient(TextureData* aData, TextureFlags aFlags, ClientIPCA , mExpectedDtRefs(0) #endif , mIsLocked(false) +, mPendingReadUnlock(false) , mInUse(false) , mAddedToCompositableClient(false) , mWorkaroundAnnoyingSharedSurfaceLifetimeIssues(false) @@ -1174,11 +1219,19 @@ public: virtual bool IsValid() const override { return true; }; - virtual bool Serialize(TileLock& aOutput) override; + virtual bool Serialize(ReadLockDescriptor& aOutput) override; int32_t mReadCount; }; +// The cross-prcess implementation of TextureReadLock. +// +// Since we don't use cross-process reference counting for the ReadLock objects, +// we use the lock's internal counter as a way to know when to deallocate the +// underlying shmem section: when the counter is equal to 1, it means that the +// lock is not "held" (the texture is writable), when the counter is equal to 0 +// it means that we can safely deallocate the shmem section without causing a race +// condition with the other process. class ShmemTextureReadLock : public TextureReadLock { public: struct ShmReadLockInfo { @@ -1199,7 +1252,7 @@ public: virtual LockType GetType() override { return TYPE_SHMEM; } - virtual bool Serialize(TileLock& aOutput) override; + virtual bool Serialize(ReadLockDescriptor& aOutput) override; mozilla::layers::ShmemSection& GetShmemSection() { return mShmemSection; } @@ -1224,31 +1277,43 @@ public: // static already_AddRefed -TextureReadLock::Open(const TileLock& aDescriptor, ISurfaceAllocator* aAllocator) +TextureReadLock::Deserialize(const ReadLockDescriptor& aDescriptor, ISurfaceAllocator* aAllocator) { - RefPtr lock; - if (aDescriptor.type() == TileLock::TShmemSection) { - const ShmemSection& section = aDescriptor.get_ShmemSection(); - MOZ_RELEASE_ASSERT(section.shmem().IsReadable()); - lock = new ShmemTextureReadLock(aAllocator, section); - } else { - if (!aAllocator->IsSameProcess()) { - // Trying to use a memory based lock instead of a shmem based one in - // the cross-process case is a bad security violation. - NS_ERROR("A client process may be trying to peek at the host's address space!"); + switch (aDescriptor.type()) { + case ReadLockDescriptor::TShmemSection: { + const ShmemSection& section = aDescriptor.get_ShmemSection(); + MOZ_RELEASE_ASSERT(section.shmem().IsReadable()); + return MakeAndAddRef(aAllocator, section); + } + case ReadLockDescriptor::Tuintptr_t: { + if (!aAllocator->IsSameProcess()) { + // Trying to use a memory based lock instead of a shmem based one in + // the cross-process case is a bad security violation. + NS_ERROR("A client process may be trying to peek at the host's address space!"); + return nullptr; + } + RefPtr lock = reinterpret_cast( + aDescriptor.get_uintptr_t() + ); + + MOZ_ASSERT(lock); + if (lock) { + // The corresponding AddRef is in MemoryTextureReadLock::Serialize + lock.get()->Release(); + } + + return lock.forget(); + } + case ReadLockDescriptor::Tnull_t: { return nullptr; } - lock = reinterpret_cast(aDescriptor.get_uintptr_t()); - MOZ_ASSERT(lock); - if (lock) { - // The corresponding AddRef is in MemoryTextureReadLock::Serialize - lock.get()->Release(); + default: { + // Invalid descriptor. + MOZ_DIAGNOSTIC_ASSERT(false); } } - - return lock.forget(); + return nullptr; } - // static already_AddRefed TextureReadLock::Create(ClientIPCAllocator* aAllocator) @@ -1276,13 +1341,13 @@ MemoryTextureReadLock::~MemoryTextureReadLock() } bool -MemoryTextureReadLock::Serialize(TileLock& aOutput) +MemoryTextureReadLock::Serialize(ReadLockDescriptor& aOutput) { // AddRef here and Release when receiving on the host side to make sure the // reference count doesn't go to zero before the host receives the message. - // see TextureReadLock::Open + // see TextureReadLock::Deserialize this->AddRef(); - aOutput = TileLock(uintptr_t(this)); + aOutput = ReadLockDescriptor(uintptr_t(this)); return true; } @@ -1339,9 +1404,9 @@ ShmemTextureReadLock::~ShmemTextureReadLock() } bool -ShmemTextureReadLock::Serialize(TileLock& aOutput) +ShmemTextureReadLock::Serialize(ReadLockDescriptor& aOutput) { - aOutput = TileLock(GetShmemSection()); + aOutput = ReadLockDescriptor(GetShmemSection()); return true; } diff --git a/gfx/layers/client/TextureClient.h b/gfx/layers/client/TextureClient.h index 5a855edec52b..f28b514685b9 100644 --- a/gfx/layers/client/TextureClient.h +++ b/gfx/layers/client/TextureClient.h @@ -177,9 +177,27 @@ struct MappedYCbCrTextureData { } }; -class TileLock; +class ReadLockDescriptor; -// A class to help implement copy-on-write semantics for shared tiles. +// A class to help implement copy-on-write semantics for shared textures. +// +// A TextureClient/Host pair can opt into using a ReadLock by calling +// TextureClient::EnableReadLock. This will equip the TextureClient with a +// ReadLock object that will be automatically ReadLock()'ed by the texture itself +// when it is written into (see TextureClient::Unlock). +// A TextureReadLock's counter starts at 1 and is expected to be equal to 1 when the +// lock is destroyed. See ShmemTextureReadLock for explanations about why we use +// 1 instead of 0 as the initial state. +// TextureReadLock is mostly internally managed by the TextureClient/Host pair, +// and the compositable only has to forward it during updates. If an update message +// contains a null_t lock, it means that the texture was not written into on the +// content side, and there is no synchronization required on the compositor side +// (or it means that the texture pair did not opt into using ReadLocks). +// On the compositor side, the TextureHost can receive a ReadLock during a +// transaction, and will both ReadUnlock() it and drop it as soon as the shared +// data is available again for writing (the texture upload is done, or the compositor +// not reading the texture anymore). The lock is dropped to make sure it is +// ReadUnlock()'ed only once. class TextureReadLock { protected: virtual ~TextureReadLock() {} @@ -196,9 +214,9 @@ public: Create(ClientIPCAllocator* aAllocator); static already_AddRefed - Open(const TileLock& aDescriptor, ISurfaceAllocator* aAllocator); + Deserialize(const ReadLockDescriptor& aDescriptor, ISurfaceAllocator* aAllocator); - virtual bool Serialize(TileLock& aOutput) = 0; + virtual bool Serialize(ReadLockDescriptor& aOutput) = 0; enum LockType { TYPE_MEMORY, @@ -632,12 +650,17 @@ public: virtual void RemoveFromCompositable(CompositableClient* aCompositable, AsyncTransactionWaiter* aWaiter = nullptr); - void SetReadLock(already_AddRefed aLock) { mReadLock = aLock; } + + void EnableReadLock(); + + void SetReadLock(TextureReadLock* aLock); TextureReadLock* GetReadLock() { return mReadLock; } bool IsReadLocked() const; + void SerializeReadLock(ReadLockDescriptor& aDescriptor); + private: static void TextureClientRecycleCallback(TextureClient* aClient, void* aClosure); @@ -687,6 +710,10 @@ protected: uint32_t mExpectedDtRefs; #endif bool mIsLocked; + // True when there has been a modification of the texture that hasn't been sent + // to the compositor yet. We keep track of this to avoid sending the texture's + // ReadLock twice after a single update. + bool mPendingReadUnlock; bool mInUse; bool mAddedToCompositableClient; diff --git a/gfx/layers/client/TiledContentClient.cpp b/gfx/layers/client/TiledContentClient.cpp index 6f97a6454be6..47873ce59a1a 100644 --- a/gfx/layers/client/TiledContentClient.cpp +++ b/gfx/layers/client/TiledContentClient.cpp @@ -119,11 +119,6 @@ MultiTiledContentClient::UpdatedBuffer(TiledBufferType aType) MOZ_ASSERT(aType != LOW_PRECISION_TILED_BUFFER || mHasLowPrecision); - // Take a ReadLock on behalf of the TiledContentHost. This - // reference will be adopted when the descriptor is opened in - // TiledLayerBufferComposite. - buffer->ReadLock(); - mForwarder->UseTiledLayerBuffer(this, buffer->GetSurfaceDescriptorTiles()); buffer->ClearPaintedRegion(); } @@ -570,10 +565,6 @@ TileClient::DiscardFrontBuffer() mAllocator->ReturnTextureClientDeferred(mFrontBuffer); if (mFrontBufferOnWhite) { - // TODO: right now we use he same ReadLock for the texture on black and - // the texture on white, but we may change that in the near future. - RefPtr lock = mFrontBuffer->GetReadLock(); - mFrontBuffer->SetReadLock(lock.forget()); mFrontBufferOnWhite->RemoveFromCompositable(mCompositableClient); mAllocator->ReturnTextureClientDeferred(mFrontBufferOnWhite); } @@ -588,38 +579,63 @@ TileClient::DiscardFrontBuffer() } } -void -TileClient::DiscardBackBuffer() +static void +DiscardTexture(TextureClient* aTexture, TextureClientAllocator* aAllocator) { - if (mBackBuffer) { - MOZ_ASSERT(mBackBuffer->GetReadLock()); - if (!mBackBuffer->HasSynchronization() && mBackBuffer->IsReadLocked()) { + if (aTexture) { + MOZ_ASSERT(aTexture->GetReadLock()); + if (!aTexture->HasSynchronization() && aTexture->IsReadLocked()) { // Our current back-buffer is still locked by the compositor. This can occur // when the client is producing faster than the compositor can consume. In // this case we just want to drop it and not return it to the pool. - mAllocator->ReportClientLost(); - if (mBackBufferOnWhite) { - mAllocator->ReportClientLost(); - } + aAllocator->ReportClientLost(); } else { - mAllocator->ReturnTextureClientDeferred(mBackBuffer); - if (mBackBufferOnWhite) { - RefPtr lock = mBackBuffer->GetReadLock(); - mBackBufferOnWhite->SetReadLock(lock.forget()); - mAllocator->ReturnTextureClientDeferred(mBackBufferOnWhite); - } + aAllocator->ReturnTextureClientDeferred(aTexture); } - if (mBackBuffer->IsLocked()) { - mBackBuffer->Unlock(); + if (aTexture->IsLocked()) { + aTexture->Unlock(); } - if (mBackBufferOnWhite && mBackBufferOnWhite->IsLocked()) { - mBackBufferOnWhite->Unlock(); - } - mBackBuffer.Set(this, nullptr); - mBackBufferOnWhite = nullptr; } } +void +TileClient::DiscardBackBuffer() +{ + DiscardTexture(mBackBuffer, mAllocator); + mBackBuffer.Set(this, nullptr); + DiscardTexture(mBackBufferOnWhite, mAllocator); + mBackBufferOnWhite = nullptr; +} + +static already_AddRefed +CreateBackBufferTexture(TextureClient* aCurrentTexture, + CompositableClient* aCompositable, + TextureClientAllocator* aAllocator) +{ + if (aCurrentTexture) { + // Our current back-buffer is still locked by the compositor. This can occur + // when the client is producing faster than the compositor can consume. In + // this case we just want to drop it and not return it to the pool. + aAllocator->ReportClientLost(); + } + + RefPtr texture = aAllocator->GetTextureClient(); + + if (!texture) { + gfxCriticalError() << "[Tiling:Client] Failed to allocate a TextureClient"; + return nullptr; + } + + texture->EnableReadLock(); + + if (!aCompositable->AddTextureClient(texture)) { + gfxCriticalError() << "[Tiling:Client] Failed to connect a TextureClient"; + return nullptr; + } + + return texture.forget(); +} + TextureClient* TileClient::GetBackBuffer(const nsIntRegion& aDirtyRegion, gfxContentType aContent, @@ -627,49 +643,53 @@ TileClient::GetBackBuffer(const nsIntRegion& aDirtyRegion, nsIntRegion& aAddPaintedRegion, RefPtr* aBackBufferOnWhite) { + if (aMode != SurfaceMode::SURFACE_COMPONENT_ALPHA) { + // It can happen that a component-alpha layer stops being on component alpha + // on the next frame, just drop the buffers on white if that happens. + if (mBackBufferOnWhite) { + mAllocator->ReportClientLost(); + mBackBufferOnWhite = nullptr; + } + if (mFrontBufferOnWhite) { + mAllocator->ReportClientLost(); + mFrontBufferOnWhite = nullptr; + } + } + // Try to re-use the front-buffer if possible - bool createdTextureClient = false; if (mFrontBuffer && mFrontBuffer->HasIntermediateBuffer() && !mFrontBuffer->IsReadLocked() && - !(aMode == SurfaceMode::SURFACE_COMPONENT_ALPHA && !mFrontBufferOnWhite)) { + (aMode != SurfaceMode::SURFACE_COMPONENT_ALPHA || ( + mFrontBufferOnWhite && !mFrontBufferOnWhite->IsReadLocked()))) { // If we had a backbuffer we no longer care about it since we'll // re-use the front buffer. DiscardBackBuffer(); Flip(); } else { if (!mBackBuffer || mBackBuffer->IsReadLocked()) { - - if (mBackBuffer) { - // Our current back-buffer is still locked by the compositor. This can occur - // when the client is producing faster than the compositor can consume. In - // this case we just want to drop it and not return it to the pool. - mAllocator->ReportClientLost(); - } - if (mBackBufferOnWhite) { - mAllocator->ReportClientLost(); - mBackBufferOnWhite = nullptr; - } - - mBackBuffer.Set(this, mAllocator->GetTextureClient()); + mBackBuffer.Set(this, + CreateBackBufferTexture(mBackBuffer, mCompositableClient, mAllocator) + ); + mInvalidBack = IntRect(0, 0, mBackBuffer->GetSize().width, mBackBuffer->GetSize().height); if (!mBackBuffer) { - gfxCriticalError() << "[Tiling:Client] Failed to allocate a TextureClient (B)"; + DiscardBackBuffer(); + DiscardFrontBuffer(); return nullptr; } + } - if (aMode == SurfaceMode::SURFACE_COMPONENT_ALPHA) { - mBackBufferOnWhite = mAllocator->GetTextureClient(); - if (!mBackBufferOnWhite) { - mBackBuffer.Set(this, nullptr); - gfxCriticalError() << "[Tiling:Client] Failed to allocate a TextureClient (W)"; - return nullptr; - } - } - - mBackBuffer->SetReadLock(TextureReadLock::Create(mManager->AsShadowForwarder())); - - createdTextureClient = true; + if (aMode == SurfaceMode::SURFACE_COMPONENT_ALPHA + && (!mBackBufferOnWhite || mBackBufferOnWhite->IsReadLocked())) { + mBackBufferOnWhite = CreateBackBufferTexture( + mBackBufferOnWhite, mCompositableClient, mAllocator + ); mInvalidBack = IntRect(0, 0, mBackBuffer->GetSize().width, mBackBuffer->GetSize().height); + if (!mBackBufferOnWhite) { + DiscardBackBuffer(); + DiscardFrontBuffer(); + return nullptr; + } } ValidateBackBufferFromFront(aDirtyRegion, aAddPaintedRegion); @@ -693,21 +713,6 @@ TileClient::GetBackBuffer(const nsIntRegion& aDirtyRegion, } } - if (createdTextureClient) { - if (!mCompositableClient->AddTextureClient(mBackBuffer)) { - gfxCriticalError() << "[Tiling:Client] Failed to connect a TextureClient (a)"; - DiscardFrontBuffer(); - DiscardBackBuffer(); - return nullptr; - } - if (mBackBufferOnWhite && !mCompositableClient->AddTextureClient(mBackBufferOnWhite)) { - gfxCriticalError() << "[Tiling:Client] Failed to connect a TextureClient (b)"; - DiscardFrontBuffer(); - DiscardBackBuffer(); - return nullptr; - } - } - *aBackBufferOnWhite = mBackBufferOnWhite; return mBackBuffer; } @@ -722,25 +727,21 @@ TileClient::GetTileDescriptor() bool wasPlaceholder = mWasPlaceholder; mWasPlaceholder = false; - TileLock lock; - mFrontBuffer->GetReadLock()->Serialize(lock); + ReadLockDescriptor lock; + mFrontBuffer->SerializeReadLock(lock); + + ReadLockDescriptor lockOnWhite = null_t(); + if (mFrontBufferOnWhite) { + mFrontBufferOnWhite->SerializeReadLock(lockOnWhite); + } return TexturedTileDescriptor(nullptr, mFrontBuffer->GetIPDLActor(), mFrontBufferOnWhite ? MaybeTexture(mFrontBufferOnWhite->GetIPDLActor()) : MaybeTexture(null_t()), mUpdateRect, - lock, + lock, lockOnWhite, wasPlaceholder); } -void -ClientMultiTiledLayerBuffer::ReadLock() { - for (TileClient& tile : mRetainedTiles) { - if (!tile.IsPlaceholderTile()) { - tile.ReadLock(); - } - } -} - void ClientMultiTiledLayerBuffer::DiscardBuffers() { diff --git a/gfx/layers/client/TiledContentClient.h b/gfx/layers/client/TiledContentClient.h index d9f71bef8d41..d2be77c38076 100644 --- a/gfx/layers/client/TiledContentClient.h +++ b/gfx/layers/client/TiledContentClient.h @@ -90,22 +90,6 @@ struct TileClient return mBackBuffer == nullptr && mFrontBuffer == nullptr; } - void ReadUnlock() - { - MOZ_ASSERT(mFrontBuffer && mFrontBuffer->GetReadLock()); - if (mFrontBuffer && mFrontBuffer->GetReadLock()) { - mFrontBuffer->GetReadLock()->ReadUnlock(); - } - } - - void ReadLock() - { - MOZ_ASSERT(mFrontBuffer && mFrontBuffer->GetReadLock()); - if (mFrontBuffer && mFrontBuffer->GetReadLock()) { - mFrontBuffer->GetReadLock()->ReadLock(); - } - } - void DiscardBuffers() { DiscardFrontBuffer(); diff --git a/gfx/layers/composite/TextureHost.cpp b/gfx/layers/composite/TextureHost.cpp index d3b8d07473ba..ffede37f978e 100644 --- a/gfx/layers/composite/TextureHost.cpp +++ b/gfx/layers/composite/TextureHost.cpp @@ -324,6 +324,26 @@ void TextureHost::Finalize() } } +void +TextureHost::UnbindTextureSource() +{ + if (mReadLock) { + auto compositor = GetCompositor(); + // This TextureHost is not used anymore. Since most compositor backends are + // working asynchronously under the hood a compositor could still be using + // this texture, so it is generally best to wait until the end of the next + // composition before calling ReadUnlock. We ask the compositor to take care + // of that for us. + if (compositor) { + compositor->UnlockAfterComposition(mReadLock.forget()); + } else { + // GetCompositor returned null which means no compositor can be using this + // texture. We can ReadUnlock right away. + ReadUnlock(); + } + } +} + void TextureHost::RecycleTexture(TextureFlags aFlags) { @@ -503,9 +523,18 @@ BufferTextureHost::Unlock() } void -TextureHost::SetReadLock(already_AddRefed aLock) +TextureHost::DeserializeReadLock(const ReadLockDescriptor& aDesc, + ISurfaceAllocator* aAllocator) { - mReadLock = aLock; + RefPtr lock = TextureReadLock::Deserialize(aDesc, aAllocator); + if (!lock) { + return; + } + + // If mReadLock is not null it means we haven't unlocked it yet and the content + // side should not have been able to write into this texture and send a new lock! + MOZ_ASSERT(!mReadLock); + mReadLock = lock.forget(); } void diff --git a/gfx/layers/composite/TextureHost.h b/gfx/layers/composite/TextureHost.h index 2d42f2180638..79650f7def52 100644 --- a/gfx/layers/composite/TextureHost.h +++ b/gfx/layers/composite/TextureHost.h @@ -41,6 +41,7 @@ namespace layers { class BufferDescriptor; class Compositor; class CompositableParentManager; +class ReadLockDescriptor; class CompositorBridgeParent; class SurfaceDescriptor; class HostIPCAllocator; @@ -409,7 +410,7 @@ public: /** * Called when another TextureHost will take over. */ - virtual void UnbindTextureSource() {} + virtual void UnbindTextureSource(); /** * Is called before compositing if the shared data has changed since last @@ -575,12 +576,15 @@ public: virtual FenceHandle GetCompositorReleaseFence() { return FenceHandle(); } - void SetReadLock(already_AddRefed aLock); + void DeserializeReadLock(const ReadLockDescriptor& aDesc, + ISurfaceAllocator* aAllocator); TextureReadLock* GetReadLock() { return mReadLock; } void ReadUnlock(); + virtual Compositor* GetCompositor() = 0; + protected: FenceHandle mReleaseFenceHandle; @@ -634,6 +638,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + /** * Return the format that is exposed to the compositor when calling * BindTextureSource. diff --git a/gfx/layers/composite/TiledContentHost.cpp b/gfx/layers/composite/TiledContentHost.cpp index f2891ff81d63..deae4b2fb673 100644 --- a/gfx/layers/composite/TiledContentHost.cpp +++ b/gfx/layers/composite/TiledContentHost.cpp @@ -186,23 +186,6 @@ UseTileTexture(CompositableTextureHostRef& aTexture, aTexture->PrepareTextureSource(aTextureSource); } -void -TiledLayerBufferComposite::MarkTilesForUnlock() -{ - // Tiles without an internal buffer will have internal locks - // held by the gpu driver until the previous draw operation has finished. - // We don't know when that will be exactly, so wait until we start the - // next composite before unlocking. - for (TileHost& tile : mRetainedTiles) { - // Tile with an internal buffer get unlocked as soon as we've uploaded, - // so won't have a lock at this point. - if (tile.mTextureHost && tile.mTextureHost->GetReadLock()) { - mDelayedUnlocks.AppendElement(tile.mTextureHost->GetReadLock()); - tile.mTextureHost->SetReadLock(nullptr); - } - } -} - class TextureSourceRecycler { public: @@ -297,15 +280,10 @@ TiledLayerBufferComposite::UseTiles(const SurfaceDescriptorTiles& aTiles, const InfallibleTArray& tileDescriptors = aTiles.tiles(); - // Step 1, unlock all the old tiles that haven't been unlocked yet. Any tiles that - // exist in both the old and new sets will have been locked again by content, so this - // doesn't result in the surface being writeable again. - MarkTilesForUnlock(); - TextureSourceRecycler oldRetainedTiles(Move(mRetainedTiles)); mRetainedTiles.SetLength(tileDescriptors.Length()); - // Step 2, deserialize the incoming set of tiles into mRetainedTiles, and attempt + // Step 1, deserialize the incoming set of tiles into mRetainedTiles, and attempt // to recycle the TextureSource for any repeated tiles. // // Since we don't have any retained 'tile' object, we have to search for instances @@ -327,16 +305,15 @@ TiledLayerBufferComposite::UseTiles(const SurfaceDescriptorTiles& aTiles, tile.mTextureHost = TextureHost::AsTextureHost(texturedDesc.textureParent()); tile.mTextureHost->SetCompositor(aCompositor); - - tile.mTextureHost->SetReadLock(TextureReadLock::Open(texturedDesc.sharedLock(), aAllocator)); - - if (!tile.mTextureHost->GetReadLock()) { - return false; - } + tile.mTextureHost->DeserializeReadLock(texturedDesc.sharedLock(), aAllocator); if (texturedDesc.textureOnWhite().type() == MaybeTexture::TPTextureParent) { - tile.mTextureHostOnWhite = - TextureHost::AsTextureHost(texturedDesc.textureOnWhite().get_PTextureParent()); + tile.mTextureHostOnWhite = TextureHost::AsTextureHost( + texturedDesc.textureOnWhite().get_PTextureParent() + ); + tile.mTextureHostOnWhite->DeserializeReadLock( + texturedDesc.sharedLockOnWhite(), aAllocator + ); } tile.mTilePosition = newTiles.TilePosition(i); @@ -360,7 +337,7 @@ TiledLayerBufferComposite::UseTiles(const SurfaceDescriptorTiles& aTiles, } } - // Step 3, attempt to recycle unused texture sources from the old tile set into new tiles. + // Step 2, attempt to recycle unused texture sources from the old tile set into new tiles. // // For gralloc, binding a new TextureHost to the existing TextureSource is the fastest way // to ensure that any implicit locking on the old gralloc image is released. @@ -371,7 +348,7 @@ TiledLayerBufferComposite::UseTiles(const SurfaceDescriptorTiles& aTiles, oldRetainedTiles.RecycleTextureSource(tile); } - // Step 4, handle the texture uploads, texture source binding and release the + // Step 3, handle the texture uploads, texture source binding and release the // copy-on-write locks for textures with an internal buffer. for (size_t i = 0; i < mRetainedTiles.Length(); i++) { TileHost& tile = mRetainedTiles[i]; @@ -393,12 +370,6 @@ TiledLayerBufferComposite::UseTiles(const SurfaceDescriptorTiles& aTiles, texturedDesc.updateRect(), aCompositor); } - - if (tile.mTextureHost->HasIntermediateBuffer()) { - // Now that we did the texture upload (in UseTileTexture), we can release - // the lock. - tile.ReadUnlock(); - } } mTiles = newTiles; @@ -412,23 +383,10 @@ TiledLayerBufferComposite::UseTiles(const SurfaceDescriptorTiles& aTiles, return true; } -void -TiledLayerBufferComposite::ProcessDelayedUnlocks() -{ - for (TextureReadLock* lock : mDelayedUnlocks) { - lock->ReadUnlock(); - } - mDelayedUnlocks.Clear(); -} - void TiledLayerBufferComposite::Clear() { - for (TileHost& tile : mRetainedTiles) { - tile.ReadUnlock(); - } mRetainedTiles.Clear(); - ProcessDelayedUnlocks(); mTiles.mFirst = TileIntPoint(); mTiles.mSize = TileIntSize(); mValidRegion = nsIntRegion(); @@ -490,8 +448,6 @@ TiledContentHost::Composite(LayerComposite* aLayer, aFilter, aClipRect, *renderRegion, aTransform); RenderLayerBuffer(mTiledBuffer, nullptr, aEffectChain, aOpacity, aFilter, aClipRect, *renderRegion, aTransform); - mLowPrecisionTiledBuffer.ProcessDelayedUnlocks(); - mTiledBuffer.ProcessDelayedUnlocks(); } diff --git a/gfx/layers/composite/TiledContentHost.h b/gfx/layers/composite/TiledContentHost.h index 4ab35f8d0a03..056b80d91ad9 100644 --- a/gfx/layers/composite/TiledContentHost.h +++ b/gfx/layers/composite/TiledContentHost.h @@ -140,9 +140,6 @@ public: void Clear(); - void MarkTilesForUnlock(); - void ProcessDelayedUnlocks(); - TileHost GetPlaceholderTile() const { return TileHost(); } // Stores the absolute resolution of the containing frame, calculated @@ -155,7 +152,6 @@ public: protected: CSSToParentLayerScale2D mFrameResolution; - nsTArray> mDelayedUnlocks; }; /** diff --git a/gfx/layers/composite/X11TextureHost.h b/gfx/layers/composite/X11TextureHost.h index 7c6cc7d72bb2..1f1d34409c32 100644 --- a/gfx/layers/composite/X11TextureHost.h +++ b/gfx/layers/composite/X11TextureHost.h @@ -34,6 +34,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual bool Lock() override; virtual gfx::SurfaceFormat GetFormat() const override; diff --git a/gfx/layers/d3d11/CompositorD3D11.cpp b/gfx/layers/d3d11/CompositorD3D11.cpp index a6f1d2b8b8b1..5653fa71c572 100644 --- a/gfx/layers/d3d11/CompositorD3D11.cpp +++ b/gfx/layers/d3d11/CompositorD3D11.cpp @@ -1205,6 +1205,8 @@ CompositorD3D11::BeginFrame(const nsIntRegion& aInvalidRegion, void CompositorD3D11::EndFrame() { + Compositor::EndFrame(); + if (!mDefaultRT) { return; } diff --git a/gfx/layers/d3d11/TextureD3D11.cpp b/gfx/layers/d3d11/TextureD3D11.cpp index 86907a8a1fef..bdeec4599523 100644 --- a/gfx/layers/d3d11/TextureD3D11.cpp +++ b/gfx/layers/d3d11/TextureD3D11.cpp @@ -684,6 +684,12 @@ DXGITextureHostD3D11::SetCompositor(Compositor* aCompositor) } } +Compositor* +DXGITextureHostD3D11::GetCompositor() +{ + return mCompositor; +} + bool DXGITextureHostD3D11::Lock() { @@ -807,6 +813,12 @@ DXGIYCbCrTextureHostD3D11::SetCompositor(Compositor* aCompositor) } } +Compositor* +DXGIYCbCrTextureHostD3D11::GetCompositor() +{ + return mCompositor; +} + bool DXGIYCbCrTextureHostD3D11::Lock() { diff --git a/gfx/layers/d3d11/TextureD3D11.h b/gfx/layers/d3d11/TextureD3D11.h index ddaa71b38aef..50e069b93b35 100644 --- a/gfx/layers/d3d11/TextureD3D11.h +++ b/gfx/layers/d3d11/TextureD3D11.h @@ -281,6 +281,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override; + virtual gfx::SurfaceFormat GetFormat() const override { return mFormat; } virtual bool Lock() override; @@ -320,6 +322,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override; + virtual gfx::SurfaceFormat GetFormat() const override{ return gfx::SurfaceFormat::YUV; } virtual bool Lock() override; diff --git a/gfx/layers/d3d9/CompositorD3D9.cpp b/gfx/layers/d3d9/CompositorD3D9.cpp index 74c62ff8c976..12f471dfb19c 100644 --- a/gfx/layers/d3d9/CompositorD3D9.cpp +++ b/gfx/layers/d3d9/CompositorD3D9.cpp @@ -716,6 +716,8 @@ CompositorD3D9::BeginFrame(const nsIntRegion& aInvalidRegion, void CompositorD3D9::EndFrame() { + Compositor::EndFrame(); + if (mDeviceManager) { device()->EndScene(); diff --git a/gfx/layers/d3d9/TextureD3D9.cpp b/gfx/layers/d3d9/TextureD3D9.cpp index 9c41b11e87a3..c0a0d65f24b2 100644 --- a/gfx/layers/d3d9/TextureD3D9.cpp +++ b/gfx/layers/d3d9/TextureD3D9.cpp @@ -935,6 +935,8 @@ TextureHostD3D9::UpdatedInternal(const nsIntRegion* aRegion) if (!mTextureSource->UpdateFromTexture(mTexture, regionToUpdate)) { gfxCriticalNote << "[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed"; } + + ReadUnlock(); } IDirect3DDevice9* @@ -959,6 +961,12 @@ TextureHostD3D9::SetCompositor(Compositor* aCompositor) } } +Compositor* +TextureHostD3D9::GetCompositor() +{ + return mCompositor; +} + bool TextureHostD3D9::BindTextureSource(CompositableTextureSourceRef& aTexture) { @@ -1085,6 +1093,12 @@ DXGITextureHostD3D9::SetCompositor(Compositor* aCompositor) } } +Compositor* +DXGITextureHostD3D9::GetCompositor() +{ + return mCompositor; +} + void DXGITextureHostD3D9::DeallocateDeviceData() { @@ -1124,6 +1138,12 @@ DXGIYCbCrTextureHostD3D9::SetCompositor(Compositor* aCompositor) } } +Compositor* +DXGIYCbCrTextureHostD3D9::GetCompositor() +{ + return mCompositor; +} + bool DXGIYCbCrTextureHostD3D9::Lock() { diff --git a/gfx/layers/d3d9/TextureD3D9.h b/gfx/layers/d3d9/TextureD3D9.h index c12a099d6c48..96c5c3e83809 100644 --- a/gfx/layers/d3d9/TextureD3D9.h +++ b/gfx/layers/d3d9/TextureD3D9.h @@ -276,6 +276,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override; + virtual gfx::SurfaceFormat GetFormat() const override { return mFormat; } virtual bool Lock() override; @@ -317,6 +319,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override; + virtual gfx::SurfaceFormat GetFormat() const override { return mFormat; } virtual gfx::IntSize GetSize() const override { return mSize; } @@ -354,6 +358,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override; + virtual gfx::SurfaceFormat GetFormat() const override { return gfx::SurfaceFormat::YUV; } virtual bool Lock() override; diff --git a/gfx/layers/ipc/LayersMessages.ipdlh b/gfx/layers/ipc/LayersMessages.ipdlh index c2a48507f68d..8cef82f13b75 100644 --- a/gfx/layers/ipc/LayersMessages.ipdlh +++ b/gfx/layers/ipc/LayersMessages.ipdlh @@ -312,9 +312,10 @@ struct ShmemSection { size_t size; }; -union TileLock { +union ReadLockDescriptor { ShmemSection; uintptr_t; + null_t; }; union MaybeTexture { @@ -326,7 +327,8 @@ struct TexturedTileDescriptor { PTexture texture; MaybeTexture textureOnWhite; IntRect updateRect; - TileLock sharedLock; + ReadLockDescriptor sharedLock; + ReadLockDescriptor sharedLockOnWhite; bool wasPlaceholder; }; diff --git a/gfx/layers/opengl/CompositorOGL.cpp b/gfx/layers/opengl/CompositorOGL.cpp index 1b674d1614e1..7e1c851d0ac5 100644 --- a/gfx/layers/opengl/CompositorOGL.cpp +++ b/gfx/layers/opengl/CompositorOGL.cpp @@ -1460,6 +1460,8 @@ CompositorOGL::EndFrame() MOZ_ASSERT(mCurrentRenderTarget == mWindowRenderTarget, "Rendering target not properly restored"); + Compositor::EndFrame(); + #ifdef MOZ_DUMP_PAINTING if (gfxEnv::DumpCompositorTextures()) { LayoutDeviceIntSize size; diff --git a/gfx/layers/opengl/GrallocTextureHost.cpp b/gfx/layers/opengl/GrallocTextureHost.cpp index 7b771d4df015..6d5f9158b66e 100644 --- a/gfx/layers/opengl/GrallocTextureHost.cpp +++ b/gfx/layers/opengl/GrallocTextureHost.cpp @@ -259,6 +259,7 @@ GrallocTextureHostOGL::GetAsSurface() { void GrallocTextureHostOGL::UnbindTextureSource() { + TextureHost::UnbindTextureSource(); // Clear the reference to the TextureSource (if any), because we know that // another TextureHost is being bound to the TextureSource. This means that // we will have to re-do gl->fEGLImageTargetTexture2D next time we go through diff --git a/gfx/layers/opengl/GrallocTextureHost.h b/gfx/layers/opengl/GrallocTextureHost.h index 8cc5e9fbd70f..1959e26f8a47 100644 --- a/gfx/layers/opengl/GrallocTextureHost.h +++ b/gfx/layers/opengl/GrallocTextureHost.h @@ -30,6 +30,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual void DeallocateSharedData() override; virtual void ForgetSharedData() override; diff --git a/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.h b/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.h index 10160db4ea00..1ffea89dbef7 100644 --- a/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.h +++ b/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.h @@ -73,6 +73,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual bool Lock() override; virtual gfx::SurfaceFormat GetFormat() const override; diff --git a/gfx/layers/opengl/TextureHostOGL.h b/gfx/layers/opengl/TextureHostOGL.h index 1b5b04a71ec2..9e6887790d3b 100644 --- a/gfx/layers/opengl/TextureHostOGL.h +++ b/gfx/layers/opengl/TextureHostOGL.h @@ -298,6 +298,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual bool Lock() override; virtual void Unlock() override {} @@ -393,6 +395,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual bool Lock() override; virtual void Unlock() override; @@ -489,6 +493,8 @@ public: virtual void SetCompositor(Compositor* aCompositor) override; + virtual Compositor* GetCompositor() override { return mCompositor; } + virtual bool Lock() override; virtual void Unlock() override;