Non-terrain-gem changes in support of upcoming terrain work. (#3345)

In preparation for a prototype Terrain Gem to get submitted, there are a few changes that are needed outside of the Terrain Gem as well:

The TerrainDataNotificationBus lives in AzFramework/Terrain, and needed to be extended to contain an optional OnTerrainDataChanged event to notify other systems when a terrain region has changed.
The HeightmapUpdateNotificationBus was removed, as this was a legacy file from the old already-removed terrain system.
The EditorWrappedComponentBase<> wrapper received a small optimization to ensure that ConfigurationChanged() is only called when the value of visibility actually changes. With prefabs, it appears that sometimes OnEntityVisibilityChanged could be called multiple times in a row with the same visibility value.
The TerrainSurfaceDataSystemComponent was updated to use the correct busses, and is ready to be moved to the Terrain Gem in a subsequent PR.

Signed-off-by: Mike Balfour 82224783+mbalfour-amzn@users.noreply.github.com

Code/Framework/AzFramework/AzFramework/Terrain/TerrainDataRequestBus.h

@@ -102,11 +102,25 @@ namespace AzFramework
             static const AZ::EBusAddressPolicy AddressPolicy = AZ::EBusAddressPolicy::Single;
             //////////////////////////////////////////////////////////////////////////
 
-            virtual void OnTerrainDataCreateBegin() {};
+            enum TerrainDataChangedMask : uint8_t
-            virtual void OnTerrainDataCreateEnd() {};
+            {
+                None        = 0b00000000,
+                Settings    = 0b00000001,
+                HeightData  = 0b00000010,
+                ColorData   = 0b00000100,
+                SurfaceData = 0b00001000
+            };
 
-            virtual void OnTerrainDataDestroyBegin() {};
+            virtual void OnTerrainDataCreateBegin() {}
-            virtual void OnTerrainDataDestroyEnd() {};
+            virtual void OnTerrainDataCreateEnd() {}
+
+            virtual void OnTerrainDataDestroyBegin() {}
+            virtual void OnTerrainDataDestroyEnd() {}
+
+            virtual void OnTerrainDataChanged(
+                [[maybe_unused]] const AZ::Aabb& dirtyRegion, [[maybe_unused]] TerrainDataChangedMask dataChangedMask)
+            {
+            }
         };
         using TerrainDataNotificationBus = AZ::EBus<TerrainDataNotifications>;
 

Code/Legacy/CryCommon/HeightmapUpdateNotificationBus.h

@@ -1,34 +0,0 @@
-/*
- * 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 <AzCore/EBus/EBus.h>
-#include <AzCore/Math/Aabb.h>
-
-namespace AZ
-{
-    /**
-    * the EBus is used to request information about potential vegetation surfaces
-    */
-    class HeightmapUpdateNotification
-        : public AZ::EBusTraits
-    {
-    public:
-        ////////////////////////////////////////////////////////////////////////
-        // EBusTraits
-        static const AZ::EBusHandlerPolicy HandlerPolicy = AZ::EBusHandlerPolicy::Multiple;
-        static const AZ::EBusAddressPolicy AddressPolicy = AZ::EBusAddressPolicy::Single;
-        ////////////////////////////////////////////////////////////////////////
-
-        // Occurs when the terrain height map is modified.
-        virtual void HeightmapModified(const AZ::Aabb& bounds) = 0;
-    };
-
-    typedef AZ::EBus<HeightmapUpdateNotification> HeightmapUpdateNotificationBus;
-}

Code/Legacy/CryCommon/crycommon_files.cmake

