Add support for VisibleObjectLists in the RPI Culling. (#15517)

* Add support for a VisibleObjectList to the RPI Culling. It will output a list of visible objects that a feature processor can process, rather than adding draw packets to views directly. This will eventually be used to generate instance buffers for mesh instancing. The previous behavior of allowing the culling system to add draw packets directly to views is still supported, and still used when instancing is disabled.

Signed-off-by: Tommy Walton <[email protected]>

* Adding basic unit test for the culling system to test visibility lists.

Signed-off-by: Tommy Walton <[email protected]>

* Rename TYPE_RPI_Visibility_List to RPI_VisibleObjectList to match the rest of the code.

Signed-off-by: Tommy Walton <[email protected]>

* Add changes missing from previous commit.

Signed-off-by: Tommy Walton <[email protected]>

* Remove unneeded sort key and init functions.

Signed-off-by: Tommy Walton <[email protected]>

* Update comments.

Signed-off-by: Tommy Walton <[email protected]>

* More comment updates.

Signed-off-by: Tommy Walton <[email protected]>

* Add missing call to Shutdown.

Signed-off-by: Tommy Walton <[email protected]>

* Update with PR feedback.

Signed-off-by: Tommy Walton <[email protected]>

* Update with PR feedback.

Signed-off-by: Tommy Walton <[email protected]>

* Fix collision between 'format' variable in SequenceBatchRenderDialog and gdiplus.h

Signed-off-by: Tommy Walton <[email protected]>

---------

Signed-off-by: Tommy Walton <[email protected]>

Code/Editor/QtUtilWin.h

@@ -22,9 +22,6 @@
 # include <QtWinExtras/QtWin>
 # include <QtGui/qpa/qplatformnativeinterface.h>
 # include <objidl.h>
-AZ_PUSH_DISABLE_WARNING(4458, "-Wunknown-warning-option")
-# include <GdiPlus.h>
-AZ_POP_DISABLE_WARNING
 #endif // Q_OS_WIN
 
 #include "Util/EditorUtils.h"

Code/Editor/TrackView/SequenceBatchRenderDialog.cpp

@@ -76,8 +76,6 @@ namespace
 
     const char debugInfo[] = "debuginfo";
 
-    const char format[] = "format";
-
     const int kBatchRenderFileVersion = 2; // This version number should be incremented every time available options like the list of formats,
     // the list of buffers change.
 
@@ -1518,7 +1516,7 @@ void CSequenceBatchRenderDialog::OnLoadBatch()
 
             itemNode->getAttr(debugInfo, item.disableDebugInfo);
 
-            item.imageFormat = itemNode->getAttr(format);
+            item.imageFormat = itemNode->getAttr("format");
 
             // cvars
             for (int k = 0; k < itemNode->getChildCount(); ++k)
@@ -1574,7 +1572,7 @@ void CSequenceBatchRenderDialog::OnSaveBatch()
 
             itemNode->setAttr(debugInfo, item.disableDebugInfo);
 
-            itemNode->setAttr(format, item.imageFormat.toUtf8().data());
+            itemNode->setAttr("format", item.imageFormat.toUtf8().data());
 
             // cvars
             for (size_t k = 0; k < item.cvars.size(); ++k)

Code/Framework/AzFramework/AzFramework/Visibility/IVisibilitySystem.h

@@ -35,7 +35,8 @@ namespace AzFramework
         {
             TYPE_None = 0,
             TYPE_Entity = 1 << 0,      // All entities
-            TYPE_RPI_Cullable = 1 << 2 // Cullable by the render system
+            TYPE_RPI_Cullable = 1 << 2, // Cullable by the render system
+            TYPE_RPI_VisibleObjectList = 1 << 3 // Culled by the render system, then output to a a list of visible objects
         };
 
         AZ::Aabb m_boundingVolume = AZ::Aabb::CreateNull();

Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessor.h

@@ -47,6 +47,10 @@ namespace AZ
 
             ObjectSrgCreatedEvent& GetObjectSrgCreatedEvent() { return m_objectSrgCreatedEvent; }
 
+            
+            using InstanceGroupHandle = StableDynamicArrayWeakHandle<MeshInstanceGroupData>;
+            using InstanceGroupHandleList = AZStd::vector<InstanceGroupHandle>;
+
         private:
             class MeshLoader
                 : private Data::AssetBus::Handler
@@ -114,8 +118,6 @@ namespace AZ
             
             // When instancing is enabled, draw packets are owned by the MeshInstanceManager,
             // and the ModelDataInstance refers to those draw packets via InstanceGroupHandles
-            using InstanceGroupHandle = StableDynamicArrayWeakHandle<MeshInstanceGroupData>;
-            using InstanceGroupHandleList = AZStd::vector<InstanceGroupHandle>;
             AZStd::vector<InstanceGroupHandleList> m_instanceGroupHandlesByLod;
             
             // AZ::Event is used to communicate back to all the objects that refer to an instance group whenever a draw packet is updated
@@ -186,6 +188,8 @@ namespace AZ
             void Deactivate() override;
             //! Updates GPU buffers with latest data from render proxies
             void Simulate(const FeatureProcessor::SimulatePacket& packet) override;
+            //! Updates ViewSrgs with per-view instance data for visible instances
+            void OnEndCulling(const RenderPacket& packet) override;
 
             // RPI::SceneNotificationBus overrides ...
             void OnBeginPrepareRender() override;
@@ -269,6 +273,9 @@ namespace AZ
             AZStd::vector<AZ::Job*> CreateUpdateCullingJobQueue();
             void ExecuteSimulateJobQueue(AZStd::span<Job*> jobQueue, Job* parentJob);
             void ExecuteCombinedJobQueue(AZStd::span<Job*> initQueue, AZStd::span<Job*> updateCullingQueue, Job* parentJob);
+            
+            
+            void ProcessVisibleObjectListForView(const RPI::ViewPtr& view);
 
             AZStd::concurrency_checker m_meshDataChecker;
             StableDynamicArray<ModelDataInstance> m_modelData;

Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp

@@ -420,6 +420,38 @@ namespace AZ
             }
         }
 
