From 8013630c8bbd3a5276919874b02fe607b9156202 Mon Sep 17 00:00:00 2001 From: sotaro Date: Mon, 12 Nov 2018 10:36:13 +0900 Subject: [PATCH] Bug 1506091 - Wait for GPU after posting NotifyDidRender r=mattwoodrow --- gfx/layers/wr/AsyncImagePipelineManager.cpp | 2 +- gfx/layers/wr/AsyncImagePipelineManager.h | 2 +- gfx/webrender_bindings/RenderCompositor.h | 1 + .../RenderCompositorANGLE.cpp | 4 ++ .../RenderCompositorANGLE.h | 1 + .../RenderCompositorOGL.cpp | 5 ++ gfx/webrender_bindings/RenderCompositorOGL.h | 1 + gfx/webrender_bindings/RenderThread.cpp | 55 +++++++++++++------ gfx/webrender_bindings/RenderThread.h | 13 +++++ gfx/webrender_bindings/RendererOGL.cpp | 11 +++- gfx/webrender_bindings/RendererOGL.h | 4 +- 11 files changed, 75 insertions(+), 24 deletions(-) diff --git a/gfx/layers/wr/AsyncImagePipelineManager.cpp b/gfx/layers/wr/AsyncImagePipelineManager.cpp index 8ab4927c988e..632e462a981c 100644 --- a/gfx/layers/wr/AsyncImagePipelineManager.cpp +++ b/gfx/layers/wr/AsyncImagePipelineManager.cpp @@ -544,7 +544,7 @@ AsyncImagePipelineManager::HoldExternalImage(const wr::PipelineId& aPipelineId, } void -AsyncImagePipelineManager::NotifyPipelinesUpdated(wr::WrPipelineInfo aInfo, bool aRender) +AsyncImagePipelineManager::NotifyPipelinesUpdated(const wr::WrPipelineInfo& aInfo, bool aRender) { // This is called on the render thread, so we just stash the data into // UpdatesQueue and process it later on the compositor thread. diff --git a/gfx/layers/wr/AsyncImagePipelineManager.h b/gfx/layers/wr/AsyncImagePipelineManager.h index b3feeaf0d1eb..22361ed62343 100644 --- a/gfx/layers/wr/AsyncImagePipelineManager.h +++ b/gfx/layers/wr/AsyncImagePipelineManager.h @@ -56,7 +56,7 @@ public: // This is called from the Renderer thread to notify this class about the // pipelines in the most recently completed render. A copy of the update // information is put into mUpdatesQueue. - void NotifyPipelinesUpdated(wr::WrPipelineInfo aInfo, bool aRender); + void NotifyPipelinesUpdated(const wr::WrPipelineInfo& aInfo, bool aRender); // This is run on the compositor thread to process mUpdatesQueue. We make // this a public entry point because we need to invoke it from other places. diff --git a/gfx/webrender_bindings/RenderCompositor.h b/gfx/webrender_bindings/RenderCompositor.h index 306af6ceac43..0c82ae739097 100644 --- a/gfx/webrender_bindings/RenderCompositor.h +++ b/gfx/webrender_bindings/RenderCompositor.h @@ -37,6 +37,7 @@ public: virtual bool BeginFrame() = 0; virtual void EndFrame() = 0; + virtual void WaitForGPU() = 0; virtual void Pause() = 0; virtual bool Resume() = 0; diff --git a/gfx/webrender_bindings/RenderCompositorANGLE.cpp b/gfx/webrender_bindings/RenderCompositorANGLE.cpp index dfc6b513b05e..fd5404f53c94 100644 --- a/gfx/webrender_bindings/RenderCompositorANGLE.cpp +++ b/gfx/webrender_bindings/RenderCompositorANGLE.cpp @@ -341,7 +341,11 @@ RenderCompositorANGLE::EndFrame() if (mCompositionDevice) { mCompositionDevice->Commit(); } +} +void +RenderCompositorANGLE::WaitForGPU() +{ // Note: this waits on the query we inserted in the previous frame, // not the one we just inserted now. Example: // Insert query #1 diff --git a/gfx/webrender_bindings/RenderCompositorANGLE.h b/gfx/webrender_bindings/RenderCompositorANGLE.h index c2cd45f6af5e..5fae7deeca0a 100644 --- a/gfx/webrender_bindings/RenderCompositorANGLE.h +++ b/gfx/webrender_bindings/RenderCompositorANGLE.h @@ -38,6 +38,7 @@ public: bool BeginFrame() override; void EndFrame() override; + void WaitForGPU() override; void Pause() override; bool Resume() override; diff --git a/gfx/webrender_bindings/RenderCompositorOGL.cpp b/gfx/webrender_bindings/RenderCompositorOGL.cpp index 78717f869e19..1083959e52ca 100644 --- a/gfx/webrender_bindings/RenderCompositorOGL.cpp +++ b/gfx/webrender_bindings/RenderCompositorOGL.cpp @@ -53,6 +53,11 @@ RenderCompositorOGL::EndFrame() mGL->SwapBuffers(); } +void +RenderCompositorOGL::WaitForGPU() +{ +} + void RenderCompositorOGL::Pause() { diff --git a/gfx/webrender_bindings/RenderCompositorOGL.h b/gfx/webrender_bindings/RenderCompositorOGL.h index c327613f1291..7037f69ec43c 100644 --- a/gfx/webrender_bindings/RenderCompositorOGL.h +++ b/gfx/webrender_bindings/RenderCompositorOGL.h @@ -24,6 +24,7 @@ public: bool BeginFrame() override; void EndFrame() override; + void WaitForGPU() override; void Pause() override; bool Resume() override; diff --git a/gfx/webrender_bindings/RenderThread.cpp b/gfx/webrender_bindings/RenderThread.cpp index 35c345a1e712..9269387eca66 100644 --- a/gfx/webrender_bindings/RenderThread.cpp +++ b/gfx/webrender_bindings/RenderThread.cpp @@ -340,7 +340,7 @@ RenderThread::RunEvent(wr::WindowId aWindowId, UniquePtr aEvent) static void NotifyDidRender(layers::CompositorBridgeParent* aBridge, - wr::WrPipelineInfo aInfo, + RefPtr aInfo, TimeStamp aStart, TimeStamp aEnd, bool aRender) @@ -352,15 +352,15 @@ NotifyDidRender(layers::CompositorBridgeParent* aBridge, aBridge->GetWrBridge()->RecordFrame(); } - for (uintptr_t i = 0; i < aInfo.epochs.length; i++) { + auto info = aInfo->Raw(); + + for (uintptr_t i = 0; i < info.epochs.length; i++) { aBridge->NotifyPipelineRendered( - aInfo.epochs.data[i].pipeline_id, - aInfo.epochs.data[i].epoch, + info.epochs.data[i].pipeline_id, + info.epochs.data[i].epoch, aStart, aEnd); } - - wr_pipeline_info_delete(aInfo); } void @@ -382,9 +382,9 @@ RenderThread::UpdateAndRender(wr::WindowId aWindowId, } auto& renderer = it->second; - + bool rendered = false; if (aRender) { - renderer->UpdateAndRender(aReadbackSize, aReadbackBuffer, aHadSlowFrame); + rendered = renderer->UpdateAndRender(aReadbackSize, aReadbackBuffer, aHadSlowFrame); } else { renderer->Update(); } @@ -392,17 +392,7 @@ RenderThread::UpdateAndRender(wr::WindowId aWindowId, renderer->CheckGraphicsResetStatus(); TimeStamp end = TimeStamp::Now(); - auto info = renderer->FlushPipelineInfo(); - RefPtr pipelineMgr = - renderer->GetCompositorBridge()->GetAsyncImagePipelineManager(); - // pipelineMgr should always be non-null here because it is only nulled out - // after the WebRenderAPI instance for the CompositorBridgeParent is - // destroyed, and that destruction blocks until the renderer thread has - // removed the relevant renderer. And after that happens we should never reach - // this code at all; it would bail out at the mRenderers.find check above. - MOZ_ASSERT(pipelineMgr); - pipelineMgr->NotifyPipelinesUpdated(info, aRender); layers::CompositorThreadHolder::Loop()->PostTask(NewRunnableFunction( "NotifyDidRenderRunnable", @@ -412,6 +402,25 @@ RenderThread::UpdateAndRender(wr::WindowId aWindowId, aStartTime, end, aRender )); + + if (rendered) { + // Wait for GPU after posting NotifyDidRender, since the wait is not + // necessary for the NotifyDidRender. + // The wait is necessary for Textures recycling of AsyncImagePipelineManager + // and for avoiding GPU queue is filled with too much tasks. + // WaitForGPU's implementation is different for each platform. + renderer->WaitForGPU(); + } + + RefPtr pipelineMgr = + renderer->GetCompositorBridge()->GetAsyncImagePipelineManager(); + // pipelineMgr should always be non-null here because it is only nulled out + // after the WebRenderAPI instance for the CompositorBridgeParent is + // destroyed, and that destruction blocks until the renderer thread has + // removed the relevant renderer. And after that happens we should never reach + // this code at all; it would bail out at the mRenderers.find check above. + MOZ_ASSERT(pipelineMgr); + pipelineMgr->NotifyPipelinesUpdated(info->Raw(), aRender); } void @@ -795,6 +804,16 @@ WebRenderShaders::~WebRenderShaders() { wr_shaders_delete(mShaders, mGL.get()); } +WebRenderPipelineInfo::WebRenderPipelineInfo(wr::WrPipelineInfo aPipelineInfo) + : mPipelineInfo(aPipelineInfo) +{ +} + +WebRenderPipelineInfo::~WebRenderPipelineInfo() +{ + wr_pipeline_info_delete(mPipelineInfo); +} + WebRenderThreadPool::WebRenderThreadPool() { mThreadPool = wr_thread_pool_new(); diff --git a/gfx/webrender_bindings/RenderThread.h b/gfx/webrender_bindings/RenderThread.h index 583fd405dc81..032683ab4934 100644 --- a/gfx/webrender_bindings/RenderThread.h +++ b/gfx/webrender_bindings/RenderThread.h @@ -72,6 +72,19 @@ protected: wr::WrShaders* mShaders; }; +class WebRenderPipelineInfo { + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(WebRenderPipelineInfo); +public: + explicit WebRenderPipelineInfo(wr::WrPipelineInfo aPipelineInfo); + + const wr::WrPipelineInfo& Raw() { return mPipelineInfo; } + +protected: + ~WebRenderPipelineInfo(); + + const wr::WrPipelineInfo mPipelineInfo; +}; + /// Base class for an event that can be scheduled to run on the render thread. /// /// The event can be passed through the same channels as regular WebRender messages diff --git a/gfx/webrender_bindings/RendererOGL.cpp b/gfx/webrender_bindings/RendererOGL.cpp index c77729e17e56..1a15d68ac1c3 100644 --- a/gfx/webrender_bindings/RendererOGL.cpp +++ b/gfx/webrender_bindings/RendererOGL.cpp @@ -194,6 +194,12 @@ RendererOGL::CheckGraphicsResetStatus() } } +void +RendererOGL::WaitForGPU() +{ + mCompositor->WaitForGPU(); +} + void RendererOGL::Pause() { @@ -229,10 +235,11 @@ RendererOGL::SetFrameStartTime(const TimeStamp& aTime) mFrameStartTime = aTime; } -wr::WrPipelineInfo +RefPtr RendererOGL::FlushPipelineInfo() { - return wr_renderer_flush_pipeline_info(mRenderer); + auto info = wr_renderer_flush_pipeline_info(mRenderer); + return new WebRenderPipelineInfo(info); } RenderTextureHost* diff --git a/gfx/webrender_bindings/RendererOGL.h b/gfx/webrender_bindings/RendererOGL.h index 5f0334963a7d..ea2c94d541da 100644 --- a/gfx/webrender_bindings/RendererOGL.h +++ b/gfx/webrender_bindings/RendererOGL.h @@ -60,7 +60,7 @@ public: bool UpdateAndRender(const Maybe& aReadbackSize, const Maybe>& aReadbackBuffer, bool aHadSlowFrame); /// This can be called on the render thread only. - bool RenderToTarget(gfx::DrawTarget& aTarget); + void WaitForGPU(); /// This can be called on the render thread only. void SetProfilerEnabled(bool aEnabled); @@ -91,7 +91,7 @@ public: layers::CompositorBridgeParent* GetCompositorBridge() { return mBridge; } - wr::WrPipelineInfo FlushPipelineInfo(); + RefPtr FlushPipelineInfo(); RenderTextureHost* GetRenderTexture(wr::WrExternalImageId aExternalImageId);