@@ -53,7 +53,6 @@ set(FILES
     HMDBus.h
     VRCommon.h
     StereoRendererBus.h
-    HeightmapUpdateNotificationBus.h
     INavigationSystem.h
     IMNM.h
     SFunctor.h

Gems/LmbrCentral/Code/include/LmbrCentral/Component/EditorWrappedComponentBase.inl

@@ -212,8 +212,11 @@ namespace LmbrCentral
     template <typename TComponent, typename TConfiguration>
     void EditorWrappedComponentBase<TComponent, TConfiguration>::OnEntityVisibilityChanged(bool visibility)
     {
-        m_visible = visibility;
+        if (m_visible != visibility)
-        ConfigurationChanged();
+        {
+            m_visible = visibility;
+            ConfigurationChanged();
+        }
     }
 
     template <typename TComponent, typename TConfiguration>

Gems/SurfaceData/Code/Include/SurfaceData/Utility/SurfaceDataUtility.h

@@ -14,7 +14,6 @@
 #include <AzCore/Math/IntersectSegment.h>
 #include <AzCore/Math/Transform.h>
 #include <AzCore/Math/Vector3.h>
-#include <MathConversion.h>
 
 namespace AZ
 {

Gems/SurfaceData/Code/Source/SurfaceDataModule.cpp

@@ -20,7 +20,7 @@ namespace SurfaceData
             SurfaceDataSystemComponent::CreateDescriptor(),
             SurfaceDataColliderComponent::CreateDescriptor(),
             SurfaceDataShapeComponent::CreateDescriptor(),
-            TerrainSurfaceDataSystemComponent::CreateDescriptor(),
+            Terrain::TerrainSurfaceDataSystemComponent::CreateDescriptor(),
         });
     }
 
@@ -28,7 +28,7 @@ namespace SurfaceData
     {
         return AZ::ComponentTypeList{
             azrtti_typeid<SurfaceDataSystemComponent>(),
-            azrtti_typeid<TerrainSurfaceDataSystemComponent>(),
+            azrtti_typeid<Terrain::TerrainSurfaceDataSystemComponent>(),
         };
     }
 }

Gems/SurfaceData/Code/Source/TerrainSurfaceDataSystemComponent.cpp

@@ -6,20 +6,16 @@
  *
  */
 
-#include "TerrainSurfaceDataSystemComponent.h"
+#include <TerrainSurfaceDataSystemComponent.h>
 #include <AzCore/Debug/Profiler.h> 
 #include <AzCore/Math/MathUtils.h>
 #include <AzCore/Serialization/EditContext.h>
 #include <AzCore/Serialization/SerializeContext.h>
-#include <AzFramework/Terrain/TerrainDataRequestBus.h>
-#include <MathConversion.h>
 #include <SurfaceData/SurfaceDataSystemRequestBus.h>
 #include <SurfaceData/SurfaceTag.h>
 #include <SurfaceData/Utility/SurfaceDataUtility.h>
 
