diff --git a/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp b/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp index 0a6a2a59c7..219d673ece 100644 --- a/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/DisplayMapper/DisplayMapperPass.cpp @@ -188,8 +188,10 @@ namespace AZ void DisplayMapperPass::InitializeInternal() { - // Force update on bindings because children of display mapper pass have their outputs connect - // to their parent's output, which is a non-conventional and non-standard workflow + // Force update on bindings because children of display mapper pass have their outputs connect to + // their parent's output, which is a non-conventional and non-standard workflow. Parent outputs are + // updated after child outputs, so post-build the child outputs do not yet point to the attachments on + // the parent bindings they are connected to. Forcing this refresh sets the attachment on the child output. for (const RPI::Ptr& child : m_children) { child->UpdateConnectedBindings(); 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 70af161e18..b154b031c2 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 @@ -79,9 +79,9 @@ namespace AZ //! //! When authoring a new pass class, inherit from Pass and override any of the virtual functions //! ending with 'Internal' to define the behavior of your passes. These virtual are recursively - //! called in Preorder order throughout the pass tree. Only FramePrepare and FrameEnd are + //! 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 and QueueForRemoval. + //! when scheduled with the PassSystem. See QueueForBuild, 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, @@ -262,15 +262,10 @@ namespace AZ PassState GetPassState() const; - - - // Update all bindings on this pass that are connected to bindings on other passes void UpdateConnectedBindings(); - - protected: explicit Pass(const PassDescriptor& descriptor); @@ -333,15 +328,15 @@ namespace AZ void Build(bool calledFromPassSystem = false); virtual void BuildInternal() { } - // Called after the pass build phase has finished. Allows passes to reset build flags. - void OnInitializationFinished(); - virtual void OnInitializationFinishedInternal() { }; - // Allows for additional pass initialization between building and rendering - // Can be queued independently of Build so as to only invoke Initialize without Build + // Can be queued independently of Build so as to only invoke Initialize() without Build() void Initialize(); virtual void InitializeInternal() { }; + // Called after the pass initialization phase has finished. Allows passes to reset various states and flags. + void OnInitializationFinished(); + virtual void OnInitializationFinishedInternal() { }; + // The Pass's 'Render' function. Called every frame, here the pass sets up it's rendering logic with // the FrameGraphBuilder. This is where your derived pass needs to call ImportScopeProducer on // the FrameGraphBuilder if it's a ScopeProducer (see ForwardPass::FrameBeginInternal for example). @@ -398,23 +393,34 @@ namespace AZ { struct { + // Whether this pass was created with a PassRequest (in which case m_request holds valid data) uint64_t m_createdByPassRequest : 1; + + // Whether the pass is enabled (behavior can be customized by overriding IsEnabled() ) uint64_t m_enabled : 1; + + // False if parent or one of it's ancestors is disabled 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; + + // If this is a parent pass, indicates whether the pass needs to create child passes uint64_t m_createChildren : 1; - // OLD SCHOOL - uint64_t m_alreadyPrepared : 1; - uint64_t m_alreadyReset : 1; - uint64_t m_queuedForBuildAttachment : 1; - - + // Whether this pass belongs to the pass hierarchy, i.e. if you can trace it's parents up to the Root pass uint64_t m_partOfHierarchy : 1; + + // Whether this pass has a DrawListTag uint64_t m_hasDrawListTag : 1; + + // Whether this pass has a PipelineViewTag uint64_t m_hasPipelineViewTag : 1; + + // Whether the pass should gather timestamp query metrics uint64_t m_timestampQueryEnabled : 1; + + // Whether the pass should gather pipeline statics uint64_t m_pipelineStatisticsQueryEnabled : 1; }; uint64_t m_allFlags = 0; @@ -436,11 +442,6 @@ namespace AZ RHI::DrawListSortType m_drawListSortType = RHI::DrawListSortType::KeyThenDepth; - - - - - private: // Return the Timestamp result of this pass virtual TimestampResult GetTimestampResultInternal() const; @@ -528,6 +529,7 @@ namespace AZ // buffers and images don't get deleted during attachment build phase AZStd::vector> m_importedAttachmentStore; + // Name of the pass. Will be concatenated with parent names to form a unique path Name m_name; // Path of the pass in the hierarchy. Example: Root.Ssao.Downsample 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 7ba7f1be4e..e34c61e3aa 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 @@ -20,31 +20,71 @@ // Enables debugging of the pass system // Set this to 1 locally on your machine to facilitate pass debugging and get extra information // about passes in the output window. DO NOT SUBMIT with value set to 1 -#define AZ_RPI_ENABLE_PASS_DEBUGGING 1 +#define AZ_RPI_ENABLE_PASS_DEBUGGING 0 namespace AZ { namespace RPI { + // This enum tracks the state of passes across build, initialization and 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, + + // 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, + + // 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 Reset, + + // Pass is currently building + // From Building, the pass can transition to Built Building, + + // Pass has been built and is awaiting initialization + // From Built, the pass can transition to Initializing Built, + + // Pass is currently being initialized + // From Initializing, the pass can transition to Initialized Initializing, + + // Pass has been initialized + // From Initialized, the pass can transition to Idle Initialized, + + // Idle state, pass is awaiting rendering + // From Idle, the pass can transition to Queued, Resetting, Building, Initializing or Rendering Idle, + + // 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 }; + // This enum keeps track of what actions the pass is queued for with the pass system enum class PassQueueState : u8 { + // 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 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 Initialization at the start of the frame. + // This state has the lowest priority and can therefore be overridden by QueueForBuild or QueueForRemoval QueuedForInitialization, }; } 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 788d00c8eb..72501fc61c 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 @@ -40,15 +40,34 @@ namespace AZ using PassCreator = AZStd::function(const PassDescriptor& descriptor)>; + // Enum to track the different execution phases of the Pass System enum class PassSystemState : u32 { + // Default state, Unitialized, - Idle, + + // Initial Pass System setup. Transitions to Idle + InitializingPassSystem, + + // Pass System is processing passes queued for Removal. Transitions to Idle RemovingPasses, - Building, - Initializing, - Validating, + + // Pass System is processing passes queued for Build (and their child passes). Transitions to Idle + BuildingPasses, + + // Pass System is processing passes queued for Initialization (and their child passes). Transitions to Idle + InitializingPasses, + + // Pass System is validating that the Pass hierarchy is in a valid state after Build and Initialization. Transitions to Idle + ValidatingPasses, + + // Pass System is idle and can transition to any other state (except FrameEnd) + Idle, + + // Pass System is currently rendering a frame. Transitions to FrameEnd Rendering, + + // Pass System is finishing rendering a frame. Transitions to Idle FrameEnd, }; 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 d03405dafd..f36673221b 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Pass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/Pass.cpp @@ -78,7 +78,6 @@ namespace AZ // Skip reset since the pass just got created m_state = PassState::Reset; - m_flags.m_alreadyReset = true; } Pass::~Pass() @@ -1040,42 +1039,13 @@ namespace AZ // --- Queuing functions with PassSystem --- -#define OLD_SCHOOL 1 - void Pass::QueueForBuild() { -#if 0//OLD_SCHOOL - // Don't queue if we're in building phase - //if (PassSystemInterface::Get()->GetState() != RPI::PassSystemState::Building) - { - if (!m_flags.m_queuedForBuildAttachment) - { - PassSystemInterface::Get()->QueueForBuild(this); - m_flags.m_queuedForBuildAttachment = true; - - // Set these two flags to false since when queue build attachments request, they should all be already be false except one use - // case that the pass system processed all queued requests when active a scene. - // m_flags.m_alreadyReset = false; - // m_flags.m_alreadyPrepared = false; - - m_queueState = PassQueueState::QueuedForBuild; - - if (m_state != PassState::Rendering) - { - m_state = PassState::Queued; - } - - } - } -#else - // Don't queue if we're in building phase + // Queue if not already queued or if queued for initialization only. Don't queue if we're currently building. if (m_state != PassState::Building && (m_queueState == PassQueueState::NoQueue || m_queueState == PassQueueState::QueuedForInitialization)) { - //if (PassSystemInterface::Get()->GetState() != RPI::PassSystemState::Building) - { - PassSystemInterface::Get()->QueueForBuild(this); - } + PassSystemInterface::Get()->QueueForBuild(this); m_queueState = PassQueueState::QueuedForBuild; if (m_state != PassState::Rendering) @@ -1083,19 +1053,12 @@ namespace AZ m_state = PassState::Queued; } } -#endif } void Pass::QueueForInitialization() { - // Pass::FrameBegin - Pass [Root.LowEndPipeline.LowEndPipelineTemplate.LightAdaptation.LookModificationTransformPass.LookModificationComposite] is attempting to render, but is not in the Idle state. - if (m_path == Name("Root.LowEndPipeline.LowEndPipelineTemplate.LightAdaptation.LookModificationTransformPass.LookModificationComposite")) - { - __nop(); - } - - // Only queue if the pass is not in any other queue - if (m_queueState == PassQueueState::NoQueue) + // Only queue if the pass is not in any other 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; @@ -1109,6 +1072,8 @@ namespace AZ void Pass::QueueForRemoval() { + // Skip only if we're already queued for removal, otherwise proceed. + // QueuedForRemoval overrides QueuedForBuild and QueuedForInitialization. if (m_queueState != PassQueueState::QueuedForRemoval) { PassSystemInterface::Get()->QueueForRemoval(this); @@ -1125,28 +1090,15 @@ namespace AZ void Pass::Reset() { - if (m_path == Name("Root.LowEndPipeline.LowEndPipelineTemplate.LightAdaptation.LookModificationTransformPass.LookModificationComposite")) - { - __nop(); - } - + // 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::QueuedForInitialization); -#if OLD_SCHOOL - AZ_Assert(!execute == m_flags.m_alreadyReset, "ANTON - EARLY OUT FLAGS do not match for pass RESET!!"); - if (m_flags.m_alreadyReset) - { - return; - } - m_flags.m_alreadyReset = true; -#else if (!execute) { return; } -#endif m_state = PassState::Resetting; @@ -1169,32 +1121,16 @@ namespace AZ void Pass::Build(bool calledFromPassSystem) { - if (m_path == Name("Root.LowEndPipeline.LowEndPipelineTemplate.LightAdaptation.LookModificationTransformPass.LookModificationComposite")) - { - __nop(); - } - AZ_RPI_BREAK_ON_TARGET_PASS; - bool execute = (m_state == PassState::Idle || m_state == PassState::Reset); - execute = execute || (m_state == PassState::Queued && m_queueState == PassQueueState::QueuedForBuild); - execute = execute || (m_state == PassState::Queued && m_queueState == PassQueueState::QueuedForInitialization); + // Ensure we're in a valid state to build. This ensures the pass won't be built multiple times in the same frame. + bool execute = (m_state == PassState::Reset); -#if OLD_SCHOOL - AZ_Assert(!execute == m_flags.m_alreadyPrepared, "ANTON - EARLY OUT FLAGS do not match for pass BUILD!!"); - if (m_flags.m_alreadyPrepared) - { - return; - } - m_flags.m_alreadyPrepared = true; -#else if (!execute) { return; } -#endif - AZ_Assert(m_state == PassState::Reset, "ANTON - BUILDING PASS BUT STATE IS NOT RESET!!"); m_state = PassState::Building; // Bindings, inputs and attachments @@ -1233,6 +1169,7 @@ namespace AZ { AZ_RPI_BREAK_ON_TARGET_PASS; + // Ensure we're in a valid state to initialize. This ensures the pass won't be initialized multiple times in the same frame. bool execute = (m_state == PassState::Idle || m_state == PassState::Built); execute = execute || (m_state == PassState::Queued && m_queueState == PassQueueState::QueuedForInitialization); @@ -1244,11 +1181,6 @@ namespace AZ m_state = PassState::Initializing; m_queueState = PassQueueState::NoQueue; - // Update -// UpdateConnectedBindings(); -// UpdateOwnedAttachments(); -// UpdateAttachmentUsageIndices(); - InitializeInternal(); m_state = PassState::Initialized; @@ -1256,12 +1188,6 @@ namespace AZ void Pass::OnInitializationFinished() { - AZ_RPI_BREAK_ON_TARGET_PASS; - - m_flags.m_alreadyReset = false; - m_flags.m_alreadyPrepared = false; - m_flags.m_queuedForBuildAttachment = false; - m_flags.m_alreadyCreated = false; m_importedAttachmentStore.clear(); OnInitializationFinishedInternal(); 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 50a2c0db91..d209d03580 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp @@ -95,7 +95,7 @@ namespace AZ void PassSystem::Init() { - m_state = PassSystemState::Initializing; + m_state = PassSystemState::InitializingPassSystem; Interface::Register(this); m_passLibrary.Init(); @@ -103,7 +103,10 @@ namespace AZ m_rootPass = CreatePass(Name{"Root"}); m_rootPass->m_flags.m_partOfHierarchy = true; - //m_targetedPassDebugName = "AcesOutputTransform"; + // Here you can specify the name of a pass you would like to break into during execution + // If you enable AZ_RPI_ENABLE_PASS_DEBUGGING, then any pass matching the specified name will debug + // break on any instance of the AZ_RPI_BREAK_ON_TARGET_PASS macro. See Pass::Build for an example + // m_targetedPassDebugName = "MyPassName"; m_state = PassSystemState::Idle; } @@ -189,12 +192,11 @@ namespace AZ void PassSystem::BuildPasses() { - m_state = PassSystemState::Building; + m_state = PassSystemState::BuildingPasses; AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzRender); AZ_ATOM_PROFILE_FUNCTION("RPI", "PassSystem: BuildPassAttachments"); m_passHierarchyChanged = m_passHierarchyChanged || !m_buildPassList.empty(); - u32 loopCounter = 0; // While loop is for the event in which passes being built add more pass to m_buildPassList while(!m_buildPassList.empty()) @@ -214,25 +216,14 @@ namespace AZ SortPassListAscending(buildListCopy); - Pass* previousPassInList = nullptr; for (const Ptr& pass : buildListCopy) { - if (pass.get() != previousPassInList) - { - pass->Reset(); - previousPassInList = pass.get(); - } + pass->Reset(); } - previousPassInList = nullptr; for (const Ptr& pass : buildListCopy) { - if (pass.get() != previousPassInList) - { - pass->Build(true); - previousPassInList = pass.get(); - } + pass->Build(true); } - loopCounter++; } if (m_passHierarchyChanged) @@ -251,12 +242,11 @@ namespace AZ void PassSystem::InitializePasses() { - m_state = PassSystemState::Initializing; + m_state = PassSystemState::InitializingPasses; AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzRender); AZ_ATOM_PROFILE_FUNCTION("RPI", "PassSystem: BuildPassAttachments"); m_passHierarchyChanged = m_passHierarchyChanged || !m_initializePassList.empty(); - u32 loopCounter = 0; while (!m_initializePassList.empty()) { @@ -273,16 +263,10 @@ namespace AZ SortPassListAscending(initListCopy); - Pass* previousPassInList = nullptr; for (const Ptr& pass : initListCopy) { - if (pass.get() != previousPassInList) - { - pass->Initialize(); - previousPassInList = pass.get(); - } + pass->Initialize(); } - loopCounter++; } if (m_passHierarchyChanged) @@ -296,7 +280,7 @@ namespace AZ void PassSystem::Validate() { - m_state = PassSystemState::Validating; + m_state = PassSystemState::ValidatingPasses; AZ_ATOM_PROFILE_FUNCTION("RPI", "PassSystem: Validate"); if (PassValidation::IsEnabled()) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp index 4e565895e9..051282a7b0 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/RenderPipeline.cpp @@ -108,7 +108,7 @@ namespace AZ pipeline->m_activeRenderSettings = desc.m_renderSettings; pipeline->m_rootPass->SetRenderPipeline(pipeline); - // Manually create the pipeline so we can gather the view tags from it's passes + // 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(); @@ -326,8 +326,10 @@ namespace AZ Ptr newRoot = m_rootPass->Recreate(); newRoot->SetRenderPipeline(this); - // Force processing of queued changes so we can validate the new pipeline - passSystem->ProcessQueuedChanges(); + // Manually build the pipeline + newRoot->Build(); + newRoot->Initialize(); + newRoot->OnInitializationFinished(); // Validate the new root PassValidationResults validation;