Terrain System cleanups and unit tests (#4119)

* Remove the "TEST_SUPPORTED" traits.
Terrain unit tests should be usable on all platforms, so they shouldn't need a platform-specific trait to enable/disable.

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

* Fix a few misc terrain bugs.
* Change Activate/Deactivate to happen immediately instead of deferring.  There were too many order-of-operation bugs caused by trying to defer this.
* Added implementation for calculating normals.
* Fixed bug where GetHeightSynchronous wasn't stopping at the highest-priority layer.
* Added locks for SurfaceData bus to help ensure we lock our mutexes in the correct order and avoid deadlocks.

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

* Add trivial TerrainSystem tests.
Tests construction, Activate(), Deactivate(), and destruction.

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

* Unified Terrain system calls on single bus.

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

* Added mock for TerrainDataNotificationBus listener.
Also added unit tests to verify the listener, and added in missing notification events.

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

* Removed extra Sampler class.
Fixed up APIs to correctly pass Sampler and terrainExistsPtr around.

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

* Switched MockTerrainSystem to be proper gmock.
This makes it for flexible to use and easier to reuse from other test environments.

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

* Fix settings bug caused by bad order of operations that occurred when the methods moved to a different bus.

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

* Eliminate extra EBus by simplifying area initialization.
Previously, there was a back-and-forth ebus signal used for the terrain system to find any terrain spawners that were created prior to the terrain system activation.  Now it uses the more simple technique of just grabbing all the spawners that are currently hooked up to the spawner ebus.

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

* Switch to NiceMock so that "uninteresting" mock calls get ignored.

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

* Addressed PR feedback.
Filled in terrainExistsPtr at the end, and added it to GetNormal as well.

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

* Fixed shader height calculation.
It was off by half a pixel, and it was interpolating, both of which were wrong.

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

Code/Editor/GameExporter.cpp

@@ -318,7 +318,7 @@ void CGameExporter::ExportLevelInfo(const QString& path)
     root->setAttr("Name", levelName.toUtf8().data());
     auto terrain = AzFramework::Terrain::TerrainDataRequestBus::FindFirstHandler();
     const AZ::Aabb terrainAabb = terrain ? terrain->GetTerrainAabb() : AZ::Aabb::CreateFromPoint(AZ::Vector3::CreateZero());
-    const AZ::Vector2 terrainGridResolution = terrain ? terrain->GetTerrainGridResolution() : AZ::Vector2::CreateOne();
+    const AZ::Vector2 terrainGridResolution = terrain ? terrain->GetTerrainHeightQueryResolution() : AZ::Vector2::CreateOne();
     const int compiledHeightmapSize = static_cast<int>(terrainAabb.GetXExtent() / terrainGridResolution.GetX());
     root->setAttr("HeightmapSize", compiledHeightmapSize);
 

Code/Framework/AzFramework/AzFramework/Terrain/TerrainDataRequestBus.cpp

@@ -51,7 +51,8 @@ namespace AzFramework
                     ->Event("GetNormal", &AzFramework::Terrain::TerrainDataRequestBus::Events::GetNormal)
                     ->Event("GetNormalFromFloats", &AzFramework::Terrain::TerrainDataRequestBus::Events::GetNormalFromFloats)
                     ->Event("GetTerrainAabb", &AzFramework::Terrain::TerrainDataRequestBus::Events::GetTerrainAabb)
-                    ->Event("GetTerrainGridResolution", &AzFramework::Terrain::TerrainDataRequestBus::Events::GetTerrainGridResolution)
+                    ->Event("GetTerrainHeightQueryResolution",
+                        &AzFramework::Terrain::TerrainDataRequestBus::Events::GetTerrainHeightQueryResolution)
                     ;
 
             }

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

@@ -59,8 +59,11 @@ namespace AzFramework
             static AZ::Vector3 GetDefaultTerrainNormal() { return AZ::Vector3::CreateAxisZ(); }
 
             // System-level queries to understand world size and resolution
-            virtual AZ::Vector2 GetTerrainGridResolution() const = 0;
+            virtual AZ::Vector2 GetTerrainHeightQueryResolution() const = 0;
+            virtual void SetTerrainHeightQueryResolution(AZ::Vector2 queryResolution) = 0;
+
             virtual AZ::Aabb GetTerrainAabb() const = 0;
+            virtual void SetTerrainAabb(const AZ::Aabb& worldBounds) = 0;
 
             //! Returns terrains height in meters at location x,y.
             //! @terrainExistsPtr: Can be nullptr. If != nullptr then, if there's no terrain at location x,y or location x,y is inside a terrain HOLE then *terrainExistsPtr will become false,

Gems/PhysX/Code/Tests/PhysXTestUtil.h

@@ -93,9 +93,26 @@ namespace PhysX
 
         ////////////////////////////////////////////////////////////////////////
         // TerrainDataRequestBus interface dummy implementation
-        AZ::Vector2 GetTerrainGridResolution() const override { return {}; }
+        AZ::Vector2 GetTerrainHeightQueryResolution() const override
-        AZ::Aabb GetTerrainAabb() const override { return {}; }
+        {
-        float GetHeight(AZ::Vector3, Sampler, bool*) const override { return {}; }
+            return {};
+        }
+        void SetTerrainHeightQueryResolution([[maybe_unused]] AZ::Vector2 queryResolution) override
+        {
+        }
+
+        AZ::Aabb GetTerrainAabb() const override
+        {
+            return {};
+        }
+        void SetTerrainAabb([[maybe_unused]] const AZ::Aabb& worldBounds) override
+        {
+        }
+
+        float GetHeight(AZ::Vector3, Sampler, bool*) const override
+        {
+            return {};
+        }
         float GetHeightFromFloats(float, float, Sampler, bool*) const override { return {}; }
         AzFramework::SurfaceData::SurfaceTagWeight GetMaxSurfaceWeight(AZ::Vector3, Sampler, bool*) const override { return {}; }
         AzFramework::SurfaceData::SurfaceTagWeight GetMaxSurfaceWeightFromFloats(float, float, Sampler, bool*) const override { return {}; }

Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli

