From f6c318715198b77ae7ee5f198a53a087b968c8b1 Mon Sep 17 00:00:00 2001 From: sphrose <82213493+sphrose@users.noreply.github.com> Date: Mon, 6 Dec 2021 09:54:00 +0000 Subject: [PATCH 1/4] LYN-7056 SurfaceDataConstants should be revised to have no terrain references Signed-off-by: sphrose <82213493+sphrose@users.noreply.github.com> --- Gems/SurfaceData/Code/CMakeLists.txt | 3 +++ .../SurfaceData/SurfaceDataConstants.h | 13 +--------- .../EditorSurfaceDataSystemComponent.cpp | 1 - .../Editor/EditorSurfaceDataSystemComponent.h | 11 ++++++++- .../Code/Tests/SurfaceDataTest.cpp | 2 +- .../Include/Terrain/TerrainDataConstants.h | 24 +++++++++++++++++++ .../TerrainSurfaceDataSystemComponent.cpp | 8 +++---- Gems/Terrain/Code/terrain_files.cmake | 1 + 8 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 Gems/Terrain/Code/Include/Terrain/TerrainDataConstants.h diff --git a/Gems/SurfaceData/Code/CMakeLists.txt b/Gems/SurfaceData/Code/CMakeLists.txt index 54363d265e..c35da595d6 100644 --- a/Gems/SurfaceData/Code/CMakeLists.txt +++ b/Gems/SurfaceData/Code/CMakeLists.txt @@ -70,6 +70,7 @@ if (PAL_TRAIT_BUILD_HOST_TOOLS) AZ::AzToolsFramework Gem::SurfaceData.Static Gem::LmbrCentral.Editor + Gem::Terrain RUNTIME_DEPENDENCIES Gem::LmbrCentral.Editor ) @@ -97,9 +98,11 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) BUILD_DEPENDENCIES PRIVATE AZ::AzTest + AZ::AzToolsFramework Legacy::CryCommon Gem::SurfaceData.Static Gem::LmbrCentral + Gem::Terrain ) ly_add_googletest( NAME Gem::SurfaceData.Tests diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataConstants.h b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataConstants.h index 3f4a1bb605..2a8f618ac8 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataConstants.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataConstants.h @@ -9,25 +9,14 @@ #pragma once #include -#include +#include namespace SurfaceData { namespace Constants { static const char* s_unassignedTagName = AzFramework::SurfaceData::Constants::s_unassignedTagName; - static const char* s_terrainHoleTagName = "terrainHole"; - static const char* s_terrainTagName = "terrain"; static const AZ::Crc32 s_unassignedTagCrc = AZ::Crc32(s_unassignedTagName); - static const AZ::Crc32 s_terrainHoleTagCrc = AZ::Crc32(s_terrainHoleTagName); - static const AZ::Crc32 s_terrainTagCrc = AZ::Crc32(s_terrainTagName); - - static const char* s_allTagNames[] = - { - s_unassignedTagName, - s_terrainHoleTagName, - s_terrainTagName, - }; } } diff --git a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp index a5bf51d0fe..0285ab5ca4 100644 --- a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp +++ b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp @@ -12,7 +12,6 @@ #include #include - namespace SurfaceData { namespace Details diff --git a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h index 603cffc56d..68f7ea7291 100644 --- a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h +++ b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h @@ -14,6 +14,7 @@ #include #include #include +#include namespace AZ::Data { @@ -22,6 +23,15 @@ namespace AZ::Data namespace SurfaceData { + namespace Constants + { + static const char* s_allTagNames[] = { + Constants::s_unassignedTagName, + Terrain::Constants::s_terrainHoleTagName, + Terrain::Constants::s_terrainTagName, + }; + } //namespace Constants + class EditorSurfaceDataSystemConfig : public AZ::ComponentConfig { @@ -47,7 +57,6 @@ namespace SurfaceData static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required); private: - void LoadAsset(const AZ::Data::AssetId& assetId); void AddAsset(AZ::Data::Asset& asset); diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp index d5384161ad..23c0a1d4c6 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp @@ -23,8 +23,8 @@ #include #include #include -#include #include +#include struct MockGlobalEnvironment { diff --git a/Gems/Terrain/Code/Include/Terrain/TerrainDataConstants.h b/Gems/Terrain/Code/Include/Terrain/TerrainDataConstants.h new file mode 100644 index 0000000000..e125e41ccf --- /dev/null +++ b/Gems/Terrain/Code/Include/Terrain/TerrainDataConstants.h @@ -0,0 +1,24 @@ +/* + * 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 +#include + +namespace Terrain +{ + namespace Constants + { + static const char* s_terrainHoleTagName = "terrainHole"; + static const char* s_terrainTagName = "terrain"; + + static const AZ::Crc32 s_terrainHoleTagCrc = AZ::Crc32(s_terrainHoleTagName); + static const AZ::Crc32 s_terrainTagCrc = AZ::Crc32(s_terrainTagName); + } +} diff --git a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp index 65e8600d62..8c75aafa65 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp +++ b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -162,8 +163,7 @@ namespace Terrain point.m_normal = terrainSurfacePoint.m_normal; // Always add a "terrain" or "terrainHole" tag. - const AZ::Crc32 terrainTag = - isHole ? SurfaceData::Constants::s_terrainHoleTagCrc : SurfaceData::Constants::s_terrainTagCrc; + const AZ::Crc32 terrainTag = isHole ? Constants::s_terrainHoleTagCrc : Constants::s_terrainTagCrc; SurfaceData::AddMaxValueForMasks(point.m_masks, terrainTag, 1.0f); // Add all of the surface tags that the terrain has at this point. @@ -189,8 +189,8 @@ namespace Terrain SurfaceData::SurfaceTagVector TerrainSurfaceDataSystemComponent::GetSurfaceTags() const { SurfaceData::SurfaceTagVector tags; - tags.push_back(SurfaceData::Constants::s_terrainHoleTagCrc); - tags.push_back(SurfaceData::Constants::s_terrainTagCrc); + tags.push_back(Constants::s_terrainHoleTagCrc); + tags.push_back(Constants::s_terrainTagCrc); return tags; } diff --git a/Gems/Terrain/Code/terrain_files.cmake b/Gems/Terrain/Code/terrain_files.cmake index 893477e10d..9e6e68d140 100644 --- a/Gems/Terrain/Code/terrain_files.cmake +++ b/Gems/Terrain/Code/terrain_files.cmake @@ -8,6 +8,7 @@ set(FILES Include/Terrain/Ebuses/TerrainAreaSurfaceRequestBus.h + Include/Terrain/TerrainDataConstants.h Source/Components/TerrainHeightGradientListComponent.cpp Source/Components/TerrainHeightGradientListComponent.h Source/Components/TerrainLayerSpawnerComponent.cpp From 195764a4274d08afe3dcf97acabce100c252e743 Mon Sep 17 00:00:00 2001 From: sphrose <82213493+sphrose@users.noreply.github.com> Date: Thu, 16 Dec 2021 15:03:53 +0000 Subject: [PATCH 2/4] review changes Signed-off-by: sphrose <82213493+sphrose@users.noreply.github.com> --- Gems/SurfaceData/Code/CMakeLists.txt | 3 --- .../Editor/EditorSurfaceDataSystemComponent.cpp | 5 +---- .../Source/Editor/EditorSurfaceDataSystemComponent.h | 10 ---------- Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp | 12 +++++++----- .../Components/TerrainSurfaceDataSystemComponent.cpp | 9 +++++++++ .../Components/TerrainSurfaceDataSystemComponent.h | 6 ++++++ 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/Gems/SurfaceData/Code/CMakeLists.txt b/Gems/SurfaceData/Code/CMakeLists.txt index c35da595d6..54363d265e 100644 --- a/Gems/SurfaceData/Code/CMakeLists.txt +++ b/Gems/SurfaceData/Code/CMakeLists.txt @@ -70,7 +70,6 @@ if (PAL_TRAIT_BUILD_HOST_TOOLS) AZ::AzToolsFramework Gem::SurfaceData.Static Gem::LmbrCentral.Editor - Gem::Terrain RUNTIME_DEPENDENCIES Gem::LmbrCentral.Editor ) @@ -98,11 +97,9 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) BUILD_DEPENDENCIES PRIVATE AZ::AzTest - AZ::AzToolsFramework Legacy::CryCommon Gem::SurfaceData.Static Gem::LmbrCentral - Gem::Terrain ) ly_add_googletest( NAME Gem::SurfaceData.Tests diff --git a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp index 0285ab5ca4..a847da4347 100644 --- a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp +++ b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.cpp @@ -123,10 +123,7 @@ namespace SurfaceData void EditorSurfaceDataSystemComponent::GetRegisteredSurfaceTagNames(SurfaceTagNameSet& masks) const { - for (const auto& tagName : Constants::s_allTagNames) - { - masks.insert(tagName); - } + masks.insert(Constants::s_unassignedTagName); for (const auto& assetPair : m_surfaceTagNameAssets) { diff --git a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h index 68f7ea7291..466da8482e 100644 --- a/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h +++ b/Gems/SurfaceData/Code/Source/Editor/EditorSurfaceDataSystemComponent.h @@ -14,7 +14,6 @@ #include #include #include -#include namespace AZ::Data { @@ -23,15 +22,6 @@ namespace AZ::Data namespace SurfaceData { - namespace Constants - { - static const char* s_allTagNames[] = { - Constants::s_unassignedTagName, - Terrain::Constants::s_terrainHoleTagName, - Terrain::Constants::s_terrainTagName, - }; - } //namespace Constants - class EditorSurfaceDataSystemConfig : public AZ::ComponentConfig { diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp index 23c0a1d4c6..8f4420bcc5 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp @@ -23,8 +23,8 @@ #include #include #include +#include #include -#include struct MockGlobalEnvironment { @@ -281,15 +281,17 @@ public: TEST_F(SurfaceDataTestApp, SurfaceData_TestRegisteredTags) { + // Check that only the unassigned tag exists if no other providers are registered. AZStd::vector> registeredTags = SurfaceData::SurfaceTag::GetRegisteredTags(); - for (const auto& searchTerm : SurfaceData::Constants::s_allTagNames) - { - ASSERT_TRUE(AZStd::find_if(registeredTags.begin(), registeredTags.end(), [searchTerm](decltype(registeredTags)::value_type pair) + const auto& searchTerm = SurfaceData::Constants::s_unassignedTagName; + + ASSERT_TRUE(AZStd::find_if( + registeredTags.begin(), registeredTags.end(), + [searchTerm](decltype(registeredTags)::value_type pair) { return pair.second == searchTerm; })); - } } #if AZ_TRAIT_DISABLE_FAILED_SURFACE_DATA_TESTS diff --git a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp index 8c75aafa65..b36283bd3c 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp +++ b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp @@ -15,6 +15,7 @@ #include #include #include +#include namespace Terrain { @@ -240,6 +241,8 @@ namespace Terrain // Start listening for surface data events AZ_Assert((m_providerHandle != SurfaceData::InvalidSurfaceDataRegistryHandle), "Invalid surface data handle"); SurfaceData::SurfaceDataProviderRequestBus::Handler::BusConnect(m_providerHandle); + + SurfaceData::SurfaceDataTagProviderRequestBus::Handler::BusConnect(); } else if (terrainValidBeforeUpdate && !terrainValidAfterUpdate) { @@ -263,4 +266,10 @@ namespace Terrain { UpdateTerrainData(dirtyRegion); } + + void TerrainSurfaceDataSystemComponent::GetRegisteredSurfaceTagNames(SurfaceData::SurfaceTagNameSet& names) const + { + names.insert(Constants::s_terrainHoleTagName); + names.insert(Constants::s_terrainTagName); + } } diff --git a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.h b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.h index d9c7893c77..4d9a5b7481 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.h +++ b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.h @@ -13,6 +13,7 @@ #include #include #include +#include namespace Terrain { @@ -32,6 +33,7 @@ namespace Terrain : public AZ::Component , private SurfaceData::SurfaceDataProviderRequestBus::Handler , private AzFramework::Terrain::TerrainDataNotificationBus::Handler + , private SurfaceData::SurfaceDataTagProviderRequestBus::Handler { friend class EditorTerrainSurfaceDataSystemComponent; TerrainSurfaceDataSystemComponent(const TerrainSurfaceDataSystemConfig&); @@ -72,5 +74,9 @@ namespace Terrain AZ::Aabb m_terrainBounds = AZ::Aabb::CreateNull(); AZStd::atomic_bool m_terrainBoundsIsValid{ false }; + + ////////////////////////////////////////////////////////////////////////// + // SurfaceData::SurfaceDataTagProviderRequestBus + void GetRegisteredSurfaceTagNames(SurfaceData::SurfaceTagNameSet& names) const override; }; } From 657e56edf7b06d986e1588aaf59c0af9c308143a Mon Sep 17 00:00:00 2001 From: sphrose <82213493+sphrose@users.noreply.github.com> Date: Tue, 21 Dec 2021 09:55:29 +0000 Subject: [PATCH 3/4] revew changes Signed-off-by: sphrose <82213493+sphrose@users.noreply.github.com> --- .../Source/Components/TerrainSurfaceDataSystemComponent.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp index b36283bd3c..a35fa50e65 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp +++ b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp @@ -98,6 +98,7 @@ namespace Terrain { m_providerHandle = SurfaceData::InvalidSurfaceDataRegistryHandle; AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusConnect(); + SurfaceData::SurfaceDataTagProviderRequestBus::Handler::BusConnect(); UpdateTerrainData(AZ::Aabb::CreateNull()); } @@ -112,6 +113,7 @@ namespace Terrain } SurfaceData::SurfaceDataProviderRequestBus::Handler::BusDisconnect(); + SurfaceData::SurfaceDataTagProviderRequestBus::Handler::BusDisconnect(); AzFramework::Terrain::TerrainDataNotificationBus::Handler::BusDisconnect(); // Clear the cached terrain bounds data @@ -241,8 +243,6 @@ namespace Terrain // Start listening for surface data events AZ_Assert((m_providerHandle != SurfaceData::InvalidSurfaceDataRegistryHandle), "Invalid surface data handle"); SurfaceData::SurfaceDataProviderRequestBus::Handler::BusConnect(m_providerHandle); - - SurfaceData::SurfaceDataTagProviderRequestBus::Handler::BusConnect(); } else if (terrainValidBeforeUpdate && !terrainValidAfterUpdate) { @@ -251,8 +251,6 @@ namespace Terrain SurfaceData::SurfaceDataSystemRequestBus::Broadcast( &SurfaceData::SurfaceDataSystemRequestBus::Events::UnregisterSurfaceDataProvider, m_providerHandle); m_providerHandle = SurfaceData::InvalidSurfaceDataRegistryHandle; - - SurfaceData::SurfaceDataProviderRequestBus::Handler::BusDisconnect(); } else { From 9aec7d311382de38d9c84a7fb151206b350a41e7 Mon Sep 17 00:00:00 2001 From: sphrose <82213493+sphrose@users.noreply.github.com> Date: Wed, 22 Dec 2021 10:43:37 +0000 Subject: [PATCH 4/4] Fix compile issues in tests Signed-off-by: sphrose <82213493+sphrose@users.noreply.github.com> --- Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp | 2 +- Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp index 8f4420bcc5..9d182312a4 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp @@ -288,7 +288,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestRegisteredTags) ASSERT_TRUE(AZStd::find_if( registeredTags.begin(), registeredTags.end(), - [searchTerm](decltype(registeredTags)::value_type pair) + [=](decltype(registeredTags)::value_type pair) { return pair.second == searchTerm; })); diff --git a/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp b/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp index 221a2da432..bb1efb5ebd 100644 --- a/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp +++ b/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp @@ -108,7 +108,7 @@ namespace UnitTest Vegetation::SurfaceMaskDepthFilterConfig config; config.m_lowerDistance = -1000.0f; config.m_upperDistance = -0.5f; - config.m_depthComparisonTags.push_back(SurfaceData::Constants::s_terrainTagCrc); + config.m_depthComparisonTags.push_back(SurfaceData::Constants::s_unassignedTagCrc); Vegetation::SurfaceMaskDepthFilterComponent* component = nullptr; auto entity = CreateEntity(config, &component, [](AZ::Entity* e) @@ -120,7 +120,7 @@ namespace UnitTest mockSurfaceHandler.m_outPosition = AZ::Vector3::CreateZero(); mockSurfaceHandler.m_outNormal = AZ::Vector3::CreateAxisZ(); mockSurfaceHandler.m_outMasks.clear(); - mockSurfaceHandler.m_outMasks[SurfaceData::Constants::s_terrainTagCrc] = 1.0f; + mockSurfaceHandler.m_outMasks[SurfaceData::Constants::s_unassignedTagCrc] = 1.0f; // passes {