+        void MeshFeatureProcessor::OnEndCulling(const MeshFeatureProcessor::RenderPacket& packet)
+        {
+            if (r_meshInstancingEnabled)
+            {
+                AZ_PROFILE_SCOPE(RPI, "MeshFeatureProcessor: OnEndCulling");
+                for (size_t viewIndex = 0; viewIndex < packet.m_views.size(); ++viewIndex)
+                {
+                    ProcessVisibleObjectListForView(packet.m_views[viewIndex]);
+                }
+            }
+        }
+        
+        void MeshFeatureProcessor::ProcessVisibleObjectListForView(const RPI::ViewPtr& view)
+        {
+            // As an interim step while mesh instancing is still not supported, when r_meshInstancingIsEnabled == true, we just submit the draw packet directly to the view
+            // In a follow up commit, we will process the lists properly and generate actual instanced draw calls
+            for (const RPI::VisibleObjectProperties& visibleObject : view->GetVisibleObjectList())
+            {
+                if (visibleObject.m_userData)
+                {
+                    const ModelDataInstance::InstanceGroupHandleList* instanceGroupHandles =
+                        reinterpret_cast<const ModelDataInstance::InstanceGroupHandleList*>(visibleObject.m_userData);
+                    // For the initial commit, we naively add each draw packet for each visible object to the view.
+                    // This will be replaced with generating instance buffers for each view to use with instanced draw calls.
+                    for (auto& instanceGroupHandle : *instanceGroupHandles)
+                    {
+                        view->AddDrawPacket(instanceGroupHandle->m_drawPacket.GetRHIDrawPacket(), visibleObject.m_depth);
+                    }
+                }
+            }
+        }
+        
         void MeshFeatureProcessor::OnBeginPrepareRender()
         {
             m_meshDataChecker.soft_lock();
@@ -2103,25 +2135,35 @@ namespace AZ
                 size_t meshCount = lodAssets[lodIndex + m_lodBias]->GetMeshes().size();
                 for (size_t meshIndex = 0; meshIndex < meshCount; ++meshIndex)
                 {
-                    const RHI::DrawPacket* rhiDrawPacket = nullptr;
                     if (!r_meshInstancingEnabled)
                     {
                         // If mesh instancing is disabled, get the draw packets directly from this ModelDataInstance
-                        rhiDrawPacket = m_drawPacketListsByLod[lodIndex + m_lodBias][meshIndex].GetRHIDrawPacket();
+                        const RHI::DrawPacket* rhiDrawPacket =
+                            m_drawPacketListsByLod[lodIndex + m_lodBias][meshIndex].GetRHIDrawPacket();
+
+                        if (rhiDrawPacket)
+                        {
+                            // OR-together all the drawListMasks (so we know which views to cull against)
+                            cullData.m_drawListMask |= rhiDrawPacket->GetDrawListMask();
+
+                            lod.m_drawPackets.push_back(rhiDrawPacket);
+                        }
                     }
                     else
                     {
-                        // If mesh instancing is enabled, get the draw packets from the mesh instance manager
-                        InstanceGroupHandle& instanceGroupHandle = m_instanceGroupHandlesByLod[lodIndex + m_lodBias][meshIndex];
-                        rhiDrawPacket = meshInstanceManager[instanceGroupHandle].m_drawPacket.GetRHIDrawPacket();
-                    }
+                        // If mesh instancing is enabled, get the draw packet from the MeshInstanceManager
+                        const RHI::DrawPacket* rhiDrawPacket =
+                            meshInstanceManager[m_instanceGroupHandlesByLod[lodIndex + m_lodBias][meshIndex]]
+                                .m_drawPacket.GetRHIDrawPacket();
 
-                    if (rhiDrawPacket)
-                    {
-                        //OR-together all the drawListMasks (so we know which views to cull against)
-                        cullData.m_drawListMask |= rhiDrawPacket->GetDrawListMask();
+                        if (rhiDrawPacket)
+                        {
+                            // OR-together all the drawListMasks (so we know which views to cull against)
+                            cullData.m_drawListMask |= rhiDrawPacket->GetDrawListMask();
+                        }
 
-                        lod.m_drawPackets.push_back(rhiDrawPacket);
+                        // Set the user data for the cullable lod to reference the intance group handles for the lod
+                        lod.m_visibleObjectUserData = static_cast<void*>(&m_instanceGroupHandlesByLod[lodIndex + m_lodBias]);
                     }
                 }
             }
@@ -2161,7 +2203,14 @@ namespace AZ
             m_cullable.m_cullData.m_boundingObb = localAabb.GetTransformedObb(localToWorld);
             m_cullable.m_cullData.m_visibilityEntry.m_boundingVolume = localAabb.GetTransformedAabb(localToWorld);
             m_cullable.m_cullData.m_visibilityEntry.m_userData = &m_cullable;