@@ -11,16 +11,16 @@ ShaderResourceGroup ObjectSrg : SRG_PerObject
 {
     Texture2D<float> m_heightmapImage;
     
-    Sampler LinearSampler
+    Sampler PointSampler
     {
-        MinFilter = Linear;
+        MinFilter = Point;
-        MagFilter = Linear;
+        MagFilter = Point;
-        MipFilter = Linear;
+        MipFilter = Point;
         AddressU = Clamp;
         AddressV = Clamp;
         AddressW = Clamp;
     };
-    
+
     row_major float3x4 m_modelToWorld;
 
     struct TerrainData
@@ -57,8 +57,8 @@ float4x4 GetObject_WorldMatrix()
 
 float GetHeight(float2 origUv)
 {
-    float2 uv = clamp(origUv, 0.0f, 1.0f);
+    float2 uv = clamp(origUv + (ObjectSrg::m_terrainData.m_uvStep * 0.5f), 0.0f, 1.0f);
-    return ObjectSrg::m_terrainData.m_heightScale * (ObjectSrg::m_heightmapImage.SampleLevel(ObjectSrg::LinearSampler, uv, 0).r - 0.5f);
+    return ObjectSrg::m_terrainData.m_heightScale * (ObjectSrg::m_heightmapImage.SampleLevel(ObjectSrg::PointSampler, uv, 0).r - 0.5f);
 }
 
 float4 GetTerrainProjectedPosition(ObjectSrg::TerrainData terrainData, float2 vertexPosition, float2 uv)

Gems/Terrain/Code/CMakeLists.txt

@@ -83,15 +83,36 @@ endif()
 ################################################################################
 # See if globally, tests are supported
 if(PAL_TRAIT_BUILD_TESTS_SUPPORTED)
-    # We globally support tests, see if we support tests on this platform for Terrain.Static
+    ly_add_target(
-    if(PAL_TRAIT_TERRAIN_TEST_SUPPORTED)
+        NAME Terrain.Tests ${PAL_TRAIT_TEST_TARGET_TYPE}
-        # We support Terrain.Tests on this platform, add Terrain.Tests target which depends on Terrain.Static
+        NAMESPACE Gem
+        FILES_CMAKE
+            terrain_files.cmake
+            terrain_tests_files.cmake
+        INCLUDE_DIRECTORIES
+            PRIVATE
+                Tests
+                Source
+        BUILD_DEPENDENCIES
+            PRIVATE
+                AZ::AzTest
+                AZ::AzFramework
+                Gem::Terrain.Static
+    )
+
+    # Add Terrain.Tests to googletest
+    ly_add_googletest(
+        NAME Gem::Terrain.Tests
+    )
+
+    # If we are a host platform we want to add tools test like editor tests here
+    if(PAL_TRAIT_BUILD_HOST_TOOLS)
+        # We support Terrain.Editor.Tests on this platform, add Terrain.Editor.Tests target which depends on Terrain.Editor
         ly_add_target(
-            NAME Terrain.Tests ${PAL_TRAIT_TEST_TARGET_TYPE}
+            NAME Terrain.Editor.Tests ${PAL_TRAIT_TEST_TARGET_TYPE}
             NAMESPACE Gem
             FILES_CMAKE
-                terrain_files.cmake
+                terrain_editor_tests_files.cmake
-                terrain_tests_files.cmake
             INCLUDE_DIRECTORIES
                 PRIVATE
                     Tests
@@ -99,40 +120,12 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED)
             BUILD_DEPENDENCIES
                 PRIVATE
                     AZ::AzTest
-                    AZ::AzFramework
+                    Gem::Terrain.Editor
-                    Gem::Terrain.Static
         )
 
-        # Add Terrain.Tests to googletest
+        # Add Terrain.Editor.Tests to googletest
         ly_add_googletest(
-            NAME Gem::Terrain.Tests
+            NAME Gem::Terrain.Editor.Tests
         )
     endif()
-
-    # If we are a host platform we want to add tools test like editor tests here
-    if(PAL_TRAIT_BUILD_HOST_TOOLS)
-        # We are a host platform, see if Editor tests are supported on this platform
-        if(PAL_TRAIT_TERRAIN_EDITOR_TEST_SUPPORTED)
-            # We support Terrain.Editor.Tests on this platform, add Terrain.Editor.Tests target which depends on Terrain.Editor
-            ly_add_target(
-                NAME Terrain.Editor.Tests ${PAL_TRAIT_TEST_TARGET_TYPE}
-                NAMESPACE Gem
-                FILES_CMAKE
-                    terrain_editor_tests_files.cmake
-                INCLUDE_DIRECTORIES
-                    PRIVATE
-                        Tests
-                        Source
-                BUILD_DEPENDENCIES
-                    PRIVATE
-                        AZ::AzTest
-                        Gem::Terrain.Editor
-            )
-
-            # Add Terrain.Editor.Tests to googletest
-            ly_add_googletest(
-                NAME Gem::Terrain.Editor.Tests
-            )
-        endif()
-    endif()
 endif()

Gems/Terrain/Code/Source/Components/TerrainHeightGradientListComponent.cpp

