From c5982c011504574154cfbc9dad982b3a154f8fb7 Mon Sep 17 00:00:00 2001 From: dmcdiar Date: Thu, 13 May 2021 19:03:38 -0700 Subject: [PATCH 1/2] Changed the RayTracingShaderTable build to be queued with the RHISystem and processed during FrameScheduler::Compile, after the Srgs are compiled. Removed the RayTracingShaderTable scopes and moved the calls to Build to the ray tracing dispatch scopes. --- .../DiffuseProbeGridRayTracingPass.cpp | 105 ++++++------------ .../DiffuseProbeGridRayTracingPass.h | 5 +- .../Code/Include/Atom/RHI/FrameScheduler.h | 8 ++ .../RHI/Code/Include/Atom/RHI/RHISystem.h | 1 + .../Include/Atom/RHI/RHISystemInterface.h | 3 + .../Include/Atom/RHI/RayTracingShaderTable.h | 19 +++- .../RHI/Code/Source/RHI/FrameScheduler.cpp | 28 +++++ Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp | 4 + .../Code/Source/RHI/RayTracingShaderTable.cpp | 25 +++-- .../Code/Source/RHI/RayTracingShaderTable.cpp | 14 ++- .../Code/Source/RHI/RayTracingShaderTable.h | 4 +- .../Code/Source/RHI/RayTracingShaderTable.h | 4 +- .../Code/Source/RHI/RayTracingShaderTable.cpp | 16 +-- .../Code/Source/RHI/RayTracingShaderTable.h | 2 +- 14 files changed, 135 insertions(+), 103 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp index 143ae1a08c..540b50f3ba 100644 --- a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp @@ -49,11 +49,6 @@ namespace AZ } } - DiffuseProbeGridRayTracingPass::~DiffuseProbeGridRayTracingPass() - { - delete m_rayTracingScopeProducerShaderTable; - } - void DiffuseProbeGridRayTracingPass::CreateRayTracingPipelineState() { RHI::Ptr device = RHI::RHISystemInterface::Get()->GetDevice(); @@ -118,19 +113,28 @@ namespace AZ void DiffuseProbeGridRayTracingPass::FrameBeginInternal(FramePrepareParams params) { + RPI::Scene* scene = m_pipeline->GetScene(); + RayTracingFeatureProcessor* rayTracingFeatureProcessor = scene->GetFeatureProcessor(); + if (!rayTracingFeatureProcessor) + { + return; + } + if (!m_initialized) { CreateRayTracingPipelineState(); - CreateShaderTableScope(); m_initialized = true; } if (!m_rayTracingShaderTable) { + RHI::Ptr device = RHI::RHISystemInterface::Get()->GetDevice(); + RHI::RayTracingBufferPools& rayTracingBufferPools = rayTracingFeatureProcessor->GetBufferPools(); + m_rayTracingShaderTable = RHI::Factory::Get().CreateRayTracingShaderTable(); + m_rayTracingShaderTable->Init(*device.get(), rayTracingBufferPools); } - RPI::Scene* scene = m_pipeline->GetScene(); DiffuseProbeGridFeatureProcessor* diffuseProbeGridFeatureProcessor = scene->GetFeatureProcessor(); if (!diffuseProbeGridFeatureProcessor || diffuseProbeGridFeatureProcessor->GetProbeGrids().empty()) { @@ -138,70 +142,9 @@ namespace AZ return; } - RayTracingFeatureProcessor* rayTracingFeatureProcessor = scene->GetFeatureProcessor(); - uint32_t rayTracingRevision = rayTracingFeatureProcessor->GetRevision(); - if (m_rayTracingRevision != rayTracingRevision) - { - // scene changed, need to rebuild the shader table - m_rayTracingRevision = rayTracingRevision; - - // [GFX TODO][ATOM-13575] Move the RHI::RayTracingShaderTable build into the RHI frame and remove this scope - params.m_frameGraphBuilder->ImportScopeProducer(*m_rayTracingScopeProducerShaderTable); - } - RenderPass::FrameBeginInternal(params); } - void DiffuseProbeGridRayTracingPass::CreateShaderTableScope() - { - struct ScopeData { }; - const auto prepareFunction = [this]([[maybe_unused]] RHI::FrameGraphInterface& scopeBuilder, [[maybe_unused]] ScopeData& scopeData) {}; - - const auto compileFunction = [this]([[maybe_unused]] const RHI::FrameGraphCompileContext& context, [[maybe_unused]] const ScopeData& scopeData) {}; - - const auto executeFunction = [this]([[maybe_unused]] const RHI::FrameGraphExecuteContext& context, [[maybe_unused]] const ScopeData& scopeData) - { - RHI::Ptr device = RHI::RHISystemInterface::Get()->GetDevice(); - RayTracingFeatureProcessor* rayTracingFeatureProcessor = m_pipeline->GetScene()->GetFeatureProcessor(); - RHI::RayTracingBufferPools& rayTracingBufferPools = rayTracingFeatureProcessor->GetBufferPools(); - - if (!rayTracingFeatureProcessor->GetSubMeshCount()) - { - m_rayTracingShaderTable = nullptr; - return; - } - - // build the ray tracing shader table descriptor - RHI::RayTracingShaderTableDescriptor descriptor; - RHI::RayTracingShaderTableDescriptor* descriptorBuild = descriptor.Build(AZ::Name("RayTracingShaderTable"), m_rayTracingPipelineState) - ->RayGenerationRecord(AZ::Name("RayGen")) - ->MissRecord(AZ::Name("Miss")); - - // add a hit group for each mesh to the shader table - for (uint32_t i = 0; i < rayTracingFeatureProcessor->GetSubMeshCount(); ++i) - { - descriptorBuild->HitGroupRecord(AZ::Name("HitGroup")); - } - - m_rayTracingShaderTable->Init(*device.get(), &descriptor, rayTracingBufferPools); - }; - - AZStd::string uuidString = AZ::Uuid::CreateRandom().ToString(); - AZStd::string scopeName = AZStd::string::format("DiffuseProbeRayTracingBuildShaderTable_%s", uuidString.c_str()); - - m_rayTracingScopeProducerShaderTable = - aznew RHI::ScopeProducerFunction< - ScopeData, - decltype(prepareFunction), - decltype(compileFunction), - decltype(executeFunction)>( - RHI::ScopeId{ scopeName }, - ScopeData{ }, - prepareFunction, - compileFunction, - executeFunction); - } - void DiffuseProbeGridRayTracingPass::SetupFrameGraphDependencies(RHI::FrameGraphInterface frameGraph) { RenderPass::SetupFrameGraphDependencies(frameGraph); @@ -211,7 +154,6 @@ namespace AZ RayTracingFeatureProcessor* rayTracingFeatureProcessor = scene->GetFeatureProcessor(); frameGraph.SetEstimatedItemCount(aznumeric_cast(diffuseProbeGridFeatureProcessor->GetProbeGrids().size())); - frameGraph.ExecuteAfter(m_rayTracingScopeProducerShaderTable->GetScopeId()); for (const auto& diffuseProbeGrid : diffuseProbeGridFeatureProcessor->GetProbeGrids()) { @@ -326,6 +268,31 @@ namespace AZ diffuseProbeGrid->GetRayTraceSrg()->Compile(); } } + + uint32_t rayTracingRevision = rayTracingFeatureProcessor->GetRevision(); + if (m_rayTracingRevision != rayTracingRevision) + { + // scene changed, need to rebuild the shader table + m_rayTracingRevision = rayTracingRevision; + + m_rayTracingShaderTableDescriptor = AZStd::make_shared(); + + if (rayTracingFeatureProcessor->GetSubMeshCount()) + { + // build the ray tracing shader table descriptor + RHI::RayTracingShaderTableDescriptor* descriptorBuild = m_rayTracingShaderTableDescriptor->Build(AZ::Name("RayTracingShaderTable"), m_rayTracingPipelineState) + ->RayGenerationRecord(AZ::Name("RayGen")) + ->MissRecord(AZ::Name("Miss")); + + // add a hit group for each mesh to the shader table + for (uint32_t i = 0; i < rayTracingFeatureProcessor->GetSubMeshCount(); ++i) + { + descriptorBuild->HitGroupRecord(AZ::Name("HitGroup")); + } + } + + m_rayTracingShaderTable->Build(m_rayTracingShaderTableDescriptor); + } } void DiffuseProbeGridRayTracingPass::BuildCommandListInternal([[maybe_unused]] const RHI::FrameGraphExecuteContext& context) diff --git a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h index 570493d573..67e5503825 100644 --- a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h +++ b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h @@ -35,8 +35,6 @@ namespace AZ AZ_RTTI(DiffuseProbeGridRayTracingPass, "{CB0DF817-3D07-4AC7-8574-F5EE529B8DCA}", RPI::RenderPass); AZ_CLASS_ALLOCATOR(DiffuseProbeGridRayTracingPass, SystemAllocator, 0); - virtual ~DiffuseProbeGridRayTracingPass() override; - //! Creates a DiffuseProbeGridRayTracingPass static RPI::Ptr Create(const RPI::PassDescriptor& descriptor); @@ -44,7 +42,6 @@ namespace AZ explicit DiffuseProbeGridRayTracingPass(const RPI::PassDescriptor& descriptor); void CreateRayTracingPipelineState(); - void CreateShaderTableScope(); // Scope producer functions void SetupFrameGraphDependencies(RHI::FrameGraphInterface frameGraph) override; @@ -65,7 +62,7 @@ namespace AZ // ray tracing shader table RHI::Ptr m_rayTracingShaderTable; - RHI::ScopeProducer* m_rayTracingScopeProducerShaderTable = nullptr; + AZStd::shared_ptr m_rayTracingShaderTableDescriptor; // ray tracing global shader resource group asset and pipeline state Data::Asset m_globalSrgAsset; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h index b43ca2124c..cc603b89ae 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h @@ -31,6 +31,7 @@ namespace AZ { class ShaderResourceGroupPool; class FrameGraphExecuteGroup; + class RayTracingShaderTable; //! @brief Fill this descriptor when initializing a FrameScheduler instance. struct FrameSchedulerDescriptor @@ -181,6 +182,9 @@ namespace AZ const TransientAttachmentPoolDescriptor* GetTransientAttachmentPoolDescriptor() const; + //! Adds a RayTracingShaderTable to be built this frame + void QueueRayTracingShaderTableForBuild(RayTracingShaderTable* rayTracingShaderTable); + private: const ScopeId m_rootScopeId{"Root"}; @@ -190,6 +194,7 @@ namespace AZ void PrepareProducers(); void CompileProducers(); void CompileShaderResourceGroups(); + void BuildRayTracingShaderTables(); ScopeProducer* FindScopeProducer(const ScopeId& scopeId); @@ -224,6 +229,9 @@ namespace AZ AZStd::unique_ptr m_rootScopeProducer; AZStd::vector m_scopeProducers; AZStd::unordered_map m_scopeProducerLookup; + + // list of RayTracingShaderTables that should be built this frame + AZStd::vector m_rayTracingShaderTablesToBuild; }; } } diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h index ccfaff3f11..4e0fcf4380 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h @@ -57,6 +57,7 @@ namespace AZ const RHI::TransientAttachmentStatistics* GetTransientAttachmentStatistics() const override; const RHI::TransientAttachmentPoolDescriptor* GetTransientAttachmentPoolDescriptor() const override; ConstPtr GetPlatformLimitsDescriptor() const override; + void QueueRayTracingShaderTableForBuild(RayTracingShaderTable* rayTracingShaderTable) override; ////////////////////////////////////////////////////////////////////////// private: diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystemInterface.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystemInterface.h index 784f9344b4..8f5f514eba 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystemInterface.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystemInterface.h @@ -26,6 +26,7 @@ namespace AZ class PipelineState; class PipelineStateCache; class PlatformLimitsDescriptor; + class RayTracingShaderTable; struct CpuTimingStatistics; struct FrameSchedulerCompileRequest; struct TransientAttachmentStatistics; @@ -61,6 +62,8 @@ namespace AZ virtual const RHI::TransientAttachmentPoolDescriptor* GetTransientAttachmentPoolDescriptor() const = 0; virtual ConstPtr GetPlatformLimitsDescriptor() const = 0; + + virtual void QueueRayTracingShaderTableForBuild(RayTracingShaderTable* rayTracingShaderTable) = 0; }; //! This bus exists to give RHI samples the ability to slot in scopes manually diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h index 5058beda68..bb16005ef5 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h @@ -97,18 +97,29 @@ namespace AZ virtual ~RayTracingShaderTable() = default; static RHI::Ptr CreateRHIRayTracingShaderTable(); + void Init(Device& device, const RayTracingBufferPools& rayTracingBufferPools); - ResultCode Init(Device& device, const RayTracingShaderTableDescriptor* descriptor, const RayTracingBufferPools& rayTracingBufferPools); + //! Queues this RayTracingShaderTable to be built by the FrameScheduler + void Build(const AZStd::shared_ptr descriptor); + + protected: + + AZStd::weak_ptr m_descriptor; + const RayTracingBufferPools* m_bufferPools = nullptr; private: - // explicit shutdown is not allowed for this type - void Shutdown() override final; + friend class FrameScheduler; + + /// Called by the FrameScheduler to validate the state prior to building + void Validate(); ////////////////////////////////////////////////////////////////////////// // Platform API - virtual RHI::ResultCode InitInternal(RHI::Device& deviceBase, const RHI::RayTracingShaderTableDescriptor* descriptor, const RayTracingBufferPools& bufferPools) = 0; + virtual RHI::ResultCode BuildInternal() = 0; ////////////////////////////////////////////////////////////////////////// + + bool m_isQueuedForBuild = false; }; } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/FrameScheduler.cpp b/Gems/Atom/RHI/Code/Source/RHI/FrameScheduler.cpp index f2fccf8da9..67cad6c4dc 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/FrameScheduler.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/FrameScheduler.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -204,6 +205,9 @@ namespace AZ // Compile all invalidated shader resource groups. CompileShaderResourceGroups(); + + // Build RayTracingShaderTables + BuildRayTracingShaderTables(); } return outcome; } @@ -314,6 +318,25 @@ namespace AZ } } + void FrameScheduler::BuildRayTracingShaderTables() + { + AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzRender); + AZ_ATOM_PROFILE_FUNCTION("RHI", "FrameScheduler: BuildRayTracingShaderTables"); + + for (auto rayTracingShaderTable : m_rayTracingShaderTablesToBuild) + { + rayTracingShaderTable->Validate(); + + [[maybe_unused]] ResultCode resultCode = rayTracingShaderTable->BuildInternal(); + AZ_Assert(resultCode == ResultCode::Success, "RayTracingShaderTable build failed"); + + rayTracingShaderTable->m_isQueuedForBuild = false; + } + + // clear the list now that all RayTracingShaderTables have been built for this frame + m_rayTracingShaderTablesToBuild.clear(); + } + ResultCode FrameScheduler::BeginFrame() { AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzRender); @@ -528,5 +551,10 @@ namespace AZ { return m_transientAttachmentPool ? &m_transientAttachmentPool->GetDescriptor() : nullptr; } + + void FrameScheduler::QueueRayTracingShaderTableForBuild(RayTracingShaderTable* rayTracingShaderTable) + { + m_rayTracingShaderTablesToBuild.push_back(rayTracingShaderTable); + } } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp b/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp index 95e0a33981..81409e2c18 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp @@ -290,5 +290,9 @@ namespace AZ return m_platformLimitsDescriptor; } + void RHISystem::QueueRayTracingShaderTableForBuild(RayTracingShaderTable* rayTracingShaderTable) + { + m_frameScheduler.QueueRayTracingShaderTableForBuild(rayTracingShaderTable); + } } //namespace RPI } //namespace AZ diff --git a/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp b/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp index 468dc89f09..07204a26f0 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp @@ -12,6 +12,7 @@ #include #include +#include namespace AZ { @@ -77,21 +78,29 @@ namespace AZ return rayTracingShaderTable; } - ResultCode RayTracingShaderTable::Init(Device& device, const RayTracingShaderTableDescriptor* descriptor, const RayTracingBufferPools& bufferPools) + void RayTracingShaderTable::Init(Device& device, const RayTracingBufferPools& bufferPools) { #if defined (AZ_RHI_ENABLE_VALIDATION) // [GFX TODO][ATOM-5217] Validate shaders in the ray tracing shader table are present in the pipeline state #endif - ResultCode resultCode = InitInternal(device, descriptor, bufferPools); - if (resultCode == ResultCode::Success) - { - DeviceObject::Init(device); - } - return resultCode; + DeviceObject::Init(device); + m_bufferPools = &bufferPools; } - void RayTracingShaderTable::Shutdown() + void RayTracingShaderTable::Build(const AZStd::shared_ptr descriptor) { + AZ_Assert(!m_isQueuedForBuild, "Attempting to build a RayTracingShaderTable that's already been queued. Only build once per frame.") + m_descriptor = descriptor; + + RHI::RHISystemInterface::Get()->QueueRayTracingShaderTableForBuild(this); + m_isQueuedForBuild = true; + } + + void RayTracingShaderTable::Validate() + { + AZ_Assert(m_isQueuedForBuild, "Attempting to build a RayTracingShaderTable that is not queued."); + AZ_Assert(!m_descriptor.expired(), "RayTracingShaderTable descriptor is no longer valid, make sure it is not freed after calling Build."); + AZ_Assert(m_bufferPools, "RayTracingBufferPools pointer is null."); } } } diff --git a/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp b/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp index 029033fd9a..7899e7dc21 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp +++ b/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp @@ -121,15 +121,17 @@ namespace AZ } #endif - RHI::ResultCode RayTracingShaderTable::InitInternal([[maybe_unused]] RHI::Device& deviceBase, [[maybe_unused]] const RHI::RayTracingShaderTableDescriptor* descriptor, [[maybe_unused]] const RHI::RayTracingBufferPools& bufferPools) + RHI::ResultCode RayTracingShaderTable::BuildInternal() { #ifdef AZ_DX12_DXR_SUPPORT // advance to the next buffer m_currentBufferIndex = (m_currentBufferIndex + 1) % BufferCount; ShaderTableBuffers& buffers = m_buffers[m_currentBufferIndex]; - // clear the shader table if a null descriptor was passed in - if (!descriptor) + AZStd::shared_ptr descriptor = m_descriptor.lock(); + + // clear the shader table if the descriptor has no ray generation shader + if (descriptor->GetRayGenerationRecord().empty()) { buffers.m_rayGenerationTable = nullptr; buffers.m_rayGenerationTableSize = 0; @@ -156,7 +158,7 @@ namespace AZ AZ_Assert(descriptor->GetRayGenerationRecord().size() == 1, "Descriptor must contain one and only one RayGeneration record"); uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(descriptor->GetRayGenerationRecord()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); - buffers.m_rayGenerationTable = BuildTable(deviceBase, bufferPools, descriptor->GetRayGenerationRecord(), shaderRecordSize, L"Ray Generation Shader Table", stateObjectProperties); + buffers.m_rayGenerationTable = BuildTable(GetDevice(), *m_bufferPools, descriptor->GetRayGenerationRecord(), shaderRecordSize, L"Ray Generation Shader Table", stateObjectProperties); buffers.m_rayGenerationTableSize = shaderRecordSize; } @@ -164,7 +166,7 @@ namespace AZ { uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(descriptor->GetMissRecords()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); - buffers.m_missTable = BuildTable(deviceBase, bufferPools, descriptor->GetMissRecords(), shaderRecordSize, L"Miss Shader Table", stateObjectProperties); + buffers.m_missTable = BuildTable(GetDevice(), *m_bufferPools, descriptor->GetMissRecords(), shaderRecordSize, L"Miss Shader Table", stateObjectProperties); buffers.m_missTableSize = shaderRecordSize * static_cast(descriptor->GetMissRecords().size()); buffers.m_missTableStride = shaderRecordSize; } @@ -173,7 +175,7 @@ namespace AZ { uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(descriptor->GetHitGroupRecords()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); - buffers.m_hitGroupTable = BuildTable(deviceBase, bufferPools, descriptor->GetHitGroupRecords(), shaderRecordSize, L"HitGroup Shader Table", stateObjectProperties); + buffers.m_hitGroupTable = BuildTable(GetDevice(), *m_bufferPools, descriptor->GetHitGroupRecords(), shaderRecordSize, L"HitGroup Shader Table", stateObjectProperties); buffers.m_hitGroupTableSize = shaderRecordSize * static_cast(descriptor->GetHitGroupRecords().size()); buffers.m_hitGroupTableStride = shaderRecordSize; } diff --git a/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.h b/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.h index 848538d222..df6b2cbc84 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.h +++ b/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.h @@ -61,8 +61,8 @@ namespace AZ #endif ////////////////////////////////////////////////////////////////////////// - // RHI::PipelineState - RHI::ResultCode InitInternal(RHI::Device& deviceBase, const RHI::RayTracingShaderTableDescriptor* descriptor, const RHI::RayTracingBufferPools& bufferPools) override; + // RHI::RayTracingShaderTable + RHI::ResultCode BuildInternal() override; ////////////////////////////////////////////////////////////////////////// static const uint32_t BufferCount = 3; diff --git a/Gems/Atom/RHI/Null/Code/Source/RHI/RayTracingShaderTable.h b/Gems/Atom/RHI/Null/Code/Source/RHI/RayTracingShaderTable.h index f36242ff22..6eb32e44ce 100644 --- a/Gems/Atom/RHI/Null/Code/Source/RHI/RayTracingShaderTable.h +++ b/Gems/Atom/RHI/Null/Code/Source/RHI/RayTracingShaderTable.h @@ -33,8 +33,8 @@ namespace AZ RayTracingShaderTable() = default; ////////////////////////////////////////////////////////////////////////// - // RHI::PipelineState - RHI::ResultCode InitInternal([[maybe_unused]] RHI::Device& deviceBase, [[maybe_unused]] const RHI::RayTracingShaderTableDescriptor* descriptor, [[maybe_unused]] const RHI::RayTracingBufferPools& bufferPools) override {return RHI::ResultCode::Success;} + // RHI::RayTracingShaderTable + RHI::ResultCode BuildInternal() override {return RHI::ResultCode::Success;} ////////////////////////////////////////////////////////////////////////// }; } diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp index 0fa63a24d8..c76cd9573a 100644 --- a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp +++ b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp @@ -75,20 +75,22 @@ namespace AZ return static_cast(shaderTableBuffer.get()); } - RHI::ResultCode RayTracingShaderTable::InitInternal([[maybe_unused]] RHI::Device& deviceBase, [[maybe_unused]] const RHI::RayTracingShaderTableDescriptor* descriptor, [[maybe_unused]] const RHI::RayTracingBufferPools& bufferPools) + RHI::ResultCode RayTracingShaderTable::BuildInternal() { - auto& device = static_cast(deviceBase); + auto& device = static_cast(GetDevice()); auto& physicalDevice = static_cast(device.GetPhysicalDevice()); const VkPhysicalDeviceRayTracingPipelinePropertiesKHR& rayTracingPipelineProperties = physicalDevice.GetPhysicalDeviceRayTracingPipelineProperties(); uint32_t shaderHandleSize = rayTracingPipelineProperties.shaderGroupHandleSize; uint32_t alignedShaderHandleSize = RHI::AlignUp(shaderHandleSize, rayTracingPipelineProperties.shaderGroupBaseAlignment); + AZStd::shared_ptr descriptor = m_descriptor.lock(); + // advance to the next buffer m_currentBufferIndex = (m_currentBufferIndex + 1) % BufferCount; ShaderTableBuffers& buffers = m_buffers[m_currentBufferIndex]; - // clear the shader table if a null descriptor was passed in - if (!descriptor) + // clear the shader table if the descriptor has no ray generation shader + if (descriptor->GetRayGenerationRecord().empty()) { buffers.m_rayGenerationTable = nullptr; buffers.m_rayGenerationTableStride = 0; @@ -118,7 +120,7 @@ namespace AZ buffers.m_rayGenerationTable = BuildTable( rayTracingPipelineProperties, rayTracingPipelineState, - bufferPools, + *m_bufferPools, descriptor->GetRayGenerationRecord(), buffers.m_rayGenerationTableStride, "RayGenerationTable"); @@ -126,7 +128,7 @@ namespace AZ buffers.m_missTable = BuildTable( rayTracingPipelineProperties, rayTracingPipelineState, - bufferPools, + *m_bufferPools, descriptor->GetMissRecords(), buffers.m_missTableStride, "MissTable"); @@ -134,7 +136,7 @@ namespace AZ buffers.m_hitGroupTable = BuildTable( rayTracingPipelineProperties, rayTracingPipelineState, - bufferPools, + *m_bufferPools, descriptor->GetHitGroupRecords(), buffers.m_hitGroupTableStride, "HitGroupTable"); diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.h b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.h index 1fc8615c5e..a931aded57 100644 --- a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.h +++ b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.h @@ -59,7 +59,7 @@ namespace AZ ////////////////////////////////////////////////////////////////////////// // RHI::RayTracingShaderTable - RHI::ResultCode InitInternal(RHI::Device& deviceBase, const RHI::RayTracingShaderTableDescriptor* descriptor, const RHI::RayTracingBufferPools& bufferPools) override; + RHI::ResultCode BuildInternal() override; ////////////////////////////////////////////////////////////////////////// static const uint32_t BufferCount = 3; From 73f438e18537ec301b9257179f4c50eac38d34a6 Mon Sep 17 00:00:00 2001 From: dmcdiar Date: Fri, 14 May 2021 19:10:25 -0700 Subject: [PATCH 2/2] Changed the descriptor a shared_ptr. --- .../DiffuseProbeGridRayTracingPass.cpp | 6 ++--- .../DiffuseProbeGridRayTracingPass.h | 1 - .../Code/Include/Atom/RHI/FrameScheduler.h | 4 ++-- .../Include/Atom/RHI/RayTracingShaderTable.h | 5 ++-- .../Code/Source/RHI/RayTracingShaderTable.cpp | 1 - .../Code/Source/RHI/RayTracingShaderTable.cpp | 24 +++++++++---------- .../Code/Source/RHI/RayTracingShaderTable.cpp | 18 +++++++------- 7 files changed, 27 insertions(+), 32 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp index 540b50f3ba..b2b5f60660 100644 --- a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.cpp @@ -275,12 +275,12 @@ namespace AZ // scene changed, need to rebuild the shader table m_rayTracingRevision = rayTracingRevision; - m_rayTracingShaderTableDescriptor = AZStd::make_shared(); + AZStd::shared_ptr descriptor = AZStd::make_shared(); if (rayTracingFeatureProcessor->GetSubMeshCount()) { // build the ray tracing shader table descriptor - RHI::RayTracingShaderTableDescriptor* descriptorBuild = m_rayTracingShaderTableDescriptor->Build(AZ::Name("RayTracingShaderTable"), m_rayTracingPipelineState) + RHI::RayTracingShaderTableDescriptor* descriptorBuild = descriptor->Build(AZ::Name("RayTracingShaderTable"), m_rayTracingPipelineState) ->RayGenerationRecord(AZ::Name("RayGen")) ->MissRecord(AZ::Name("Miss")); @@ -291,7 +291,7 @@ namespace AZ } } - m_rayTracingShaderTable->Build(m_rayTracingShaderTableDescriptor); + m_rayTracingShaderTable->Build(descriptor); } } diff --git a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h index 67e5503825..bb35803e51 100644 --- a/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h +++ b/Gems/Atom/Feature/Common/Code/Source/DiffuseProbeGrid/DiffuseProbeGridRayTracingPass.h @@ -62,7 +62,6 @@ namespace AZ // ray tracing shader table RHI::Ptr m_rayTracingShaderTable; - AZStd::shared_ptr m_rayTracingShaderTableDescriptor; // ray tracing global shader resource group asset and pipeline state Data::Asset m_globalSrgAsset; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h index cc603b89ae..4cb0280837 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/FrameScheduler.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -31,7 +32,6 @@ namespace AZ { class ShaderResourceGroupPool; class FrameGraphExecuteGroup; - class RayTracingShaderTable; //! @brief Fill this descriptor when initializing a FrameScheduler instance. struct FrameSchedulerDescriptor @@ -231,7 +231,7 @@ namespace AZ AZStd::unordered_map m_scopeProducerLookup; // list of RayTracingShaderTables that should be built this frame - AZStd::vector m_rayTracingShaderTablesToBuild; + AZStd::vector> m_rayTracingShaderTablesToBuild; }; } } diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h index bb16005ef5..57ab9b87fc 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/RayTracingShaderTable.h @@ -99,12 +99,13 @@ namespace AZ static RHI::Ptr CreateRHIRayTracingShaderTable(); void Init(Device& device, const RayTracingBufferPools& rayTracingBufferPools); - //! Queues this RayTracingShaderTable to be built by the FrameScheduler + //! Queues this RayTracingShaderTable to be built by the FrameScheduler. + //! Note that the descriptor must be heap allocated, preferably using make_shared. void Build(const AZStd::shared_ptr descriptor); protected: - AZStd::weak_ptr m_descriptor; + AZStd::shared_ptr m_descriptor; const RayTracingBufferPools* m_bufferPools = nullptr; private: diff --git a/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp b/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp index 07204a26f0..5934cb2e22 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/RayTracingShaderTable.cpp @@ -99,7 +99,6 @@ namespace AZ void RayTracingShaderTable::Validate() { AZ_Assert(m_isQueuedForBuild, "Attempting to build a RayTracingShaderTable that is not queued."); - AZ_Assert(!m_descriptor.expired(), "RayTracingShaderTable descriptor is no longer valid, make sure it is not freed after calling Build."); AZ_Assert(m_bufferPools, "RayTracingBufferPools pointer is null."); } } diff --git a/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp b/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp index 7899e7dc21..f7d32f9392 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp +++ b/Gems/Atom/RHI/DX12/Code/Source/RHI/RayTracingShaderTable.cpp @@ -128,10 +128,8 @@ namespace AZ m_currentBufferIndex = (m_currentBufferIndex + 1) % BufferCount; ShaderTableBuffers& buffers = m_buffers[m_currentBufferIndex]; - AZStd::shared_ptr descriptor = m_descriptor.lock(); - // clear the shader table if the descriptor has no ray generation shader - if (descriptor->GetRayGenerationRecord().empty()) + if (m_descriptor->GetRayGenerationRecord().empty()) { buffers.m_rayGenerationTable = nullptr; buffers.m_rayGenerationTableSize = 0; @@ -146,7 +144,7 @@ namespace AZ // retrieve the ID3D12StateObjectProperties interface from the raytracing pipeline state object // this is needed to get the shader identifiers to put in the table - const RayTracingPipelineState* rayTracingPipelineState = static_cast(descriptor->GetPipelineState().get()); + const RayTracingPipelineState* rayTracingPipelineState = static_cast(m_descriptor->GetPipelineState().get()); Microsoft::WRL::ComPtr stateObjectProperties; [[maybe_unused]] HRESULT hr = rayTracingPipelineState->Get()->QueryInterface(IID_GRAPHICS_PPV_ARGS(stateObjectProperties.GetAddressOf())); @@ -155,28 +153,28 @@ namespace AZ // ray generation shader table { // RayGeneration table must have one and only one record - AZ_Assert(descriptor->GetRayGenerationRecord().size() == 1, "Descriptor must contain one and only one RayGeneration record"); - uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(descriptor->GetRayGenerationRecord()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); + AZ_Assert(m_descriptor->GetRayGenerationRecord().size() == 1, "Descriptor must contain one and only one RayGeneration record"); + uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(m_descriptor->GetRayGenerationRecord()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); - buffers.m_rayGenerationTable = BuildTable(GetDevice(), *m_bufferPools, descriptor->GetRayGenerationRecord(), shaderRecordSize, L"Ray Generation Shader Table", stateObjectProperties); + buffers.m_rayGenerationTable = BuildTable(GetDevice(), *m_bufferPools, m_descriptor->GetRayGenerationRecord(), shaderRecordSize, L"Ray Generation Shader Table", stateObjectProperties); buffers.m_rayGenerationTableSize = shaderRecordSize; } // miss shader table { - uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(descriptor->GetMissRecords()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); + uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(m_descriptor->GetMissRecords()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); - buffers.m_missTable = BuildTable(GetDevice(), *m_bufferPools, descriptor->GetMissRecords(), shaderRecordSize, L"Miss Shader Table", stateObjectProperties); - buffers.m_missTableSize = shaderRecordSize * static_cast(descriptor->GetMissRecords().size()); + buffers.m_missTable = BuildTable(GetDevice(), *m_bufferPools, m_descriptor->GetMissRecords(), shaderRecordSize, L"Miss Shader Table", stateObjectProperties); + buffers.m_missTableSize = shaderRecordSize * static_cast(m_descriptor->GetMissRecords().size()); buffers.m_missTableStride = shaderRecordSize; } // hit group shader table { - uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(descriptor->GetHitGroupRecords()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); + uint32_t shaderRecordSize = RHI::AlignUp(FindLargestRecordSize(m_descriptor->GetHitGroupRecords()), D3D12_RAYTRACING_SHADER_RECORD_BYTE_ALIGNMENT); - buffers.m_hitGroupTable = BuildTable(GetDevice(), *m_bufferPools, descriptor->GetHitGroupRecords(), shaderRecordSize, L"HitGroup Shader Table", stateObjectProperties); - buffers.m_hitGroupTableSize = shaderRecordSize * static_cast(descriptor->GetHitGroupRecords().size()); + buffers.m_hitGroupTable = BuildTable(GetDevice(), *m_bufferPools, m_descriptor->GetHitGroupRecords(), shaderRecordSize, L"HitGroup Shader Table", stateObjectProperties); + buffers.m_hitGroupTableSize = shaderRecordSize * static_cast(m_descriptor->GetHitGroupRecords().size()); buffers.m_hitGroupTableStride = shaderRecordSize; } #endif diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp index c76cd9573a..d6d6f3b3c0 100644 --- a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp +++ b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/RayTracingShaderTable.cpp @@ -83,14 +83,12 @@ namespace AZ uint32_t shaderHandleSize = rayTracingPipelineProperties.shaderGroupHandleSize; uint32_t alignedShaderHandleSize = RHI::AlignUp(shaderHandleSize, rayTracingPipelineProperties.shaderGroupBaseAlignment); - AZStd::shared_ptr descriptor = m_descriptor.lock(); - // advance to the next buffer m_currentBufferIndex = (m_currentBufferIndex + 1) % BufferCount; ShaderTableBuffers& buffers = m_buffers[m_currentBufferIndex]; // clear the shader table if the descriptor has no ray generation shader - if (descriptor->GetRayGenerationRecord().empty()) + if (m_descriptor->GetRayGenerationRecord().empty()) { buffers.m_rayGenerationTable = nullptr; buffers.m_rayGenerationTableStride = 0; @@ -110,18 +108,18 @@ namespace AZ buffers.m_hitGroupTableStride = RHI::AlignUp(alignedShaderHandleSize, rayTracingPipelineProperties.shaderGroupBaseAlignment); // calculate sub-table sizes - buffers.m_rayGenerationTableSize = buffers.m_rayGenerationTableStride * aznumeric_cast(descriptor->GetRayGenerationRecord().size()); - buffers.m_missTableSize = buffers.m_missTableStride * aznumeric_cast(descriptor->GetMissRecords().size()); - buffers.m_hitGroupTableSize = buffers.m_hitGroupTableStride * aznumeric_cast(descriptor->GetHitGroupRecords().size()); + buffers.m_rayGenerationTableSize = buffers.m_rayGenerationTableStride * aznumeric_cast(m_descriptor->GetRayGenerationRecord().size()); + buffers.m_missTableSize = buffers.m_missTableStride * aznumeric_cast(m_descriptor->GetMissRecords().size()); + buffers.m_hitGroupTableSize = buffers.m_hitGroupTableStride * aznumeric_cast(m_descriptor->GetHitGroupRecords().size()); - const RayTracingPipelineState* rayTracingPipelineState = static_cast(descriptor->GetPipelineState().get()); + const RayTracingPipelineState* rayTracingPipelineState = static_cast(m_descriptor->GetPipelineState().get()); // build sub-tables buffers.m_rayGenerationTable = BuildTable( rayTracingPipelineProperties, rayTracingPipelineState, *m_bufferPools, - descriptor->GetRayGenerationRecord(), + m_descriptor->GetRayGenerationRecord(), buffers.m_rayGenerationTableStride, "RayGenerationTable"); @@ -129,7 +127,7 @@ namespace AZ rayTracingPipelineProperties, rayTracingPipelineState, *m_bufferPools, - descriptor->GetMissRecords(), + m_descriptor->GetMissRecords(), buffers.m_missTableStride, "MissTable"); @@ -137,7 +135,7 @@ namespace AZ rayTracingPipelineProperties, rayTracingPipelineState, *m_bufferPools, - descriptor->GetHitGroupRecords(), + m_descriptor->GetHitGroupRecords(), buffers.m_hitGroupTableStride, "HitGroupTable");