-            m_cullable.m_cullData.m_visibilityEntry.m_typeFlags = AzFramework::VisibilityEntry::TYPE_RPI_Cullable;
+            if (!r_meshInstancingEnabled)
+            {
+                m_cullable.m_cullData.m_visibilityEntry.m_typeFlags = AzFramework::VisibilityEntry::TYPE_RPI_Cullable;
+            }
+            else
+            {
+                m_cullable.m_cullData.m_visibilityEntry.m_typeFlags = AzFramework::VisibilityEntry::TYPE_RPI_VisibleObjectList;
+            }
             m_scene->GetCullingScene()->RegisterOrUpdateCullable(m_cullable);
 
             m_flags.m_cullBoundsNeedsUpdate = false;

Gems/Atom/RHI/Code/Source/RHI/DrawListContext.cpp

@@ -43,27 +43,28 @@ namespace AZ
 
         void DrawListContext::AddDrawPacket(const DrawPacket* drawPacket, float depth)
         {
-            if (Validation::IsEnabled())
+            if (drawPacket)
             {
-                if (!drawPacket)
+                DrawListsByTag& threadListsByTag = m_threadListsByTag.GetStorage();
+
+                for (size_t i = 0; i < drawPacket->GetDrawItemCount(); ++i)
                 {
-                    AZ_Error("DrawListContext", false, "Null draw packet was added to a draw list context. This is not permitted and will crash if validation is disabled.");
-                    return;
+                    const DrawListTag drawListTag = drawPacket->GetDrawListTag(i);
+
+                    if (m_drawListMask[drawListTag.GetIndex()])
+                    {
+                        DrawItemProperties drawItem = drawPacket->GetDrawItem(i);
+                        drawItem.m_depth = depth;
+                        threadListsByTag[drawListTag.GetIndex()].push_back(drawItem);
+                    }
                 }
             }
-
-            DrawListsByTag& threadListsByTag = m_threadListsByTag.GetStorage();
-
-            for (size_t i = 0; i < drawPacket->GetDrawItemCount(); ++i)
+            else
             {
-                const DrawListTag drawListTag = drawPacket->GetDrawListTag(i);
-
-                if (m_drawListMask[drawListTag.GetIndex()])
-                {
-                    DrawItemProperties drawItem = drawPacket->GetDrawItem(i);
-                    drawItem.m_depth = depth;
-                    threadListsByTag[drawListTag.GetIndex()].push_back(drawItem);
-                }
+                AZ_Error(
+                    "DrawListContext",
+                    false,
+                    "Null draw packet was added to a draw list context. Visible object will be ignored.");
             }
         }
 

Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Culling.h

@@ -96,9 +96,10 @@ namespace AZ
             {
                 struct Lod
                 {
-                    float m_screenCoverageMin;
-                    float m_screenCoverageMax;
+                    float m_screenCoverageMin = 0.0f;
+                    float m_screenCoverageMax = 1.0f;
                     AZStd::vector<const RHI::DrawPacket*> m_drawPackets;
+                    void* m_visibleObjectUserData = nullptr;
                 };
 
                 AZStd::vector<Lod> m_lods;
@@ -222,7 +223,7 @@ namespace AZ
         };
 
         //! Selects an lod (based on size-in-screen-space) and adds the appropriate DrawPackets to the view.
-        uint32_t AddLodDataToView(const Vector3& pos, const Cullable::LodData& lodData, RPI::View& view);
+        uint32_t AddLodDataToView(const Vector3& pos, const Cullable::LodData& lodData, RPI::View& view, AzFramework::VisibilityEntry::TypeFlags typeFlags);
 
         //! Centralized manager for culling-related processing for a given scene.
         //! There is one CullingScene owned by each Scene, so external systems (such as FeatureProcessors) should

Gems/Atom/RPI/Code/Include/Atom/RPI.Public/FeatureProcessor.h

@@ -119,7 +119,13 @@ namespace AZ
             //! 
             //!  - This is called every frame.
             //!  - This may be called in parallel with other feature processors.
+            //!  - This may be called in parallel with culling
             virtual void Render(const RenderPacket&) {}
+            
+            //! Notifies when culling is finished, but draw lists have not been finalized or sorted
+            //! If a feature processor uses visibility lists instead of letting the culling system submit draw items
+            //! it should access the visibility lists here
+            virtual void OnEndCulling(const RenderPacket&) {}
 
             //! The feature processor may do clean up when the current render frame is finished
             //!  - This is called every RPI::RenderTick.

Gems/Atom/RPI/Code/Include/Atom/RPI.Public/View.h

@@ -14,6 +14,7 @@
 #include <Atom/RPI.Public/Base.h>
 #include <Atom/RPI.Public/Pass/Pass.h>
 #include <Atom/RPI.Public/Shader/ShaderResourceGroup.h>
+#include <Atom/RPI.Public/VisibleObjectContext.h>
 
 #include <AzCore/Math/Matrix4x4.h>
 #include <AzCore/Memory/SystemAllocator.h>
@@ -74,7 +75,15 @@ namespace AZ
             void AddDrawPacket(const RHI::DrawPacket* drawPacket, float depth = 0.0f);
 
             //! Similar to previous AddDrawPacket() but calculates depth from packet position