@@ -166,14 +166,20 @@ namespace Terrain
     }
 
     void TerrainHeightGradientListComponent::GetHeight(
-        const AZ::Vector3& inPosition, AZ::Vector3& outPosition, [[maybe_unused]] Sampler sampleFilter = Sampler::DEFAULT)
+        const AZ::Vector3& inPosition,
+        AZ::Vector3& outPosition,
+        [[maybe_unused]] AzFramework::Terrain::TerrainDataRequests::Sampler sampleFilter =
+            AzFramework::Terrain::TerrainDataRequests::Sampler::DEFAULT)
     {
         const float height = GetHeight(inPosition.GetX(), inPosition.GetY());
         outPosition.SetZ(height);
     }
 
     void TerrainHeightGradientListComponent::GetNormal(
-        const AZ::Vector3& inPosition, AZ::Vector3& outNormal, [[maybe_unused]] Sampler sampleFilter = Sampler::DEFAULT)
+        const AZ::Vector3& inPosition,
+        AZ::Vector3& outNormal,
+        [[maybe_unused]] AzFramework::Terrain::TerrainDataRequests::Sampler sampleFilter =
+            AzFramework::Terrain::TerrainDataRequests::Sampler::DEFAULT)
     {
         const float x = inPosition.GetX();
         const float y = inPosition.GetY();
@@ -206,7 +212,7 @@ namespace Terrain
         // Get the height range of the entire world
         m_cachedHeightQueryResolution = AZ::Vector2(1.0f);
         AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult(
-            m_cachedHeightQueryResolution, &AzFramework::Terrain::TerrainDataRequestBus::Events::GetTerrainGridResolution);
+            m_cachedHeightQueryResolution, &AzFramework::Terrain::TerrainDataRequestBus::Events::GetTerrainHeightQueryResolution);
 
         AZ::Aabb worldBounds = AZ::Aabb::CreateNull();
         AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult(

Gems/Terrain/Code/Source/Components/TerrainHeightGradientListComponent.h

@@ -64,8 +64,14 @@ namespace Terrain
         TerrainHeightGradientListComponent() = default;
         ~TerrainHeightGradientListComponent() = default;
 
-        void GetHeight(const AZ::Vector3& inPosition, AZ::Vector3& outPosition, Sampler sampleFilter) override;
+        void GetHeight(
-        void GetNormal(const AZ::Vector3& inPosition, AZ::Vector3& outNormal, Sampler sampleFilter) override;
+            const AZ::Vector3& inPosition,
+            AZ::Vector3& outPosition,
+            AzFramework::Terrain::TerrainDataRequests::Sampler sampleFilter) override;
+        void GetNormal(
+            const AZ::Vector3& inPosition,
+            AZ::Vector3& outNormal,
+            AzFramework::Terrain::TerrainDataRequests::Sampler sampleFilter) override;
 
         //////////////////////////////////////////////////////////////////////////
         // AZ::Component interface implementation

Gems/Terrain/Code/Source/Components/TerrainLayerSpawnerComponent.cpp

@@ -104,7 +104,6 @@ namespace Terrain
     {
         AZ::TransformNotificationBus::Handler::BusConnect(GetEntityId());
         LmbrCentral::ShapeComponentNotificationsBus::Handler::BusConnect(GetEntityId());
-        TerrainAreaRequestBus::Handler::BusConnect(GetEntityId());
         TerrainSpawnerRequestBus::Handler::BusConnect(GetEntityId());
 
         TerrainSystemServiceRequestBus::Broadcast(&TerrainSystemServiceRequestBus::Events::RegisterArea, GetEntityId());
@@ -114,7 +113,6 @@ namespace Terrain
     {
         TerrainSystemServiceRequestBus::Broadcast(&TerrainSystemServiceRequestBus::Events::UnregisterArea, GetEntityId());
         TerrainSpawnerRequestBus::Handler::BusDisconnect();
-        TerrainAreaRequestBus::Handler::BusDisconnect();
         LmbrCentral::ShapeComponentNotificationsBus::Handler::BusDisconnect();
         AZ::TransformNotificationBus::Handler::BusDisconnect();
         
@@ -161,11 +159,6 @@ namespace Terrain
         return m_configuration.m_useGroundPlane;
     }
 
-    void TerrainLayerSpawnerComponent::RegisterArea()
-    {
-        TerrainSystemServiceRequestBus::Broadcast(&TerrainSystemServiceRequestBus::Events::RegisterArea, GetEntityId());
-    }
-
     void TerrainLayerSpawnerComponent::RefreshArea()
     {
         TerrainSystemServiceRequestBus::Broadcast(&TerrainSystemServiceRequestBus::Events::RefreshArea, GetEntityId());

Gems/Terrain/Code/Source/Components/TerrainLayerSpawnerComponent.h

@@ -58,7 +58,6 @@ namespace Terrain
         : public AZ::Component
         , private AZ::TransformNotificationBus::Handler
         , private LmbrCentral::ShapeComponentNotificationsBus::Handler
-        , private Terrain::TerrainAreaRequestBus::Handler
         , private Terrain::TerrainSpawnerRequestBus::Handler
     {
     public:
@@ -81,6 +80,7 @@ namespace Terrain
         bool ReadInConfig(const AZ::ComponentConfig* baseConfig) override;
         bool WriteOutConfig(AZ::ComponentConfig* outBaseConfig) const override;
 
+    protected:
         //////////////////////////////////////////////////////////////////////////
         // AZ::TransformNotificationBus::Handler
         void OnTransformChanged(const AZ::Transform& local, const AZ::Transform& world) override;
@@ -92,8 +92,7 @@ namespace Terrain
         void GetPriority(AZ::u32& outLayer, AZ::u32& outPriority) override;
         bool GetUseGroundPlane() override;
         
-        void RegisterArea() override;
+        void RefreshArea();
-        void RefreshArea() override;
 
     private:
         TerrainLayerSpawnerConfig m_configuration;

Gems/Terrain/Code/Source/Components/TerrainWorldComponent.cpp

@@ -13,6 +13,7 @@
 #include <AzCore/RTTI/BehaviorContext.h>
 #include <AzCore/Serialization/EditContext.h>
 #include <AzCore/Serialization/SerializeContext.h>
+#include <AzFramework/Terrain/TerrainDataRequestBus.h>
 
 namespace Terrain
 {
@@ -85,17 +86,16 @@ namespace Terrain
 
     void TerrainWorldComponent::Activate()
     {
-        TerrainSystemServiceRequestBus::Broadcast(
-            &TerrainSystemServiceRequestBus::Events::SetWorldBounds,
-            AZ::Aabb::CreateFromMinMax(m_configuration.m_worldMin, m_configuration.m_worldMax)
-        );
-        TerrainSystemServiceRequestBus::Broadcast(
-            &TerrainSystemServiceRequestBus::Events::SetHeightQueryResolution, m_configuration.m_heightQueryResolution);
-
         // Currently, the Terrain System Component owns the Terrain System instance because the Terrain World component gets recreated
         // every time an entity is added or removed to a level.  If this ever changes, the Terrain System ownership could move into
         // the level component.
         TerrainSystemServiceRequestBus::Broadcast(&TerrainSystemServiceRequestBus::Events::Activate);
+
+        AzFramework::Terrain::TerrainDataRequestBus::Broadcast(
+            &AzFramework::Terrain::TerrainDataRequestBus::Events::SetTerrainAabb,
+            AZ::Aabb::CreateFromMinMax(m_configuration.m_worldMin, m_configuration.m_worldMax));
+        AzFramework::Terrain::TerrainDataRequestBus::Broadcast(
+            &AzFramework::Terrain::TerrainDataRequestBus::Events::SetTerrainHeightQueryResolution, m_configuration.m_heightQueryResolution);
     }
 
     void TerrainWorldComponent::Deactivate()

Gems/Terrain/Code/Source/Components/TerrainWorldDebuggerComponent.cpp

@@ -171,7 +171,7 @@ namespace Terrain
                 // Determine how far to draw in each direction in world space based on our MaxSectorsToDraw
                 AZ::Vector2 queryResolution = AZ::Vector2(1.0f);
                 AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult(
-                    queryResolution, &AzFramework::Terrain::TerrainDataRequests::GetTerrainGridResolution);
+                    queryResolution, &AzFramework::Terrain::TerrainDataRequests::GetTerrainHeightQueryResolution);
                 AZ::Vector3 viewDistance(
                     queryResolution.GetX() * SectorSizeInGridPoints * sqrtf(MaxSectorsToDraw),
                     queryResolution.GetY() * SectorSizeInGridPoints * sqrtf(MaxSectorsToDraw),
@@ -214,7 +214,7 @@ namespace Terrain
 
         AZ::Vector2 queryResolution = AZ::Vector2(1.0f);
         AzFramework::Terrain::TerrainDataRequestBus::BroadcastResult(
-            queryResolution, &AzFramework::Terrain::TerrainDataRequests::GetTerrainGridResolution);
+            queryResolution, &AzFramework::Terrain::TerrainDataRequests::GetTerrainHeightQueryResolution);
 
         // Calculate the world size of each sector.  Note that this size actually ends at the last point, not the last square.
         // So for example, the sector size for 3 points will go from (*--*--*) even though it will be used to draw (*--*--*--).

Gems/Terrain/Code/Source/TerrainSystem/TerrainSystem.cpp

@@ -9,6 +9,7 @@
 #include <TerrainSystem/TerrainSystem.h>
 #include <AzCore/std/parallel/shared_mutex.h>
 #include <SurfaceData/SurfaceDataTypes.h>
+#include <SurfaceData/SurfaceDataSystemRequestBus.h>
 #include <LmbrCentral/Shape/ShapeComponentBus.h>
 
 #include <Atom/RPI.Public/Scene.h>
@@ -53,7 +54,7 @@ TerrainSystem::TerrainSystem()
     m_currentSettings.m_worldBounds = AZ::Aabb::CreateNull();
 
     m_requestedSettings = m_currentSettings;
-    m_requestedSettings.m_worldBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3(4096.0f, 4096.0f, 2048.0f));
+    m_requestedSettings.m_worldBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(-512.0f), AZ::Vector3(512.0f));
 }
 
 TerrainSystem::~TerrainSystem()
@@ -66,23 +67,76 @@ TerrainSystem::~TerrainSystem()
 
 void TerrainSystem::Activate()
 {
-    m_requestedSettings.m_systemActive = true;
+    AzFramework::Terrain::TerrainDataNotificationBus::Broadcast(
+        &AzFramework::Terrain::TerrainDataNotificationBus::Events::OnTerrainDataCreateBegin);
+
+    m_dirtyRegion = AZ::Aabb::CreateNull();
+    m_terrainHeightDirty = true;
     m_terrainSettingsDirty = true;
+    m_requestedSettings.m_systemActive = true;
+
+    {
+        AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
+        m_registeredAreas.clear();
+    }
+
+    AzFramework::Terrain::TerrainDataRequestBus::Handler::BusConnect();
+
+    // Register any terrain spawners that were already active before the terrain system activated.
+    auto enumerationCallback = [&]([[maybe_unused]] Terrain::TerrainSpawnerRequests* terrainSpawner) -> bool
+    {
+        AZ::EntityId areaId = *(Terrain::TerrainSpawnerRequestBus::GetCurrentBusId());
+        RegisterArea(areaId);
+
+        // Keep Enumerating
+        return true;
+    };
+    Terrain::TerrainSpawnerRequestBus::EnumerateHandlers(enumerationCallback);
+
+    AzFramework::Terrain::TerrainDataNotificationBus::Broadcast(
+        &AzFramework::Terrain::TerrainDataNotificationBus::Events::OnTerrainDataCreateEnd);
 }
 
 void TerrainSystem::Deactivate()
 {
-    m_requestedSettings.m_systemActive = false;
+    AzFramework::Terrain::TerrainDataNotificationBus::Broadcast(
+        &AzFramework::Terrain::TerrainDataNotificationBus::Events::OnTerrainDataDestroyBegin);
+
+    AzFramework::Terrain::TerrainDataRequestBus::Handler::BusDisconnect();
+
+    {
+        AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
+        m_registeredAreas.clear();
+    }
+
+    m_dirtyRegion = AZ::Aabb::CreateNull();
+    m_terrainHeightDirty = true;
     m_terrainSettingsDirty = true;
+    m_requestedSettings.m_systemActive = false;
+
+    if (auto rpi = AZ::RPI::RPISystemInterface::Get(); rpi)
+    {
+        if (auto defaultScene = rpi->GetDefaultScene(); defaultScene)
+        {
+            const AZ::RPI::Scene* scene = defaultScene.get();
+            if (auto terrainFeatureProcessor = scene->GetFeatureProcessor<TerrainFeatureProcessor>(); terrainFeatureProcessor)
+            {
+                terrainFeatureProcessor->RemoveTerrainData();
+            }
+        }
+    }
+
+    AzFramework::Terrain::TerrainDataNotificationBus::Broadcast(
+        &AzFramework::Terrain::TerrainDataNotificationBus::Events::OnTerrainDataDestroyEnd);
 }
 
-void TerrainSystem::SetWorldBounds(const AZ::Aabb& worldBounds)
+void TerrainSystem::SetTerrainAabb(const AZ::Aabb& worldBounds)
 {   
     m_requestedSettings.m_worldBounds = worldBounds;
     m_terrainSettingsDirty = true;
 }
 
-void TerrainSystem::SetHeightQueryResolution(AZ::Vector2 queryResolution)
+void TerrainSystem::SetTerrainHeightQueryResolution(AZ::Vector2 queryResolution)
 {
     m_requestedSettings.m_heightQueryResolution = queryResolution;
     m_terrainSettingsDirty = true;
@@ -93,13 +147,15 @@ AZ::Aabb TerrainSystem::GetTerrainAabb() const
     return m_currentSettings.m_worldBounds;
 }
 
-AZ::Vector2 TerrainSystem::GetTerrainGridResolution() const
+AZ::Vector2 TerrainSystem::GetTerrainHeightQueryResolution() const
 {
     return m_currentSettings.m_heightQueryResolution;
 }
 
-float TerrainSystem::GetHeightSynchronous(float x, float y) const
+float TerrainSystem::GetHeightSynchronous(float x, float y, Sampler sampler, bool* terrainExistsPtr) const
 {
+    bool terrainExists = false;
+
     AZ::Vector3 inPosition((float)x, (float)y, m_currentSettings.m_worldBounds.GetMin().GetZ());
     AZ::Vector3 outPosition((float)x, (float)y, m_currentSettings.m_worldBounds.GetMin().GetZ());
 
@@ -111,67 +167,77 @@ float TerrainSystem::GetHeightSynchronous(float x, float y) const
         if (areaBounds.Contains(inPosition))
         {
             Terrain::TerrainAreaHeightRequestBus::Event(
-                areaId, &Terrain::TerrainAreaHeightRequestBus::Events::GetHeight, inPosition, outPosition,
+                areaId, &Terrain::TerrainAreaHeightRequestBus::Events::GetHeight, inPosition, outPosition, sampler);
-                Terrain::TerrainAreaHeightRequestBus::Events::Sampler::DEFAULT);
+
+            terrainExists = true;
+
+            break;
         }
     }
 
