From 2a982fa4b2e9c8a5d33b6bb4d9753f5eee0299ad Mon Sep 17 00:00:00 2001 From: antonmic Date: Thu, 10 Jun 2021 12:50:38 -0700 Subject: [PATCH] Pass changes: Addressing PR feedback --- .../CoreLights/CascadedShadowmapsPass.cpp | 4 +- .../DirectionalLightFeatureProcessor.cpp | 2 +- .../CoreLights/ProjectedShadowmapsPass.cpp | 2 +- .../DisplayMapper/DisplayMapperPass.cpp | 2 +- .../Code/Source/LuxCore/RenderTexturePass.cpp | 2 +- .../ProjectedShadowFeatureProcessor.cpp | 2 +- .../Code/Include/Atom/RPI.Public/Pass/Pass.h | 17 ++-- .../Atom/RPI.Public/Pass/PassDefines.h | 77 +++++++++++++------ .../Source/RPI.Public/Pass/ParentPass.cpp | 6 +- .../RPI/Code/Source/RPI.Public/Pass/Pass.cpp | 38 ++++++--- .../Pass/Specific/RenderToTexturePass.cpp | 2 +- .../RPI.Public/Pass/Specific/SelectorPass.cpp | 4 +- .../Pass/Specific/SwapChainPass.cpp | 2 +- .../Code/Source/RPI.Public/RenderPipeline.cpp | 8 +- 14 files changed, 109 insertions(+), 59 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/CoreLights/CascadedShadowmapsPass.cpp b/Gems/Atom/Feature/Common/Code/Source/CoreLights/CascadedShadowmapsPass.cpp index 4c7e69f2ae..b9beac4285 100644 --- a/Gems/Atom/Feature/Common/Code/Source/CoreLights/CascadedShadowmapsPass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/CoreLights/CascadedShadowmapsPass.cpp @@ -100,7 +100,7 @@ namespace AZ m_arraySize = arraySize; m_updateChildren = true; - QueueForBuild(); + QueueForBuildAndInitialization(); m_atlas.Initialize(); for (size_t cascadeIndex = 0; cascadeIndex < m_arraySize; ++cascadeIndex) @@ -215,7 +215,7 @@ namespace AZ AZ_RPI_PASS_WARNING(child, "CascadedShadowmapsPass child Pass creation failed for %d", cascadeIndex); if (child) { - child->QueueForBuild(); + child->QueueForBuildAndInitialization(); AddChild(child); } } diff --git a/Gems/Atom/Feature/Common/Code/Source/CoreLights/DirectionalLightFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/CoreLights/DirectionalLightFeatureProcessor.cpp index cdbe431949..c4f4bc54b3 100644 --- a/Gems/Atom/Feature/Common/Code/Source/CoreLights/DirectionalLightFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/CoreLights/DirectionalLightFeatureProcessor.cpp @@ -1106,7 +1106,7 @@ namespace AZ { for (EsmShadowmapsPass* pass : it.second) { - pass->QueueForBuild(); + pass->QueueForBuildAndInitialization(); } } } diff --git a/Gems/Atom/Feature/Common/Code/Source/CoreLights/ProjectedShadowmapsPass.cpp b/Gems/Atom/Feature/Common/Code/Source/CoreLights/ProjectedShadowmapsPass.cpp index 87cacbb345..2f1badaae7 100644 --- a/Gems/Atom/Feature/Common/Code/Source/CoreLights/ProjectedShadowmapsPass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/CoreLights/ProjectedShadowmapsPass.cpp @@ -73,7 +73,7 @@ namespace AZ { m_sizes = sizes; m_updateChildren = true; - QueueForBuild(); + QueueForBuildAndInitialization(); m_atlas.Initialize(); for (const auto& it : m_sizes) diff --git a/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp b/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp index 219d673ece..b37c218c21 100644 --- a/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp @@ -513,7 +513,7 @@ namespace AZ desc.m_acesParameterOverrides.m_overrideDefaults != m_displayMapperConfigurationDescriptor.m_acesParameterOverrides.m_overrideDefaults) { m_flags.m_createChildren = true; - QueueForBuild(); + QueueForBuildAndInitialization(); } m_displayMapperConfigurationDescriptor = desc; } diff --git a/Gems/Atom/Feature/Common/Code/Source/LuxCore/RenderTexturePass.cpp b/Gems/Atom/Feature/Common/Code/Source/LuxCore/RenderTexturePass.cpp index aa991e270a..6e0444dca4 100644 --- a/Gems/Atom/Feature/Common/Code/Source/LuxCore/RenderTexturePass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/LuxCore/RenderTexturePass.cpp @@ -38,7 +38,7 @@ namespace AZ m_attachmentSize = image->GetRHIImage()->GetDescriptor().m_size; m_attachmentFormat = format; m_shaderResourceGroup->SetImage(m_textureIndex, image); - QueueForBuild(); + QueueForBuildAndInitialization(); } void RenderTexturePass::BuildInternal() diff --git a/Gems/Atom/Feature/Common/Code/Source/Shadows/ProjectedShadowFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Shadows/ProjectedShadowFeatureProcessor.cpp index 8484fdddc2..da174ab0d5 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Shadows/ProjectedShadowFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Shadows/ProjectedShadowFeatureProcessor.cpp @@ -526,7 +526,7 @@ namespace AZ::Render for (EsmShadowmapsPass* esmPass : m_esmShadowmapsPasses) { - esmPass->QueueForBuild(); + esmPass->QueueForBuildAndInitialization(); } for (ProjectedShadowmapsPass* shadowPass : m_projectedShadowmapsPasses) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/Pass.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/Pass.h index b154b031c2..9d0b36d126 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/Pass.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/Pass.h @@ -81,7 +81,7 @@ namespace AZ //! ending with 'Internal' to define the behavior of your passes. These virtual are recursively //! called in preorder traversal throughout the pass tree. Only FrameBegin and FrameEnd are //! guaranteed to be called per frame. The other override-able functions are called as needed - //! when scheduled with the PassSystem. See QueueForBuild, QueueForRemoval and QueueForInitialization. + //! when scheduled with the PassSystem. See QueueForBuildAndInitialization, QueueForRemoval and QueueForInitialization. //! //! Passes are created by the PassFactory. They can be created using either Pass Name, //! a PassTemplate, or a PassRequest. To register your pass class with the PassFactory, @@ -154,8 +154,8 @@ namespace AZ // --- Utility functions --- - //! Queues the pass to have Build() called by the PassSystem on frame update - void QueueForBuild(); + //! Queues the pass to have Build() and Initilize() called by the PassSystem on frame update + void QueueForBuildAndInitialization(); //! Queues the pass to have RemoveFromParent() called by the PassSystem on frame update void QueueForRemoval(); @@ -319,12 +319,12 @@ namespace AZ // customize it's behavior, hence why these functions are called the pass behavior functions. // Resets everything in the pass (like Attachments). - // Called from PassSystem when pass is QueueForBuild. + // Called from PassSystem when pass is QueueForBuildAndInitialization. void Reset(); virtual void ResetInternal() { } // Builds and sets up any attachments and input/output connections the pass needs. - // Called from PassSystem when pass is QueueForBuild. + // Called from PassSystem when pass is QueueForBuildAndInitialization. void Build(bool calledFromPassSystem = false); virtual void BuildInternal() { } @@ -403,7 +403,8 @@ namespace AZ uint64_t m_parentEnabled : 1; // If this is a parent pass, indicates if the pass has already created children this frame - uint64_t m_alreadyCreated : 1; + // Prevents ParentPass::CreateChildPasses from executing multiple times in the same pass + uint64_t m_alreadyCreatedChildren : 1; // If this is a parent pass, indicates whether the pass needs to create child passes uint64_t m_createChildren : 1; @@ -453,6 +454,10 @@ namespace AZ // buffers and images don't get deleted during attachment build phase void StoreImportedAttachmentReferences(); + // Used by the RenderPipeline to create it's passes immediately instead of waiting on + // the next Pass System update. The function internally build and initializes the pass. + void ManualPipelineBuildAndInitialize(); + // --- Hierarchy related functions --- // Called when the pass gets a new spot in the pass hierarchy diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassDefines.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassDefines.h index e34c61e3aa..a7513cc53c 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassDefines.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassDefines.h @@ -27,46 +27,75 @@ namespace AZ namespace RPI { // This enum tracks the state of passes across build, initialization and rendering + // + // Standard order of state progression: + // + // Uninitialized -> Queued -> Resetting -> Reset -> Building -> Built -> Initializing -> Initialized -> Idle -> Rendering -> Idle ... + // + // Addition state transitions: + // + // Queued -> Resetting + // -> Building + // -> Initializing + // + // Idle -> Queued + // -> Resetting + // -> Building + // -> Initializing + // -> Rendering + // + // Rendering -> Idle + // -> Queued (Rendering will transition to Queued if a pass was queued with the PassSystem during Rendering) + // enum class PassState : u8 { // Default value, you should only ever see this in the Pass constructor // Once the constructor is done, the Pass will set it's state to Reset Uninitialized, - + // | + // | + // V // Pass is queued with the Pass System for an update (see PassQueueState below) - // From Queued, the pass can transition into Resetting, Building or Initializing depending on the PassQueueState Queued, - + // | + // | + // V // Pass is currently in the process of resetting - // From Resetting, the pass can transition into Resetting, - - // Pass has been reset and is await build - // From Reset, the pass can transition to Building + // | + // | + // V + // Pass has been reset and is awaiting build Reset, - + // | + // | + // V // Pass is currently building - // From Building, the pass can transition to Built Building, - + // | + // | + // V // Pass has been built and is awaiting initialization - // From Built, the pass can transition to Initializing Built, - + // | + // | + // V // Pass is currently being initialized - // From Initializing, the pass can transition to Initialized Initializing, - + // | + // | + // V // Pass has been initialized - // From Initialized, the pass can transition to Idle Initialized, - + // | + // | + // V // Idle state, pass is awaiting rendering - // From Idle, the pass can transition to Queued, Resetting, Building, Initializing or Rendering Idle, - + // | + // | + // V // Pass is currently rendering. Pass must be in Idle state before entering this state - // From Rendering, the pass can transition to Idle or Queue if the pass was queued with the Pass System during Rendering Rendering }; @@ -76,15 +105,15 @@ namespace AZ // The pass is currently not in any queued state and may therefore transition to any queued state NoQueue, - // The pass is queued for Removal at the start of the next frame. Cannot be overridden by any other queue state + // The pass is queued for Removal at the start of the next frame. Has the highest priority and cannot be overridden by any other queue state QueuedForRemoval, - // The pass is queued for Build at the start of the frame. Note that any pass built at the start of the frame will also be initialized. - // This state can be overridden by QueuedForRemoval - QueuedForBuild, + // The pass is queued for Build at the start of the frame. Note that any pass built at the start of the frame will also be Initialized. + // This state can be overridden by QueuedForRemoval, as we don't want to build a pass that has been removed. + QueuedForBuildAndInitialization, // The pass is queued for Initialization at the start of the frame. - // This state has the lowest priority and can therefore be overridden by QueueForBuild or QueueForRemoval + // This state has the lowest priority and can therefore be overridden by QueueForBuildAndInitialization or QueueForRemoval. QueuedForInitialization, }; } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/ParentPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/ParentPass.cpp index 773deafe68..cbae56e809 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/ParentPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/ParentPass.cpp @@ -59,7 +59,7 @@ namespace AZ child->m_parent = this; child->OnHierarchyChange(); - QueueForBuild(); + QueueForBuildAndInitialization(); // Notify pipeline if (m_pipeline) @@ -249,11 +249,11 @@ namespace AZ void ParentPass::CreateChildPasses() { // The already created flag prevents this function from executing multiple times a frame - if (!m_flags.m_createChildren || m_flags.m_alreadyCreated) + if (!m_flags.m_createChildren || m_flags.m_alreadyCreatedChildren) { return; } - m_flags.m_alreadyCreated = true; + m_flags.m_alreadyCreatedChildren = true; RemoveChildren(); CreatePassesFromTemplate(); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Pass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Pass.cpp index 7ae3559e38..99a6e23914 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Pass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Pass.cpp @@ -73,7 +73,7 @@ namespace AZ } PassSystemInterface::Get()->RegisterPass(this); - QueueForBuild(); + QueueForBuildAndInitialization(); // Skip reset since the pass just got created m_state = PassState::Reset; @@ -1038,15 +1038,22 @@ namespace AZ // --- Queuing functions with PassSystem --- - void Pass::QueueForBuild() + void Pass::QueueForBuildAndInitialization() { - // Queue if not already queued or if queued for initialization only. Don't queue if we're currently building. + // Don't queue if we're currently building. Don't queue if we're already queued for Build or Removal if (m_state != PassState::Building && - (m_queueState == PassQueueState::NoQueue || m_queueState == PassQueueState::QueuedForInitialization)) + m_queueState != PassQueueState::QueuedForBuildAndInitialization && + m_queueState != PassQueueState::QueuedForRemoval) { + // NOTE: We only queue for Build here, the queue for Initialization happens at the end of Pass::Build + // (doing it this way is an optimization to minimize the number of passes queued for initialization, + // as many passes will be initialized by their parent passes and thus don't need to be queued) PassSystemInterface::Get()->QueueForBuild(this); - m_queueState = PassQueueState::QueuedForBuild; + m_queueState = PassQueueState::QueuedForBuildAndInitialization; + + // Transition state + // If we are Rendering, the state will transition [Rendering -> Queued] in Pass::FrameEnd if (m_state != PassState::Rendering) { m_state = PassState::Queued; @@ -1056,12 +1063,16 @@ namespace AZ void Pass::QueueForInitialization() { - // Only queue if the pass is not in any other queue. Don't queue if we're currently initializing. + // Only queue if the pass is not in any queue. Don't queue if we're currently initializing. if (m_queueState == PassQueueState::NoQueue && m_state != PassState::Initializing) { PassSystemInterface::Get()->QueueForInitialization(this); m_queueState = PassQueueState::QueuedForInitialization; + // Transition state + // If we are Rendering, the state will transition [Rendering -> Queued] in Pass::FrameEnd + // If the state is Built, preserve the state since [Built -> Initializing] is a valid transition + // Preserving PassState::Built lets the pass ignore subsequent build calls in the same frame if (m_state != PassState::Rendering && m_state != PassState::Built) { m_state = PassState::Queued; @@ -1072,12 +1083,14 @@ namespace AZ void Pass::QueueForRemoval() { // Skip only if we're already queued for removal, otherwise proceed. - // QueuedForRemoval overrides QueuedForBuild and QueuedForInitialization. + // QueuedForRemoval overrides QueuedForBuildAndInitialization and QueuedForInitialization. if (m_queueState != PassQueueState::QueuedForRemoval) { PassSystemInterface::Get()->QueueForRemoval(this); m_queueState = PassQueueState::QueuedForRemoval; + // Transition state + // If we are Rendering, the state will transition [Rendering -> Queued] in Pass::FrameEnd if (m_state != PassState::Rendering) { m_state = PassState::Queued; @@ -1091,7 +1104,7 @@ namespace AZ { // Ensure we're in a valid state to reset. This ensures the pass won't be reset multiple times in the same frame. bool execute = (m_state == PassState::Idle); - execute = execute || (m_state == PassState::Queued && m_queueState == PassQueueState::QueuedForBuild); + execute = execute || (m_state == PassState::Queued && m_queueState == PassQueueState::QueuedForBuildAndInitialization); execute = execute || (m_state == PassState::Queued && m_queueState == PassQueueState::QueuedForInitialization); if (!execute) @@ -1187,7 +1200,7 @@ namespace AZ void Pass::OnInitializationFinished() { - m_flags.m_alreadyCreated = false; + m_flags.m_alreadyCreatedChildren = false; m_importedAttachmentStore.clear(); OnInitializationFinishedInternal(); @@ -1305,6 +1318,13 @@ namespace AZ return m_pipeline; } + void Pass::ManualPipelineBuildAndInitialize() + { + Build(); + Initialize(); + OnInitializationFinished(); + } + Scene* Pass::GetScene() const { if (m_pipeline) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/RenderToTexturePass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/RenderToTexturePass.cpp index c0a4785365..7306bf5b6a 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/RenderToTexturePass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/RenderToTexturePass.cpp @@ -100,7 +100,7 @@ namespace AZ m_passData.m_width = width; m_passData.m_height = height; OnUpdateOutputSize(); - QueueForBuild(); + QueueForBuildAndInitialization(); } void RenderToTexturePass::OnUpdateOutputSize() diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SelectorPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SelectorPass.cpp index 90390b78d7..82cea31ca1 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SelectorPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SelectorPass.cpp @@ -72,7 +72,7 @@ namespace AZ m_connections[outputSlotIndex] = inputSlotIndex; // Queue to rebuild attachment connections - QueueForBuild(); + QueueForBuildAndInitialization(); } void SelectorPass::Connect(const AZ::Name& inputSlot, const AZ::Name& outputSlot) @@ -113,7 +113,7 @@ namespace AZ m_connections[outputIdx] = inputIdx; // Queue to rebuild attachment connections - QueueForBuild(); + QueueForBuildAndInitialization(); } } // namespace RPI diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SwapChainPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SwapChainPass.cpp index a7add17042..7b79efbaec 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SwapChainPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Specific/SwapChainPass.cpp @@ -150,7 +150,7 @@ namespace AZ void SwapChainPass::OnWindowResized([[maybe_unused]] uint32_t width, [[maybe_unused]] uint32_t height) { - QueueForBuild(); + QueueForBuildAndInitialization(); } void SwapChainPass::ReadbackSwapChain(AZStd::shared_ptr readback) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp index 051282a7b0..891190738f 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp @@ -109,9 +109,7 @@ namespace AZ pipeline->m_rootPass->SetRenderPipeline(pipeline); // Manually build the pipeline so we can gather the view tags from it's passes - pipeline->m_rootPass->Build(); - pipeline->m_rootPass->Initialize(); - pipeline->m_rootPass->OnInitializationFinished(); + pipeline->m_rootPass->ManualPipelineBuildAndInitialize(); pipeline->BuildPipelineViews(); } @@ -327,9 +325,7 @@ namespace AZ newRoot->SetRenderPipeline(this); // Manually build the pipeline - newRoot->Build(); - newRoot->Initialize(); - newRoot->OnInitializationFinished(); + newRoot->ManualPipelineBuildAndInitialize(); // Validate the new root PassValidationResults validation;