-#include <ISystem.h>
+namespace Terrain
-
-namespace SurfaceData
 {
     //////////////////////////////////////////////////////////////////////////
     // TerrainSurfaceDataSystemConfig
@@ -98,26 +94,23 @@ namespace SurfaceData
 
     void TerrainSurfaceDataSystemComponent::Activate()
     {
-        m_providerHandle = InvalidSurfaceDataRegistryHandle;
+        m_providerHandle = SurfaceData::InvalidSurfaceDataRegistryHandle;
-        m_system = GetISystem();
+        AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusConnect();
-        CrySystemEventBus::Handler::BusConnect();
-        AZ::HeightmapUpdateNotificationBus::Handler::BusConnect();
 
         UpdateTerrainData(AZ::Aabb::CreateNull());
     }
 
     void TerrainSurfaceDataSystemComponent::Deactivate()
     {
-        if (m_providerHandle != InvalidSurfaceDataRegistryHandle)
+        if (m_providerHandle != SurfaceData::InvalidSurfaceDataRegistryHandle)
         {
-            SurfaceDataSystemRequestBus::Broadcast(&SurfaceDataSystemRequestBus::Events::UnregisterSurfaceDataProvider, m_providerHandle);
+            SurfaceData::SurfaceDataSystemRequestBus::Broadcast(
-            m_providerHandle = InvalidSurfaceDataRegistryHandle;
+                &SurfaceData::SurfaceDataSystemRequestBus::Events::UnregisterSurfaceDataProvider, m_providerHandle);
+            m_providerHandle = SurfaceData::InvalidSurfaceDataRegistryHandle;
         }
 
-        SurfaceDataProviderRequestBus::Handler::BusDisconnect();
+        SurfaceData::SurfaceDataProviderRequestBus::Handler::BusDisconnect();
-        AZ::HeightmapUpdateNotificationBus::Handler::BusDisconnect();
+        AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusDisconnect();
-        CrySystemEventBus::Handler::BusDisconnect();
-        m_system = nullptr;
 
         // Clear the cached terrain bounds data
         {
@@ -146,17 +139,8 @@ namespace SurfaceData
         return false;
     }
 
-    void TerrainSurfaceDataSystemComponent::OnCrySystemInitialized(ISystem& system, [[maybe_unused]] const SSystemInitParams& systemInitParams)
+    void TerrainSurfaceDataSystemComponent::GetSurfacePoints(
-    {
+        const AZ::Vector3& inPosition, SurfaceData::SurfacePointList& surfacePointList) const
-        m_system = &system;
-    }
-
-    void TerrainSurfaceDataSystemComponent::OnCrySystemShutdown([[maybe_unused]] ISystem& system)
-    {
-        m_system = nullptr;
-    }
-
-    void TerrainSurfaceDataSystemComponent::GetSurfacePoints(const AZ::Vector3& inPosition, SurfacePointList& surfacePointList) const
     {
         if (m_terrainBoundsIsValid)
         {
@@ -168,12 +152,13 @@ namespace SurfaceData
                     const float terrainHeight = terrain->GetHeight(inPosition, AzFramework::Terrain::TerrainDataRequests::Sampler::BILINEAR, &isTerrainValidAtPoint);
                     const bool isHole = !isTerrainValidAtPoint;
 
-                    SurfacePoint point;
+                    SurfaceData::SurfacePoint point;
                     point.m_entityId = GetEntityId();
                     point.m_position = AZ::Vector3(inPosition.GetX(), inPosition.GetY(), terrainHeight);
                     point.m_normal = terrain->GetNormal(inPosition);
-                    const AZ::Crc32 terrainTag = isHole ? Constants::s_terrainHoleTagCrc : Constants::s_terrainTagCrc;
+                    const AZ::Crc32 terrainTag =
-                    AddMaxValueForMasks(point.m_masks, terrainTag, 1.0f);
+                        isHole ? SurfaceData::Constants::s_terrainHoleTagCrc : SurfaceData::Constants::s_terrainTagCrc;
+                    SurfaceData::AddMaxValueForMasks(point.m_masks, terrainTag, 1.0f);
                     surfacePointList.push_back(point);
                 }
                 // Only one handler should exist.
@@ -189,11 +174,11 @@ namespace SurfaceData
         return terrain ? terrain->GetTerrainAabb() : AZ::Aabb::CreateNull();
     }
 
-    SurfaceTagVector TerrainSurfaceDataSystemComponent::GetSurfaceTags() const
+    SurfaceData::SurfaceTagVector TerrainSurfaceDataSystemComponent::GetSurfaceTags() const
     {
-        SurfaceTagVector tags;
+        SurfaceData::SurfaceTagVector tags;
-        tags.push_back(Constants::s_terrainHoleTagCrc);
+        tags.push_back(SurfaceData::Constants::s_terrainHoleTagCrc);
-        tags.push_back(Constants::s_terrainTagCrc);
+        tags.push_back(SurfaceData::Constants::s_terrainTagCrc);
         return tags;
     }
 
@@ -203,7 +188,7 @@ namespace SurfaceData
         bool terrainValidAfterUpdate = false;
         AZ::Aabb terrainBoundsBeforeUpdate = m_terrainBounds;
 
-        SurfaceDataRegistryEntry registryEntry;
+        SurfaceData::SurfaceDataRegistryEntry registryEntry;
         registryEntry.m_entityId = GetEntityId();
         registryEntry.m_bounds = GetSurfaceAabb();
         registryEntry.m_tags = GetSurfaceTags();
@@ -215,38 +200,44 @@ namespace SurfaceData
 
         if (terrainValidBeforeUpdate && terrainValidAfterUpdate)
         {
-            AZ_Assert((m_providerHandle != InvalidSurfaceDataRegistryHandle), "Invalid surface data handle");
+            AZ_Assert((m_providerHandle != SurfaceData::InvalidSurfaceDataRegistryHandle), "Invalid surface data handle");
 
             // Our terrain was valid before and after, it just changed in some way.  If we have a valid dirty region passed in
             // then it's possible that the heightmap has been modified in the Editor.  Otherwise, just notify that the entire
             // terrain has changed in some way.
             if (dirtyRegion.IsValid())
             {
-                SurfaceDataSystemRequestBus::Broadcast(&SurfaceDataSystemRequestBus::Events::RefreshSurfaceData, dirtyRegion);
+                SurfaceData::SurfaceDataSystemRequestBus::Broadcast(
+                    &SurfaceData::SurfaceDataSystemRequestBus::Events::RefreshSurfaceData, dirtyRegion);
             }
             else
             {
-                SurfaceDataSystemRequestBus::Broadcast(&SurfaceDataSystemRequestBus::Events::UpdateSurfaceDataProvider, m_providerHandle, registryEntry);
+                SurfaceData::SurfaceDataSystemRequestBus::Broadcast(
+                    &SurfaceData::SurfaceDataSystemRequestBus::Events::UpdateSurfaceDataProvider, m_providerHandle, registryEntry);
             }
         }
         else if (!terrainValidBeforeUpdate && terrainValidAfterUpdate)
         {
             // Our terrain has become valid, so register as a provider and save off the registry handles
-            AZ_Assert((m_providerHandle == InvalidSurfaceDataRegistryHandle), "Surface Provider data handle is initialized before our terrain became valid");
+            AZ_Assert(
-            SurfaceDataSystemRequestBus::BroadcastResult(m_providerHandle, &SurfaceDataSystemRequestBus::Events::RegisterSurfaceDataProvider, registryEntry);
+                (m_providerHandle == SurfaceData::InvalidSurfaceDataRegistryHandle),
+                "Surface Provider data handle is initialized before our terrain became valid");
+            SurfaceData::SurfaceDataSystemRequestBus::BroadcastResult(
+                m_providerHandle, &SurfaceData::SurfaceDataSystemRequestBus::Events::RegisterSurfaceDataProvider, registryEntry);
 
             // Start listening for surface data events
-            AZ_Assert((m_providerHandle != InvalidSurfaceDataRegistryHandle), "Invalid surface data handle");
+            AZ_Assert((m_providerHandle != SurfaceData::InvalidSurfaceDataRegistryHandle), "Invalid surface data handle");
-            SurfaceDataProviderRequestBus::Handler::BusConnect(m_providerHandle);
+            SurfaceData::SurfaceDataProviderRequestBus::Handler::BusConnect(m_providerHandle);
         }
         else if (terrainValidBeforeUpdate && !terrainValidAfterUpdate)
         {
             // Our terrain has stopped being valid, so unregister and stop listening for surface data events
-            AZ_Assert((m_providerHandle != InvalidSurfaceDataRegistryHandle), "Invalid surface data handle");
+            AZ_Assert((m_providerHandle != SurfaceData::InvalidSurfaceDataRegistryHandle), "Invalid surface data handle");
-            SurfaceDataSystemRequestBus::Broadcast(&SurfaceDataSystemRequestBus::Events::UnregisterSurfaceDataProvider, m_providerHandle);
+            SurfaceData::SurfaceDataSystemRequestBus::Broadcast(
-            m_providerHandle = InvalidSurfaceDataRegistryHandle;
+                &SurfaceData::SurfaceDataSystemRequestBus::Events::UnregisterSurfaceDataProvider, m_providerHandle);
+            m_providerHandle = SurfaceData::InvalidSurfaceDataRegistryHandle;
 
-            SurfaceDataProviderRequestBus::Handler::BusDisconnect();
+            SurfaceData::SurfaceDataProviderRequestBus::Handler::BusDisconnect();
         }
         else
         {
@@ -255,8 +246,9 @@ namespace SurfaceData
 
     }
 
-    void TerrainSurfaceDataSystemComponent::HeightmapModified(const AZ::Aabb& bounds)
+    void TerrainSurfaceDataSystemComponent::OnTerrainDataChanged(
+        const AZ::Aabb& dirtyRegion, [[maybe_unused]] TerrainDataChangedMask dataChangedMask)
     {
-        UpdateTerrainData(bounds);
+        UpdateTerrainData(dirtyRegion);
     }
 }

Gems/SurfaceData/Code/Source/TerrainSurfaceDataSystemComponent.h

@@ -10,12 +10,11 @@
 #include <AzCore/Component/Component.h>
 #include <AzCore/Component/Entity.h>
 #include <AzCore/EBus/EBus.h>
-#include <CrySystemBus.h>
+#include <AzFramework/Terrain/TerrainDataRequestBus.h>
-#include <HeightmapUpdateNotificationBus.h>
 #include <SurfaceData/SurfaceDataModifierRequestBus.h>
 #include <SurfaceData/SurfaceDataProviderRequestBus.h>
 
-namespace SurfaceData
+namespace Terrain
 {
     class TerrainSurfaceDataSystemConfig
         : public AZ::ComponentConfig
@@ -31,9 +30,8 @@ namespace SurfaceData
     */
     class TerrainSurfaceDataSystemComponent
         : public AZ::Component
-        , private SurfaceDataProviderRequestBus::Handler
+        , private SurfaceData::SurfaceDataProviderRequestBus::Handler
-        , private AZ::HeightmapUpdateNotificationBus::Handler
+        , private AzFramework::Terrain::TerrainDataNotificationBus::Handler
-        , private CrySystemEventBus::Handler
     {
         friend class EditorTerrainSurfaceDataSystemComponent;
         TerrainSurfaceDataSystemComponent(const TerrainSurfaceDataSystemConfig&);
@@ -58,25 +56,19 @@ namespace SurfaceData
 
         //////////////////////////////////////////////////////////////////////////
         // SurfaceDataProviderRequestBus
-        void GetSurfacePoints(const AZ::Vector3& inPosition, SurfacePointList& surfacePointList) const;
+        void GetSurfacePoints(const AZ::Vector3& inPosition, SurfaceData::SurfacePointList& surfacePointList) const;
-
-        ////////////////////////////////////////////////////////////////////////////
-        // CrySystemEvents
-        void OnCrySystemInitialized(ISystem& system, const SSystemInitParams& systemInitParams) override;
-        void OnCrySystemShutdown(ISystem& system) override;
 
         //////////////////////////////////////////////////////////////////////////
-        // AZ::HeightmapUpdateNotificationBus
+        // AzFramework::Terrain::TerrainDataNotificationBus
-        void HeightmapModified(const AZ::Aabb& bounds) override;
+        void OnTerrainDataChanged(const AZ::Aabb& dirtyRegion, TerrainDataChangedMask dataChangedMask) override;
 
     private:
         void UpdateTerrainData(const AZ::Aabb& dirtyRegion);
         AZ::Aabb GetSurfaceAabb() const;
-        SurfaceTagVector GetSurfaceTags() const;
+        SurfaceData::SurfaceTagVector GetSurfaceTags() const;
-        SurfaceDataRegistryHandle m_providerHandle = InvalidSurfaceDataRegistryHandle;
+        SurfaceData::SurfaceDataRegistryHandle m_providerHandle = SurfaceData::InvalidSurfaceDataRegistryHandle;
 
         TerrainSurfaceDataSystemConfig m_configuration;
-        ISystem* m_system = nullptr;
 
         AZ::Aabb m_terrainBounds = AZ::Aabb::CreateNull();
         AZStd::atomic_bool m_terrainBoundsIsValid{ false };