-    return AZ::GetClamp(
-        outPosition.GetZ(), m_currentSettings.m_worldBounds.GetMin().GetZ(), m_currentSettings.m_worldBounds.GetMax().GetZ());
-}
-
-float TerrainSystem::GetHeight(AZ::Vector3 position, [[maybe_unused]] Sampler sampler, [[maybe_unused]] bool* terrainExistsPtr) const
-{
     if (terrainExistsPtr)
     {
-        *terrainExistsPtr = true;
+        *terrainExistsPtr = terrainExists;
     }
 
-    return GetHeightSynchronous(position.GetX(), position.GetY());
+    return AZ::GetClamp(
+        outPosition.GetZ(), m_currentSettings.m_worldBounds.GetMin().GetZ(), m_currentSettings.m_worldBounds.GetMax().GetZ());
 }
 
-float TerrainSystem::GetHeightFromFloats(
+float TerrainSystem::GetHeight(AZ::Vector3 position, Sampler sampler, bool* terrainExistsPtr) const
-    float x, float y, [[maybe_unused]] Sampler sampler, [[maybe_unused]] bool* terrainExistsPtr) const
 {
-    if (terrainExistsPtr)
+    return GetHeightSynchronous(position.GetX(), position.GetY(), sampler, terrainExistsPtr);
-    {
-        *terrainExistsPtr = true;
-    }
-
-    return GetHeightSynchronous(x, y);
 }
 