-            void AddDrawPacket(const RHI::DrawPacket* drawPacket, Vector3 worldPosition);
+            void AddDrawPacket(const RHI::DrawPacket* drawPacket, const Vector3& worldPosition);
+            
+            //! Similar to AddDrawPacket, but the view will not submit any draw items for rendering. It will just
+            //! maintain a list of visible objects for the current frame, and the caller must get that list, reinterpret the
+            //! userData, and submit the draw calls.
+            void AddVisibleObject(const void* userData, float depth = 0.0f);
+
+            //! Similar to previous AddVisibleObject() but calculates depth from object position
+            void AddVisibleObject(const void* userData, const Vector3& worldPosition);
 
             //! Add a draw item to this view with its associated draw list tag
             void AddDrawItem(RHI::DrawListTag drawListTag, const RHI::DrawItemProperties& drawItemProperties);
@@ -123,6 +132,10 @@ namespace AZ
             //! Get the camera's world transform, converted from the viewToWorld matrix's native y-up to z-up
             AZ::Transform GetCameraTransform() const;
 
+            //! Finalize visible object lists in this view. This function should only be called when all
+            //! visible objects for current frame are added, but before FinalizeDrawLists is called. 
+            void FinalizeVisibleObjectList();
+
             //! Finalize draw lists in this view. This function should only be called when all
             //! draw packets for current frame are added. 
             void FinalizeDrawListsJob(AZ::Job* parentJob);
@@ -131,6 +144,7 @@ namespace AZ
             bool HasDrawListTag(RHI::DrawListTag drawListTag);
 
             RHI::DrawListView GetDrawList(RHI::DrawListTag drawListTag);
+            VisibleObjectListView GetVisibleObjectList();
 
             //! Helper function to generate a sort key from a given position in world
             RHI::DrawItemSortKey GetSortKeyForPosition(const Vector3& positionInWorld) const;
@@ -200,6 +214,8 @@ namespace AZ
             RHI::DrawListContext m_drawListContext;
             RHI::DrawListMask m_drawListMask;
 
+            RPI::VisibleObjectContext m_visibleObjectContext;
+
             Matrix4x4 m_worldToViewMatrix;
             Matrix4x4 m_viewToWorldMatrix;
             Matrix4x4 m_viewToClipMatrix;

Gems/Atom/RPI/Code/Include/Atom/RPI.Public/VisibleObjectContext.h

@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+#pragma once
+
+#include <Atom/RHI/DrawItem.h>
+#include <Atom/RHI/ThreadLocalContext.h>
+
+namespace AZ
+{
+    namespace RPI
+    {
+        // A struct representing an objet to be culled by the RPI culling system,
+        // with the visible objects written to a visiblity list instead of being rendered
+        // directly by the RPI
+        struct VisibleObjectProperties
+        {
+            //! A pointer to the custom data for this object
+            const void* m_userData = nullptr;
+            //! A depth value this object which can be used for sorting draw calls
+            float m_depth = 0.0f;
+        };
+        using VisibleObjectList = AZStd::vector<VisibleObjectProperties>;
+        using VisibleObjectListView = AZStd::span<const VisibleObjectProperties>;
+
+        /**
+         * This class is a context for filling and accessing visible object lists. It is designed to be thread-safe
+         * and low-contention. The API is partitioned into two phases: append and consume.
+         *
+         * In the append phase, visible object entries (or void* pointers to user data) are added to the context.
+         * This is thread-safe and low contention. 
+         *
+         * Call FinalizeLists to transition to the consume phase. This combines the per-thread data into a single list.
+         *
+         * Finally, in the consume phase, the context is immutable and lists are accessible via GetList.
+         */
+        class VisibleObjectContext final
+        {
+        public:
+            VisibleObjectContext() = default;
+
+            /// Copies and moves are disabled to enforce thread safety.
+            AZ_DISABLE_COPY_MOVE(VisibleObjectContext);
+
+            void Shutdown();
+
+            /// Adds visible objects to the thread local visible object lists.
+            /// The depth value here is the depth of the object from the perspective of the view.
+            void AddVisibleObject(const void* userData, float depth = 0.0f);
+
+            /// Coalesces the thread local visible object lists in preparation for access via GetList. This should
+            /// be called from a single thread as a sync point between the append / consume phases.
+            void FinalizeLists();
+
+            /// Returns the visible object list for the view.
+            VisibleObjectListView GetList() const;
+
+        private:
+            // Thread local storage of visible objects during the append phase
+            RHI::ThreadLocalContext<VisibleObjectList> m_visibleObjectListContext;
+            // Combined results from the thread local list to be used during the consume phase
+            VisibleObjectList m_finalizedVisibleObjectList;
+        };
+    }
+}

Gems/Atom/RPI/Code/Source/RPI.Public/Culling.cpp

