From 72f808876c9bdaa4127edb2aae4b73e746858baa Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Wed, 28 Jul 2021 16:35:51 -0700 Subject: [PATCH 1/4] Improvements to pass statistics and SRG debugability Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- .../Atom/RHI.Reflect/ConstantsLayout.h | 5 +- .../Atom/RHI.Reflect/NameIdReflectionMap.h | 9 +++ .../RHI/Code/Include/Atom/RHI/ConstantsData.h | 8 +++ .../Atom/RHI/ShaderResourceGroupData.h | 3 + .../Atom/RHI/ShaderResourceGroupDebug.h | 21 ++++++ .../Source/RHI.Reflect/ConstantsLayout.cpp | 35 ++++++---- .../RHI/Code/Source/RHI/ConstantsData.cpp | 66 +++++++++++++++++++ .../Source/RHI/ShaderResourceGroupData.cpp | 5 ++ .../Source/RHI/ShaderResourceGroupDebug.cpp | 59 +++++++++++++++++ .../Atom/RHI/Code/atom_rhi_public_files.cmake | 2 + .../Include/Atom/RPI.Public/Pass/PassSystem.h | 15 ++++- .../RPI.Public/Pass/PassSystemInterface.h | 41 ++++++++---- .../Include/Atom/RPI.Public/Pass/RasterPass.h | 3 + .../Source/RPI.Public/Pass/PassSystem.cpp | 24 +++++++ .../Source/RPI.Public/Pass/RasterPass.cpp | 7 ++ .../Source/RPI.Public/Pass/RenderPass.cpp | 2 + ...AtomViewportDisplayInfoSystemComponent.cpp | 25 +++---- 17 files changed, 289 insertions(+), 41 deletions(-) create mode 100644 Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h create mode 100644 Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h index 09415f1bc2..003a7dcd23 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h @@ -81,6 +81,10 @@ namespace AZ //! If validation is disabled, true is always returned. bool ValidateAccess(ShaderInputConstantIndex inputIndex) const; + //! Prints to the console the shader input names specified by input list of indices + //! Will ignore any indices outside of the inputs array bounds + void DebugPrintNames(AZStd::array_view constantList) const; + protected: ConstantsLayout() = default; @@ -94,7 +98,6 @@ namespace AZ AZStd::vector m_inputs; IdReflectionMapForConstants m_idReflection; - AZStd::vector m_intervals; uint32_t m_sizeInBytes = 0; HashValue64 m_hash = InvalidHash; }; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h index 7756e7d290..b6042ab34c 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h @@ -68,6 +68,9 @@ namespace AZ /// Return the number of entries size_t Size() const; + // Returns true if size is zero + bool IsEmpty() const; + class NameIdReflectionMapSerializationEvents : public SerializeContext::IEventHandler { @@ -168,6 +171,12 @@ namespace AZ return m_reflectionMap.size(); } + template + bool NameIdReflectionMap::IsEmpty() const + { + return Size() == 0; + } + template void NameIdReflectionMap::Sort() { diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h index 5fd10212d4..27bd418b8a 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h @@ -84,6 +84,14 @@ namespace AZ //! Returns the constants layout. const ConstantsLayout* GetLayout() const; + //! Returns whether other constant data and this have the same value at the specified shader input index + bool ConstantIsEqual(const ConstantsData& other, ShaderInputConstantIndex inputIndex) const; + + //! Performs a diff between this and input constant data and returns a list of all the shader input indices + //! for which the constants are not the same between the two. If one of the two has more constants than the + //! other, these additional constants will be added to the end of the returned list. + AZStd::vector GetIndicesOfDifferingConstants(const ConstantsData& other) const; + private: enum class ValidateConstantAccessExpect : uint32_t { diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h index a145ba7ece..17ed3a64f1 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h @@ -172,6 +172,9 @@ namespace AZ //! Different platforms might follow different packing rules for the internally-managed SRG constant buffer. AZStd::array_view GetConstantData() const; + //! Returns the underlying ConstantsData struct + const ConstantsData& GetConstantsData() const; + //! Returns the shader resource layout for this group. const ShaderResourceGroupLayout* GetLayout() const; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h new file mode 100644 index 0000000000..b81f9e0c0b --- /dev/null +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h @@ -0,0 +1,21 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +namespace AZ +{ + namespace RHI + { + class ConstantsData; + struct DrawItem; + class ShaderResourceGroup; + + void PrintConstantDataDiff(const ShaderResourceGroup& shaderResourceGroup, ConstantsData& referenceData, bool updateReferenceData = false); + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData = false); + + } +} diff --git a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp index 3f9d9d9d63..fa85fbd884 100644 --- a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp @@ -17,10 +17,9 @@ namespace AZ if (SerializeContext* serializeContext = azrtti_cast(context)) { serializeContext->Class() - ->Version(0) + ->Version(1) // Version 1: Adding debug helper functions to Shader Resource Groups ->Field("m_inputs", &ConstantsLayout::m_inputs) ->Field("m_idReflection", &ConstantsLayout::m_idReflection) - ->Field("m_intervals", &ConstantsLayout::m_intervals) ->Field("m_sizeInBytes", &ConstantsLayout::m_sizeInBytes) ->Field("m_hash", &ConstantsLayout::m_hash); } @@ -42,7 +41,6 @@ namespace AZ { m_inputs.clear(); m_idReflection.Clear(); - m_intervals.clear(); m_sizeInBytes = 0; m_hash = InvalidHash; } @@ -66,11 +64,8 @@ namespace AZ return false; } - // The constant data size is the maximum offset + size from the start of the struct. - constantDataSize = AZStd::max(constantDataSize, constantDescriptor.m_constantByteOffset + constantDescriptor.m_constantByteCount); - - // Add the [min, max) interval for the inline constant. - m_intervals.emplace_back(constantDescriptor.m_constantByteOffset, constantDescriptor.m_constantByteOffset + constantDescriptor.m_constantByteCount); + uint32_t end = constantDescriptor.m_constantByteOffset + constantDescriptor.m_constantByteCount; + constantDataSize = AZStd::max(constantDataSize, end); ++constantInputIndex; m_hash = TypeHash64(constantDescriptor.GetHash(), m_hash); @@ -99,7 +94,10 @@ namespace AZ Interval ConstantsLayout::GetInterval(ShaderInputConstantIndex inputIndex) const { - return m_intervals[inputIndex.GetIndex()]; + const ShaderInputConstantDescriptor& desc = GetShaderInput(inputIndex); + uint32_t start = desc.m_constantByteOffset; + uint32_t end = start + desc.m_constantByteCount; + return Interval(start, end); } const ShaderInputConstantDescriptor& ConstantsLayout::GetShaderInput(ShaderInputConstantIndex inputIndex) const @@ -138,8 +136,8 @@ namespace AZ { if (!m_sizeInBytes) { - AZ_Assert(m_intervals.empty(), "Constants size is not valid."); - return m_intervals.empty(); + AZ_Assert(m_idReflection.IsEmpty(), "Constants size is not valid."); + return m_idReflection.IsEmpty(); } } @@ -148,5 +146,20 @@ namespace AZ return true; } + + void ConstantsLayout::DebugPrintNames(AZStd::array_view constantList) const + { + AZStd::string output; + for (const ShaderInputConstantIndex& constandIdx : constantList) + { + if (constandIdx.GetIndex() < m_inputs.size()) + { + output += m_inputs[constandIdx.GetIndex()].m_name.GetCStr(); + output += " - "; + } + } + AZ_Printf("RHI", output.c_str()); + } + } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp index e00896d44e..29d451b4d5 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp @@ -404,5 +404,71 @@ namespace AZ AZ_Assert(m_layout, "Constants layout is null"); return m_layout.get(); } + + bool ConstantsData::ConstantIsEqual(const ConstantsData& other, ShaderInputConstantIndex inputIndex) const + { + AZStd::array_view myConstans = GetConstantRaw(inputIndex); + AZStd::array_view otherConstans = other.GetConstantRaw(inputIndex); + + // If they point to the same data, they are equal + if (myConstans == otherConstans) + { + return true; + } + + // If they point to data of different size, they are not equal + if (myConstans.size() != otherConstans.size()) + { + return false; + } + + // If they point to differing data of same size, compare the data + // Note: due to small size of data this loop will be faster than a mem compare + for(uint32_t i = 0; i < myConstans.size(); ++i) + { + if (myConstans[i] != otherConstans[i]) + { + return false; + } + } + + // Arrays point to different locations in memory but all bytes match, return true + return true; + } + + AZStd::vector ConstantsData::GetIndicesOfDifferingConstants(const ConstantsData& other) const + { + AZStd::vector differingIndices; + + if (m_layout == nullptr || other.m_layout == nullptr) + { + return differingIndices; + } + + AZStd::array_view myShaderInputs = m_layout->GetShaderInputList(); + AZStd::array_view otherShaderInputs = other.m_layout->GetShaderInputList(); + + size_t minSize = AZStd::min(myShaderInputs.size(), otherShaderInputs.size()); + size_t maxSize = AZStd::max(myShaderInputs.size(), otherShaderInputs.size()); + + for (size_t idx = 0; idx < minSize; ++idx) + { + const ShaderInputConstantIndex inputIndex(idx); + if (!ConstantIsEqual(other, inputIndex)) + { + differingIndices.push_back(inputIndex); + } + } + + // If sizes are different, add difference at the end + for (size_t idx = minSize; idx < maxSize; ++idx) + { + const ShaderInputConstantIndex inputIndex(idx); + differingIndices.push_back(inputIndex); + } + + return differingIndices; + } + } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp index f2b57bf1e5..65499fb57d 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp @@ -334,5 +334,10 @@ namespace AZ return m_constantsData.GetConstantData(); } + const ConstantsData& ShaderResourceGroupData::GetConstantsData() const + { + return m_constantsData; + } + } // namespace RHI } // namespace AZ diff --git a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp new file mode 100644 index 0000000000..4bd8d0aab8 --- /dev/null +++ b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp @@ -0,0 +1,59 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ + +#include +#include +#include +#include + +namespace AZ +{ + namespace RHI + { + + void PrintConstantDataDiff(const ShaderResourceGroup& shaderResourceGroup, ConstantsData& referenceData, bool updateReferenceData) + { + const RHI::ConstantsData& currentData = shaderResourceGroup.GetData().GetConstantsData(); + + AZStd::vector differingIndices = currentData.GetIndicesOfDifferingConstants(referenceData); + + if (differingIndices.size() > 0) + { + AZ_Printf("RHI", "Detected different SRG values for the following fields:\n"); + if (currentData.GetLayout()) + { + currentData.GetLayout()->DebugPrintNames(differingIndices); + } + } + + if (updateReferenceData) + { + referenceData = currentData; + } + } + + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData) + { + s32 srgIndex = -1; + for (u32 i = 0; i < drawItem.m_shaderResourceGroupCount; ++i) + { + if (drawItem.m_shaderResourceGroups[i]->GetBindingSlot() == srgBindingSlot) + { + srgIndex = i; + break; + } + } + + if (srgIndex != -1) + { + const ShaderResourceGroup& srg = *drawItem.m_shaderResourceGroups[srgIndex]; + PrintConstantDataDiff(srg, referenceData, updateReferenceData); + } + } + + } +} diff --git a/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake b/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake index b489cbbbb4..2bf2acd892 100644 --- a/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake +++ b/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake @@ -156,10 +156,12 @@ set(FILES Source/RHI/ScopeAttachment.cpp Include/Atom/RHI/ShaderResourceGroup.h Include/Atom/RHI/ShaderResourceGroupData.h + Include/Atom/RHI/ShaderResourceGroupDebug.h Include/Atom/RHI/ShaderResourceGroupInvalidateRegistry.h Include/Atom/RHI/ShaderResourceGroupPool.h Source/RHI/ShaderResourceGroup.cpp Source/RHI/ShaderResourceGroupData.cpp + Source/RHI/ShaderResourceGroupDebug.cpp Source/RHI/ShaderResourceGroupInvalidateRegistry.cpp Source/RHI/ShaderResourceGroupPool.cpp Include/Atom/RHI/MemoryStatisticsBuilder.h diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h index de57e2e3f6..5998798ae8 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h @@ -74,6 +74,12 @@ namespace AZ const AZ::Name& GetTargetedPassDebuggingName() const override; void ConnectEvent(OnReadyLoadTemplatesEvent::Handler& handler) override; PassSystemState GetState() const override; + SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const override; + + // PassSystemInterface statistics related functions + void IncrementFrameDrawItemCount(u32 numDrawItems) override; + void IncrementFrameRenderPassCount() override; + PassSystemFrameStatistics GetFrameStatistics() override; // PassSystemInterface factory related functions... void AddPassCreator(Name className, PassCreator createFunction) override; @@ -92,7 +98,6 @@ namespace AZ void RegisterPass(Pass* pass) override; void UnregisterPass(Pass* pass) override; AZStd::vector FindPasses(const PassFilter& passFilter) const override; - SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const override; private: // Returns the root of the pass tree hierarchy @@ -115,6 +120,9 @@ namespace AZ void QueueForRemoval(Pass* pass) override; void QueueForInitialization(Pass* pass) override; + // Resets the frame statistic counters + void ResetFrameStatistics(); + // Lists for queuing passes for various function calls // Name of the list reflects the pass function it will call AZStd::vector< Ptr > m_buildPassList; @@ -140,13 +148,16 @@ namespace AZ AZ::Name m_targetedPassDebugName; // Counts the number of passes - int32_t m_passCounter = 0; + u32 m_passCounter = 0; // Events OnReadyLoadTemplatesEvent m_loadTemplatesEvent; // Used to track what phase of execution the pass system is in PassSystemState m_state = PassSystemState::Unitialized; + + // Counters used to gather statistics about the frame + PassSystemFrameStatistics m_frameStatistics; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h index cc66a7c228..246e43a2f7 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h @@ -66,6 +66,14 @@ namespace AZ FrameEnd, }; + //! Frame counters used for collecting statistics + struct PassSystemFrameStatistics + { + u32 m_numRenderPassesExecuted = 0; + u32 m_totalDrawItemsRendered = 0; + u32 m_maxDrawItemsRenderedInAPass = 0; + }; + class PassSystemInterface { friend class Pass; @@ -115,6 +123,27 @@ namespace AZ virtual void SetTargetedPassDebuggingName(const AZ::Name& targetPassName) = 0; virtual const AZ::Name& GetTargetedPassDebuggingName() const = 0; + //! Find the SwapChainPass associated with window Handle + virtual SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const = 0; + + using OnReadyLoadTemplatesEvent = AZ::Event<>; + //! Connect a handler to listen to the event that the pass system is ready to load pass templates + //! The event is triggered when pass system is initialized and asset system is ready. + //! The handler can add new pass templates or load pass template mappings from assets + virtual void ConnectEvent(OnReadyLoadTemplatesEvent::Handler& handler) = 0; + + virtual PassSystemState GetState() const = 0; + + // Passes call this function to notify the pass system that they are drawing X draw items this frame + // Used for Pass System statistics + virtual void IncrementFrameDrawItemCount(u32 numDrawItems) = 0; + + // Increments the counter for the number of render passes executed this frame (does not include passes that are disabled) + virtual void IncrementFrameRenderPassCount() = 0; + + // Get frame statistics from the Pass System + virtual PassSystemFrameStatistics GetFrameStatistics() = 0; + // --- Pass Factory related functionality --- //! Directly creates a pass given a PassDescriptor @@ -171,17 +200,6 @@ namespace AZ //! Find matching passes from registered passes with specified filter virtual AZStd::vector FindPasses(const PassFilter& passFilter) const = 0; - //! Find the SwapChainPass associated with window Handle - virtual SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const = 0; - - using OnReadyLoadTemplatesEvent = AZ::Event<>; - //! Connect a handler to listen to the event that the pass system is ready to load pass templates - //! The event is triggered when pass system is initialized and asset system is ready. - //! The handler can add new pass templates or load pass template mappings from assets - virtual void ConnectEvent(OnReadyLoadTemplatesEvent::Handler& handler) = 0; - - virtual PassSystemState GetState() const = 0; - private: // These functions are only meant to be used by the Pass class @@ -199,7 +217,6 @@ namespace AZ //! Unregisters the pass with the pass library. Called in the Pass destructor. virtual void UnregisterPass(Pass* pass) = 0; - }; namespace PassSystemEvents diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h index 735b3930c8..b0d941083b 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h @@ -42,6 +42,8 @@ namespace AZ //! Expose shader resource group. ShaderResourceGroup* GetShaderResourceGroup(); + u32 GetDrawItemCount(); + protected: explicit RasterPass(const PassDescriptor& descriptor); @@ -68,6 +70,7 @@ namespace AZ RHI::Viewport m_viewportState; bool m_overrideScissorSate = false; bool m_overrideViewportState = false; + u32 m_drawItemCount = 0; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp index 91cc645947..3092b9ecd0 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp @@ -308,6 +308,7 @@ namespace AZ AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzRender); AZ_ATOM_PROFILE_FUNCTION("RPI", "PassSystem: FrameUpdate"); + ResetFrameStatistics(); ProcessQueuedChanges(); m_state = PassSystemState::Rendering; @@ -392,6 +393,29 @@ namespace AZ handler.Connect(m_loadTemplatesEvent); } + void PassSystem::ResetFrameStatistics() + { + m_frameStatistics.m_numRenderPassesExecuted = 0; + m_frameStatistics.m_totalDrawItemsRendered = 0; + m_frameStatistics.m_maxDrawItemsRenderedInAPass = 0; + } + + PassSystemFrameStatistics PassSystem::GetFrameStatistics() + { + return m_frameStatistics; + } + + void PassSystem::IncrementFrameDrawItemCount(u32 numDrawItems) + { + m_frameStatistics.m_totalDrawItemsRendered += numDrawItems; + m_frameStatistics.m_maxDrawItemsRenderedInAPass = AZStd::max(m_frameStatistics.m_maxDrawItemsRenderedInAPass, numDrawItems); + } + + void PassSystem::IncrementFrameRenderPassCount() + { + ++m_frameStatistics.m_numRenderPassesExecuted; + } + // --- Pass Factory Functions --- void PassSystem::AddPassCreator(Name className, PassCreator createFunction) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index ee38c1e5ef..ae74e355f2 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -112,6 +112,11 @@ namespace AZ return m_shaderResourceGroup.get(); } + u32 RasterPass::GetDrawItemCount() + { + return m_drawItemCount; + } + // --- Pass behaviour overrides --- void RasterPass::Validate(PassValidationResults& validationResults) @@ -145,6 +150,8 @@ namespace AZ // Draw List m_drawListView = view->GetDrawList(m_drawListTag); + m_drawItemCount = m_drawListView.size(); + PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); } RenderPass::FrameBeginInternal(params); 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 96649573b2..92e6d8b2b6 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp @@ -200,6 +200,8 @@ namespace AZ m_attachmentCopy.lock()->FrameBegin(params); } CollectSrgs(); + + PassSystemInterface::Get()->IncrementFrameRenderPassCount(); } diff --git a/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp b/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp index af9fd7fdd8..ef341f8dc8 100644 --- a/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp +++ b/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp @@ -229,25 +229,20 @@ namespace AZ::Render 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) - { - int count = 1; - if (auto passAsParent = pass->AsParent()) - { - for (const auto& child : passAsParent->GetChildren()) - { - count += containingPassCount(child); - } - } - return count; - }; - const int numPasses = containingPassCount(rootPass); + + RPI::PassSystemFrameStatistics passSystemFrameStatistics = AZ::RPI::PassSystemInterface::Get()->GetFrameStatistics(); + DrawLine(AZStd::string::format( - "Total Passes: %d Vertex Count: %lld Primitive Count: %lld", - numPasses, + "RenderPasses: %d Vertex Count: %lld Primitive Count: %lld", + passSystemFrameStatistics.m_numRenderPassesExecuted, aznumeric_cast(stats.m_vertexCount), aznumeric_cast(stats.m_primitiveCount) )); + DrawLine(AZStd::string::format( + "Total Draw Item Count: %d Max Draw Items in a Pass: %d", + passSystemFrameStatistics.m_totalDrawItemsRendered, + passSystemFrameStatistics.m_maxDrawItemsRenderedInAPass + )); } void AtomViewportDisplayInfoSystemComponent::UpdateFramerate() From 64cff38f8294338371555ef737008be3879f2a28 Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Sun, 8 Aug 2021 16:46:42 -0700 Subject: [PATCH 2/4] Addressing PR feedback and fixed a small issue with the draw item count display Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- .../Code/Include/Atom/RHI/ShaderResourceGroupDebug.h | 12 +++++++++++- .../RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp | 6 +++--- Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp | 12 ++++++------ .../RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp | 6 +++--- .../RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp | 4 +++- 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h index b81f9e0c0b..d9ea77d823 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h @@ -14,8 +14,18 @@ namespace AZ struct DrawItem; class ShaderResourceGroup; + /// Given a ShaderResourceGroup and a reference ConstantsData input, this function will fetch the ConstantsData on the SRG and compare it + /// to the reference ConstantsData. It will print the names of any constants that are different between the two. + /// The parameter updateReferenceData can be used to set the reference data to the SRG's constant data after the comparison. This is + /// useful for keeping track of differences in between calls to the function, such as between frames. void PrintConstantDataDiff(const ShaderResourceGroup& shaderResourceGroup, ConstantsData& referenceData, bool updateReferenceData = false); - void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData = false); + + /// Given a DrawItem, an SRG binding slot on that draw item and a reference ConstantsData input, this function will fetch the ConstantsData + /// from the draw item's SRG at the binding slot and compare it to the reference ConstantsData. It will print the names of any constants + /// that are different between the two. + /// The parameter updateReferenceData can be used to set the reference data to the draw item's constant data after the comparison. This is + /// useful for keeping track of differences in between calls to the function, such as between frames. + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, uint32_t srgBindingSlot, bool updateReferenceData = false); } } diff --git a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp index f14d1d4c4e..8be053e4ef 100644 --- a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp @@ -151,11 +151,11 @@ namespace AZ void ConstantsLayout::DebugPrintNames(AZStd::array_view constantList) const { AZStd::string output; - for (const ShaderInputConstantIndex& constandIdx : constantList) + for (const ShaderInputConstantIndex& constantIdx : constantList) { - if (constandIdx.GetIndex() < m_inputs.size()) + if (constantIdx.GetIndex() < m_inputs.size()) { - output += m_inputs[constandIdx.GetIndex()].m_name.GetCStr(); + output += m_inputs[constantIdx.GetIndex()].m_name.GetCStr(); output += " - "; } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp index d4caa3dd66..1524883e54 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp @@ -408,26 +408,26 @@ namespace AZ bool ConstantsData::ConstantIsEqual(const ConstantsData& other, ShaderInputConstantIndex inputIndex) const { - AZStd::array_view myConstans = GetConstantRaw(inputIndex); - AZStd::array_view otherConstans = other.GetConstantRaw(inputIndex); + AZStd::array_view myConstant = GetConstantRaw(inputIndex); + AZStd::array_view otherConstant = other.GetConstantRaw(inputIndex); // If they point to the same data, they are equal - if (myConstans == otherConstans) + if (myConstant == otherConstant) { return true; } // If they point to data of different size, they are not equal - if (myConstans.size() != otherConstans.size()) + if (myConstant.size() != otherConstant.size()) { return false; } // If they point to differing data of same size, compare the data // Note: due to small size of data this loop will be faster than a mem compare - for(uint32_t i = 0; i < myConstans.size(); ++i) + for(uint32_t i = 0; i < myConstant.size(); ++i) { - if (myConstans[i] != otherConstans[i]) + if (myConstant[i] != otherConstant[i]) { return false; } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp index 4bd8d0aab8..54399eba88 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp @@ -36,10 +36,10 @@ namespace AZ } } - void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData) + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, uint32_t srgBindingSlot, bool updateReferenceData) { - s32 srgIndex = -1; - for (u32 i = 0; i < drawItem.m_shaderResourceGroupCount; ++i) + int srgIndex = -1; + for (uint32_t i = 0; i < drawItem.m_shaderResourceGroupCount; ++i) { if (drawItem.m_shaderResourceGroups[i]->GetBindingSlot() == srgBindingSlot) { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index 13f2215c8b..2e61542c36 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -166,11 +166,14 @@ namespace AZ // clean up data m_drawListView = {}; m_combinedDrawList.clear(); + m_drawItemCount = 0; // draw list from view was sorted and if it's the only draw list then we can use it directly if (viewDrawList.size() > 0 && drawLists.size() == 0) { m_drawListView = viewDrawList; + m_drawItemCount += viewDrawList.size(); + PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); return; } @@ -178,7 +181,6 @@ namespace AZ drawLists.push_back(viewDrawList); // combine draw items from mutiple draw lists to one draw list and sort it. - m_drawItemCount = 0; for (auto drawList : drawLists) { m_drawItemCount += drawList.size(); From 43ae25b49b40b888d2e99d8d570e4fd23c0fdb13 Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Tue, 10 Aug 2021 10:00:42 -0700 Subject: [PATCH 3/4] fixed compiler loss of precision warning Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index 2e61542c36..4da5560908 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -172,7 +172,7 @@ namespace AZ if (viewDrawList.size() > 0 && drawLists.size() == 0) { m_drawListView = viewDrawList; - m_drawItemCount += viewDrawList.size(); + m_drawItemCount += (u32)viewDrawList.size(); PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); return; } @@ -183,7 +183,7 @@ namespace AZ // combine draw items from mutiple draw lists to one draw list and sort it. for (auto drawList : drawLists) { - m_drawItemCount += drawList.size(); + m_drawItemCount += (u32)drawList.size(); } PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); m_combinedDrawList.resize(m_drawItemCount); From 5472d6768e8a3ceb81f901251858bb58600439d0 Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Tue, 10 Aug 2021 12:48:26 -0700 Subject: [PATCH 4/4] Minor improvements Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- .../RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h | 6 +++--- .../RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h index cdbf983561..2c884ceedf 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h @@ -38,12 +38,12 @@ namespace AZ void SetDrawListTag(Name drawListName); - void SetPipelineStateDataIndex(u32 index); + void SetPipelineStateDataIndex(uint32_t index); //! Expose shader resource group. ShaderResourceGroup* GetShaderResourceGroup(); - u32 GetDrawItemCount(); + uint32_t GetDrawItemCount(); protected: explicit RasterPass(const PassDescriptor& descriptor); @@ -78,7 +78,7 @@ namespace AZ RHI::Viewport m_viewportState; bool m_overrideScissorSate = false; bool m_overrideViewportState = false; - u32 m_drawItemCount = 0; + uint32_t m_drawItemCount = 0; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index 4da5560908..ce02e1c570 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -104,7 +104,7 @@ namespace AZ m_flags.m_hasDrawListTag = true; } - void RasterPass::SetPipelineStateDataIndex(u32 index) + void RasterPass::SetPipelineStateDataIndex(uint32_t index) { m_pipelineStateDataIndex.m_index = index; } @@ -114,7 +114,7 @@ namespace AZ return m_shaderResourceGroup.get(); } - u32 RasterPass::GetDrawItemCount() + uint32_t RasterPass::GetDrawItemCount() { return m_drawItemCount; } @@ -172,7 +172,7 @@ namespace AZ if (viewDrawList.size() > 0 && drawLists.size() == 0) { m_drawListView = viewDrawList; - m_drawItemCount += (u32)viewDrawList.size(); + m_drawItemCount += static_cast(viewDrawList.size()); PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); return; } @@ -183,7 +183,7 @@ namespace AZ // combine draw items from mutiple draw lists to one draw list and sort it. for (auto drawList : drawLists) { - m_drawItemCount += (u32)drawList.size(); + m_drawItemCount += static_cast(drawList.size()); } PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); m_combinedDrawList.resize(m_drawItemCount); @@ -211,7 +211,7 @@ namespace AZ void RasterPass::SetupFrameGraphDependencies(RHI::FrameGraphInterface frameGraph) { RenderPass::SetupFrameGraphDependencies(frameGraph); - frameGraph.SetEstimatedItemCount(static_cast(m_drawListView.size())); + frameGraph.SetEstimatedItemCount(static_cast(m_drawListView.size())); } void RasterPass::CompileResources(const RHI::FrameGraphCompileContext& context)