From 60db6b55d065ddd34efea67d09307cdfe7e40c80 Mon Sep 17 00:00:00 2001 From: qingtao Date: Tue, 27 Apr 2021 13:26:43 -0700 Subject: [PATCH] Address feedback of PR for ATOM-6088. * Rename DrawItemKeyPair to DrawItemProperties. * Switch back to use Ptr instead of DrawListTagRegistry * Updated some comment. --- .../Atom/RHI/Code/Include/Atom/RHI/DrawItem.h | 18 +++-- .../Atom/RHI/Code/Include/Atom/RHI/DrawList.h | 4 +- .../Code/Include/Atom/RHI/DrawListContext.h | 2 +- .../RHI/Code/Include/Atom/RHI/DrawPacket.h | 8 +-- .../Code/Include/Atom/RHI/DrawPacketBuilder.h | 2 +- .../RHI/Code/Include/Atom/RHI/RHISystem.h | 2 +- .../RHI/Code/Include/Atom/RHI/TagRegistry.h | 19 ++++-- Gems/Atom/RHI/Code/Source/RHI/DrawList.cpp | 12 ++-- .../RHI/Code/Source/RHI/DrawListContext.cpp | 6 +- Gems/Atom/RHI/Code/Source/RHI/DrawPacket.cpp | 4 +- Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp | 7 +- Gems/Atom/RHI/Code/Tests/DrawPacketTests.cpp | 66 ++++++++++--------- .../DynamicDraw/DynamicDrawInterface.h | 3 +- .../Include/Atom/RPI.Public/RenderPipeline.h | 5 +- .../RPI/Code/Include/Atom/RPI.Public/Scene.h | 2 +- .../RPI/Code/Include/Atom/RPI.Public/View.h | 2 +- .../DynamicDraw/DynamicDrawContext.cpp | 10 +-- .../Source/RPI.Public/Pass/RasterPass.cpp | 6 +- .../Atom/RPI/Code/Source/RPI.Public/Scene.cpp | 5 +- Gems/Atom/RPI/Code/Source/RPI.Public/View.cpp | 4 +- 20 files changed, 104 insertions(+), 83 deletions(-) diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawItem.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawItem.h index 8043e5aa67..23fa2e691c 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawItem.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawItem.h @@ -160,18 +160,18 @@ namespace AZ constexpr uint32_t DrawFilterMaskDefaultValue = uint32_t(-1); // Default all bit to 1. static_assert(sizeof(DrawFilterMask) * 8 >= Limits::Pipeline::DrawFilterTagCountMax, "DrawFilterMask doesn't have enough bits for maximum tag count"); - struct DrawItemKeyPair + struct DrawItemProperties { - DrawItemKeyPair() = default; + DrawItemProperties() = default; - DrawItemKeyPair(const DrawItem* item, DrawItemSortKey sortKey = 0, DrawFilterMask filterMask = DrawFilterMaskDefaultValue) + DrawItemProperties(const DrawItem* item, DrawItemSortKey sortKey = 0, DrawFilterMask filterMask = DrawFilterMaskDefaultValue) : m_item{item} , m_sortKey{sortKey} , m_drawFilterMask{filterMask} { } - bool operator == (const DrawItemKeyPair& rhs) const + bool operator==(const DrawItemProperties& rhs) const { return m_item == rhs.m_item && m_sortKey == rhs.m_sortKey && @@ -180,19 +180,25 @@ namespace AZ ; } - bool operator != (const DrawItemKeyPair& rhs) const + bool operator!=(const DrawItemProperties& rhs) const { return !(*this == rhs); } - bool operator < (const DrawItemKeyPair& rhs) const + bool operator<(const DrawItemProperties& rhs) const { return m_sortKey < rhs.m_sortKey; } + //! A pointer to the draw item const DrawItem* m_item = nullptr; + //! A sorting key of this draw item which is used for sorting draw items in DrawList + // Check RHI::SortDrawList() function for detail DrawItemSortKey m_sortKey = 0; + //! A depth value this draw item which is used for sorting draw items in DrawList + //! Check RHI::SortDrawList() function for detail float m_depth = 0.0f; + //! A filter mask which helps decide whether to submit this draw item to a Scope's command list or not DrawFilterMask m_drawFilterMask = DrawFilterMaskDefaultValue; }; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawList.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawList.h index 71c1cc026c..e232b1cf07 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawList.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawList.h @@ -39,8 +39,8 @@ namespace AZ using DrawListTag = Handle; using DrawListMask = AZStd::bitset; - using DrawList = AZStd::vector; - using DrawListView = AZStd::array_view; + using DrawList = AZStd::vector; + using DrawListView = AZStd::array_view; /// Contains a table of draw lists, indexed by the tag. using DrawListsByTag = AZStd::array; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawListContext.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawListContext.h index 00514ef3ef..c8f792ac49 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawListContext.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawListContext.h @@ -53,7 +53,7 @@ namespace AZ /// Adds an individual draw item to the draw list associated with the provided tag. This will /// no-op if the tag is not present in the internal draw list mask. - void AddDrawItem(DrawListTag drawListTag, DrawItemKeyPair drawItemKeyPair); + void AddDrawItem(DrawListTag drawListTag, DrawItemProperties drawItemProperties); /// Coalesces the draw lists in preparation for access via GetList. This should /// be called from a single thread as a sync point between the append / consume phases. diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacket.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacket.h index fc7fa90dd6..988c3a73dd 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacket.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacket.h @@ -25,7 +25,7 @@ namespace AZ //! DrawPacket is a packed data structure (one contiguous allocation) containing a collection of //! DrawItems and their associated array data. Each draw item in the packet is associated //! with a DrawListTag. All draw items in the packet share the same set of shader resource - //! groups, index buffer, and draw arguments. + //! groups, index buffer, one DrawFilterMask, and draw arguments. //! //! Some notes about design and usage: //! - Draw packets should be used to 'broadcast' variations of the same 'object' to multiple passes. @@ -42,7 +42,7 @@ namespace AZ { friend class DrawPacketBuilder; public: - using DrawItemVisitor = AZStd::function; + using DrawItemVisitor = AZStd::function; //! Draw packets cannot be move constructed or copied, as they contain an additional memory payload. AZ_DISABLE_COPY_MOVE(DrawPacket); @@ -53,8 +53,8 @@ namespace AZ //! Returns the number of draw items stored in the packet. size_t GetDrawItemCount() const; - //! Returns the draw item / sort key associated with the provided index. - DrawItemKeyPair GetDrawItem(size_t index) const; + //! Returns the draw item and its properties associated with the provided index. + DrawItemProperties GetDrawItem(size_t index) const; //! Returns the draw list tag associated with the provided index. DrawListTag GetDrawListTag(size_t index) const; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacketBuilder.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacketBuilder.h index 5d475d2ccb..15303158f9 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacketBuilder.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/DrawPacketBuilder.h @@ -47,7 +47,7 @@ namespace AZ //! The sort key assigned to this draw item. DrawItemSortKey m_sortKey = 0; - //! The filter associates to this draw item. + //! The filter associated to this draw item. DrawFilterMask m_drawFilterMask = DrawFilterMaskDefaultValue; }; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h index efaaad1967..ccfaff3f11 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/RHISystem.h @@ -65,7 +65,7 @@ namespace AZ RHI::Ptr InitInternalDevice(); RHI::Ptr m_device; - RHI::DrawListTagRegistry m_drawListTagRegistry; + RHI::Ptr m_drawListTagRegistry; RHI::Ptr m_pipelineStateCache; RHI::FrameScheduler m_frameScheduler; RHI::FrameSchedulerCompileRequest m_compileRequest; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/TagRegistry.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/TagRegistry.h index 277afdd1e3..429a49657b 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/TagRegistry.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/TagRegistry.h @@ -36,6 +36,10 @@ namespace AZ : public AZStd::intrusive_base { public: + AZ_CLASS_ALLOCATOR(TagRegistry, AZ::SystemAllocator, 0); + AZ_DISABLE_COPY_MOVE(TagRegistry); + + static Ptr Create(); //! Resets the registry back to an empty state. All references are released. void Reset(); @@ -43,24 +47,25 @@ namespace AZ //! Acquires a tag from the provided name (case sensitive). If the tag already existed, it is ref-counted. //! Returns a valid tag on success; returns a null tag if the registry is at full capacity. You must //! call ReleaseTag() if successful. - TagType AcquireTag(const Name& drawListName); + TagType AcquireTag(const Name& tagName); //! Releases a reference to a tag. Tags are ref-counted, so it's necessary to maintain ownership of the //! tag and release when its no longer needed. - void ReleaseTag(TagType drawListTag); + void ReleaseTag(TagType tagName); //! Finds the tag associated with the provided name (case sensitive). If a tag exists with that name, the tag //! is returned. The reference count is NOT incremented on success; ownership is not passed to the user. If //! the tag does not exist, a null tag is returned. - TagType FindTag(const Name& drawListName) const; + TagType FindTag(const Name& tagName) const; - //! Returns the name of the given DrawListTag, or empty string if the tag is not registered. + //! Returns the name of the given tag, or empty string if the tag is not registered. Name GetName(TagType tag) const; //! Returns the number of allocated tags in the registry. size_t GetAllocatedTagCount() const; private: + TagRegistry() = default; struct Entry { @@ -73,6 +78,12 @@ namespace AZ size_t m_allocatedTagCount = 0; }; + template + Ptr> TagRegistry::Create() + { + return aznew TagRegistry(); + } + template void TagRegistry::Reset() { diff --git a/Gems/Atom/RHI/Code/Source/RHI/DrawList.cpp b/Gems/Atom/RHI/Code/Source/RHI/DrawList.cpp index 518ec59ac5..c50255224f 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/DrawList.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/DrawList.cpp @@ -35,8 +35,7 @@ namespace AZ switch (sortType) { case DrawListSortType::KeyThenDepth: - AZStd::sort(drawList.begin(), drawList.end(), - [](const DrawItemKeyPair& a, const DrawItemKeyPair& b) + AZStd::sort(drawList.begin(), drawList.end(), [](const DrawItemProperties& a, const DrawItemProperties& b) { if (a.m_sortKey != b.m_sortKey) { @@ -48,8 +47,7 @@ namespace AZ break; case DrawListSortType::KeyThenReverseDepth: - AZStd::sort(drawList.begin(), drawList.end(), - [](const DrawItemKeyPair& a, const DrawItemKeyPair& b) + AZStd::sort(drawList.begin(), drawList.end(), [](const DrawItemProperties& a, const DrawItemProperties& b) { if (a.m_sortKey != b.m_sortKey) { @@ -61,8 +59,7 @@ namespace AZ break; case DrawListSortType::DepthThenKey: - AZStd::sort(drawList.begin(), drawList.end(), - [](const DrawItemKeyPair& a, const DrawItemKeyPair& b) + AZStd::sort(drawList.begin(), drawList.end(), [](const DrawItemProperties& a, const DrawItemProperties& b) { if (a.m_depth != b.m_depth) { @@ -74,8 +71,7 @@ namespace AZ break; case DrawListSortType::ReverseDepthThenKey: - AZStd::sort(drawList.begin(), drawList.end(), - [](const DrawItemKeyPair& a, const DrawItemKeyPair& b) + AZStd::sort(drawList.begin(), drawList.end(), [](const DrawItemProperties& a, const DrawItemProperties& b) { if (a.m_depth != b.m_depth) { diff --git a/Gems/Atom/RHI/Code/Source/RHI/DrawListContext.cpp b/Gems/Atom/RHI/Code/Source/RHI/DrawListContext.cpp index dbd06c6969..8cec5ba39b 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/DrawListContext.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/DrawListContext.cpp @@ -63,14 +63,14 @@ namespace AZ if (m_drawListMask[drawListTag.GetIndex()]) { - DrawItemKeyPair drawItem = drawPacket->GetDrawItem(i); + DrawItemProperties drawItem = drawPacket->GetDrawItem(i); drawItem.m_depth = depth; threadListsByTag[drawListTag.GetIndex()].push_back(drawItem); } } } - void DrawListContext::AddDrawItem(DrawListTag drawListTag, DrawItemKeyPair drawItemKeyPair) + void DrawListContext::AddDrawItem(DrawListTag drawListTag, DrawItemProperties drawItemProperties) { if (Validation::IsEnabled()) { @@ -84,7 +84,7 @@ namespace AZ if (m_drawListMask[drawListTag.GetIndex()]) { DrawListsByTag& drawListsByTag = m_threadListsByTag.GetStorage(); - drawListsByTag[drawListTag.GetIndex()].push_back(drawItemKeyPair); + drawListsByTag[drawListTag.GetIndex()].push_back(drawItemProperties); } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/DrawPacket.cpp b/Gems/Atom/RHI/Code/Source/RHI/DrawPacket.cpp index 5fa5c563bd..85580fdc84 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/DrawPacket.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/DrawPacket.cpp @@ -24,10 +24,10 @@ namespace AZ return m_drawItemCount; } - DrawItemKeyPair DrawPacket::GetDrawItem(size_t index) const + DrawItemProperties DrawPacket::GetDrawItem(size_t index) const { AZ_Assert(index < GetDrawItemCount(), "Out of bounds array access!"); - return DrawItemKeyPair(&m_drawItems[index], m_drawItemSortKeys[index], m_drawFilterMask); + return DrawItemProperties(&m_drawItems[index], m_drawItemSortKeys[index], m_drawFilterMask); } DrawListTag DrawPacket::GetDrawListTag(size_t index) const diff --git a/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp b/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp index 550a7765bf..f92d2621b1 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/RHISystem.cpp @@ -60,7 +60,8 @@ namespace AZ AZ_Assert(false, "RHISystem", "Unable to initialize RHI! \n"); return; } - + + m_drawListTagRegistry = RHI::DrawListTagRegistry::Create(); m_pipelineStateCache = RHI::PipelineStateCache::Create(*m_device); frameSchedulerDescriptor.m_transientAttachmentPoolDescriptor.m_renderTargetBudgetInBytes = m_platformLimitsDescriptor->m_transientAttachmentPoolBudgets.m_renderTargetBudgetInBytes; @@ -106,7 +107,7 @@ namespace AZ // Register draw list tags declared from content. for (const Name& drawListName : descriptor.m_drawListTags) { - RHI::DrawListTag drawListTag = m_drawListTagRegistry.AcquireTag(drawListName); + RHI::DrawListTag drawListTag = m_drawListTagRegistry->AcquireTag(drawListName); AZ_Warning("RHISystem", drawListTag.IsValid(), "Failed to register draw list tag '%s'. Registry at capacity.", drawListName.GetCStr()); } @@ -250,7 +251,7 @@ namespace AZ RHI::DrawListTagRegistry* RHISystem::GetDrawListTagRegistry() { - return &m_drawListTagRegistry; + return m_drawListTagRegistry.get(); } const RHI::FrameSchedulerCompileRequest& RHISystem::GetFrameSchedulerCompileRequest() const diff --git a/Gems/Atom/RHI/Code/Tests/DrawPacketTests.cpp b/Gems/Atom/RHI/Code/Tests/DrawPacketTests.cpp index 47830dfa2b..7fc393cd0c 100644 --- a/Gems/Atom/RHI/Code/Tests/DrawPacketTests.cpp +++ b/Gems/Atom/RHI/Code/Tests/DrawPacketTests.cpp @@ -80,11 +80,11 @@ namespace UnitTest m_indexBufferView = RHI::IndexBufferView(*m_bufferEmpty, random.GetRandom(), random.GetRandom(), RHI::IndexFormat::Uint16); } - void ValidateDrawItem(const DrawItemData& drawItemData, RHI::DrawItemKeyPair itemKeyPair) const + void ValidateDrawItem(const DrawItemData& drawItemData, RHI::DrawItemProperties itemProperties) const { - const RHI::DrawItem* drawItem = itemKeyPair.m_item; + const RHI::DrawItem* drawItem = itemProperties.m_item; - EXPECT_EQ(itemKeyPair.m_sortKey, drawItemData.m_sortKey); + EXPECT_EQ(itemProperties.m_sortKey, drawItemData.m_sortKey); EXPECT_EQ(drawItem->m_stencilRef, drawItemData.m_stencilRef); EXPECT_EQ(drawItem->m_pipelineState, drawItemData.m_pipelineState); @@ -170,10 +170,12 @@ namespace UnitTest RHITestFixture::SetUp(); m_factory.reset(aznew Factory()); + m_drawListTagRegistry = RHI::DrawListTagRegistry::Create(); } void TearDown() override { + m_drawListTagRegistry = nullptr; m_factory.reset(); RHITestFixture::TearDown(); @@ -182,7 +184,7 @@ namespace UnitTest protected: static const uint32_t s_randomSeed = 1234; - RHI::DrawListTagRegistry m_drawListTagRegistry; + RHI::Ptr m_drawListTagRegistry; RHI::DrawListContext m_drawListContext; AZStd::unique_ptr m_factory; @@ -190,12 +192,12 @@ namespace UnitTest TEST_F(DrawPacketTest, TestDrawListTagRegistryNullCase) { - RHI::DrawListTag nullTag = m_drawListTagRegistry.AcquireTag(Name()); + RHI::DrawListTag nullTag = m_drawListTagRegistry->AcquireTag(Name()); EXPECT_TRUE(nullTag.IsNull()); - EXPECT_EQ(m_drawListTagRegistry.GetAllocatedTagCount(), 0); + EXPECT_EQ(m_drawListTagRegistry->GetAllocatedTagCount(), 0); - m_drawListTagRegistry.ReleaseTag(nullTag); - EXPECT_EQ(m_drawListTagRegistry.GetAllocatedTagCount(), 0); + m_drawListTagRegistry->ReleaseTag(nullTag); + EXPECT_EQ(m_drawListTagRegistry->GetAllocatedTagCount(), 0); } TEST_F(DrawPacketTest, TestDrawListTagRegistrySimple) @@ -203,39 +205,39 @@ namespace UnitTest const Name forwardName1("Forward"); const Name forwardName2("forward"); - RHI::DrawListTag forwardTag1 = m_drawListTagRegistry.AcquireTag(forwardName1); - RHI::DrawListTag forwardTag2 = m_drawListTagRegistry.AcquireTag(forwardName2); + RHI::DrawListTag forwardTag1 = m_drawListTagRegistry->AcquireTag(forwardName1); + RHI::DrawListTag forwardTag2 = m_drawListTagRegistry->AcquireTag(forwardName2); EXPECT_FALSE(forwardTag1.IsNull()); EXPECT_FALSE(forwardTag2.IsNull()); EXPECT_NE(forwardTag1, forwardTag2); - RHI::DrawListTag forwardTag3 = m_drawListTagRegistry.AcquireTag(forwardName1); + RHI::DrawListTag forwardTag3 = m_drawListTagRegistry->AcquireTag(forwardName1); EXPECT_EQ(forwardTag1, forwardTag3); - m_drawListTagRegistry.ReleaseTag(forwardTag1); - m_drawListTagRegistry.ReleaseTag(forwardTag2); - m_drawListTagRegistry.ReleaseTag(forwardTag3); + m_drawListTagRegistry->ReleaseTag(forwardTag1); + m_drawListTagRegistry->ReleaseTag(forwardTag2); + m_drawListTagRegistry->ReleaseTag(forwardTag3); - EXPECT_EQ(m_drawListTagRegistry.GetAllocatedTagCount(), 0); + EXPECT_EQ(m_drawListTagRegistry->GetAllocatedTagCount(), 0); } TEST_F(DrawPacketTest, TestDrawListTagRegistryDeAllocateAssert) { AZ_TEST_START_ASSERTTEST; - EXPECT_EQ(m_drawListTagRegistry.GetAllocatedTagCount(), 0); + EXPECT_EQ(m_drawListTagRegistry->GetAllocatedTagCount(), 0); const Name tagName{"Test"}; - RHI::DrawListTag tag = m_drawListTagRegistry.AcquireTag(tagName); - m_drawListTagRegistry.AcquireTag(tagName); - m_drawListTagRegistry.AcquireTag(tagName); - m_drawListTagRegistry.ReleaseTag(tag); - m_drawListTagRegistry.ReleaseTag(tag); - m_drawListTagRegistry.ReleaseTag(tag); + RHI::DrawListTag tag = m_drawListTagRegistry->AcquireTag(tagName); + m_drawListTagRegistry->AcquireTag(tagName); + m_drawListTagRegistry->AcquireTag(tagName); + m_drawListTagRegistry->ReleaseTag(tag); + m_drawListTagRegistry->ReleaseTag(tag); + m_drawListTagRegistry->ReleaseTag(tag); // One additional forfeit should assert. - m_drawListTagRegistry.ReleaseTag(tag); + m_drawListTagRegistry->ReleaseTag(tag); AZ_TEST_STOP_ASSERTTEST(1); } @@ -254,15 +256,15 @@ namespace UnitTest // Acquire if (random.GetRandom() % 2) { - RHI::DrawListTag tag = m_drawListTagRegistry.AcquireTag(tagNameUnique); + RHI::DrawListTag tag = m_drawListTagRegistry->AcquireTag(tagNameUnique); if (tag.IsNull()) { - EXPECT_EQ(m_drawListTagRegistry.GetAllocatedTagCount(), RHI::Limits::Pipeline::DrawListTagCountMax); + EXPECT_EQ(m_drawListTagRegistry->GetAllocatedTagCount(), RHI::Limits::Pipeline::DrawListTagCountMax); } else { - EXPECT_LT(m_drawListTagRegistry.GetAllocatedTagCount(), RHI::Limits::Pipeline::DrawListTagCountMax); + EXPECT_LT(m_drawListTagRegistry->GetAllocatedTagCount(), RHI::Limits::Pipeline::DrawListTagCountMax); acquiredTags.emplace_back(tag); } } @@ -274,26 +276,26 @@ namespace UnitTest RHI::DrawListTag tag = acquiredTags[tagIndex]; - size_t allocationCountBefore = m_drawListTagRegistry.GetAllocatedTagCount(); - m_drawListTagRegistry.ReleaseTag(tag); - size_t allocationCountAfter = m_drawListTagRegistry.GetAllocatedTagCount(); + size_t allocationCountBefore = m_drawListTagRegistry->GetAllocatedTagCount(); + m_drawListTagRegistry->ReleaseTag(tag); + size_t allocationCountAfter = m_drawListTagRegistry->GetAllocatedTagCount(); EXPECT_EQ(allocationCountBefore - allocationCountAfter, 1); acquiredTags.erase(acquiredTags.begin() + tagIndex); } - EXPECT_EQ(acquiredTags.size(), m_drawListTagRegistry.GetAllocatedTagCount()); + EXPECT_EQ(acquiredTags.size(), m_drawListTagRegistry->GetAllocatedTagCount()); } // Erase all references, make sure the registry is empty again. for (RHI::DrawListTag tag : acquiredTags) { - m_drawListTagRegistry.ReleaseTag(tag); + m_drawListTagRegistry->ReleaseTag(tag); } acquiredTags.clear(); - EXPECT_EQ(m_drawListTagRegistry.GetAllocatedTagCount(), 0); + EXPECT_EQ(m_drawListTagRegistry->GetAllocatedTagCount(), 0); } TEST_F(DrawPacketTest, DrawPacketEmpty) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/DynamicDraw/DynamicDrawInterface.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/DynamicDraw/DynamicDrawInterface.h index ce686f5c00..50b13277af 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/DynamicDraw/DynamicDrawInterface.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/DynamicDraw/DynamicDrawInterface.h @@ -58,7 +58,8 @@ namespace AZ virtual RHI::Ptr CreateDynamicDrawContext(Scene* scene) = 0; //! Create a DynamicDrawContext for specified render pipeline - //! This allows draw calls are only submitted to selected render pipeline (viewport) + //! Draw calls submitted through the context created by this function are only submitted + //! to the supplied render pipeline (viewport) virtual RHI::Ptr CreateDynamicDrawContext(RenderPipeline* pipeline) = 0; //! Get a DynamicBuffer from DynamicDrawSystem. diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/RenderPipeline.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/RenderPipeline.h index d82e575493..5595c1d2ac 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/RenderPipeline.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/RenderPipeline.h @@ -187,6 +187,7 @@ namespace AZ //! Get draw filter tag RHI::DrawFilterTag GetDrawFilterTag() const; + //! Get draw filter mask RHI::DrawFilterMask GetDrawFilterMask() const; private: @@ -261,7 +262,9 @@ namespace AZ // A tag to filter draw items submitted by passes of this render pipeline. // This tag is allocated when it's added to a scene. It's set to invalid when it's removed to the scene. RHI::DrawFilterTag m_drawFilterTag; - RHI::DrawFilterMask m_drawFilterMask = 0; // The corresponding mask of the m_drawFilterTag + // A mask to filter draw items submitted by passes of this render pipeline. + // This mask is created from the value of m_drawFilterTag. + RHI::DrawFilterMask m_drawFilterMask = 0; }; } // namespace RPI diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Scene.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Scene.h index fd80ea4f74..3238785d81 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Scene.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Scene.h @@ -237,7 +237,7 @@ namespace AZ DynamicDrawSystem* m_dynamicDrawSystem = nullptr; // Registry which allocates draw filter tag for RenderPipeline - RHI::DrawFilterTagRegistry m_drawFilterTagRegistry; + RHI::Ptr m_drawFilterTagRegistry; }; // --- Template functions --- diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/View.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/View.h index ba652a2b94..f64983126d 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/View.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/View.h @@ -75,7 +75,7 @@ namespace AZ void AddDrawPacket(const RHI::DrawPacket* drawPacket, Vector3 worldPosition); //! Add a draw item to this view with its associated draw list tag - void AddDrawItem(RHI::DrawListTag drawListTag, const RHI::DrawItemKeyPair& drawItemKeyPair); + void AddDrawItem(RHI::DrawListTag drawListTag, const RHI::DrawItemProperties& drawItemProperties); //! Sets the worldToView matrix and recalculates the other matrices. void SetWorldToViewMatrix(const AZ::Matrix4x4& worldToView); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/DynamicDraw/DynamicDrawContext.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/DynamicDraw/DynamicDrawContext.cpp index 78b06aedf8..65b698f9ac 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/DynamicDraw/DynamicDrawContext.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/DynamicDraw/DynamicDrawContext.cpp @@ -602,11 +602,11 @@ namespace AZ drawItemInfo.m_drawItem.m_streamBufferViews = &m_cachedStreamBufferViews[drawItemInfo.m_vertexBufferViewIndex]; } - RHI::DrawItemKeyPair drawItemKeyPair; - drawItemKeyPair.m_sortKey = sortKey; - drawItemKeyPair.m_item = &drawItemInfo.m_drawItem; - drawItemKeyPair.m_drawFilterMask = m_drawFilter; - view->AddDrawItem(m_drawListTag, drawItemKeyPair); + RHI::DrawItemProperties drawItemProperties; + drawItemProperties.m_sortKey = sortKey; + drawItemProperties.m_item = &drawItemInfo.m_drawItem; + drawItemProperties.m_drawFilterMask = m_drawFilter; + view->AddDrawItem(m_drawListTag, drawItemProperties); sortKey++; } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index 1ff6c8ea14..baf1d22da5 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -195,11 +195,11 @@ namespace AZ SetSrgsForDraw(commandList); } - for (const RHI::DrawItemKeyPair& drawItemKeyPair : drawListViewPartition) + for (const RHI::DrawItemProperties& drawItemProperties : drawListViewPartition) { - if (drawItemKeyPair.m_drawFilterMask & m_pipeline->GetDrawFilterMask()) + if (drawItemProperties.m_drawFilterMask & m_pipeline->GetDrawFilterMask()) { - commandList->Submit(*drawItemKeyPair.m_item); + commandList->Submit(*drawItemProperties.m_item); } } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Scene.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Scene.cpp index 2d9b5f035a..2c11ef9a41 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Scene.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Scene.cpp @@ -87,6 +87,7 @@ namespace AZ m_id = Uuid::CreateRandom(); m_cullingScene = aznew CullingScene(); SceneRequestBus::Handler::BusConnect(m_id); + m_drawFilterTagRegistry = RHI::DrawFilterTagRegistry::Create(); } Scene::~Scene() @@ -269,7 +270,7 @@ namespace AZ return; } - pipeline->SetDrawFilterTag(m_drawFilterTagRegistry.AcquireTag(pipelineId)); + pipeline->SetDrawFilterTag(m_drawFilterTagRegistry->AcquireTag(pipelineId)); m_pipelines.push_back(pipeline); @@ -305,7 +306,7 @@ namespace AZ m_defaultPipeline = nullptr; } - m_drawFilterTagRegistry.ReleaseTag(pipelineToRemove->GetDrawFilterTag()); + m_drawFilterTagRegistry->ReleaseTag(pipelineToRemove->GetDrawFilterTag()); pipelineToRemove->OnRemovedFromScene(this); m_pipelines.erase(it); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/View.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/View.cpp index 4fe9843534..8a8f27b4b5 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/View.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/View.cpp @@ -90,9 +90,9 @@ namespace AZ AddDrawPacket(drawPacket, depth); } - void View::AddDrawItem(RHI::DrawListTag drawListTag, const RHI::DrawItemKeyPair& drawItemKeyPair) + void View::AddDrawItem(RHI::DrawListTag drawListTag, const RHI::DrawItemProperties& drawItemProperties) { - m_drawListContext.AddDrawItem(drawListTag, drawItemKeyPair); + m_drawListContext.AddDrawItem(drawListTag, drawItemProperties); } void View::SetWorldToViewMatrix(const AZ::Matrix4x4& worldToView)