@@ -234,7 +234,8 @@ namespace AZ
             {
                 AzFramework::VisibilityEntry* visibleEntry = entries[i];
 
-                if (visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_Cullable)
+                if (visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_Cullable ||
+                    visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_VisibleObjectList)
                 {
                     Cullable* c = static_cast<Cullable*>(visibleEntry->m_userData);
 
@@ -262,8 +263,8 @@ namespace AZ
                         // There are ways to write this without [[maybe_unused]], but they are brittle.
                         // For example, using #else could cause a bug where the function's parameter
                         // is changed in #ifdef but not in #else.
-                        [[maybe_unused]]
-                        const uint32_t drawPacketCount = AddLodDataToView(c->m_cullData.m_boundingSphere.GetCenter(), c->m_lodData, *worklistData->m_view);
+                        [[maybe_unused]] const uint32_t drawPacketCount = AddLodDataToView(
+                            c->m_cullData.m_boundingSphere.GetCenter(), c->m_lodData, *worklistData->m_view, visibleEntry->m_typeFlags);
                         c->m_isVisible = true;
                         worklistData->m_view->ApplyFlags(c->m_flags);
 
@@ -284,7 +285,8 @@ namespace AZ
                 {
                     for (AzFramework::VisibilityEntry* visibleEntry : entries)
                     {
-                        if (visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_Cullable)
+                        if (visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_Cullable ||
+                            visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_VisibleObjectList)
                         {
                             Cullable* c = static_cast<Cullable*>(visibleEntry->m_userData);
                             if (worklistData->m_debugCtx->m_drawBoundingBoxes)
@@ -736,7 +738,8 @@ namespace AZ
             ProcessCullables(scene, view, nullptr, &taskGraph, &taskGraphEvent);
         }
 
-        uint32_t AddLodDataToView(const Vector3& pos, const Cullable::LodData& lodData, RPI::View& view)
+        uint32_t AddLodDataToView(
+            const Vector3& pos, const Cullable::LodData& lodData, RPI::View& view, AzFramework::VisibilityEntry::TypeFlags typeFlags)
         {
 #ifdef AZ_CULL_PROFILE_DETAILED
             AZ_PROFILE_SCOPE(RPI, "AddLodDataToView");
@@ -750,9 +753,20 @@ namespace AZ
                 AZ_PROFILE_SCOPE(RPI, "add draw packets: %zu", lod.m_drawPackets.size());
 #endif
                 numVisibleDrawPackets += static_cast<uint32_t>(lod.m_drawPackets.size());   //don't want to pay the cost of aznumeric_cast<> here so using static_cast<> instead
-                for (const RHI::DrawPacket* drawPacket : lod.m_drawPackets)
+                if (typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_VisibleObjectList)
                 {
-                    view.AddDrawPacket(drawPacket, pos);
+                    view.AddVisibleObject(lod.m_visibleObjectUserData, pos);
+                }
+                else if (typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_Cullable)
+                {
+                    for (const RHI::DrawPacket* drawPacket : lod.m_drawPackets)
+                    {
+                        view.AddDrawPacket(drawPacket, pos);
+                    }
+                }
+                else
+                {
+                    AZ_Assert(false, "Invalid cullable type flags.")
                 }
             };
 
@@ -761,7 +775,8 @@ namespace AZ
                 case Cullable::LodType::SpecificLod:
                     if (lodData.m_lodConfiguration.m_lodOverride < lodData.m_lods.size())
                     {
-                        addLodToDrawPacket(lodData.m_lods.at(lodData.m_lodConfiguration.m_lodOverride));
+                    addLodToDrawPacket(
+                        lodData.m_lods.at(lodData.m_lodConfiguration.m_lodOverride));
                     }
                     break;
                 case Cullable::LodType::ScreenCoverage:
@@ -777,8 +792,9 @@ namespace AZ
                     const float approxScreenPercentage =
                         ModelLodUtils::ApproxScreenPercentage(pos, lodData.m_lodSelectionRadius, cameraPos, yScale, isPerspective);
 
-                    for (const Cullable::LodData::Lod& lod : lodData.m_lods)
+                    for (uint32_t lodIndex = 0; lodIndex < static_cast<uint32_t>(lodData.m_lods.size()); ++lodIndex)
                     {
+                        const Cullable::LodData::Lod& lod = lodData.m_lods[lodIndex];
                         // Note that this supports overlapping lod ranges (to suport cross-fading lods, for example)
                         if (approxScreenPercentage >= lod.m_screenCoverageMin && approxScreenPercentage <= lod.m_screenCoverageMax)
                         {
@@ -927,7 +943,8 @@ namespace AZ
                 {
                     for (AzFramework::VisibilityEntry* visibleEntry : nodeData.m_entries)
                     {
-                        if (visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_Cullable)
+                        if (visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_Cullable ||
+                            visibleEntry->m_typeFlags & AzFramework::VisibilityEntry::TYPE_RPI_VisibleObjectList)
                         {
                             ++numObjects;
                         }

Gems/Atom/RPI/Code/Source/RPI.Public/Scene.cpp

@@ -824,6 +824,23 @@ namespace AZ
                 }
             }
 
+            {
+                AZ_PROFILE_SCOPE(RPI, "Scene FinalizeVisibleObjectLists");
+
+                for (auto& view : m_renderPacket.m_views)
+                {
+                    view->FinalizeVisibleObjectList();
+                }
+            }
+
+            {
+                AZ_PROFILE_SCOPE(RPI, "Scene OnEndCulling");
+                for (auto& fp : m_featureProcessors)
+                {
+                    fp->OnEndCulling(m_renderPacket);
+                }
+            }
+
             {
                 AZ_PROFILE_SCOPE(RPI, "FinalizeDrawLists");
                 if (jobPolicy == RHI::JobPolicy::Serial || 

Gems/Atom/RPI/Code/Source/RPI.Public/View.cpp

@@ -94,6 +94,7 @@ namespace AZ
         {
             m_drawListMask.reset();
             m_drawListContext.Shutdown();
+            m_visibleObjectContext.Shutdown();
             m_passesByDrawList = nullptr;
         }
 
@@ -113,13 +114,26 @@ namespace AZ
             m_drawListContext.AddDrawPacket(drawPacket, depth);
         }        
 
-        void View::AddDrawPacket(const RHI::DrawPacket* drawPacket, Vector3 worldPosition)
+        void View::AddDrawPacket(const RHI::DrawPacket* drawPacket, const Vector3& worldPosition)
         {
             Vector3 cameraToObject = worldPosition - m_position;
             float depth = cameraToObject.Dot(-m_viewToWorldMatrix.GetBasisZAsVector3());
             AddDrawPacket(drawPacket, depth);
         }
 
+        void View::AddVisibleObject(const void* userData, float depth)
+        {
+            // This function is thread safe since VisibleObjectContext has storage per thread for draw item data.
+            m_visibleObjectContext.AddVisibleObject(userData, depth);
+        }
+
+        void View::AddVisibleObject(const void* userData, const Vector3& worldPosition)
+        {
+            Vector3 cameraToObject = worldPosition - m_position;
+            float depth = cameraToObject.Dot(-m_viewToWorldMatrix.GetBasisZAsVector3());
+            AddVisibleObject(userData, depth);
+        }
+
         void View::AddDrawItem(RHI::DrawListTag drawListTag, const RHI::DrawItemProperties& drawItemProperties)
         {
             m_drawListContext.AddDrawItem(drawListTag, drawItemProperties);
@@ -356,6 +370,16 @@ namespace AZ
             return m_drawListContext.GetList(drawListTag);
         }
 
+        VisibleObjectListView View::GetVisibleObjectList()
+        {
+            return m_visibleObjectContext.GetList();
+        }
+
+        void View::FinalizeVisibleObjectList()
+        {
+            m_visibleObjectContext.FinalizeLists();
+        }
+
         void View::FinalizeDrawListsTG(AZ::TaskGraphEvent& finalizeDrawListsTGEvent)
         {
             AZ_PROFILE_SCOPE(RPI, "View: FinalizeDrawLists");

Gems/Atom/RPI/Code/Source/RPI.Public/VisibleObjectContext.cpp

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+#include <Atom/RPI.Public/VisibleObjectContext.h>
+#include <Atom/RPI.Reflect/Base.h>
+#include <AzCore/Debug/Profiler.h>
+#include <AzCore/std/sort.h>
+
+AZ_DECLARE_BUDGET(RPI);
+
+namespace AZ
+{
+    namespace RPI
+    {
+        void VisibleObjectContext::Shutdown()
+        {
+            m_visibleObjectListContext.Clear();
+
+            m_finalizedVisibleObjectList.clear();
+        }
+
+        void VisibleObjectContext::AddVisibleObject(const void* userData, float depth)
+        {
+            if (userData)
+            {
+                VisibleObjectList& visibleObjectList = m_visibleObjectListContext.GetStorage();
+                VisibleObjectProperties visibleObject;
+                visibleObject.m_userData = userData;
+                visibleObject.m_depth = depth;
+                visibleObjectList.push_back(visibleObject);
+            }
+            else
+            {
+                AZ_Error(
+                    "VisibleObjectContext",
+                    false,
+                    "Null userData was added to a VisibleObjectContext. Visible object will be ignored.");
+            }
+        }
+
+        void VisibleObjectContext::FinalizeLists()
+        {
+            AZ_PROFILE_SCOPE(RPI, "VisibleObjectContext: FinalizeLists");
+            m_finalizedVisibleObjectList.clear();
+
+            // Reserve enough memory for all the visible objects
+            size_t objectCount = 0;
+            m_visibleObjectListContext.ForEach(
+                [&objectCount](const VisibleObjectList& visibleObjectList)
+                {
+                    objectCount += visibleObjectList.size();
+                });
+            m_finalizedVisibleObjectList.reserve(objectCount);
+
+            // Concatenate all the per-thread lists into a single list
+            m_visibleObjectListContext.ForEach(
+                [this](VisibleObjectList& visibleObjectList)
+                {
+                    m_finalizedVisibleObjectList.insert(m_finalizedVisibleObjectList.end(), visibleObjectList.begin(), visibleObjectList.end());
+                    visibleObjectList.clear();
+            });
+        }
+
+        VisibleObjectListView VisibleObjectContext::GetList() const
+        {
+            return m_finalizedVisibleObjectList;
+        }
+    }
+}

Gems/Atom/RPI/Code/Tests/System/CullingTests.cpp

@@ -0,0 +1,254 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+#include <AzCore/Math/MatrixUtils.h>
+#include <AzCore/Task/TaskExecutor.h>
+#include <AzCore/Task/TaskGraph.h>
+#include <AzFramework/Visibility/OctreeSystemComponent.h>
+
+#include <Atom/RPI.Public/Culling.h>
+#include <Atom/RPI.Public/Scene.h>
+#include <Atom/RPI.Public/View.h>
+#include <Common/RPITestFixture.h>
+
+namespace UnitTest
+{
+    using namespace AZ;
+    using namespace RPI;
+
+    // The CullingTests fixture sets up a culling scene for testing culling.
+    // It also creates some views and a varying number of cullable objects visible in each view.
+    // It does not register the cullables with the culling scene, so their properties can be overridden
+    // before registering in order to test different scenarios
+    class CullingTests : public RPITestFixture
+    {
+        void SetUp() override
+        {
+            RPITestFixture::SetUp();
+
+            m_executor = aznew TaskExecutor{};
+            TaskExecutor::SetInstance(m_executor);
+
+            m_octreeSystemComponent = new AzFramework::OctreeSystemComponent;
+            m_testScene = Scene::CreateScene(SceneDescriptor{});
+            m_cullingScene = m_testScene->GetCullingScene();
+            m_cullingScene->Activate(m_testScene.get());
+
+            CreateTestViews();
+            CreateTestObjects();
+        }
+
+        void TearDown() override
+        {
+            m_views.clear();
+            m_cullingScene->Deactivate();
+            m_testScene = nullptr;
+            delete m_octreeSystemComponent;
+
+            if (&TaskExecutor::Instance() == m_executor) // if this test created the default instance unset it before destroying it
+            {
+                TaskExecutor::SetInstance(nullptr);
+            }
+            azdestroy(m_executor);
+
+            RPITestFixture::TearDown();
+        }
+
+    protected:
+        static constexpr size_t testCameraCount = 4;
+        static constexpr size_t visibleObjectUserDataOffset = 100;
+        using TestCameraList = AZStd::vector<ViewPtr>;
+
+        void Cull(TestCameraList& views)
+        {
+            m_cullingScene->BeginCulling(views);
+
+            // Create and submit work to the culling scene in a similar style as RPI::Scene::PrepareRender
+            static const TaskDescriptor processCullablesDescriptor{ "RPI::Scene::ProcessCullables", "Graphics" };
+            TaskGraphEvent processCullablesTGEvent{ "ProcessCullables Wait" };
+            TaskGraph processCullablesTG{ "ProcessCullables" };
+            for (ViewPtr& viewPtr : views)
+            {
+                processCullablesTG.AddTask(
+                    processCullablesDescriptor,
+                    [this, &viewPtr, &processCullablesTGEvent]()
+                    {
+                        TaskGraph subTaskGraph{ "ProcessCullables Subgraph" };
+                        m_cullingScene->ProcessCullablesTG(*m_testScene, *viewPtr, subTaskGraph, processCullablesTGEvent);
+                        if (!subTaskGraph.IsEmpty())
+                        {
+                            subTaskGraph.Detach();
+                            subTaskGraph.Submit(&processCullablesTGEvent);
+                        }
+                    });
+            }
+
+            processCullablesTG.Submit(&processCullablesTGEvent);
+            processCullablesTGEvent.Wait();
+            m_cullingScene->EndCulling();
+
+            for (ViewPtr& viewPtr : views)
+            {
+                viewPtr->FinalizeVisibleObjectList();
+            }
+        }
+
+        enum ViewIndex
+        {
+            YPositive = 0,
+            XNegative,
+            YNegative,
+            XPositive
+        };
+
+        // Create four test cameras
+        // Top down view of the cameras
+        //    ___        +y
+        //    \0/        |  
+        //  |1>X<3|      | 
+        //    /2\        |_____+x
+        //    ---
+        //
+        void CreateTestViews()
+        {
+            ViewPtr viewYPositive = View::CreateView(Name("TestViewYPositive"), RPI::View::UsageCamera);
+            ViewPtr viewXNegative = View::CreateView(Name("TestViewXNegative"), RPI::View::UsageShadow);
+            ViewPtr viewYNegative = View::CreateView(Name("TestViewYNegative"), RPI::View::UsageShadow);
+            ViewPtr viewXPositive = View::CreateView(Name("TestViewXPositive"), RPI::View::UsageReflectiveCubeMap);
+
+            // Render everything by default
+            RHI::DrawListMask drawListMask;
+            drawListMask.reset();
+            drawListMask.flip();
+
+            viewYPositive->SetDrawListMask(drawListMask);
+            viewXNegative->SetDrawListMask(drawListMask);
+            viewYNegative->SetDrawListMask(drawListMask);
+            viewXPositive->SetDrawListMask(drawListMask);
+
+            const float fovY = DegToRad(90.0f);
+            const float aspectRatio = 1.0f;
+            const float nearDist = 0.1f;
+            const float farDist = 100.0f;
+
+            // These rotations represent a right-handed rotation around the z-up axis.
+            // Starting with a view pointed straight down the y-forward axis,
+            // these will rotate counter clockwise
+            viewYPositive->SetCameraTransform(Matrix3x4::CreateIdentity());
+            viewXNegative->SetCameraTransform(Matrix3x4::CreateRotationZ(DegToRad(90.0f)));
+            viewYNegative->SetCameraTransform(Matrix3x4::CreateRotationZ(DegToRad(180.0f)));
+            viewXPositive->SetCameraTransform(Matrix3x4::CreateRotationZ(DegToRad(270.0f)));
+
+            // Matrix4x4::CreateProjection creates a view pointing up the positive z-axis.
+            // Combine that with the rotation matrices above to get the 4 views.
+            Matrix4x4 viewToClipZPositive = Matrix4x4::CreateIdentity();
+            bool reverseDepth = true;
+            MakePerspectiveFovMatrixRH(viewToClipZPositive, fovY, aspectRatio, nearDist, farDist, reverseDepth);
+            viewYPositive->SetViewToClipMatrix(viewToClipZPositive);
+            viewXNegative->SetViewToClipMatrix(viewToClipZPositive);
+            viewYNegative->SetViewToClipMatrix(viewToClipZPositive);
+            viewXPositive->SetViewToClipMatrix(viewToClipZPositive);
+
+            m_views.resize(testCameraCount);
+            m_views[YPositive] = viewYPositive;
+            m_views[XNegative] = viewXNegative;
+            m_views[YNegative] = viewYNegative;
+            m_views[XPositive] = viewXPositive;
+        }
+
+        static void InitializeCullableFromAabb(Cullable& cullable, const Aabb& aabb, size_t index)
+        {
+            cullable.m_cullData.m_boundingObb = Obb::CreateFromAabb(aabb);
+            cullable.m_cullData.m_boundingSphere = Sphere::CreateFromAabb(aabb);
+            cullable.m_cullData.m_visibilityEntry.m_boundingVolume = aabb;
+            cullable.m_cullData.m_visibilityEntry.m_typeFlags = AzFramework::VisibilityEntry::TYPE_RPI_VisibleObjectList;
+
+            // Set all bits in the draw list mask by default, so everything will be rendered
+            cullable.m_cullData.m_drawListMask.reset();
+            cullable.m_cullData.m_drawListMask.flip();
+
+            cullable.m_cullData.m_visibilityEntry.m_userData = &cullable;
+            cullable.m_lodData.m_lodSelectionRadius = 0.5f * aabb.GetExtents().GetMaxElement();
+            Cullable::LodData::Lod lod;
+            lod.m_screenCoverageMin = 0.0f;
+            lod.m_screenCoverageMax = 1.0f;
+
+            // We're not actually using the user data for anything, but it needs to be non-null or the
+            // VisibleObjectContext will assert. We'll use the index here, which could potentially be
+            // used for validation in the tests (e.g., validate the Nth object was culled/visible).
+            // However, we must also add an offset, or the 0th object will be treated as a nullptr.
+            lod.m_visibleObjectUserData = reinterpret_cast<void*>(index + visibleObjectUserDataOffset);
+            cullable.m_lodData.m_lods.push_back(lod);
+        }
+
+        // Create test objects visible to the cameras
+        // (objects represented as dots in the diagram below)
+        // Top down view of the cameras:
+        //        ....
+        //        ___         +y
+        //        \0/         |  
+        // ... |1> X <3| .    | 
+        //        /2\         |_____+x
+        //        ---
+        //        ..
+        //
+        void CreateTestObjects()
+        {
+            // Four objects visible by the first camera
+            Aabb aabb = Aabb::CreateCenterRadius(Vector3::CreateAxisY(10.0), 1.0);
+            for (size_t i = 0; i < 10; ++i)
+            {
+                if (i == 4)
+                {
+                    // Three objects visible by the second camera
+                    aabb = Aabb::CreateCenterRadius(Vector3::CreateAxisX(-10.0), 1.0);
+                }
+                if (i == 7)
+                {
+                    // Two objects visible by the third camera
+                    aabb = Aabb::CreateCenterRadius(Vector3::CreateAxisY(-10.0), 1.0);
+                }
+                if (i == 9)
+                {
+                    // One object visible by the final camera
+                    aabb = Aabb::CreateCenterRadius(Vector3::CreateAxisX(10.0), 1.0);
+                }
+
+                // We're initializing these cullables in place because the copy constructor for RPI::Cullbable is implicitely deleted
+                InitializeCullableFromAabb(m_testObjects[i], aabb, i);
+            }
+        }
+
+        TaskExecutor* m_executor;
+        AzFramework::OctreeSystemComponent* m_octreeSystemComponent;
+        ScenePtr m_testScene;
+        CullingScene* m_cullingScene;
+        AZStd::vector<ViewPtr> m_views;
+        AZStd::array<Cullable, 10> m_testObjects;
+    };
+
+    TEST_F(CullingTests, VisibleObjectListTest)
+    {
+        for (Cullable& object : m_testObjects)
+        {
+            m_cullingScene->RegisterOrUpdateCullable(object);
+        }
+
+        Cull(m_views);
+
+        EXPECT_EQ(m_views[YPositive]->GetVisibleObjectList().size(), 4);
+        EXPECT_EQ(m_views[XNegative]->GetVisibleObjectList().size(), 3);
+        EXPECT_EQ(m_views[YNegative]->GetVisibleObjectList().size(), 2);
+        EXPECT_EQ(m_views[XPositive]->GetVisibleObjectList().size(), 1);
+
+        for (Cullable& object : m_testObjects)
+        {
+            m_cullingScene->UnregisterCullable(object);
+        }
+    }
+}

Gems/Atom/RPI/Code/atom_rpi_public_files.cmake

@@ -30,6 +30,7 @@ set(FILES
     Include/Atom/RPI.Public/ViewportContextBus.h
     Include/Atom/RPI.Public/ViewportContextManager.h
     Include/Atom/RPI.Public/ViewProviderBus.h
+    Include/Atom/RPI.Public/VisibleObjectContext.h
     Include/Atom/RPI.Public/WindowContext.h
     Include/Atom/RPI.Public/WindowContextBus.h
     Include/Atom/RPI.Public/AuxGeom/AuxGeomDraw.h
@@ -119,6 +120,7 @@ set(FILES
     Source/RPI.Public/ViewGroup.cpp
     Source/RPI.Public/ViewportContext.cpp
     Source/RPI.Public/ViewportContextManager.cpp
+    Source/RPI.Public/VisibleObjectContext.cpp
     Source/RPI.Public/WindowContext.cpp
     Source/RPI.Public/AuxGeomFeatureProcessorInterface.cpp
     Source/RPI.Public/Buffer/Buffer.cpp

Gems/Atom/RPI/Code/atom_rpi_tests_files.cmake

@@ -45,6 +45,7 @@ set(FILES
     Tests/ShaderResourceGroup/ShaderResourceGroupConstantBufferTests.cpp
     Tests/ShaderResourceGroup/ShaderResourceGroupImageTests.cpp
     Tests/ShaderResourceGroup/ShaderResourceGroupGeneralTests.cpp
+    Tests/System/CullingTests.cpp
     Tests/System/FeatureProcessorFactoryTests.cpp
     Tests/System/GpuQueryTests.cpp
     Tests/System/RenderPipelineTests.cpp