[Pointcloud] Fixes in Pointcloud Gem (#62)

* Enable pointcloud bus for editor component * Query for FeatureProcessor before enabling (to avoid log flooding) * Empty Pointcloud asset breaks viewport selection - fix by not applying transforms on invalid bounds * Add ChangeEvent to PointcloudFeatureProcessor to update cached local bounds * Code cleanup Signed-off-by: Mateusz Żak <[email protected]>
RobotecAI · Sep 18, 2024 · 6d6a10b · 6d6a10b
1 parent 288a29c
commit 6d6a10b
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 46 deletions.
diff --git a/...loudComponentControllerConfigurationBus.h → ...e/Pointcloud/PointcloudConfigurationBus.h b/...loudComponentControllerConfigurationBus.h → ...e/Pointcloud/PointcloudConfigurationBus.h
diff --git a/Gems/Pointcloud/Code/Include/Pointcloud/PointcloudFeatureProcessorInterface.h b/Gems/Pointcloud/Code/Include/Pointcloud/PointcloudFeatureProcessorInterface.h
@@ -24,6 +24,7 @@ namespace Pointcloud
     {
     public:
         using PointcloudHandle = int;
+        using PointcloudChangedEvent = AZ::Event<PointcloudHandle>;
         constexpr static PointcloudHandle InvalidPointcloudHandle = -1;
         AZ_RTTI(PointcloudFeatureProcessorInterface, "{8597AF27-EB4E-4363-8889-3BFC2AF5D2EC}", AZ::RPI::FeatureProcessor);
 
@@ -67,5 +68,8 @@ namespace Pointcloud
         //! Get the bounds of a pointcloud
         //! @param handle The handle of the pointcloud obtained from AcquirePointcloud
         virtual AZStd::optional<AZ::Aabb> GetBounds(const PointcloudHandle& handle) const = 0;
+
+        //! Connects a handler to any changes to a Pointcloud. Changes include loading and reloading
+        virtual void ConnectChangeEventHandler(const PointcloudHandle& meshHandle, PointcloudChangedEvent::Handler& handler) = 0;
     };
 } // namespace Pointcloud
diff --git a/Gems/Pointcloud/Code/Source/Clients/PointcloudComponent.cpp b/Gems/Pointcloud/Code/Source/Clients/PointcloudComponent.cpp
@@ -8,11 +8,7 @@
 
 #include "PointcloudComponent.h"
 #include <Atom/RPI.Public/Scene.h>
-#include <AzCore/Serialization/EditContext.h>
 #include <AzCore/Serialization/SerializeContext.h>
-#include <Pointcloud/PointcloudComponentControllerConfigurationBus.h>
-#include <Pointcloud/PointcloudTypeIds.h>
-#include <Render/PointcloudFeatureProcessor.h>
 
 namespace Pointcloud
 {

diff --git a/Gems/Pointcloud/Code/Source/Render/PointcloudFeatureProcessor.cpp b/Gems/Pointcloud/Code/Source/Render/PointcloudFeatureProcessor.cpp
@@ -128,6 +128,7 @@ namespace Pointcloud
         pcData.m_bounds = aabb;
 
         UpdateDrawPacket();
+        m_pointcloudChangedEvent.Signal(PointcloudDataIndex);
     }
 
     PointcloudFeatureProcessorInterface::PointcloudHandle PointcloudFeatureProcessor::AcquirePointcloudFromAsset(
@@ -366,4 +367,12 @@ namespace Pointcloud
         }
         return AZStd::nullopt;
     }
+    void PointcloudFeatureProcessor::ConnectChangeEventHandler(
+        const PointcloudHandle& pointcloudHandle, PointcloudChangedEvent::Handler& handler)
+    {
+        if (pointcloudHandle != InvalidPointcloudHandle)
+        {
+            handler.Connect(m_pointcloudChangedEvent);
+        }
+    }
 } // namespace Pointcloud
diff --git a/Gems/Pointcloud/Code/Source/Render/PointcloudFeatureProcessor.h b/Gems/Pointcloud/Code/Source/Render/PointcloudFeatureProcessor.h
@@ -45,6 +45,8 @@ namespace Pointcloud
         uint32_t GetPointCount(const PointcloudHandle& handle) const override;
         AZStd::optional<AZ::Aabb> GetBounds(const PointcloudHandle& handle) const override;
 
