From 85222130d4137d79c994b273f88dc7c39fc12929 Mon Sep 17 00:00:00 2001 From: Qing Tao <55564570+VickyAtAZ@users.noreply.github.com> Date: Thu, 27 May 2021 13:56:19 -0700 Subject: [PATCH] LYN-4007 Editor crashes when entering the Game Mode with two Box Shape entities with Game View enabled (#974) The crash was caused by using gpu query across command lists. --- .../Code/Source/RPI.Public/GpuQuery/Query.cpp | 12 --------- .../Source/RPI.Public/Pass/RenderPass.cpp | 26 ++++++++++++++++--- .../RPI/Code/Tests/System/GpuQueryTests.cpp | 12 ++++++--- ...AtomViewportDisplayInfoSystemComponent.cpp | 8 +++--- 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/GpuQuery/Query.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/GpuQuery/Query.cpp index 227e096594..1d6e8cdc65 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/GpuQuery/Query.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/GpuQuery/Query.cpp @@ -96,12 +96,6 @@ namespace AZ return QueryResultCode::Fail; } - // Limit calling BeginQuery() to the first CommandList in the array. - if (context.GetCommandListIndex() != 0) - { - return QueryResultCode::Success; - } - const auto rhiQueryIndices = GetRhiQueryIndicesFromCurrentFrame(); if (!rhiQueryIndices) { @@ -124,12 +118,6 @@ namespace AZ return QueryResultCode::Fail; } - // Limit calling EndQuery() to the last CommandList in the array. - if (context.GetCommandListIndex() != context.GetCommandListCount() - 1) - { - return QueryResultCode::Success; - } - // Validate that the queries are recorded for the same scope. if (m_cachedScopeId != context.GetScopeId()) { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp index 9c6a95e582..3a2a556429 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp @@ -522,8 +522,11 @@ namespace AZ } }; - ExecuteOnTimestampQuery(beginQuery); - ExecuteOnPipelineStatisticsQuery(beginQuery); + if (context.GetCommandListIndex() == 0) + { + ExecuteOnTimestampQuery(beginQuery); + ExecuteOnPipelineStatisticsQuery(beginQuery); + } } void RenderPass::EndScopeQuery(const RHI::FrameGraphExecuteContext& context) @@ -533,8 +536,23 @@ namespace AZ query->EndQuery(context); }; - ExecuteOnTimestampQuery(endQuery); - ExecuteOnPipelineStatisticsQuery(endQuery); + // This scopy query implmentation should be replaced by + // [ATOM-5407] [RHI][Core] - Add GPU timestamp and pipeline statistic support for scopes + + // For timestamp query, it's okay to execute across different command lists + if (context.GetCommandListIndex() == context.GetCommandListCount() - 1) + { + ExecuteOnTimestampQuery(endQuery); + } + // For all the other types of queries except timestamp, the query start and end has to be in the same command list + // Here only tracks the PipelineStatistics for the first command list due to that we don't know how many queries are + // needed when AddScopeQueryToFrameGraph is called. + // This implementation leads to an issue that we may not get accurate pipeline statistic data + // for passes which were executed with more than one command list + if (context.GetCommandListIndex() == 0) + { + ExecuteOnPipelineStatisticsQuery(endQuery); + } } void RenderPass::ReadbackScopeQueryResults() diff --git a/Gems/Atom/RPI/Code/Tests/System/GpuQueryTests.cpp b/Gems/Atom/RPI/Code/Tests/System/GpuQueryTests.cpp index e55ec8012b..98ff6befdd 100644 --- a/Gems/Atom/RPI/Code/Tests/System/GpuQueryTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/System/GpuQueryTests.cpp @@ -159,7 +159,9 @@ namespace UnitTest const uint32_t ResultSize = sizeof(uint64_t); uint64_t mockData; - const RHI::FrameGraphExecuteContext::Descriptor desc = {}; + RHI::FrameGraphExecuteContext::Descriptor desc = {}; + uint64_t dummyCommandList; + desc.m_commandList = reinterpret_cast(&dummyCommandList); RHI::FrameGraphExecuteContext context(desc); RHI::Scope scope; @@ -209,7 +211,9 @@ namespace UnitTest const uint32_t ResultSize = sizeof(uint64_t) * 4u; uint64_t mockData; - const RHI::FrameGraphExecuteContext::Descriptor desc = {}; + RHI::FrameGraphExecuteContext::Descriptor desc = {}; + uint64_t dummyCommandList; + desc.m_commandList = reinterpret_cast(&dummyCommandList); RHI::FrameGraphExecuteContext context(desc); RHI::Scope scope; @@ -273,7 +277,9 @@ namespace UnitTest const uint32_t ResultSize = sizeof(uint64_t) * 2u; uint64_t mockData; - const RHI::FrameGraphExecuteContext::Descriptor desc = {}; + RHI::FrameGraphExecuteContext::Descriptor desc = {}; + uint64_t dummyCommandList; + desc.m_commandList = reinterpret_cast(&dummyCommandList); RHI::FrameGraphExecuteContext context(desc); RHI::Scope scope; diff --git a/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp b/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp index 672c26a9ab..7d830b4ca9 100644 --- a/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp +++ b/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp @@ -18,10 +18,11 @@ #include #include +#include +#include #include #include #include -#include #include #include @@ -146,7 +147,7 @@ namespace AZ::Render if (m_updateRootPassQuery) { - if (auto rootPass = AZ::RPI::PassSystemInterface::Get()->GetRootPass()) + if (auto rootPass = viewportContext->GetCurrentPipeline()->GetRootPass()) { rootPass->SetPipelineStatisticsQueryEnabled(displayLevel != AtomBridge::ViewportInfoDisplayState::CompactInfo); m_updateRootPassQuery = false; @@ -226,7 +227,8 @@ namespace AZ::Render void AtomViewportDisplayInfoSystemComponent::DrawPassInfo() { - auto rootPass = AZ::RPI::PassSystemInterface::Get()->GetRootPass(); + AZ::RPI::ViewportContextPtr viewportContext = GetViewportContext(); + auto rootPass = viewportContext->GetCurrentPipeline()->GetRootPass(); const RPI::PipelineStatisticsResult stats = rootPass->GetLatestPipelineStatisticsResult(); AZStd::function)> containingPassCount = [&containingPassCount](const AZ::RPI::Ptr pass) {