-bool TerrainSystem::GetIsHoleFromFloats(
+float TerrainSystem::GetHeightFromFloats(float x, float y, Sampler sampler, bool* terrainExistsPtr) const
-    [[maybe_unused]] float x, [[maybe_unused]] float y, [[maybe_unused]] Sampler sampleFilter) const
 {
-    return false;
+    return GetHeightSynchronous(x, y, sampler, terrainExistsPtr);
 }
 
-AZ::Vector3 TerrainSystem::GetNormalSynchronous([[maybe_unused]] float x, [[maybe_unused]] float y) const
+bool TerrainSystem::GetIsHoleFromFloats(float x, float y, Sampler sampler) const
 {
-    return AZ::Vector3::CreateAxisZ();
+    bool terrainExists = false;
+    GetHeightSynchronous(x, y, sampler, &terrainExists);
+    return !terrainExists;
 }
 
-AZ::Vector3 TerrainSystem::GetNormal(
+AZ::Vector3 TerrainSystem::GetNormalSynchronous(float x, float y, Sampler sampler, bool* terrainExistsPtr) const
-    AZ::Vector3 position, [[maybe_unused]] Sampler sampleFilter, [[maybe_unused]] bool* terrainExistsPtr) const
 {
+    bool terrainExists = false;
+
+    AZ::Vector3 inPosition((float)x, (float)y, m_currentSettings.m_worldBounds.GetMin().GetZ());
+    AZ::Vector3 outNormal = AZ::Vector3::CreateAxisZ();
+
+    AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
+
+    for (auto& [areaId, areaBounds] : m_registeredAreas)
+    {
+        inPosition.SetZ(areaBounds.GetMin().GetZ());
+        if (areaBounds.Contains(inPosition))
+        {
+            Terrain::TerrainAreaHeightRequestBus::Event(
+                areaId, &Terrain::TerrainAreaHeightRequestBus::Events::GetNormal, inPosition, outNormal, sampler);
+            terrainExists = true;
+            break;
+        }
+    }
+
     if (terrainExistsPtr)
     {
-        *terrainExistsPtr = true;
+        *terrainExistsPtr = terrainExists;
     }
 
-    return GetNormalSynchronous(position.GetX(), position.GetY());
+    return outNormal;
 }
 
-AZ::Vector3 TerrainSystem::GetNormalFromFloats(
+AZ::Vector3 TerrainSystem::GetNormal(AZ::Vector3 position, Sampler sampler, bool* terrainExistsPtr) const
-    float x, float y, [[maybe_unused]] Sampler sampleFilter, [[maybe_unused]] bool* terrainExistsPtr) const
 {
-    if (terrainExistsPtr)
+    return GetNormalSynchronous(position.GetX(), position.GetY(), sampler, terrainExistsPtr);
-    {
+}
-        *terrainExistsPtr = true;
-    }
 
-    return GetNormalSynchronous(x, y);
+AZ::Vector3 TerrainSystem::GetNormalFromFloats(float x, float y, Sampler sampler, bool* terrainExistsPtr) const
+{
+    return GetNormalSynchronous(x, y, sampler, terrainExistsPtr);
 }
 
 
@@ -298,35 +364,6 @@ void TerrainSystem::ProcessSurfacePointsFromRegion(const AZ::Aabb& inRegion, con
 }
 */
 
