Pass changes: Addressing PR feedback

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);
                 }
             }

Gems/Atom/Feature/Common/Code/Source/CoreLights/DirectionalLightFeatureProcessor.cpp

@@ -1106,7 +1106,7 @@ namespace AZ
                     {
                         for (EsmShadowmapsPass* pass : it.second)
                         {
-                            pass->QueueForBuild();
+                            pass->QueueForBuildAndInitialization();
                         }
                     }
                 }

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)

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;
                 }

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()

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)

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 
+            //! Queues the pass to have Build() and Initilize() called by the PassSystem on frame update 
-            void QueueForBuild();
+            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

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.
+            // 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
+            // This state can be overridden by QueuedForRemoval, as we don't want to build a pass that has been removed.
-            QueuedForBuild,
+            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,
         };
     }

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();

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)

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()

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

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<AttachmentReadback> readback)

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->ManualPipelineBuildAndInitialize();
-            pipeline->m_rootPass->Initialize();
-            pipeline->m_rootPass->OnInitializationFinished();
 
             pipeline->BuildPipelineViews();
         }
@@ -327,9 +325,7 @@ namespace AZ
                 newRoot->SetRenderPipeline(this);
 
                 // Manually build the pipeline
-                newRoot->Build();
+                newRoot->ManualPipelineBuildAndInitialize();
-                newRoot->Initialize();
-                newRoot->OnInitializationFinished();
 
                 // Validate the new root
                 PassValidationResults validation;