From 4374f75f51d0606934ada78b9552301b7a43348c Mon Sep 17 00:00:00 2001 From: Barret Rennie Date: Wed, 31 Jul 2019 18:28:34 +0000 Subject: [PATCH] Bug 1566915 - Write collected frames from the composition recorder on the render thread r=kvark On macOS, if we try to write the collected frames from the `CompositorBridgeParent` we will not have an active GL context, resulting in a crash. Writing the frames from the `RenderThread` solves this problem. Differential Revision: https://phabricator.services.mozilla.com/D39789 --HG-- extra : moz-landing-system : lando --- gfx/layers/ipc/CompositorBridgeParent.cpp | 11 +++--- gfx/layers/wr/WebRenderBridgeParent.cpp | 44 +++++++++++------------ gfx/layers/wr/WebRenderBridgeParent.h | 11 ++++-- gfx/webrender_bindings/RenderThread.cpp | 15 ++++++++ gfx/webrender_bindings/RenderThread.h | 2 ++ gfx/webrender_bindings/WebRenderAPI.cpp | 19 ++++++++++ gfx/webrender_bindings/WebRenderAPI.h | 7 ++++ 7 files changed, 78 insertions(+), 31 deletions(-) diff --git a/gfx/layers/ipc/CompositorBridgeParent.cpp b/gfx/layers/ipc/CompositorBridgeParent.cpp index 70dab0822f88..65d01b616316 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.cpp +++ b/gfx/layers/ipc/CompositorBridgeParent.cpp @@ -2642,13 +2642,14 @@ mozilla::ipc::IPCResult CompositorBridgeParent::RecvEndRecording( if (mLayerManager) { mLayerManager->SetCompositionRecorder(nullptr); + mCompositionRecorder->WriteCollectedFrames(); + } else if (mWrBridge) { + // If we are using WebRender, the |RenderThread| will have a handle to this + // |WebRenderCompositionRecorder|, which it will release once the frames + // have been written. + mWrBridge->WriteCollectedFrames(); } - // If we are using WebRender, the |RenderThread| will have a handle to this - // |WebRenderCompositionRecorder|, which it will release once the frames have - // been written. - - mCompositionRecorder->WriteCollectedFrames(); mCompositionRecorder = nullptr; *aOutSuccess = true; return IPC_OK(); diff --git a/gfx/layers/wr/WebRenderBridgeParent.cpp b/gfx/layers/wr/WebRenderBridgeParent.cpp index 2730dc38e97c..f3b332872af8 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.cpp +++ b/gfx/layers/wr/WebRenderBridgeParent.cpp @@ -384,7 +384,6 @@ void WebRenderBridgeParent::RemoveDeferredPipeline(wr::PipelineId aPipelineId) { } } - /* static */ WebRenderBridgeParent* WebRenderBridgeParent::CreateDestroyed( const wr::PipelineId& aPipelineId) { @@ -415,11 +414,8 @@ bool WebRenderBridgeParent::HandleDeferredPipelineData( wr::Epoch wrEpoch = GetNextWrEpoch(); bool validTransaction = data.mIdNamespace == mIdNamespace; - if (!ProcessRenderRootDisplayListData(data, - wrEpoch, - aTxnStartTime, - validTransaction, - false)){ + if (!ProcessRenderRootDisplayListData(data, wrEpoch, aTxnStartTime, + validTransaction, false)) { return false; } @@ -521,13 +517,11 @@ bool WebRenderBridgeParent::MaybeHandleDeferredPipelineData( aTxnStartTime)) { return false; } - } return true; } - mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEnsureConnected( TextureFactoryIdentifier* aTextureFactoryIdentifier, MaybeIdNamespace* aMaybeIdNamespace) { @@ -995,6 +989,10 @@ void WebRenderBridgeParent::SetCompositionRecorder( Api(wr::RenderRoot::Default)->SetCompositionRecorder(std::move(aRecorder)); } +void WebRenderBridgeParent::WriteCollectedFrames() { + Api(wr::RenderRoot::Default)->WriteCollectedFrames(); +} + CompositorBridgeParent* WebRenderBridgeParent::GetRootCompositorBridgeParent() const { if (!mCompositorBridge) { @@ -1165,10 +1163,8 @@ bool WebRenderBridgeParent::SetDisplayList( } bool WebRenderBridgeParent::ProcessRenderRootDisplayListData( - RenderRootDisplayListData& aDisplayList, - wr::Epoch aWrEpoch, - const TimeStamp& aTxnStartTime, - bool aValidTransaction, + RenderRootDisplayListData& aDisplayList, wr::Epoch aWrEpoch, + const TimeStamp& aTxnStartTime, bool aValidTransaction, bool aObserveLayersUpdate) { wr::TransactionBuilder txn; Maybe sender; @@ -1276,7 +1272,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList( // Only non-root WRBPs will ever have an unresolved mRenderRoot MOZ_ASSERT(!IsRootWebRenderBridgeParent()); if (aDisplayLists.Length() != 1) { - return IPC_FAIL(this, "Well-behaved content processes must only send a DL for a single renderRoot"); + return IPC_FAIL(this, + "Well-behaved content processes must only send a DL for " + "a single renderRoot"); } PushDeferredPipelineData(AsVariant(std::move(aDisplayLists[0]))); aDisplayLists.Clear(); @@ -1284,10 +1282,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList( for (auto& displayList : aDisplayLists) { if (IsRootWebRenderBridgeParent()) { - if (!MaybeHandleDeferredPipelineData( - displayList.mRenderRoot, - displayList.mRemotePipelineIds, - aTxnStartTime)) { + if (!MaybeHandleDeferredPipelineData(displayList.mRenderRoot, + displayList.mRemotePipelineIds, + aTxnStartTime)) { return IPC_FAIL(this, "Failed processing deferred pipeline data"); } } else { @@ -1309,12 +1306,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList( bool observeLayersUpdate = ShouldParentObserveEpoch(); for (auto& displayList : aDisplayLists) { - if (!ProcessRenderRootDisplayListData( - displayList, - wrEpoch, - aTxnStartTime, - validTransaction, - observeLayersUpdate)) { + if (!ProcessRenderRootDisplayListData(displayList, wrEpoch, aTxnStartTime, + validTransaction, + observeLayersUpdate)) { return IPC_FAIL(this, "Failed to process RenderRootDisplayListData."); } } @@ -1456,7 +1450,9 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEmptyTransaction( // Only non-root WRBPs will ever have an unresolved mRenderRoot MOZ_ASSERT(!IsRootWebRenderBridgeParent()); if (aRenderRootUpdates.Length() != 1) { - return IPC_FAIL(this, "Well-behaved content processes must only send a DL for a single renderRoot"); + return IPC_FAIL(this, + "Well-behaved content processes must only send a DL for " + "a single renderRoot"); } PushDeferredPipelineData(AsVariant(std::move(aRenderRootUpdates[0]))); aRenderRootUpdates.Clear(); diff --git a/gfx/layers/wr/WebRenderBridgeParent.h b/gfx/layers/wr/WebRenderBridgeParent.h index ac6beff4c129..34c20547f2cb 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.h +++ b/gfx/layers/wr/WebRenderBridgeParent.h @@ -282,8 +282,8 @@ class WebRenderBridgeParent final const TimeStamp& aTxnStartTime); /** - * See MaybeHandleDeferredPipelineData - this is the implementation of that for - * a single pipeline. + * See MaybeHandleDeferredPipelineData - this is the implementation of that + * for a single pipeline. */ bool MaybeHandleDeferredPipelineDataForPipeline( wr::RenderRoot aRenderRoot, wr::PipelineId aPipelineId, @@ -300,6 +300,13 @@ class WebRenderBridgeParent final void SetCompositionRecorder( RefPtr&& aRecorder); + /** + * Write the frames collected by the |WebRenderCompositionRecorder| to disk. + * + * If there is not currently a recorder, this is a no-op. + */ + void WriteCollectedFrames(); + private: class ScheduleSharedSurfaceRelease; diff --git a/gfx/webrender_bindings/RenderThread.cpp b/gfx/webrender_bindings/RenderThread.cpp index cdc25f507c42..7154a0c57fd5 100644 --- a/gfx/webrender_bindings/RenderThread.cpp +++ b/gfx/webrender_bindings/RenderThread.cpp @@ -267,6 +267,21 @@ void RenderThread::SetCompositionRecorderForWindow( mCompositionRecorders[aWindowId] = std::move(aCompositionRecorder); } +void RenderThread::WriteCollectedFramesForWindow(wr::WindowId aWindowId) { + MOZ_ASSERT(IsInRenderThread()); + MOZ_ASSERT(GetRenderer(aWindowId)); + + auto it = mCompositionRecorders.find(aWindowId); + MOZ_DIAGNOSTIC_ASSERT( + it != mCompositionRecorders.end(), + "Attempted to write frames from a window that was not recording."); + if (it != mCompositionRecorders.end()) { + it->second->WriteCollectedFrames(); + + mCompositionRecorders.erase(it); + } +} + void RenderThread::HandleFrame(wr::WindowId aWindowId, bool aRender) { if (mHasShutdown) { return; diff --git a/gfx/webrender_bindings/RenderThread.h b/gfx/webrender_bindings/RenderThread.h index 07c886f79ac6..00816d40f653 100644 --- a/gfx/webrender_bindings/RenderThread.h +++ b/gfx/webrender_bindings/RenderThread.h @@ -271,6 +271,8 @@ class RenderThread final { wr::WindowId aWindowId, RefPtr&& aCompositionRecorder); + void WriteCollectedFramesForWindow(wr::WindowId aWindowId); + private: explicit RenderThread(base::Thread* aThread); diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp index 9636c5c2f895..b91489b852f1 100644 --- a/gfx/webrender_bindings/WebRenderAPI.cpp +++ b/gfx/webrender_bindings/WebRenderAPI.cpp @@ -606,6 +606,25 @@ void WebRenderAPI::SetCompositionRecorder( auto event = MakeUnique(std::move(aRecorder)); RunOnRenderThread(std::move(event)); } + +void WebRenderAPI::WriteCollectedFrames() { + class WriteCollectedFramesEvent final : public RendererEvent { + public: + explicit WriteCollectedFramesEvent() { + MOZ_COUNT_CTOR(WriteCollectedFramesEvent); + } + + ~WriteCollectedFramesEvent() { MOZ_COUNT_DTOR(WriteCollectedFramesEvent); } + + void Run(RenderThread& aRenderThread, WindowId aWindowId) override { + aRenderThread.WriteCollectedFramesForWindow(aWindowId); + } + }; + + auto event = MakeUnique(); + RunOnRenderThread(std::move(event)); +} + void TransactionBuilder::Clear() { wr_resource_updates_clear(mTxn); } void TransactionBuilder::Notify(wr::Checkpoint aWhen, diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h index a34712846e38..36b3a360eb8e 100644 --- a/gfx/webrender_bindings/WebRenderAPI.h +++ b/gfx/webrender_bindings/WebRenderAPI.h @@ -268,6 +268,13 @@ class WebRenderAPI final { void SetCompositionRecorder( RefPtr&& aRecorder); + /** + * Write the frames collected by the |WebRenderCompositionRecorder| to disk. + * + * If there is not currently a recorder, this is a no-op. + */ + void WriteCollectedFrames(); + protected: WebRenderAPI(wr::DocumentHandle* aHandle, wr::WindowId aId, uint32_t aMaxTextureSize, bool aUseANGLE, bool aUseDComp,