-void TerrainSystem::SystemActivate()
-{
-    {
-        AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
-        m_registeredAreas.clear();
-    }
-
-    AzFramework::Terrain::TerrainDataRequestBus::Handler::BusConnect();
-
-    TerrainAreaRequestBus::Broadcast(&TerrainAreaRequestBus::Events::RegisterArea);
-}
-
-void TerrainSystem::SystemDeactivate()
-{
-    AzFramework::Terrain::TerrainDataRequestBus::Handler::BusDisconnect();
-
-    {
-        AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
-        m_registeredAreas.clear();
-    }
-
-    const AZ::RPI::Scene* scene = AZ::RPI::RPISystemInterface::Get()->GetDefaultScene().get();
-    auto terrainFeatureProcessor = scene->GetFeatureProcessor<TerrainFeatureProcessor>();
-    if (terrainFeatureProcessor)
-    {
-        terrainFeatureProcessor->RemoveTerrainData();
-    }
-}
-
 void TerrainSystem::RegisterArea(AZ::EntityId areaId)
 {
     AZStd::unique_lock<AZStd::shared_mutex> lock(m_areaMutex);
@@ -383,6 +420,7 @@ void TerrainSystem::OnTick(float /*deltaTime*/, AZ::ScriptTimePoint /*time*/)
 
     if (m_terrainSettingsDirty)
     {
+        terrainSettingsChanged = true;
         m_terrainSettingsDirty = false;
 
         // This needs to happen before the "system active" check below, because activating the system will cause the various
@@ -393,24 +431,12 @@ void TerrainSystem::OnTick(float /*deltaTime*/, AZ::ScriptTimePoint /*time*/)
             m_dirtyRegion.AddAabb(m_requestedSettings.m_worldBounds);
             m_terrainHeightDirty = true;
             m_currentSettings.m_worldBounds = m_requestedSettings.m_worldBounds;
-            terrainSettingsChanged = true;
         }
 
         if (m_requestedSettings.m_heightQueryResolution != m_currentSettings.m_heightQueryResolution)
         {
             m_dirtyRegion = AZ::Aabb::CreateNull();
             m_terrainHeightDirty = true;
-            terrainSettingsChanged = true;
-        }
-
-        if (m_requestedSettings.m_systemActive != m_currentSettings.m_systemActive)
-        {
-            m_requestedSettings.m_systemActive ? SystemActivate() : SystemDeactivate();
-
-            // Null dirty region will be interpreted as updating everything
-            m_dirtyRegion = AZ::Aabb::CreateNull();
-            m_terrainHeightDirty = true;
-            terrainSettingsChanged = true;
         }
 
         m_currentSettings = m_requestedSettings;
@@ -420,6 +446,14 @@ void TerrainSystem::OnTick(float /*deltaTime*/, AZ::ScriptTimePoint /*time*/)
     {
         AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
 
+        // Block other threads from accessing the surface data bus while we are in GetValue (which may call into the SurfaceData bus).
+        // We lock our surface data mutex *before* checking / setting "isRequestInProgress" so that we prevent race conditions
+        // that create false detection of cyclic dependencies when multiple requests occur on different threads simultaneously.
+        // (One case where this was previously able to occur was in rapid updating of the Preview widget on the
+        // GradientSurfaceDataComponent in the Editor when moving the threshold sliders back and forth rapidly)
+        auto& surfaceDataContext = SurfaceData::SurfaceDataSystemRequestBus::GetOrCreateContext(false);
+        typename SurfaceData::SurfaceDataSystemRequestBus::Context::DispatchLockGuard scopeLock(surfaceDataContext.m_contextMutex);
+
         AZ::Transform transform = AZ::Transform::CreateTranslation(m_currentSettings.m_worldBounds.GetCenter());
 
         uint32_t width = aznumeric_cast<uint32_t>(
@@ -449,7 +483,8 @@ void TerrainSystem::OnTick(float /*deltaTime*/, AZ::ScriptTimePoint /*time*/)
                     }
 
                     AZ::Vector3 outPosition;
-                    const Terrain::TerrainAreaHeightRequests::Sampler sampleFilter = Terrain::TerrainAreaHeightRequests::Sampler::DEFAULT;
+                    const AzFramework::Terrain::TerrainDataRequestBus::Events::Sampler sampleFilter =
+                        AzFramework::Terrain::TerrainDataRequestBus::Events::Sampler::DEFAULT;
 
                     Terrain::TerrainAreaHeightRequestBus::Event(
                         areaId, &Terrain::TerrainAreaHeightRequestBus::Events::GetHeight, inPosition, outPosition, sampleFilter);
@@ -476,6 +511,14 @@ void TerrainSystem::OnTick(float /*deltaTime*/, AZ::ScriptTimePoint /*time*/)
 
     if (terrainSettingsChanged || m_terrainHeightDirty)
     {
+        // Block other threads from accessing the surface data bus while we are in GetValue (which may call into the SurfaceData bus).
+        // We lock our surface data mutex *before* checking / setting "isRequestInProgress" so that we prevent race conditions
+        // that create false detection of cyclic dependencies when multiple requests occur on different threads simultaneously.
+        // (One case where this was previously able to occur was in rapid updating of the Preview widget on the
+        // GradientSurfaceDataComponent in the Editor when moving the threshold sliders back and forth rapidly)
+        auto& surfaceDataContext = SurfaceData::SurfaceDataSystemRequestBus::GetOrCreateContext(false);
+        typename SurfaceData::SurfaceDataSystemRequestBus::Context::DispatchLockGuard scopeLock(surfaceDataContext.m_contextMutex);
+
         AzFramework::Terrain::TerrainDataNotifications::TerrainDataChangedMask changeMask =
             AzFramework::Terrain::TerrainDataNotifications::TerrainDataChangedMask::None;
 

Gems/Terrain/Code/Source/TerrainSystem/TerrainSystem.h

@@ -42,10 +42,6 @@ namespace Terrain
 
         ///////////////////////////////////////////
         // TerrainSystemServiceRequestBus::Handler Impl
-
-        void SetWorldBounds(const AZ::Aabb& worldBounds) override;
-        void SetHeightQueryResolution(AZ::Vector2 queryResolution) override;
-
         void Activate() override;
         void Deactivate() override;
 
@@ -55,8 +51,12 @@ namespace Terrain
 
         ///////////////////////////////////////////
         // TerrainDataRequestBus::Handler Impl
-        AZ::Vector2 GetTerrainGridResolution() const override;
+        AZ::Vector2 GetTerrainHeightQueryResolution() const override;
+        void SetTerrainHeightQueryResolution(AZ::Vector2 queryResolution) override;
+
         AZ::Aabb GetTerrainAabb() const override;
+        void SetTerrainAabb(const AZ::Aabb& worldBounds) override;
+
 
         //! Returns terrains height in meters at location x,y.
         //! @terrainExistsPtr: Can be nullptr. If != nullptr then, if there's no terrain at location x,y or location x,y is inside a terrain
@@ -94,15 +94,12 @@ namespace Terrain
             float x, float y, Sampler sampleFilter = Sampler::BILINEAR, bool* terrainExistsPtr = nullptr) const override;
 
     private:
-        float GetHeightSynchronous(float x, float y) const;
+        float GetHeightSynchronous(float x, float y, Sampler sampler, bool* terrainExistsPtr) const;
-        AZ::Vector3 GetNormalSynchronous(float x, float y) const;
+        AZ::Vector3 GetNormalSynchronous(float x, float y, Sampler sampler, bool* terrainExistsPtr) const;
 
         // AZ::TickBus::Handler overrides ...
         void OnTick(float deltaTime, AZ::ScriptTimePoint time) override;
 
-        void SystemActivate();
-        void SystemDeactivate();
-
         struct TerrainSystemSettings
         {
             AZ::Aabb m_worldBounds;

Gems/Terrain/Code/Source/TerrainSystem/TerrainSystemBus.h

@@ -16,6 +16,8 @@
 #include <AzCore/EBus/EBus.h>
 #include <AzCore/Component/ComponentBus.h>
 
+#include <AzFramework/Terrain/TerrainDataRequestBus.h>
+
 namespace Terrain
 {
     /**
@@ -39,9 +41,6 @@ namespace Terrain
         virtual void Activate() = 0;
         virtual void Deactivate() = 0;
 
-        virtual void SetWorldBounds(const AZ::Aabb& worldBounds) = 0;
-        virtual void SetHeightQueryResolution(AZ::Vector2 queryResolution) = 0;
-
         // register an area to override terrain
         virtual void RegisterArea(AZ::EntityId areaId) = 0;
         virtual void UnregisterArea(AZ::EntityId areaId) = 0;
@@ -50,27 +49,6 @@ namespace Terrain
 
     using TerrainSystemServiceRequestBus = AZ::EBus<TerrainSystemServiceRequests>;
 
-    /**
-    * A bus to signal the life times of terrain areas
-    * Note: all the API are meant to be queued events
-    */
-    class TerrainAreaRequests
-        : public AZ::ComponentBus
-    {
-    public:
-        ////////////////////////////////////////////////////////////////////////
-        // EBusTraits
-        using MutexType = AZStd::recursive_mutex;
-        ////////////////////////////////////////////////////////////////////////
-
-        virtual ~TerrainAreaRequests() = default;
-
-        virtual void RegisterArea() = 0;
-        virtual void RefreshArea() = 0;
-
-    };
-
-    using TerrainAreaRequestBus = AZ::EBus<TerrainAreaRequests>;
 
     /**
     * A bus to signal the life times of terrain areas
@@ -87,15 +65,6 @@ namespace Terrain
 
         virtual ~TerrainAreaHeightRequests() = default;
 
-        enum class Sampler
-        {
-            BILINEAR,   // Get the value at the requested location, using terrain sample grid to bilinear filter between sample grid points
-            CLAMP,      // Clamp the input point to the terrain sample grid, then get the exact value
-            EXACT,       // Directly get the value at the location, regardless of terrain sample grid density
-
-            DEFAULT = BILINEAR
-        };
-
         enum SurfacePointDataMask
         {
             POSITION = 0x01,
@@ -107,8 +76,16 @@ namespace Terrain
 
         // Synchronous single input location.  The Vector3 input position versions are defined to ignore the input Z value.
 
-        virtual void GetHeight(const AZ::Vector3& inPosition, AZ::Vector3& outPosition, Sampler sampleFilter = Sampler::DEFAULT) = 0;
+        virtual void GetHeight(
-        virtual void GetNormal(const AZ::Vector3& inPosition, AZ::Vector3& outNormal, Sampler sampleFilter = Sampler::DEFAULT) = 0;
+            const AZ::Vector3& inPosition,
+            AZ::Vector3& outPosition,
+            AzFramework::Terrain::TerrainDataRequests::Sampler sampleFilter =
+                AzFramework::Terrain::TerrainDataRequests::Sampler::DEFAULT) = 0;
+        virtual void GetNormal(
+            const AZ::Vector3& inPosition,
+            AZ::Vector3& outNormal,
+            AzFramework::Terrain::TerrainDataRequests::Sampler sampleFilter =
+                AzFramework::Terrain::TerrainDataRequests::Sampler::DEFAULT) = 0;
     };
 
     using TerrainAreaHeightRequestBus = AZ::EBus<TerrainAreaHeightRequests>;

Gems/Terrain/Code/Tests/LayerSpawnerTests.cpp

@@ -17,6 +17,10 @@
 
 #include <TerrainMocks.h>
 
+using ::testing::NiceMock;
+using ::testing::AtLeast;
+using ::testing::_;
+
 class LayerSpawnerComponentTest
     : public ::testing::Test
 {
@@ -26,7 +30,7 @@ protected:
     AZStd::unique_ptr<AZ::Entity> m_entity;
     Terrain::TerrainLayerSpawnerComponent* m_layerSpawnerComponent;
     UnitTest::MockBoxShapeComponent* m_shapeComponent;
-    AZStd::unique_ptr<UnitTest::MockTerrainSystem> m_terrainSystem;
+    AZStd::unique_ptr<NiceMock<UnitTest::MockTerrainSystemService>> m_terrainSystem;
 
     void SetUp() override
     {
@@ -40,10 +44,8 @@ protected:
 
     void TearDown() override
     {
-        if (m_terrainSystem)
+        m_entity.reset();
-        {
+        m_terrainSystem.reset();
-            m_terrainSystem->Deactivate();
-        }
         m_app.Destroy();
     }
 
@@ -72,16 +74,9 @@ protected:
         ASSERT_TRUE(m_shapeComponent);
     }
 
-    void ResetEntity()
-    {
-        m_entity->Deactivate();
-        m_entity->Reset();
-    }
-
     void CreateMockTerrainSystem()
     {
-        m_terrainSystem = AZStd::make_unique<UnitTest::MockTerrainSystem>();
+        m_terrainSystem = AZStd::make_unique<NiceMock<UnitTest::MockTerrainSystemService>>();
-        m_terrainSystem->Activate();
     }
 };
 
@@ -93,7 +88,7 @@ TEST_F(LayerSpawnerComponentTest, ActivatEntityActivateSuccess)
     m_entity->Activate();
     EXPECT_EQ(m_entity->GetState(), AZ::Entity::State::Active);
      
-    ResetEntity();
+    m_entity->Deactivate();
 }
 
 TEST_F(LayerSpawnerComponentTest, LayerSpawnerDefaultValuesCorrect)
@@ -115,7 +110,7 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerDefaultValuesCorrect)
 
     EXPECT_TRUE(useGroundPlane);
 
-    ResetEntity();
+    m_entity->Deactivate();
 }
 
 TEST_F(LayerSpawnerComponentTest, LayerSpawnerConfigValuesCorrect)
@@ -147,7 +142,7 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerConfigValuesCorrect)
 
     EXPECT_FALSE(useGroundPlane);
 
-    ResetEntity();
+    m_entity->Deactivate();
 }
 
 TEST_F(LayerSpawnerComponentTest, LayerSpawnerRegisterAreaUpdatesTerrainSystem)
@@ -156,14 +151,14 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerRegisterAreaUpdatesTerrainSystem)
 
     CreateMockTerrainSystem();
 
+    // The Activate call should register the area.
+    EXPECT_CALL(*m_terrainSystem, RegisterArea(_)).Times(1);
+
     AddLayerSpawnerAndShapeComponentToEntity();
 
     m_entity->Activate();
 
-    // The Activate call should have registered the area.
+    m_entity->Deactivate();
-    EXPECT_EQ(1, m_terrainSystem->m_registerAreaCalledCount);
-
-    ResetEntity();
 }
 
 TEST_F(LayerSpawnerComponentTest, LayerSpawnerUnregisterAreaUpdatesTerrainSystem)
@@ -172,16 +167,14 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerUnregisterAreaUpdatesTerrainSystem
 
     CreateMockTerrainSystem();
 
+    // The Deactivate call should unregister the area.
+    EXPECT_CALL(*m_terrainSystem, UnregisterArea(_)).Times(1);
+
     AddLayerSpawnerAndShapeComponentToEntity();
 
     m_entity->Activate();
 
-    m_layerSpawnerComponent->Deactivate();
+    m_entity->Deactivate();
-
-    // The Deactivate call should have unregistered the area.
-    EXPECT_EQ(1, m_terrainSystem->m_unregisterAreaCalledCount);
-
-    ResetEntity();
 }
 
 TEST_F(LayerSpawnerComponentTest, LayerSpawnerTransformChangedUpdatesTerrainSystem)
@@ -190,6 +183,9 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerTransformChangedUpdatesTerrainSyst
 
     CreateMockTerrainSystem();
 
+    // The TransformChanged call should refresh the area.
+    EXPECT_CALL(*m_terrainSystem, RefreshArea(_)).Times(1);
+
     AddLayerSpawnerAndShapeComponentToEntity();
 
     m_entity->Activate();
@@ -197,9 +193,7 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerTransformChangedUpdatesTerrainSyst
     AZ::TransformNotificationBus::Event(
         m_entity->GetId(), &AZ::TransformNotificationBus::Events::OnTransformChanged, AZ::Transform(), AZ::Transform());
 
-    EXPECT_EQ(1, m_terrainSystem->m_refreshAreaCalledCount);
+    m_entity->Deactivate();
-
-    ResetEntity();
 }
 
 TEST_F(LayerSpawnerComponentTest, LayerSpawnerShapeChangedUpdatesTerrainSystem)
@@ -208,6 +202,9 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerShapeChangedUpdatesTerrainSystem)
 
     CreateMockTerrainSystem();
 
+    // The ShapeChanged call should refresh the area.
+    EXPECT_CALL(*m_terrainSystem, RefreshArea(_)).Times(1);
+
     AddLayerSpawnerAndShapeComponentToEntity();
 
     m_entity->Activate();
@@ -216,7 +213,5 @@ TEST_F(LayerSpawnerComponentTest, LayerSpawnerShapeChangedUpdatesTerrainSystem)
         m_entity->GetId(), &LmbrCentral::ShapeComponentNotificationsBus::Events::OnShapeChanged,
         LmbrCentral::ShapeComponentNotifications::ShapeChangeReasons::ShapeChanged);
 
-    EXPECT_EQ(1, m_terrainSystem->m_refreshAreaCalledCount);
+    m_entity->Deactivate();
-
-    ResetEntity();
 }

Gems/Terrain/Code/Tests/TerrainMocks.h

@@ -7,7 +7,10 @@
  */
 #pragma once
 
+#include <gmock/gmock.h>
+
 #include <AzCore/Component/ComponentApplication.h>
+#include <AzFramework/Terrain/TerrainDataRequestBus.h>
 #include <LmbrCentral/Shape/ShapeComponentBus.h>
 
 namespace UnitTest
@@ -62,44 +65,45 @@ namespace UnitTest
         }
     };
 
-    class MockTerrainSystem : private Terrain::TerrainSystemServiceRequestBus::Handler
+    class MockTerrainSystemService : private Terrain::TerrainSystemServiceRequestBus::Handler
     {
     public:
-        void Activate() override
+        MockTerrainSystemService()
         {
             Terrain::TerrainSystemServiceRequestBus::Handler::BusConnect();
         }
 
-        void Deactivate() override
+        ~MockTerrainSystemService()
         {
             Terrain::TerrainSystemServiceRequestBus::Handler::BusDisconnect();
         }
 
-        void SetWorldBounds([[maybe_unused]] const AZ::Aabb& worldBounds) override
+        MOCK_METHOD0(Activate, void());
-        {
+        MOCK_METHOD0(Deactivate, void());
-        }
-
-        void SetHeightQueryResolution([[maybe_unused]] AZ::Vector2 queryResolution) override
-        {
-        }
 
-        void RegisterArea([[maybe_unused]] AZ::EntityId areaId) override
+        MOCK_METHOD1(RegisterArea, void(AZ::EntityId areaId));
-        {
+        MOCK_METHOD1(UnregisterArea, void(AZ::EntityId areaId));
-            m_registerAreaCalledCount++;
+        MOCK_METHOD1(RefreshArea, void(AZ::EntityId areaId));
-        }
+    };
 
-        void UnregisterArea([[maybe_unused]] AZ::EntityId areaId) override
+    class MockTerrainDataNotificationListener : public AzFramework::Terrain::TerrainDataNotificationBus::Handler
+    {
+    public:
+        MockTerrainDataNotificationListener()
         {
-            m_unregisterAreaCalledCount++;
+            AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusConnect();
         }
 
-        void RefreshArea([[maybe_unused]] AZ::EntityId areaId) override
+        ~MockTerrainDataNotificationListener()
         {
-            m_refreshAreaCalledCount++;
+            AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusDisconnect();
         }
 
-        int m_registerAreaCalledCount = 0;
+        MOCK_METHOD0(OnTerrainDataCreateBegin, void());
-        int m_refreshAreaCalledCount = 0;
+        MOCK_METHOD0(OnTerrainDataCreateEnd, void());
-        int m_unregisterAreaCalledCount = 0;
+        MOCK_METHOD0(OnTerrainDataDestroyBegin, void());
+        MOCK_METHOD0(OnTerrainDataDestroyEnd, void());
+        MOCK_METHOD2(OnTerrainDataChanged, void(const AZ::Aabb& dirtyRegion, TerrainDataChangedMask dataChangedMask));
     };
+
 }

Gems/Terrain/Code/Tests/TerrainSystemTest.cpp

@@ -0,0 +1,92 @@
+/*
+ * 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/Component/ComponentApplication.h>
+#include <AzCore/Memory/MemoryComponent.h>
+
+#include <TerrainSystem/TerrainSystem.h>
+
+#include <AzTest/AzTest.h>
+#include <TerrainMocks.h>
+
+using ::testing::AtLeast;
+using ::testing::NiceMock;
+
+class TerrainSystemTest : public ::testing::Test
+{
+protected:
+    AZ::ComponentApplication m_app;
+
+    AZStd::unique_ptr<AZ::Entity> m_entity;
+    AZStd::unique_ptr<Terrain::TerrainSystem> m_terrainSystem;
+
+    void SetUp() override
+    {
+        AZ::ComponentApplication::Descriptor appDesc;
+        appDesc.m_memoryBlocksByteSize = 20 * 1024 * 1024;
+        appDesc.m_recordingMode = AZ::Debug::AllocationRecords::RECORD_NO_RECORDS;
+        appDesc.m_stackRecordLevels = 20;
+
+        m_app.Create(appDesc);
+    }
+
+    void TearDown() override
+    {
+        m_terrainSystem.reset();
+        m_app.Destroy();
+    }
+
+    void CreateEntity()
+    {
+        m_entity = AZStd::make_unique<AZ::Entity>();
+        m_entity->Init();
+
+        ASSERT_TRUE(m_entity);
+    }
+
+    void ResetEntity()
+    {
+        m_entity->Deactivate();
+        m_entity->Reset();
+    }
+};
+
+TEST_F(TerrainSystemTest, TrivialCreateDestroy)
+{
+    m_terrainSystem = AZStd::make_unique<Terrain::TerrainSystem>();
+}
+
+TEST_F(TerrainSystemTest, TrivialActivateDeactivate)
+{
+    m_terrainSystem = AZStd::make_unique<Terrain::TerrainSystem>();
+    m_terrainSystem->Activate();
+    m_terrainSystem->Deactivate();
+}
+
+TEST_F(TerrainSystemTest, CreateEventsCalledOnActivation)
+{
+    NiceMock<UnitTest::MockTerrainDataNotificationListener> mockTerrainListener;
+    EXPECT_CALL(mockTerrainListener, OnTerrainDataCreateBegin()).Times(AtLeast(1));
+    EXPECT_CALL(mockTerrainListener, OnTerrainDataCreateEnd()).Times(AtLeast(1));
+
+    m_terrainSystem = AZStd::make_unique<Terrain::TerrainSystem>();
+    m_terrainSystem->Activate();
+}
+
+TEST_F(TerrainSystemTest, DestroyEventsCalledOnDeactivation)
+{
+    NiceMock<UnitTest::MockTerrainDataNotificationListener> mockTerrainListener;
+    EXPECT_CALL(mockTerrainListener, OnTerrainDataDestroyBegin()).Times(AtLeast(1));
+    EXPECT_CALL(mockTerrainListener, OnTerrainDataDestroyEnd()).Times(AtLeast(1));
+
+    m_terrainSystem = AZStd::make_unique<Terrain::TerrainSystem>();
+    m_terrainSystem->Activate();
+    m_terrainSystem->Deactivate();
+}
+
+

Gems/Terrain/Code/Tests/TerrainTest.cpp

@@ -8,24 +8,4 @@
 
 #include <AzTest/AzTest.h>
 
-class TerrainTest
-    : public ::testing::Test
-{
-protected:
-    void SetUp() override
-    {
-
-    }
-
-    void TearDown() override
-    {
-
-    }
-};
-
-TEST_F(TerrainTest, SanityTest)
-{
-    ASSERT_TRUE(true);
-}
-
 AZ_UNIT_TEST_HOOK(DEFAULT_UNIT_TEST_ENV);

Gems/Terrain/Code/terrain_tests_files.cmake

@@ -9,5 +9,6 @@
 set(FILES
     Tests/TerrainMocks.h
     Tests/TerrainTest.cpp
+    Tests/TerrainSystemTest.cpp
     Tests/LayerSpawnerTests.cpp
 )