+        void ConnectChangeEventHandler(const PointcloudHandle& meshHandle, PointcloudChangedEvent::Handler& handler) override;
+
     protected:
         // RPI::SceneNotificationBus overrides
         void OnRenderPipelineChanged(
@@ -101,5 +103,6 @@ namespace Pointcloud
         AZStd::unordered_map<PointcloudHandle, PointcloudData> m_pointcloudData; //!< Map of pointcloud data
         PointcloudHandle m_currentPointcloudDataIndex = 0; //!< Index to the next pointcloud data to be created
         AZStd::unordered_map<AZ::Data::AssetId, PointcloudHandle> m_pointcloudAssets;
+        PointcloudChangedEvent m_pointcloudChangedEvent;
     };
 } // namespace Pointcloud
diff --git a/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudComponentController.cpp b/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudComponentController.cpp
@@ -1,4 +1,5 @@
 #include "PointcloudComponentController.h"
+
 #include "Clients/PointcloudComponent.h"
 #include <Atom/RPI.Public/Scene.h>
 #include <AzCore/Component/ComponentBus.h>
@@ -8,6 +9,7 @@
 #include <AzCore/Serialization/EditContextConstants.inl>
 #include <AzCore/Serialization/SerializeContext.h>
 #include <AzFramework/Entity/EntityContext.h>
+#include <AzFramework/Visibility/EntityBoundsUnionBus.h>
 #include <Render/PointcloudFeatureProcessor.h>
 
 namespace Pointcloud
@@ -25,17 +27,25 @@ namespace Pointcloud
 
     void PointcloudComponentController::Init()
     {
+        m_changeEventHandler = AZ::EventHandler<PointcloudFeatureProcessorInterface::PointcloudHandle>(
+            [&](PointcloudFeatureProcessorInterface::PointcloudHandle handle)
+            {
+                this->HandleChange(handle);
+            });
+
         AZ::SystemTickBus::QueueFunction(
             [this]()
             {
-                m_scene = AZ::RPI::Scene::GetSceneForEntityId(m_config.m_editorEntityId);
-                if (m_scene)
+                m_featureProcessor = AZ::RPI::Scene::GetFeatureProcessorForEntity<PointcloudFeatureProcessor>(m_config.m_editorEntityId);
+                if (!m_featureProcessor)
                 {
-                    m_featureProcessor = m_scene->EnableFeatureProcessor<PointcloudFeatureProcessor>();
-
-                    AZ_Assert(m_featureProcessor, "Failed to enable PointcloudFeatureProcessorInterface.");
-                    OnAssetChanged();
+                    if (auto* scene = AZ::RPI::Scene::GetSceneForEntityId(m_config.m_editorEntityId))
+                    {
+                        m_featureProcessor = scene->EnableFeatureProcessor<PointcloudFeatureProcessor>();
+                        AZ_Assert(m_featureProcessor, "Failed to enable PointcloudFeatureProcessorInterface.");
+                    }
                 }
+                OnAssetChanged();
             });
     }
 
@@ -119,14 +129,17 @@ namespace Pointcloud
         AZ::SystemTickBus::QueueFunction(
             [this]()
             {
-                m_scene = AZ::RPI::Scene::GetSceneForEntityId(m_config.m_editorEntityId);
-                if (m_scene)
+                m_featureProcessor = AZ::RPI::Scene::GetFeatureProcessorForEntity<PointcloudFeatureProcessor>(m_config.m_editorEntityId);
+                if (!m_featureProcessor)
                 {
-                    m_featureProcessor = m_scene->EnableFeatureProcessor<PointcloudFeatureProcessor>();
-
-                    AZ_Assert(m_featureProcessor, "Failed to enable PointcloudFeatureProcessorInterface.");
-                    OnAssetChanged();
+                    if (auto* scene = AZ::RPI::Scene::GetSceneForEntityId(m_config.m_editorEntityId))
+                    {
+                        m_featureProcessor = scene->EnableFeatureProcessor<PointcloudFeatureProcessor>();
+                        AZ_Assert(m_featureProcessor, "Failed to enable PointcloudFeatureProcessorInterface.");
+                    }
                 }
+                m_featureProcessor->ConnectChangeEventHandler(m_config.m_pointcloudHandle, m_changeEventHandler);
+                OnAssetChanged();
             });
     }
 
