From 2bc381811d0a5d5fd508d7c4443371e8a04f8f36 Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Thu, 16 Dec 2021 10:42:53 -0800 Subject: [PATCH] addressed PR feedback Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- .../Feature/Common/Assets/Passes/PassTemplates.azasset | 4 ++-- .../Common/Assets/Passes/{Clear.pass => SlowClear.pass} | 6 +++++- .../Common/Assets/atom_feature_common_asset_files.cmake | 2 +- .../RPI/Code/Include/Atom/RPI.Public/Pass/ClearPass.h | 4 +++- .../RPI/Code/Include/Atom/RPI.Public/Pass/ParentPass.h | 9 +++++++++ Gems/Atom/RPI/Code/Source/RPI.Public/Pass/ParentPass.cpp | 2 +- 6 files changed, 21 insertions(+), 6 deletions(-) rename Gems/Atom/Feature/Common/Assets/Passes/{Clear.pass => SlowClear.pass} (79%) diff --git a/Gems/Atom/Feature/Common/Assets/Passes/PassTemplates.azasset b/Gems/Atom/Feature/Common/Assets/Passes/PassTemplates.azasset index b16536b276..96cf769690 100644 --- a/Gems/Atom/Feature/Common/Assets/Passes/PassTemplates.azasset +++ b/Gems/Atom/Feature/Common/Assets/Passes/PassTemplates.azasset @@ -65,8 +65,8 @@ "Path": "Passes/CascadedShadowmaps.pass" }, { - "Name": "ClearPassTemplate", - "Path": "Passes/Clear.pass" + "Name": "SlowClearPassTemplate", + "Path": "Passes/SlowClear.pass" }, { "Name": "FullscreenCopyTemplate", diff --git a/Gems/Atom/Feature/Common/Assets/Passes/Clear.pass b/Gems/Atom/Feature/Common/Assets/Passes/SlowClear.pass similarity index 79% rename from Gems/Atom/Feature/Common/Assets/Passes/Clear.pass rename to Gems/Atom/Feature/Common/Assets/Passes/SlowClear.pass index a36c2ef03d..417cf94eac 100644 --- a/Gems/Atom/Feature/Common/Assets/Passes/Clear.pass +++ b/Gems/Atom/Feature/Common/Assets/Passes/SlowClear.pass @@ -4,7 +4,11 @@ "ClassName": "PassAsset", "ClassData": { "PassTemplate": { - "Name": "ClearPassTemplate", + + // This is for debug purposes and edge cases only + // If you want to clear an attachment you should + // use the LoadStoreAction on your pass slot. + "Name": "SlowClearPassTemplate", "PassClass": "ClearPass", "Slots": [ { diff --git a/Gems/Atom/Feature/Common/Assets/atom_feature_common_asset_files.cmake b/Gems/Atom/Feature/Common/Assets/atom_feature_common_asset_files.cmake index a684b4b299..a03058e2bf 100644 --- a/Gems/Atom/Feature/Common/Assets/atom_feature_common_asset_files.cmake +++ b/Gems/Atom/Feature/Common/Assets/atom_feature_common_asset_files.cmake @@ -86,7 +86,6 @@ set(FILES Passes/CascadedShadowmaps.pass Passes/CheckerboardResolveColor.pass Passes/CheckerboardResolveDepth.pass - Passes/Clear.pass Passes/ContrastAdaptiveSharpening.pass Passes/ConvertToAcescg.pass Passes/DebugOverlayParent.pass @@ -201,6 +200,7 @@ set(FILES Passes/Skinning.pass Passes/SkyBox.pass Passes/SkyBox_TwoOutputs.pass + Passes/SlowClear.pass Passes/SMAA1xApplyLinearHDRColor.pass Passes/SMAA1xApplyPerceptualColor.pass Passes/SMAABlendingWeightCalculation.pass diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ClearPass.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ClearPass.h index 3599e6de44..96252ca82c 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ClearPass.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ClearPass.h @@ -14,7 +14,9 @@ namespace AZ { namespace RPI { - //! A simple pass to clear a render target + //! Only use this for debug purposes and edge cases + //! The correct and efficient way to clear a pass is through the LoadStoreAction on the pass slot + //! This will clear a given image attachment to the specified clear value. class ClearPass : public RenderPass { diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ParentPass.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ParentPass.h index 6ee0b43ce5..00b7a76f77 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ParentPass.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/ParentPass.h @@ -133,6 +133,15 @@ namespace AZ void CreatePassesFromTemplate(); // Generates child clear passes to clear input and input/output attachments + // TODO: These two functions are a workaround for a complicated edge case: + // Let Parent Pass P1 have two children, C1 and C2. C1 writes to an attachment that C2 reads, + // but C1 can be disabled, in which case we just want C2 to read the cleared texture. + // Because of this, the attachment is owned by the parent pass, that way it is always available for C2 + // to read even when C1 is disabled. However we still want to clear the attachment before C2 reads it. + // We tried overriding the LoadStoreAction to clear on C2's slot when C1 is disabled, but the RHI + // doesn't allow for clears on Input only slots. Changing the slot to InputOutput was in conflict with + // the texture definition in the SRG, and it couldn't be changed to RW because it was an MSAA texture. + // So now we detect clear actions on parent slots and generate a clear pass for them. void CreateClearPassFromBinding(PassAttachmentBinding& binding, PassRequest& clearRequest); void CreateClearPassesFromBindings(); }; 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 f0154b371a..fff6fae0a3 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/ParentPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/ParentPass.cpp @@ -247,7 +247,7 @@ namespace AZ void ParentPass::CreateClearPassesFromBindings() { PassRequest clearRequest; - clearRequest.m_templateName = Name("ClearPassTemplate"); + clearRequest.m_templateName = Name("SlowClearPassTemplate"); clearRequest.m_passData = AZStd::make_shared(); clearRequest.m_connections.push_back(); clearRequest.m_connections[0].m_localSlot = Name("ClearInputOutput");