@@ -153,7 +166,6 @@ namespace Pointcloud
     {
         if (m_featureProcessor)
         {
-            printf("Feature processor exists\n");
             m_featureProcessor->ReleasePointcloud(m_config.m_pointcloudHandle);
             if (m_config.m_pointcloudAsset.GetId().IsValid())
             {
@@ -169,6 +181,15 @@ namespace Pointcloud
         return AZ::Edit::PropertyRefreshLevels::EntireTree;
     }
 
+    void PointcloudComponentController::HandleChange(PointcloudFeatureProcessorInterface::PointcloudHandle handle)
+    {
+        if (m_config.m_pointcloudHandle == handle)
+        {
+            // Refresh cached local bounds
+            AZ::Interface<AzFramework::IEntityBoundsUnion>::Get()->RefreshEntityLocalBoundsUnion(m_config.m_editorEntityId);
+        }
+    }
+
     void PointcloudComponentController::OnEntityInfoUpdatedVisibility(AZ::EntityId entityId, bool visible)
     {
         if (entityId == m_config.m_editorEntityId)
@@ -189,6 +210,7 @@ namespace Pointcloud
     {
         m_config.m_pointcloudAsset = asset;
     }
+
     void PointcloudComponentController::SetPointSize(float pointSize)
     {
         m_config.m_pointSize = pointSize;

diff --git a/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudComponentController.h b/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudComponentController.h
@@ -8,18 +8,12 @@
 
 #pragma once
 
-#include "AzCore/Math/Aabb.h"
-
 #include <AzCore/Component/ComponentBus.h>
 #include <AzCore/Component/EntityId.h>
 #include <AzCore/Component/TransformBus.h>
-#include <AzCore/Memory/Memory_fwd.h>
-#include <AzCore/Memory/SystemAllocator.h>
-#include <AzCore/base.h>
-#include <AzFramework/Spawnable/Spawnable.h>
-#include <Pointcloud/PointcloudComponentControllerConfigurationBus.h>
+#include <AzCore/Math/Aabb.h>
+#include <Pointcloud/PointcloudConfigurationBus.h>
 #include <Pointcloud/PointcloudFeatureProcessorInterface.h>
-#include <Pointcloud/PointcloudTypeIds.h>
 
 namespace Pointcloud
 {
@@ -37,7 +31,7 @@ namespace Pointcloud
         float m_pointSize = 1.0f;
         PointcloudFeatureProcessorInterface::PointcloudHandle m_pointcloudHandle =
             PointcloudFeatureProcessorInterface::InvalidPointcloudHandle;
-        AZ::Data::Asset<PointcloudAsset> m_pointcloudAsset;
+        AZ::Data::Asset<PointcloudAsset> m_pointcloudAsset = {};
     };
 
     class PointcloudComponentController
@@ -61,7 +55,8 @@ namespace Pointcloud
         //////////////////////////////////////////////////////////////////////////
 
         //////////////////////////////////////////////////////////////////////////
-        void OnTransformChanged(const AZ::Transform& local, const AZ::Transform& world);
+        // AZ::TransformNotificationBus::Handler overrides ...
+        void OnTransformChanged(const AZ::Transform& local, const AZ::Transform& world) override;
 
         // AzToolsFramework::EditorEntityInfoNotificationBus overrides ...
         void OnEntityInfoUpdatedVisibility(AZ::EntityId entityId, bool visible);
@@ -76,8 +71,10 @@ namespace Pointcloud
         AZ::Crc32 OnAssetChanged();
 
     private:
+        PointcloudFeatureProcessorInterface::PointcloudChangedEvent::Handler m_changeEventHandler;
+        void HandleChange(PointcloudFeatureProcessorInterface::PointcloudHandle handle);
+
         PointcloudFeatureProcessorInterface* m_featureProcessor = nullptr;
-        AZ::RPI::Scene* m_scene = nullptr;
         PointcloudComponentConfig m_config;
         bool visibility = true;
     };

diff --git a/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudEditorComponent.cpp b/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudEditorComponent.cpp
@@ -1,7 +1,6 @@
 #include "PointcloudEditorComponent.h"
 #include "Clients/PointcloudComponent.h"
 
-#include <Atom/RPI.Public/Scene.h>
 #include <AzToolsFramework/Entity/EditorEntityInfoBus.h>
 #include <AzToolsFramework/ToolsComponents/EditorComponentBase.h>
 #include <Render/PointcloudFeatureProcessor.h>
@@ -61,17 +60,16 @@ namespace Pointcloud
         AzToolsFramework::EditorComponentSelectionRequestsBus::Handler::BusDisconnect();
         AzFramework::BoundsRequestBus::Handler::BusDisconnect();
     }
-    bool PointcloudEditorComponent::ShouldActivateController() const
-    {
-        return false;
-    }
 
     AZ::Aabb PointcloudEditorComponent::GetWorldBounds()
     {
         AZ::Transform transform = AZ::Transform::CreateIdentity();
         AZ::TransformBus::EventResult(transform, GetEntityId(), &AZ::TransformBus::Events::GetWorldTM);
         AZ::Aabb bounds = m_controller.GetBounds();
-        bounds.ApplyTransform(transform);
+        if (bounds.IsValid())
+        {
+            bounds.ApplyTransform(transform);
+        }
         return bounds;
     }
 
@@ -80,7 +78,10 @@ namespace Pointcloud
         AZ::Transform transform = AZ::Transform::CreateIdentity();
         AZ::TransformBus::EventResult(transform, GetEntityId(), &AZ::TransformBus::Events::GetLocalTM);
         AZ::Aabb bounds = m_controller.GetBounds();
-        bounds.ApplyTransform(transform);
+        if (bounds.IsValid())
+        {
+            bounds.ApplyTransform(transform);
+        }
         return bounds;
     }
 

diff --git a/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudEditorComponent.h b/Gems/Pointcloud/Code/Source/Tools/Components/PointcloudEditorComponent.h
@@ -2,22 +2,15 @@
 
 #include "PointcloudComponentController.h"
 
-#include "../../Clients/PointcloudComponent.h"
-#include <AzToolsFramework/API/ComponentEntitySelectionBus.h>
-
-#include <AzFramework/Visibility/BoundsBus.h>
+#include "Clients/PointcloudComponent.h"
 
-#include <AzCore/Asset/AssetCommon.h>
-#include <AzCore/Component/TickBus.h>
 #include <AzCore/Component/TransformBus.h>
-#include <AzFramework/Entity/EntityContextBus.h>
 #include <AzFramework/Entity/EntityDebugDisplayBus.h>
-#include <AzFramework/Scene/Scene.h>
-#include <AzFramework/Spawnable/SpawnableEntitiesInterface.h>
+#include <AzFramework/Visibility/BoundsBus.h>
+#include <AzToolsFramework/API/ComponentEntitySelectionBus.h>
 #include <AzToolsFramework/Entity/EditorEntityInfoBus.h>
 #include <AzToolsFramework/ToolsComponents/EditorComponentAdapter.h>
 #include <AzToolsFramework/ToolsComponents/EditorComponentBase.h>
-#include <Pointcloud/PointcloudComponentControllerConfigurationBus.h>
 #include <Pointcloud/PointcloudFeatureProcessorInterface.h>
 #include <Pointcloud/PointcloudTypeIds.h>
 
@@ -47,18 +40,20 @@ namespace Pointcloud
         // EditorComponentBase interface overrides ...
         void Activate() override;
         void Deactivate() override;
-        bool ShouldActivateController() const override;
 
+        // AzFramework::BoundsRequestBus overrides ...
         AZ::Aabb GetWorldBounds() override;
         AZ::Aabb GetLocalBounds() override;
 
+        // AzToolsFramework::EditorComponentSelectionRequestsBus overrides ...
         AZ::Aabb GetEditorSelectionBoundsViewport(const AzFramework::ViewportInfo& viewportInfo) override;
         bool EditorSelectionIntersectRayViewport(
             const AzFramework::ViewportInfo& viewportInfo, const AZ::Vector3& src, const AZ::Vector3& dir, float& distance) override;
 
         bool SupportsEditorRayIntersect() override;
         bool SupportsEditorRayIntersectViewport(const AzFramework::ViewportInfo& viewportInfo) override;
 
+        // AzFramework::EntityDebugDisplayEventBus overrides ...
         void DisplayEntityViewport(const AzFramework::ViewportInfo& viewportInfo, AzFramework::DebugDisplayRequests& debugDisplay) override;
 
     private: