diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp index 2686ca8a16..117af76325 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp @@ -178,7 +178,7 @@ namespace SurfaceData AZ::Vector3 hitNormal; if (DoRayTrace(inPosition, hitPosition, hitNormal)) { - surfacePointList.AddSurfacePoint(GetEntityId(), hitPosition, hitNormal, m_newPointWeights); + surfacePointList.AddSurfacePoint(GetEntityId(), inPosition, hitPosition, hitNormal, m_newPointWeights); } } @@ -255,6 +255,7 @@ namespace SurfaceData registryEntry.m_entityId = GetEntityId(); registryEntry.m_bounds = GetSurfaceAabb(); registryEntry.m_tags = GetSurfaceTags(); + registryEntry.m_maxPointsCreatedPerInput = 1; if (!meshValidBeforeUpdate && !meshValidAfterUpdate) { diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h index d8e902a0d5..2984495979 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -107,16 +108,16 @@ namespace GradientSignal void AddTag(AZStd::string tag) override; private: - static float CalculateAltitudeRatio(const SurfaceData::SurfacePointList& points, float altitudeMin, float altitudeMax) + static float CalculateAltitudeRatio(const SurfaceData::SurfacePointList& points, size_t inPositionIndex, float altitudeMin, float altitudeMax) { - if (points.IsEmpty()) + if (points.IsEmpty(inPositionIndex)) { return 0.0f; } // GetSurfacePoints (which was used to populate the points list) always returns points in decreasing height order, so the // first point in the list contains the highest altitude. - const float highestAltitude = points.GetHighestSurfacePoint().m_position.GetZ(); + const float highestAltitude = points.GetHighestSurfacePoint(inPositionIndex).m_position.GetZ(); // Turn the absolute altitude value into a 0-1 value by returning the % of the given altitude range that it falls at. return GetRatio(altitudeMin, altitudeMax, highestAltitude); @@ -126,5 +127,10 @@ namespace GradientSignal SurfaceAltitudeGradientConfig m_configuration; LmbrCentral::DependencyMonitor m_dependencyMonitor; AZStd::atomic_bool m_dirty{ false }; + + // m_surfacePointList exists here because it will more efficiently reuse memory across GetValue() calls if it stays constructed. + // The mutex exists to ensure that we don't try to use it from GetValue() in multiple threads at once. + mutable AZStd::recursive_mutex m_surfacePointListMutex; + mutable SurfaceData::SurfacePointList m_surfacePointList; }; } diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h index eb009e6b92..bfb5767a1a 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h @@ -14,6 +14,7 @@ #include #include #include +#include #include namespace LmbrCentral @@ -84,10 +85,9 @@ namespace GradientSignal static float GetMaxSurfaceWeight(const SurfaceData::SurfacePointList& points) { float result = 0.0f; - points.EnumeratePoints([&result]( - [[maybe_unused]] const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, - const SurfaceData::SurfaceTagWeights& masks) -> bool + [[maybe_unused]] size_t inPositionIndex, [[maybe_unused]] const AZ::Vector3& position, + [[maybe_unused]] const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { masks.EnumerateWeights( [&result]([[maybe_unused]] AZ::Crc32 surfaceType, float weight) -> bool diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h index e7a55a2bbc..f54e58d0fc 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -125,17 +126,19 @@ namespace GradientSignal private: float GetSlopeRatio(const SurfaceData::SurfacePointList& points, float angleMin, float angleMax) const { - if (points.IsEmpty()) + constexpr size_t inPositionIndex = 0; + if (points.IsEmpty(inPositionIndex)) { return 0.0f; } // Assuming our surface normal vector is actually normalized, we can get the slope // by just grabbing the Z value. It's the same thing as normal.Dot(AZ::Vector3::CreateAxisZ()). + auto highestSurfacePoint = points.GetHighestSurfacePoint(inPositionIndex); AZ_Assert( - points.GetHighestSurfacePoint().m_normal.GetNormalized().IsClose(points.GetHighestSurfacePoint().m_normal), + highestSurfacePoint.m_normal.GetNormalized().IsClose(highestSurfacePoint.m_normal), "Surface normals are expected to be normalized"); - const float slope = points.GetHighestSurfacePoint().m_normal.GetZ(); + const float slope = highestSurfacePoint.m_normal.GetZ(); // Convert slope back to an angle so that we can lerp in "angular space", not "slope value space". // (We want our 0-1 range to be linear across the range of angles) const float slopeAngle = acosf(slope); diff --git a/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp b/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp index b79606818b..41ccbafc7a 100644 --- a/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp @@ -202,13 +202,18 @@ namespace GradientSignal float SurfaceAltitudeGradientComponent::GetValue(const GradientSampleParams& sampleParams) const { + // For GetValue(), we reuse our SurfacePointList for the GetSurfacePoints query to avoid the repeated cost of memory allocation + // and deallocation across GetValue() calls. However, this also means we can only use it from one thread at a time, so lock a + // mutex to ensure no other threads call GetValue() at the same time. + AZStd::unique_lock surfacePointLock(m_surfacePointListMutex); + AZStd::shared_lock lock(m_cacheMutex); - SurfaceData::SurfacePointList points; SurfaceData::SurfaceDataSystemRequestBus::Broadcast(&SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, - sampleParams.m_position, m_configuration.m_surfaceTagsToSample, points); + sampleParams.m_position, m_configuration.m_surfaceTagsToSample, m_surfacePointList); - return CalculateAltitudeRatio(points, m_configuration.m_altitudeMin, m_configuration.m_altitudeMax); + constexpr size_t inPositionIndex = 0; + return CalculateAltitudeRatio(m_surfacePointList, inPositionIndex, m_configuration.m_altitudeMin, m_configuration.m_altitudeMax); } void SurfaceAltitudeGradientComponent::GetValues(AZStd::span positions, AZStd::span outValues) const @@ -220,31 +225,25 @@ namespace GradientSignal } AZStd::shared_lock lock(m_cacheMutex); - bool valuesFound = false; - // Rather than calling GetSurfacePoints on the EBus repeatedly in a loop, we instead pass a lambda into the EBus that contains - // the loop within it so that we can avoid the repeated EBus-calling overhead. + SurfaceData::SurfacePointList points; SurfaceData::SurfaceDataSystemRequestBus::Broadcast( - [this, positions, &outValues, &valuesFound](SurfaceData::SurfaceDataSystemRequestBus::Events* surfaceDataRequests) - { - // It's possible that there's nothing connected to the EBus, so keep track of the fact that we have valid results. - valuesFound = true; - SurfaceData::SurfacePointList points; - - // For each position, call GetSurfacePoints() and turn the height into a 0-1 value based on our min/max altitudes. - for (size_t index = 0; index < positions.size(); index++) - { - points.Clear(); - surfaceDataRequests->GetSurfacePoints(positions[index], m_configuration.m_surfaceTagsToSample, points); - outValues[index] = CalculateAltitudeRatio(points, m_configuration.m_altitudeMin, m_configuration.m_altitudeMax); - } - }); - - if (!valuesFound) + &SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePointsFromList, positions, m_configuration.m_surfaceTagsToSample, + points); + + if (points.IsEmpty()) { // No surface data, so no output values. AZStd::fill(outValues.begin(), outValues.end(), 0.0f); } + else + { + // For each position, turn the height into a 0-1 value based on our min/max altitudes. + for (size_t index = 0; index < positions.size(); index++) + { + outValues[index] = CalculateAltitudeRatio(points, index, m_configuration.m_altitudeMin, m_configuration.m_altitudeMax); + } + } } void SurfaceAltitudeGradientComponent::OnCompositionChanged() diff --git a/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp b/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp index 692576b3b3..91f1a1b661 100644 --- a/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp @@ -12,6 +12,7 @@ #include #include #include +#include namespace GradientSignal { @@ -107,7 +108,7 @@ namespace GradientSignal // Create a fake surface point with the position we're sampling. AzFramework::SurfaceData::SurfacePoint point; point.m_position = params.m_position; - SurfaceData::SurfacePointList pointList = { { point } }; + SurfaceData::SurfacePointList pointList = AZStd::span(&point, 1); // Send it into the component, see what emerges m_component.ModifySurfacePoints(pointList); @@ -117,8 +118,8 @@ namespace GradientSignal // the underlying logic ever changes to allow separate ranges per tag. float result = 0.0f; pointList.EnumeratePoints([&result]( - [[maybe_unused]] const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, - const SurfaceData::SurfaceTagWeights& masks) -> bool + [[maybe_unused]] size_t inPositionIndex, [[maybe_unused]] const AZ::Vector3& position, + [[maybe_unused]] const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { masks.EnumerateWeights( [&result]([[maybe_unused]] AZ::Crc32 surfaceType, float weight) -> bool diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp index 2e602e8c80..3eef714518 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp @@ -79,7 +79,8 @@ namespace UnitTest { float angle = AZ::DegToRad(inputAngles[(y * dataSize) + x]); point.m_normal = AZ::Vector3(sinf(angle), 0.0f, cosf(angle)); - mockSurface->m_surfacePoints[AZStd::make_pair(static_cast(x), static_cast(y))] = { { point } }; + mockSurface->m_surfacePoints[AZStd::make_pair(static_cast(x), static_cast(y))] = + AZStd::span(&point, 1); } } ActivateEntity(surfaceEntity.get()); @@ -546,10 +547,22 @@ namespace UnitTest auto surfaceEntity = CreateEntity(); auto mockSurface = surfaceEntity->CreateComponent(); mockSurface->m_bounds = mockShapeComponentHandler.m_GetEncompassingAabb; - mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() } }; - mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() } }; - mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() } }; - mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() } }; + AzFramework::SurfaceData::SurfacePoint mockOutputs[] = + { + { AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() }, + }; + + mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 0.0f)] = + AZStd::span(&mockOutputs[0], 1); + mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 0.0f)] = + AZStd::span(&mockOutputs[1], 1); + mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 1.0f)] = + AZStd::span(&mockOutputs[2], 1); + mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 1.0f)] = + AZStd::span(&mockOutputs[3], 1); ActivateEntity(surfaceEntity.get()); // We set the min/max to values other than 0-10 to help validate that they aren't used in the case of the pinned shape. @@ -583,10 +596,21 @@ namespace UnitTest auto surfaceEntity = CreateEntity(); auto mockSurface = surfaceEntity->CreateComponent(); mockSurface->m_bounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f), AZ::Vector3(1.0f)); - mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() } }; - mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() } }; - mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() } }; - mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() } }; + AzFramework::SurfaceData::SurfacePoint mockOutputs[] = { + { AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() }, + }; + + mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 0.0f)] = + AZStd::span(&mockOutputs[0], 1); + mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 0.0f)] = + AZStd::span(&mockOutputs[1], 1); + mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 1.0f)] = + AZStd::span(&mockOutputs[2], 1); + mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 1.0f)] = + AZStd::span(&mockOutputs[3], 1); ActivateEntity(surfaceEntity.get()); // We set the min/max to 0-10, but don't set a shape. @@ -642,14 +666,25 @@ namespace UnitTest auto surfaceEntity = CreateEntity(); auto mockSurface = surfaceEntity->CreateComponent(); mockSurface->m_bounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f), AZ::Vector3(1.0f)); + AzFramework::SurfaceData::SurfacePoint mockOutputs[] = { + { AZ::Vector3(0.0f, 0.0f, -10.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, -5.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 15.0f), AZ::Vector3::CreateZero() }, + { AZ::Vector3(0.0f, 0.0f, 20.0f), AZ::Vector3::CreateZero() }, + }; + // Altitude value below min - should result in 0.0f. - mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, -10.0f), AZ::Vector3::CreateZero() } }; + mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 0.0f)] = + AZStd::span(&mockOutputs[0], 1); // Altitude value at exactly min - should result in 0.0f. - mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, -5.0f), AZ::Vector3::CreateZero() } }; + mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 0.0f)] = + AZStd::span(&mockOutputs[1], 1); // Altitude value at exactly max - should result in 1.0f. - mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 15.0f), AZ::Vector3::CreateZero() } }; + mockSurface->m_surfacePoints[AZStd::make_pair(0.0f, 1.0f)] = + AZStd::span(&mockOutputs[2], 1); // Altitude value above max - should result in 1.0f. - mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 20.0f), AZ::Vector3::CreateZero() } }; + mockSurface->m_surfacePoints[AZStd::make_pair(1.0f, 1.0f)] = + AZStd::span(&mockOutputs[3], 1); ActivateEntity(surfaceEntity.get()); // We set the min/max to -5 - 15. By using a range without 0 at either end, and not having 0 as the midpoint, @@ -691,7 +726,8 @@ namespace UnitTest { point.m_surfaceTags.clear(); point.m_surfaceTags.emplace_back(AZ_CRC_CE("test_mask"), expectedOutput[(y * dataSize) + x]); - mockSurface->m_surfacePoints[AZStd::make_pair(static_cast(x), static_cast(y))] = { { point } }; + mockSurface->m_surfacePoints[AZStd::make_pair(static_cast(x), static_cast(y))] = + AZStd::span(&point, 1); } } ActivateEntity(surfaceEntity.get()); diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp index 4ab4c76d56..45bf818c71 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp @@ -108,12 +108,13 @@ namespace UnitTest EXPECT_TRUE(modifierHandle != SurfaceData::InvalidSurfaceDataRegistryHandle); // Call ModifySurfacePoints and verify the results - SurfaceData::SurfacePointList pointList = { { input } }; + SurfaceData::SurfacePointList pointList; + pointList.StartListConstruction(AZStd::span(&input, 1)); SurfaceData::SurfaceDataModifierRequestBus::Event(modifierHandle, &SurfaceData::SurfaceDataModifierRequestBus::Events::ModifySurfacePoints, pointList); + pointList.EndListConstruction(); ASSERT_EQ(pointList.GetSize(), 1); - pointList.EnumeratePoints( - [this, expectedOutput]( - const AZ::Vector3& position, const AZ::Vector3& normal, + pointList.EnumeratePoints([this, expectedOutput]( + [[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) { EXPECT_TRUE(SurfacePointsAreEqual(position, normal, masks, expectedOutput)); diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h b/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h index c8e69e0d51..466eca2a07 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h +++ b/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h @@ -132,6 +132,18 @@ namespace UnitTest providerRegistryEntry.m_bounds = m_bounds; providerRegistryEntry.m_tags = m_tags; + // Run through the set of surface points that have been set on this component to find out the maximum number + // that we'll return for any given input point. + providerRegistryEntry.m_maxPointsCreatedPerInput = 1; + for (auto& pointEntry : m_surfacePoints) + { + for (size_t index = 0; index < pointEntry.second.GetInputPositionSize(); index++) + { + providerRegistryEntry.m_maxPointsCreatedPerInput = + AZ::GetMax(providerRegistryEntry.m_maxPointsCreatedPerInput, pointEntry.second.GetSize(index)); + } + } + SurfaceData::SurfaceDataSystemRequestBus::BroadcastResult( m_providerHandle, &SurfaceData::SurfaceDataSystemRequestBus::Events::RegisterSurfaceDataProvider, providerRegistryEntry); SurfaceData::SurfaceDataProviderRequestBus::Handler::BusConnect(m_providerHandle); @@ -160,7 +172,15 @@ namespace UnitTest if (surfacePoints != m_surfacePoints.end()) { - surfacePointList = surfacePoints->second; + // If we have an entry for this input position, run through all of its points and add them to the passed-in list. + surfacePoints->second.EnumeratePoints( + [inPosition, &surfacePointList]( + [[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& weights) -> bool + { + surfacePointList.AddSurfacePoint(AZ::EntityId(), inPosition, position, normal, weights); + return true; + }); } } diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/Components/SurfaceDataSystemComponent.h b/Gems/SurfaceData/Code/Include/SurfaceData/Components/SurfaceDataSystemComponent.h index 3e1291e524..820a1296c5 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/Components/SurfaceDataSystemComponent.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/Components/SurfaceDataSystemComponent.h @@ -42,11 +42,17 @@ namespace SurfaceData void GetSurfacePoints(const AZ::Vector3& inPosition, const SurfaceTagVector& desiredTags, SurfacePointList& surfacePointList) const override; void GetSurfacePointsFromRegion( const AZ::Aabb& inRegion, const AZ::Vector2 stepSize, const SurfaceTagVector& desiredTags, - SurfacePointLists& surfacePointListPerPosition) const override; + SurfacePointList& surfacePointListPerPosition) const override; void GetSurfacePointsFromList( AZStd::span inPositions, const SurfaceTagVector& desiredTags, - SurfacePointLists& surfacePointLists) const override; + SurfacePointList& surfacePointLists) const override; + + void GetSurfacePointsFromListInternal( + AZStd::span inPositions, + const AZ::Aabb& inBounds, + const SurfaceTagVector& desiredTags, + SurfacePointList& surfacePointLists) const; SurfaceDataRegistryHandle RegisterSurfaceDataProvider(const SurfaceDataRegistryEntry& entry) override; void UnregisterSurfaceDataProvider(const SurfaceDataRegistryHandle& handle) override; diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataModifierRequestBus.h b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataModifierRequestBus.h index 19cc84fae2..6fc6766f5c 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataModifierRequestBus.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataModifierRequestBus.h @@ -11,6 +11,7 @@ #include #include #include +#include namespace SurfaceData { diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataProviderRequestBus.h b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataProviderRequestBus.h index b3c8667679..3d42f6061b 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataProviderRequestBus.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataProviderRequestBus.h @@ -9,7 +9,9 @@ #pragma once #include +#include #include +#include namespace SurfaceData { @@ -32,9 +34,19 @@ namespace SurfaceData //! Get all of the surface points that this provider has at the given input position. //! @param inPosition - The input position to query. Only XY are guaranteed to be valid, Z should be ignored. - //! @param surfacePointList - The output list of surface points generated, if any. Each provider is expected to - //! append to this list, not overwrite it. + //! @param surfacePointList - The input/output SurfacePointList to add any generated surface points to. virtual void GetSurfacePoints(const AZ::Vector3& inPosition, SurfacePointList& surfacePointList) const = 0; + + //! Get all of the surface points that this provider has at the given input positions. + //! @param inPositions - The input positions to query. Only XY are guaranteed to be valid, Z should be ignored. + //! @param surfacePointList - The input/output SurfacePointList to add any generated surface points to. + virtual void GetSurfacePointsFromList(AZStd::span inPositions, SurfacePointList& surfacePointList) const + { + for (auto& inPosition : inPositions) + { + GetSurfacePoints(inPosition, surfacePointList); + } + } }; typedef AZ::EBus SurfaceDataProviderRequestBus; diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataSystemRequestBus.h b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataSystemRequestBus.h index d4e84c5974..c3309707b6 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataSystemRequestBus.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataSystemRequestBus.h @@ -14,6 +14,7 @@ #include #include #include +#include namespace SurfaceData { @@ -40,13 +41,13 @@ namespace SurfaceData // The input positions are chosen by starting at the min sides of inRegion and incrementing by stepSize. This method is inclusive // on the min sides of the AABB, and exclusive on the max sides (i.e. for a box of (0,0) - (4,4), the point (0,0) is included but (4,4) isn't). virtual void GetSurfacePointsFromRegion(const AZ::Aabb& inRegion, const AZ::Vector2 stepSize, const SurfaceTagVector& desiredTags, - SurfacePointLists& surfacePointLists) const = 0; + SurfacePointList& surfacePointLists) const = 0; // Get all surface points for every passed-in input position. Only the XY dimensions of each position are used. virtual void GetSurfacePointsFromList( AZStd::span inPositions, const SurfaceTagVector& desiredTags, - SurfacePointLists& surfacePointLists) const = 0; + SurfacePointList& surfacePointLists) const = 0; virtual SurfaceDataRegistryHandle RegisterSurfaceDataProvider(const SurfaceDataRegistryEntry& entry) = 0; virtual void UnregisterSurfaceDataProvider(const SurfaceDataRegistryHandle& handle) = 0; diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h index 1d49fff3dc..95f64bcae3 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -166,7 +167,7 @@ namespace SurfaceData //! Check to see if the collection contains any of the given tags. //! @param sampleTags - The tags to look for. //! @return True if any of the tags is found, false if none are found. - bool HasAnyMatchingTags(const SurfaceTagVector& sampleTags) const; + bool HasAnyMatchingTags(AZStd::span sampleTags) const; //! Check to see if the collection contains the given tag with the given weight range. //! The range check is inclusive on both sides of the range: [weightMin, weightMax] @@ -174,7 +175,7 @@ namespace SurfaceData //! @param weightMin - The minimum weight for this tag. //! @param weightMax - The maximum weight for this tag. //! @return True if any of the tags is found, false if none are found. - bool HasAnyMatchingTags(const SurfaceTagVector& sampleTags, float weightMin, float weightMax) const; + bool HasAnyMatchingTags(AZStd::span sampleTags, float weightMin, float weightMax) const; private: //! Search for the given tag entry. @@ -185,79 +186,21 @@ namespace SurfaceData AZStd::fixed_vector m_weights; }; - //! SurfacePointList stores a collection of surface point data, which consists of positions, normals, and surface tag weights. - class SurfacePointList - { - public: - AZ_CLASS_ALLOCATOR(SurfacePointList, AZ::SystemAllocator, 0); - AZ_TYPE_INFO(SurfacePointList, "{DBA02848-2131-4279-BDEF-3581B76AB736}"); - - SurfacePointList() = default; - ~SurfacePointList() = default; - - //! Constructor for creating a SurfacePointList from a list of SurfacePoint data. - //! Primarily used as a convenience for unit tests. - //! @param surfacePoints - An initial set of SurfacePoint points to store in the SurfacePointList. - SurfacePointList(AZStd::initializer_list surfacePoints); - - //! Add a surface point to the list. - //! @param entityId - The entity creating the surface point. - //! @param position - The position of the surface point. - //! @param normal - The normal for the surface point. - //! @param weights - The surface tags and weights for this surface point. - void AddSurfacePoint(const AZ::EntityId& entityId, - const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& weights); - - //! Clear the surface point list. - void Clear(); - - //! Preallocate space in the list based on the maximum number of output points per input point we can generate. - //! @param maxPointsPerInput - The maximum number of output points per input point. - void ReserveSpace(size_t maxPointsPerInput); - - //! Check if the surface point list is empty. - //! @return - true if empty, false if it contains points. - bool IsEmpty() const; - - //! Get the size of the surface point list. - //! @return - The number of valid points in the list. - size_t GetSize() const; - - //! Enumerate every surface point and call a callback for each point found. - void EnumeratePoints(AZStd::function< - bool(const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& surfaceWeights)> pointCallback) const; - - //! Modify the surface weights for each surface point in the list. - void ModifySurfaceWeights( - const AZ::EntityId& currentEntityId, - AZStd::function modificationWeightCallback); - - //! Get the surface point with the highest Z value. - AzFramework::SurfaceData::SurfacePoint GetHighestSurfacePoint() const; - - //! Remove any points that don't contain any of the provided surface tags. - void FilterPoints(const SurfaceTagVector& desiredTags); - - protected: - // These are kept in separate parallel vectors instead of a single struct so that it's possible to pass just specific data - // "channels" into other methods as span<> without having to pass the full struct into the span<>. Specifically, we want to be - // able to pass spans of the positions down through nesting gradient/surface calls. - // A side benefit is that profiling showed the data access to be faster than packing all the fields into a single struct. - AZStd::vector m_surfaceCreatorIdList; - AZStd::vector m_surfacePositionList; - AZStd::vector m_surfaceNormalList; - AZStd::vector m_surfaceWeightsList; - - AZ::Aabb m_pointBounds = AZ::Aabb::CreateNull(); - }; - - using SurfacePointLists = AZStd::vector; struct SurfaceDataRegistryEntry { + //! The entity ID of the surface provider / modifier AZ::EntityId m_entityId; + + //! The AABB bounds that this surface provider / modifier can affect, or null if it has infinite bounds. AZ::Aabb m_bounds = AZ::Aabb::CreateNull(); + + //! The set of surface tags that this surface provider / modifier can create or add to a point. SurfaceTagVector m_tags; + + //! The maximum number of surface points that this will create per input position. + //! For surface modifiers, this is always expected to be 0, and for surface providers it's expected to be > 0. + size_t m_maxPointsCreatedPerInput = 0; }; using SurfaceDataRegistryHandle = AZ::u32; diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/SurfacePointList.h b/Gems/SurfaceData/Code/Include/SurfaceData/SurfacePointList.h new file mode 100644 index 0000000000..83fb02c30a --- /dev/null +++ b/Gems/SurfaceData/Code/Include/SurfaceData/SurfacePointList.h @@ -0,0 +1,229 @@ +/* + * 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 +#include +#include +#include +#include +#include +#include +#include + +namespace SurfaceData +{ + //! SurfacePointList stores a collection of surface point data, which consists of positions, normals, and surface tag weights. + //! This class is specifically designed to be used in the following ways. + //! + //! List construction: + //! StartListConstruction() - This clears the structure, temporarily holds on to the list of input positions, and preallocates the data. + //! AddSurfacePoint() - Add surface points to the list. They're expected to get added in input position order. + //! ModifySurfaceWeights() - Modify the surface weights for the set of input points. + //! FilterPoints() - Remove any generated surface points that don't fit the filter criteria + //! EndListConstruction() - "Freeze" and compact the data. + //! + //! List usage: + //! Any of the query APIs can be used in any order after the list has finished being constructed. + //! + //! This class is specifically designed around the usage patterns described above to minimize the amount of allocations and data + //! shifting that needs to occur. There are some tricky bits that need to be accounted for: + //! * Tracking which input positions each output point belongs to. + //! * Support for merging similar surface points together, which causes us to keep them sorted for easier comparisons. + //! * Each surface provider will add points in input position order, but we call each provider separately, so the added points will + //! show up like (0, 1, 2, 3), (0, 1, 3), (0, 0, 1, 2, 3), etc. We don't want to call each surface provider per-point, because that + //! incurs a lot of avoidable overhead in each provider. + //! * Output points get optionally filtered out at the very end if they don't match any of the filter tags passed in. + //! + //! The solution is that we always add new surface point data to the end of their respective vectors, but we also keep a helper + //! structure that's a list of lists of sorted indices. We can incrementally re-sort the indices quickly without having to shift + //! all the surface point data around. + class SurfacePointList + { + public: + AZ_CLASS_ALLOCATOR(SurfacePointList, AZ::SystemAllocator, 0); + AZ_TYPE_INFO(SurfacePointList, "{DBA02848-2131-4279-BDEF-3581B76AB736}"); + + SurfacePointList() = default; + ~SurfacePointList() = default; + + // ---------- List Construction APIs ------------- + + //! Clear the surface point list. + void Clear(); + + //! Constructor for creating a SurfacePointList from a list of SurfacePoint data. + //! Primarily used as a convenience for unit tests. + //! @param surfacePoints - A set of SurfacePoint points to store in the SurfacePointList. + //! Each point that's passed in will be treated as both the input and output position. + //! The list will be fully constructed and queryable after this runs. + SurfacePointList(AZStd::span surfacePoints); + + //! Start construction of a SurfacePointList from a list of SurfacePoint data. + //! Primarily used as a convenience for unit tests. + //! @param surfacePoints - A set of SurfacePoint points to store in the SurfacePointList. + //! The list will remain in the "constructing" state after this is called, so it will still be possible to add/modify + //! points, and EndListConstruction() will still need to be called. + void StartListConstruction(AZStd::span surfacePoints); + + //! Start construction of a SurfacePointList. + //! @param inPositions - the list of input positions that will be used to generate this list. This list is expected to remain + //! valid until EndListConstruction() is called. + //! @param maxPointsPerInput - the maximum number of potential surface points that will be generated for each input. This is used + //! for allocating internal structures during list construction and is enforced to be correct. + //! @param filterTags - optional list of tags to filter the generated surface points by. If this list is provided, every surface + //! point remaining in the list after construction will contain at least one of these tags. If the list is empty, all generated + //! points will remain in the list. The filterTags list is expected to remain valid until EndListConstruction() is called. + void StartListConstruction(AZStd::span inPositions, size_t maxPointsPerInput, + AZStd::span filterTags); + + //! Add a surface point to the list. + //! To use this method optimally, the points should get added in increasing inPosition index order. + //! @param entityId - The entity creating the surface point. + //! @param inPosition - The input position that produced this surface point. + //! @param position - The position of the surface point. + //! @param normal - The normal for the surface point. + //! @param weights - The surface tags and weights for this surface point. + void AddSurfacePoint(const AZ::EntityId& entityId, const AZ::Vector3& inPosition, + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& weights); + + //! Modify the surface weights for each surface point in the list. + //! @param currentEntityId - The entity currently modifying the surface weights. This is used to prevent entities from modifying + //! any points they created. + //! @param modificationWeightCallback - The function to call for each surface point to process. + void ModifySurfaceWeights( + const AZ::EntityId& currentEntityId, + AZStd::function modificationWeightCallback); + + //! End construction of the SurfacePointList. + //! After this is called, surface points can no longer be added or modified, and all of the query APIs can start getting used. + void EndListConstruction(); + + // ---------- List Query APIs ------------- + + //! Return whether or not the entire surface point list is empty. + //! @return True if empty, false if it contains any points. + bool IsEmpty() const; + + //! Return whether or not a given input position index has any output points associated with it. + //! @param inputPositionIndex - The input position to look for output points for. + //! @return True if empty, false if there is at least one output point associated with the input position. + bool IsEmpty(size_t inputPositionIndex) const; + + //! Return the total number of output points generated. + //! @return The total number of output points generated. + size_t GetSize() const; + + //! Return the total number of output points generated from a specific input position index. + //! @param inputPositionIndex - The input position to look for output points for. + //! @return The total number of output points generated from a specific input position index. + size_t GetSize(size_t inputPositionIndex) const; + + //! Return the total number of input positions. + //! Normally the caller would already be expected to know this, but in the case of using region-based queries, the number of + //! input positions might not be entirely obvious. + //! @return The total number of input positions used to generate the outputs. + size_t GetInputPositionSize() const + { + return m_inputPositionSize; + } + + //! Enumerate every surface point and call a callback for each point found. + //! Note: There is no guaranteed order to which the points will be enumerated. + //! @pointCallback - The method to call with each surface point. + void EnumeratePoints( + AZStd::function + pointCallback) const; + + //! Enumerate every surface point for a given input position and call a callback for each point found. + //! Note: There is no guaranteed order to which the points will be enumerated. + //! @inputPositionIndex - The input position to get the outputs for. + //! @pointCallback - The method to call with each surface point. + void EnumeratePoints( + size_t inputPositionIndex, + AZStd::function pointCallback) const; + + //! Get the surface point with the highest Z value for a given input position. + //! @inputPositionIndex - The input position to get the highest surface point for. + //! @return The surface point with the highest Z value for the given input position. + AzFramework::SurfaceData::SurfacePoint GetHighestSurfacePoint(size_t inputPositionIndex) const; + + //! Get the AABB that encapsulates all of the generated output surface points. + //! @return The AABB surrounding all the output surface points. + AZ::Aabb GetSurfacePointAabb() const + { + return m_surfacePointBounds; + } + + protected: + // Remove any output surface points that don't contain any of the provided surface tags. + void FilterPoints(AZStd::span desiredTags); + + // Get the input position index associated with a specific input position. + size_t GetInPositionIndexFromPosition(const AZ::Vector3& inPosition) const; + + // Get the first entry in the sortedSurfacePointIndices list for the given input position index. + size_t GetSurfacePointStartIndexFromInPositionIndex(size_t inPositionIndex) const; + + // During list construction, keep track of the tags to filter the output points to. + // These will be used at the end of list construction to remove any output points that don't contain any of these tags. + // (If the list is empty, all output points will be retained) + AZStd::span m_filterTags; + + // During list construction, keep track of all the input positions that we'll generate outputs for. + // Note that after construction is complete, we'll only know how *many* input positions, but not their values. + // This keeps us from copying data that the caller should already have. We can't assume the lifetime of that data though, + // so we won't hold on to the span<> after construction. + AZStd::span m_inputPositions; + + // The total number of input positions that we have. We keep this value separately so that we can still know the quantity + // after list construction when our m_inputPositions span<> has become invalid. + size_t m_inputPositionSize = 0; + + // The last input position index that we used when adding points. + // This is used by GetInPositionIndexFromPosition() as an optimization to reduce search times for converting input positions + // to indices without needing to construct a separate search structure. + // Because we know surface points will get added in input position order, we'll always start looking for our next input position + // with the last one we used. + mutable size_t m_lastInputPositionIndex = 0; + + // This list is the size of m_inputPositions.size() and contains the number of output surface points that we've generated for + // each input point. + AZStd::vector m_numSurfacePointsPerInput; + + // The AABB surrounding all the surface points. We build this up incrementally as we add each surface point into the list. + AZ::Aabb m_surfacePointBounds = AZ::Aabb::CreateNull(); + + // The maximum number of output points that can be generated for each input. + size_t m_maxSurfacePointsPerInput = 0; + + // State tracker to determine whether or not the list is currently under construction. + // This is used to verify that the construction APIs are only used during construction, and the query APIs are only used + // after construction is complete. + bool m_listIsBeingConstructed = false; + + // List of lists that's used to index into our storage vectors for all the surface point data. + // The surface points are stored sequentially in creation order in the storage vectors. + // This should be thought of as a nested array - m_sortedSurfacePointIndices[m_inputPositionSize][m_maxSurfacePointsPerInput]. + // For each input position, the list of indices are kept sorted in decreasing Z order. + AZStd::vector m_sortedSurfacePointIndices; + + // Storage vectors for keeping track of all the created surface point data. + // These are kept in separate parallel vectors instead of a single struct so that it's possible to pass just specific data + // "channels" into other methods as span<> without having to pass the full struct into the span<>. Specifically, we want to be + // able to pass spans of the positions down through nesting gradient/surface calls. + AZStd::vector m_surfacePositionList; + AZStd::vector m_surfaceNormalList; + AZStd::vector m_surfaceWeightsList; + AZStd::vector m_surfaceCreatorIdList; + }; +} diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/Tests/SurfaceDataTestMocks.h b/Gems/SurfaceData/Code/Include/SurfaceData/Tests/SurfaceDataTestMocks.h index bafe215402..e07fad98e8 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/Tests/SurfaceDataTestMocks.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/Tests/SurfaceDataTestMocks.h @@ -205,14 +205,14 @@ namespace UnitTest } void GetSurfacePointsFromRegion([[maybe_unused]] const AZ::Aabb& inRegion, [[maybe_unused]] const AZ::Vector2 stepSize, [[maybe_unused]] const SurfaceData::SurfaceTagVector& desiredTags, - [[maybe_unused]] SurfaceData::SurfacePointLists& surfacePointListPerPosition) const override + [[maybe_unused]] SurfaceData::SurfacePointList& surfacePointListPerPosition) const override { } void GetSurfacePointsFromList( [[maybe_unused]] AZStd::span inPositions, [[maybe_unused]] const SurfaceData::SurfaceTagVector& desiredTags, - [[maybe_unused]] SurfaceData::SurfacePointLists& surfacePointLists) const override + [[maybe_unused]] SurfaceData::SurfacePointList& surfacePointLists) const override { } diff --git a/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp b/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp index 35f377bfe0..5506d7640b 100644 --- a/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp +++ b/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp @@ -237,7 +237,7 @@ namespace SurfaceData if (DoRayTrace(inPosition, queryPointOnly, hitPosition, hitNormal)) { - surfacePointList.AddSurfacePoint(GetEntityId(), hitPosition, hitNormal, m_newPointWeights); + surfacePointList.AddSurfacePoint(GetEntityId(), inPosition, hitPosition, hitNormal, m_newPointWeights); } } @@ -317,10 +317,11 @@ namespace SurfaceData providerRegistryEntry.m_entityId = GetEntityId(); providerRegistryEntry.m_bounds = m_colliderBounds; providerRegistryEntry.m_tags = m_configuration.m_providerTags; + providerRegistryEntry.m_maxPointsCreatedPerInput = 1; SurfaceDataRegistryEntry modifierRegistryEntry(providerRegistryEntry); modifierRegistryEntry.m_tags = m_configuration.m_modifierTags; - + modifierRegistryEntry.m_maxPointsCreatedPerInput = 0; if (!colliderValidBeforeUpdate && !colliderValidAfterUpdate) { diff --git a/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp b/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp index 973fca232a..ec81f06134 100644 --- a/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp +++ b/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp @@ -146,7 +146,7 @@ namespace SurfaceData { AZStd::shared_lock lock(m_cacheMutex); - if (m_shapeBoundsIsValid) + if (m_shapeBoundsIsValid && SurfaceData::AabbContains2D(m_shapeBounds, inPosition)) { const AZ::Vector3 rayOrigin = AZ::Vector3(inPosition.GetX(), inPosition.GetY(), m_shapeBounds.GetMax().GetZ()); const AZ::Vector3 rayDirection = -AZ::Vector3::CreateAxisZ(); @@ -156,7 +156,7 @@ namespace SurfaceData if (hitShape) { AZ::Vector3 position = rayOrigin + intersectionDistance * rayDirection; - surfacePointList.AddSurfacePoint(GetEntityId(), position, AZ::Vector3::CreateAxisZ(), m_newPointWeights); + surfacePointList.AddSurfacePoint(GetEntityId(), inPosition, position, AZ::Vector3::CreateAxisZ(), m_newPointWeights); } } } @@ -238,9 +238,11 @@ namespace SurfaceData providerRegistryEntry.m_entityId = GetEntityId(); providerRegistryEntry.m_bounds = m_shapeBounds; providerRegistryEntry.m_tags = m_configuration.m_providerTags; + providerRegistryEntry.m_maxPointsCreatedPerInput = 1; SurfaceDataRegistryEntry modifierRegistryEntry(providerRegistryEntry); modifierRegistryEntry.m_tags = m_configuration.m_modifierTags; + modifierRegistryEntry.m_maxPointsCreatedPerInput = 0; if (shapeValidBeforeUpdate && shapeValidAfterUpdate) { diff --git a/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp b/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp index 7dc6711b72..572b330579 100644 --- a/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp +++ b/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp @@ -205,54 +205,12 @@ namespace SurfaceData void SurfaceDataSystemComponent::GetSurfacePoints(const AZ::Vector3& inPosition, const SurfaceTagVector& desiredTags, SurfacePointList& surfacePointList) const { - const bool useTagFilters = HasValidTags(desiredTags); - const bool hasModifierTags = useTagFilters && HasAnyMatchingTags(desiredTags, m_registeredModifierTags); - - AZStd::shared_lock registrationLock(m_registrationMutex); - - surfacePointList.Clear(); - surfacePointList.ReserveSpace(m_registeredSurfaceDataProviders.size()); - - //gather all intersecting points - for (const auto& entryPair : m_registeredSurfaceDataProviders) - { - const AZ::u32 entryAddress = entryPair.first; - const SurfaceDataRegistryEntry& entry = entryPair.second; - if (!entry.m_bounds.IsValid() || AabbContains2D(entry.m_bounds, inPosition)) - { - if (!useTagFilters || hasModifierTags || HasAnyMatchingTags(desiredTags, entry.m_tags)) - { - SurfaceDataProviderRequestBus::Event(entryAddress, &SurfaceDataProviderRequestBus::Events::GetSurfacePoints, inPosition, surfacePointList); - } - } - } - - if (!surfacePointList.IsEmpty()) - { - //modify or annotate reported points - for (const auto& entryPair : m_registeredSurfaceDataModifiers) - { - const AZ::u32 entryAddress = entryPair.first; - const SurfaceDataRegistryEntry& entry = entryPair.second; - if (!entry.m_bounds.IsValid() || AabbContains2D(entry.m_bounds, inPosition)) - { - SurfaceDataModifierRequestBus::Event(entryAddress, &SurfaceDataModifierRequestBus::Events::ModifySurfacePoints, surfacePointList); - } - } - - // After we've finished creating and annotating all the surface points, combine any points together that have effectively the - // same XY coordinates and extremely similar Z values. This produces results that are sorted in decreasing Z order. - // Also, this filters out any remaining points that don't match the desired tag list. This can happen when a surface provider - // doesn't add a desired tag, and a surface modifier has the *potential* to add it, but then doesn't. - if (useTagFilters) - { - surfacePointList.FilterPoints(desiredTags); - } - } + GetSurfacePointsFromListInternal( + AZStd::span(&inPosition, 1), AZ::Aabb::CreateFromPoint(inPosition), desiredTags, surfacePointList); } void SurfaceDataSystemComponent::GetSurfacePointsFromRegion(const AZ::Aabb& inRegion, const AZ::Vector2 stepSize, - const SurfaceTagVector& desiredTags, SurfacePointLists& surfacePointLists) const + const SurfaceTagVector& desiredTags, SurfacePointList& surfacePointLists) const { const size_t totalQueryPositions = aznumeric_cast(ceil(inRegion.GetXExtent() / stepSize.GetX())) * aznumeric_cast(ceil(inRegion.GetYExtent() / stepSize.GetY())); @@ -270,47 +228,89 @@ namespace SurfaceData } } - GetSurfacePointsFromList(inPositions, desiredTags, surfacePointLists); + GetSurfacePointsFromListInternal(inPositions, inRegion, desiredTags, surfacePointLists); } void SurfaceDataSystemComponent::GetSurfacePointsFromList( - AZStd::span inPositions, const SurfaceTagVector& desiredTags, SurfacePointLists& surfacePointLists) const + AZStd::span inPositions, + const SurfaceTagVector& desiredTags, + SurfacePointList& surfacePointLists) const { - AZStd::shared_lock registrationLock(m_registrationMutex); - - const size_t totalQueryPositions = inPositions.size(); - - surfacePointLists.clear(); - surfacePointLists.resize(totalQueryPositions); - - for (auto& surfacePointList : surfacePointLists) + AZ::Aabb inBounds = AZ::Aabb::CreateNull(); + for (auto& position : inPositions) { - surfacePointList.ReserveSpace(m_registeredSurfaceDataProviders.size()); + inBounds.AddPoint(position); } + GetSurfacePointsFromListInternal(inPositions, inBounds, desiredTags, surfacePointLists); + } + + void SurfaceDataSystemComponent::GetSurfacePointsFromListInternal( + AZStd::span inPositions, const AZ::Aabb& inPositionBounds, + const SurfaceTagVector& desiredTags, SurfacePointList& surfacePointLists) const + { + AZStd::shared_lock registrationLock(m_registrationMutex); + const bool useTagFilters = HasValidTags(desiredTags); const bool hasModifierTags = useTagFilters && HasAnyMatchingTags(desiredTags, m_registeredModifierTags); - // Loop through each data provider, and query all the points for each one. This allows us to check the tags and the overall - // AABB bounds just once per provider, instead of once per point. It also allows for an eventual optimization in which we could - // send the list of points directly into each SurfaceDataProvider. - for (const auto& [providerHandle, provider] : m_registeredSurfaceDataProviders) + // Clear our output structure. + surfacePointLists.Clear(); + + auto ProviderIsApplicable = [useTagFilters, hasModifierTags, &desiredTags, &inPositionBounds] + (const SurfaceDataRegistryEntry& provider) -> bool { bool hasInfiniteBounds = !provider.m_bounds.IsValid(); + // Only allow surface providers that match our tag filters. However, if we aren't using tag filters, + // or if there's at least one surface modifier that can *add* a filtered tag to a created point, then + // allow all the surface providers. if (!useTagFilters || hasModifierTags || HasAnyMatchingTags(desiredTags, provider.m_tags)) { - for (size_t index = 0; index < totalQueryPositions; index++) + // Only allow surface providers that overlap the input position area. + if (hasInfiniteBounds || AabbOverlaps2D(provider.m_bounds, inPositionBounds)) { - bool inBounds = hasInfiniteBounds || AabbContains2D(provider.m_bounds, inPositions[index]); - if (inBounds) - { - SurfaceDataProviderRequestBus::Event( - providerHandle, &SurfaceDataProviderRequestBus::Events::GetSurfacePoints, - inPositions[index], surfacePointLists[index]); - } + return true; } } + + return false; + }; + + // Gather up the subset of surface providers that overlap the input positions. + size_t maxPointsCreatedPerInput = 0; + for (const auto& [providerHandle, provider] : m_registeredSurfaceDataProviders) + { + if (ProviderIsApplicable(provider)) + { + maxPointsCreatedPerInput += provider.m_maxPointsCreatedPerInput; + } + } + + // If we don't have any surface providers that will create any new surface points, then there's nothing more to do. + if (maxPointsCreatedPerInput == 0) + { + return; + } + + // Notify our output structure that we're starting to build up the list of output points. + // This will reserve memory and allocate temporary structures to help build up the list efficiently. + AZStd::span tagFilters; + if (useTagFilters) + { + tagFilters = desiredTags; + } + surfacePointLists.StartListConstruction(inPositions, maxPointsCreatedPerInput, tagFilters); + + // Loop through each data provider and generate surface points from the set of input positions. + // Any generated points that have the same XY coordinates and extremely similar Z values will get combined together. + for (const auto& [providerHandle, provider] : m_registeredSurfaceDataProviders) + { + if (ProviderIsApplicable(provider)) + { + SurfaceDataProviderRequestBus::Event( + providerHandle, &SurfaceDataProviderRequestBus::Events::GetSurfacePointsFromList, inPositions, surfacePointLists); + } } // Once we have our list of surface points created, run through the list of surface data modifiers to potentially add @@ -318,43 +318,28 @@ namespace SurfaceData // create new surface points, but surface data *modifiers* simply annotate points that have already been created. The modifiers // are used to annotate points that occur within a volume. A common example is marking points as "underwater" for points that occur // within a water volume. - for (const auto& entryPair : m_registeredSurfaceDataModifiers) + for (const auto& [modifierHandle, modifier] : m_registeredSurfaceDataModifiers) { - const SurfaceDataRegistryEntry& entry = entryPair.second; - bool hasInfiniteBounds = !entry.m_bounds.IsValid(); + bool hasInfiniteBounds = !modifier.m_bounds.IsValid(); - for (size_t index = 0; index < totalQueryPositions; index++) + if (hasInfiniteBounds || AabbOverlaps2D(modifier.m_bounds, surfacePointLists.GetSurfacePointAabb())) { - const auto& inPosition = inPositions[index]; - SurfacePointList& surfacePointList = surfacePointLists[index]; - if (!surfacePointList.IsEmpty()) - { - if (hasInfiniteBounds || AabbContains2D(entry.m_bounds, inPosition)) - { - SurfaceDataModifierRequestBus::Event( - entryPair.first, &SurfaceDataModifierRequestBus::Events::ModifySurfacePoints, - surfacePointList); - } - } + SurfaceDataModifierRequestBus::Event( + modifierHandle, &SurfaceDataModifierRequestBus::Events::ModifySurfacePoints, + surfacePointLists); } } - // After we've finished creating and annotating all the surface points, combine any points together that have effectively the - // same XY coordinates and extremely similar Z values. This produces results that are sorted in decreasing Z order. - // Also, this filters out any remaining points that don't match the desired tag list. This can happen when a surface provider + // Notify the output structure that we're done building up the list. + // This will filter out any remaining points that don't match the desired tag list. This can happen when a surface provider // doesn't add a desired tag, and a surface modifier has the *potential* to add it, but then doesn't. - if (useTagFilters) - { - for (auto& surfacePointList : surfacePointLists) - { - surfacePointList.FilterPoints(desiredTags); - } - } - + // It may also compact the memory and free any temporary structures. + surfacePointLists.EndListConstruction(); } SurfaceDataRegistryHandle SurfaceDataSystemComponent::RegisterSurfaceDataProviderInternal(const SurfaceDataRegistryEntry& entry) { + AZ_Assert(entry.m_maxPointsCreatedPerInput > 0, "Surface data providers should always create at least 1 point."); AZStd::unique_lock registrationLock(m_registrationMutex); SurfaceDataRegistryHandle handle = ++m_registeredSurfaceDataProviderHandleCounter; m_registeredSurfaceDataProviders[handle] = entry; @@ -376,6 +361,7 @@ namespace SurfaceData bool SurfaceDataSystemComponent::UpdateSurfaceDataProviderInternal(const SurfaceDataRegistryHandle& handle, const SurfaceDataRegistryEntry& entry, AZ::Aabb& oldBounds) { + AZ_Assert(entry.m_maxPointsCreatedPerInput > 0, "Surface data providers should always create at least 1 point."); AZStd::unique_lock registrationLock(m_registrationMutex); auto entryItr = m_registeredSurfaceDataProviders.find(handle); if (entryItr != m_registeredSurfaceDataProviders.end()) @@ -389,6 +375,7 @@ namespace SurfaceData SurfaceDataRegistryHandle SurfaceDataSystemComponent::RegisterSurfaceDataModifierInternal(const SurfaceDataRegistryEntry& entry) { + AZ_Assert(entry.m_maxPointsCreatedPerInput == 0, "Surface data modifiers cannot create any points."); AZStd::unique_lock registrationLock(m_registrationMutex); SurfaceDataRegistryHandle handle = ++m_registeredSurfaceDataModifierHandleCounter; m_registeredSurfaceDataModifiers[handle] = entry; @@ -411,6 +398,7 @@ namespace SurfaceData bool SurfaceDataSystemComponent::UpdateSurfaceDataModifierInternal(const SurfaceDataRegistryHandle& handle, const SurfaceDataRegistryEntry& entry, AZ::Aabb& oldBounds) { + AZ_Assert(entry.m_maxPointsCreatedPerInput == 0, "Surface data modifiers cannot create any points."); AZStd::unique_lock registrationLock(m_registrationMutex); auto entryItr = m_registeredSurfaceDataModifiers.find(handle); if (entryItr != m_registeredSurfaceDataModifiers.end()) diff --git a/Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp b/Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp index 3d6378aa7c..06dda90992 100644 --- a/Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp +++ b/Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp @@ -7,7 +7,6 @@ */ #include -#include namespace SurfaceData { @@ -118,7 +117,7 @@ namespace SurfaceData return FindTag(sampleTag) != m_weights.end(); } - bool SurfaceTagWeights::HasAnyMatchingTags(const SurfaceTagVector& sampleTags) const + bool SurfaceTagWeights::HasAnyMatchingTags(AZStd::span sampleTags) const { for (const auto& sampleTag : sampleTags) { @@ -137,7 +136,7 @@ namespace SurfaceData return weightEntry != m_weights.end() && weightMin <= weightEntry->m_weight && weightMax >= weightEntry->m_weight; } - bool SurfaceTagWeights::HasAnyMatchingTags(const SurfaceTagVector& sampleTags, float weightMin, float weightMax) const + bool SurfaceTagWeights::HasAnyMatchingTags(AZStd::span sampleTags, float weightMin, float weightMax) const { for (const auto& sampleTag : sampleTags) { @@ -169,151 +168,4 @@ namespace SurfaceData // The tag wasn't found, so return end(). return m_weights.end(); } - - - SurfacePointList::SurfacePointList(AZStd::initializer_list surfacePoints) - { - ReserveSpace(surfacePoints.size()); - - for (auto& point : surfacePoints) - { - SurfaceTagWeights weights(point.m_surfaceTags); - AddSurfacePoint(AZ::EntityId(), point.m_position, point.m_normal, weights); - } - } - - void SurfacePointList::AddSurfacePoint(const AZ::EntityId& entityId, - const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& masks) - { - // When adding a surface point, we'll either merge it with a similar existing point, or else add it in order of - // decreasing Z, so that our final results are sorted. - - for (size_t index = 0; index < m_surfacePositionList.size(); ++index) - { - // (Someday we should add a configurable tolerance for comparison) - if (m_surfacePositionList[index].IsClose(position) && m_surfaceNormalList[index].IsClose(normal)) - { - // consolidate points with similar attributes by adding masks/weights to the similar point instead of adding a new one. - m_surfaceWeightsList[index].AddSurfaceTagWeights(masks); - return; - } - else if (m_surfacePositionList[index].GetZ() < position.GetZ()) - { - m_pointBounds.AddPoint(position); - m_surfacePositionList.insert(m_surfacePositionList.begin() + index, position); - m_surfaceNormalList.insert(m_surfaceNormalList.begin() + index, normal); - m_surfaceWeightsList.insert(m_surfaceWeightsList.begin() + index, masks); - m_surfaceCreatorIdList.insert(m_surfaceCreatorIdList.begin() + index, entityId); - return; - } - } - - // The point wasn't merged and the sort puts it at the end, so just add the point to the end of the list. - m_pointBounds.AddPoint(position); - m_surfacePositionList.emplace_back(position); - m_surfaceNormalList.emplace_back(normal); - m_surfaceWeightsList.emplace_back(masks); - m_surfaceCreatorIdList.emplace_back(entityId); - } - - void SurfacePointList::Clear() - { - m_surfacePositionList.clear(); - m_surfaceNormalList.clear(); - m_surfaceWeightsList.clear(); - m_surfaceCreatorIdList.clear(); - } - - void SurfacePointList::ReserveSpace(size_t maxPointsPerInput) - { - AZ_Assert(m_surfacePositionList.empty(), "Trying to reserve space on a list that is already being used."); - - m_surfaceCreatorIdList.reserve(maxPointsPerInput); - m_surfacePositionList.reserve(maxPointsPerInput); - m_surfaceNormalList.reserve(maxPointsPerInput); - m_surfaceWeightsList.reserve(maxPointsPerInput); - } - - bool SurfacePointList::IsEmpty() const - { - return m_surfacePositionList.empty(); - } - - size_t SurfacePointList::GetSize() const - { - return m_surfacePositionList.size(); - } - - void SurfacePointList::EnumeratePoints( - AZStd::function - pointCallback) const - { - for (size_t index = 0; index < m_surfacePositionList.size(); index++) - { - if (!pointCallback(m_surfacePositionList[index], m_surfaceNormalList[index], m_surfaceWeightsList[index])) - { - break; - } - } - } - - void SurfacePointList::ModifySurfaceWeights( - const AZ::EntityId& currentEntityId, - AZStd::function modificationWeightCallback) - { - for (size_t index = 0; index < m_surfacePositionList.size(); index++) - { - if (m_surfaceCreatorIdList[index] != currentEntityId) - { - modificationWeightCallback(m_surfacePositionList[index], m_surfaceWeightsList[index]); - } - } - } - - AzFramework::SurfaceData::SurfacePoint SurfacePointList::GetHighestSurfacePoint() const - { - AzFramework::SurfaceData::SurfacePoint point; - point.m_position = m_surfacePositionList.front(); - point.m_normal = m_surfaceNormalList.front(); - point.m_surfaceTags = m_surfaceWeightsList.front().GetSurfaceTagWeightList(); - - return point; - } - - void SurfacePointList::FilterPoints(const SurfaceTagVector& desiredTags) - { - // Filter out any points that don't match our search tags. - // This has to be done after the Surface Modifiers have processed the points, not at point insertion time, because - // Surface Modifiers add tags to existing points. - size_t listSize = m_surfacePositionList.size(); - size_t index = 0; - for (; index < listSize; index++) - { - if (!m_surfaceWeightsList[index].HasAnyMatchingTags(desiredTags)) - { - break; - } - } - - if (index != listSize) - { - size_t next = index + 1; - for (; next < listSize; ++next) - { - if (m_surfaceWeightsList[index].HasAnyMatchingTags(desiredTags)) - { - m_surfaceCreatorIdList[index] = m_surfaceCreatorIdList[next]; - m_surfacePositionList[index] = m_surfacePositionList[next]; - m_surfaceNormalList[index] = m_surfaceNormalList[next]; - m_surfaceWeightsList[index] = m_surfaceWeightsList[next]; - ++index; - } - } - - m_surfaceCreatorIdList.resize(index); - m_surfacePositionList.resize(index); - m_surfaceNormalList.resize(index); - m_surfaceWeightsList.resize(index); - } - } } diff --git a/Gems/SurfaceData/Code/Source/SurfacePointList.cpp b/Gems/SurfaceData/Code/Source/SurfacePointList.cpp new file mode 100644 index 0000000000..4e9d618ba4 --- /dev/null +++ b/Gems/SurfaceData/Code/Source/SurfacePointList.cpp @@ -0,0 +1,361 @@ +/* + * 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 +#include + +namespace SurfaceData +{ + size_t SurfacePointList::GetInPositionIndexFromPosition(const AZ::Vector3& inPosition) const + { + // Given an input position, find the input position index that's associated with it. + // We'll bias towards always having a position that's the same or further in our input list than before, + // so we'll do a linear search that starts with the last input position we used, and goes forward (and wraps around) + // until we've searched them all. + // Our expectation is that most of the time, we'll only have to compare 0-1 input positions. + + size_t inPositionIndex = m_lastInputPositionIndex; + bool foundMatch = false; + for (size_t indexCounter = 0; indexCounter < m_inputPositions.size(); indexCounter++) + { + if (m_inputPositions[inPositionIndex] == inPosition) + { + foundMatch = true; + break; + } + + inPositionIndex = (inPositionIndex + 1) % m_inputPositions.size(); + } + + AZ_Assert(foundMatch, "Couldn't find input position!"); + m_lastInputPositionIndex = inPositionIndex; + return inPositionIndex; + } + + size_t SurfacePointList::GetSurfacePointStartIndexFromInPositionIndex(size_t inPositionIndex) const + { + // Index to the first output surface point for this input position. + return inPositionIndex * m_maxSurfacePointsPerInput; + } + + SurfacePointList::SurfacePointList(AZStd::span surfacePoints) + { + // Construct and finalize the list with the set of passed-in surface points. + // This is primarily a convenience for unit tests. + StartListConstruction(surfacePoints); + EndListConstruction(); + } + + void SurfacePointList::StartListConstruction(AZStd::span surfacePoints) + { + // Construct the list with the set of passed-in surface points but don't finalize it. + // This is primarily a convenience for unit tests that want to test surface modifiers with specific inputs. + + surfacePoints.begin(); + StartListConstruction(AZStd::span(&(surfacePoints.begin()->m_position), 1), surfacePoints.size(), {}); + + for (auto& point : surfacePoints) + { + SurfaceTagWeights weights(point.m_surfaceTags); + AddSurfacePoint(AZ::EntityId(), point.m_position, point.m_position, point.m_normal, weights); + } + } + + void SurfacePointList::StartListConstruction( + AZStd::span inPositions, size_t maxPointsPerInput, AZStd::span filterTags) + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to start list construction on a list currently under construction."); + AZ_Assert(m_surfacePositionList.empty(), "Trying to reserve space on a list that is already being used."); + + Clear(); + + m_listIsBeingConstructed = true; + + // Save off working references to the data we'll need during list construction. + // These references need to remain valid during construction, but not afterwards. + m_filterTags = filterTags; + m_inputPositions = inPositions; + m_inputPositionSize = inPositions.size(); + m_maxSurfacePointsPerInput = maxPointsPerInput; + + size_t outputReserveSize = inPositions.size() * m_maxSurfacePointsPerInput; + + // Reserve enough space to have one value per input position, and initialize it to 0. + m_numSurfacePointsPerInput.resize(m_inputPositionSize); + + // Reserve enough space to have maxSurfacePointsPerInput entries per input position, and initialize them all to 0. + m_sortedSurfacePointIndices.resize(outputReserveSize); + + // Reserve enough space for all our possible output surface points, but don't initialize them. + m_surfaceCreatorIdList.reserve(outputReserveSize); + m_surfacePositionList.reserve(outputReserveSize); + m_surfaceNormalList.reserve(outputReserveSize); + m_surfaceWeightsList.reserve(outputReserveSize); + } + + void SurfacePointList::Clear() + { + m_listIsBeingConstructed = false; + + m_lastInputPositionIndex = 0; + m_inputPositionSize = 0; + m_maxSurfacePointsPerInput = 0; + + m_filterTags = {}; + m_inputPositions = {}; + + m_sortedSurfacePointIndices.clear(); + m_numSurfacePointsPerInput.clear(); + m_surfacePositionList.clear(); + m_surfaceNormalList.clear(); + m_surfaceWeightsList.clear(); + m_surfaceCreatorIdList.clear(); + + m_surfacePointBounds = AZ::Aabb::CreateNull(); + } + + void SurfacePointList::AddSurfacePoint( + const AZ::EntityId& entityId, const AZ::Vector3& inPosition, + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& masks) + { + AZ_Assert(m_listIsBeingConstructed, "Trying to add surface points to a SurfacePointList that isn't under construction."); + + // Find the inPositionIndex that matches the inPosition. + size_t inPositionIndex = GetInPositionIndexFromPosition(inPosition); + + // Find the first SurfacePoint that either matches the inPosition, or that starts the range for the next inPosition after this one. + size_t surfacePointStartIndex = GetSurfacePointStartIndexFromInPositionIndex(inPositionIndex); + + // When adding a surface point, we'll either merge it with a similar existing point, or else add it in order of + // decreasing Z, so that our final results are sorted. + size_t surfacePointInsertIndex = surfacePointStartIndex; + + for (; surfacePointInsertIndex < (surfacePointStartIndex + m_numSurfacePointsPerInput[inPositionIndex]); ++surfacePointInsertIndex) + { + // (Someday we should add a configurable tolerance for comparison) + if (m_surfacePositionList[m_sortedSurfacePointIndices[surfacePointInsertIndex]].IsClose(position) && + m_surfaceNormalList[m_sortedSurfacePointIndices[surfacePointInsertIndex]].IsClose(normal)) + { + // consolidate points with similar attributes by adding masks/weights to the similar point instead of adding a new one. + m_surfaceWeightsList[m_sortedSurfacePointIndices[surfacePointInsertIndex]].AddSurfaceTagWeights(masks); + return; + } + else if (m_surfacePositionList[m_sortedSurfacePointIndices[surfacePointInsertIndex]].GetZ() < position.GetZ()) + { + break; + } + } + + // If we've made it here, we're adding the point, not merging it. + + // Verify we aren't adding more points than expected. + AZ_Assert(m_numSurfacePointsPerInput[inPositionIndex] < m_maxSurfacePointsPerInput, "Adding too many surface points."); + + // Expand our output AABB to include this point. + m_surfacePointBounds.AddPoint(position); + + // If this isn't the first output for this input position, shift our sorted indices for this input position to make room for + // the new entry. + if (m_numSurfacePointsPerInput[inPositionIndex] > 0) + { + size_t startIndex = surfacePointInsertIndex; + size_t endIndex = surfacePointStartIndex + m_numSurfacePointsPerInput[inPositionIndex]; + + AZStd::move_backward( + m_sortedSurfacePointIndices.begin() + startIndex, m_sortedSurfacePointIndices.begin() + endIndex, + m_sortedSurfacePointIndices.begin() + endIndex + 1); + } + + m_numSurfacePointsPerInput[inPositionIndex]++; + + // Insert the new sorted index that references into our storage vectors. + m_sortedSurfacePointIndices[surfacePointInsertIndex] = m_surfacePositionList.size(); + + // Add the new point to the back of our storage vectors. + m_surfacePositionList.emplace_back(position); + m_surfaceNormalList.emplace_back(normal); + m_surfaceWeightsList.emplace_back(masks); + m_surfaceCreatorIdList.emplace_back(entityId); + } + + void SurfacePointList::ModifySurfaceWeights( + const AZ::EntityId& currentEntityId, + AZStd::function modificationWeightCallback) + { + AZ_Assert(m_listIsBeingConstructed, "Trying to modify surface weights on a SurfacePointList that isn't under construction."); + + // For every valid output point, call the modification callback only if it doesn't match the entity that created the point. + for (size_t inputIndex = 0; (inputIndex < m_inputPositionSize); inputIndex++) + { + size_t surfacePointStartIndex = GetSurfacePointStartIndexFromInPositionIndex(inputIndex); + for (size_t index = surfacePointStartIndex; (index < (surfacePointStartIndex + m_numSurfacePointsPerInput[inputIndex])); + index++) + { + if (m_surfaceCreatorIdList[m_sortedSurfacePointIndices[index]] != currentEntityId) + { + modificationWeightCallback( + m_surfacePositionList[m_sortedSurfacePointIndices[index]], + m_surfaceWeightsList[m_sortedSurfacePointIndices[index]]); + } + } + } + } + + void SurfacePointList::FilterPoints(AZStd::span desiredTags) + { + AZ_Assert(m_listIsBeingConstructed, "Trying to filter a SurfacePointList that isn't under construction."); + + // Filter out any points that don't match our search tags. + // This has to be done after the Surface Modifiers have processed the points, not at point insertion time, because + // Surface Modifiers add tags to existing points. + // The algorithm below is basically an "erase_if" that's operating across multiple storage vectors and using one level of + // indirection to keep our sorted indices valid. + // At some point we might want to consider modifying this to compact the final storage to the minimum needed. + for (size_t inputIndex = 0; (inputIndex < m_inputPositionSize); inputIndex++) + { + size_t surfacePointStartIndex = GetSurfacePointStartIndexFromInPositionIndex(inputIndex); + size_t listSize = (surfacePointStartIndex + m_numSurfacePointsPerInput[inputIndex]); + size_t index = surfacePointStartIndex; + for (; index < listSize; index++) + { + if (!m_surfaceWeightsList[m_sortedSurfacePointIndices[index]].HasAnyMatchingTags(desiredTags)) + { + break; + } + } + + if (index != listSize) + { + size_t next = index + 1; + for (; next < listSize; ++next) + { + if (m_surfaceWeightsList[m_sortedSurfacePointIndices[index]].HasAnyMatchingTags(desiredTags)) + { + m_sortedSurfacePointIndices[index] = AZStd::move(m_sortedSurfacePointIndices[next]); + m_surfaceCreatorIdList[m_sortedSurfacePointIndices[index]] = + AZStd::move(m_surfaceCreatorIdList[m_sortedSurfacePointIndices[next]]); + m_surfacePositionList[m_sortedSurfacePointIndices[index]] = + AZStd::move(m_surfacePositionList[m_sortedSurfacePointIndices[next]]); + m_surfaceNormalList[m_sortedSurfacePointIndices[index]] = + AZStd::move(m_surfaceNormalList[m_sortedSurfacePointIndices[next]]); + m_surfaceWeightsList[m_sortedSurfacePointIndices[index]] = + AZStd::move(m_surfaceWeightsList[m_sortedSurfacePointIndices[next]]); + + m_numSurfacePointsPerInput[inputIndex]--; + + ++index; + } + } + } + } + } + + void SurfacePointList::EndListConstruction() + { + AZ_Assert(m_listIsBeingConstructed, "Trying to end list construction on a SurfacePointList that isn't under construction."); + + // Now that we've finished adding and modifying points, filter out any points that don't match the filterTags list, if we have one. + if (!m_filterTags.empty()) + { + FilterPoints(m_filterTags); + } + + m_listIsBeingConstructed = false; + m_inputPositions = {}; + m_filterTags = {}; + } + + bool SurfacePointList::IsEmpty() const + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to query a SurfacePointList that's still under construction."); + + return m_surfacePositionList.empty(); + } + + bool SurfacePointList::IsEmpty(size_t inputPositionIndex) const + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to query a SurfacePointList that's still under construction."); + + return (m_inputPositionSize == 0) || (m_numSurfacePointsPerInput[inputPositionIndex] == 0); + } + + size_t SurfacePointList::GetSize() const + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to query a SurfacePointList that's still under construction."); + + return m_surfacePositionList.size(); + } + + size_t SurfacePointList::GetSize(size_t inputPositionIndex) const + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to query a SurfacePointList that's still under construction."); + + return (m_inputPositionSize == 0) ? 0 : (m_numSurfacePointsPerInput[inputPositionIndex]); + } + + void SurfacePointList::EnumeratePoints( + size_t inputPositionIndex, + AZStd::function + pointCallback) const + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to query a SurfacePointList that's still under construction."); + + size_t surfacePointStartIndex = GetSurfacePointStartIndexFromInPositionIndex(inputPositionIndex); + for (size_t index = surfacePointStartIndex; + (index < (surfacePointStartIndex + m_numSurfacePointsPerInput[inputPositionIndex])); index++) + { + if (!pointCallback( + m_surfacePositionList[m_sortedSurfacePointIndices[index]], m_surfaceNormalList[m_sortedSurfacePointIndices[index]], + m_surfaceWeightsList[m_sortedSurfacePointIndices[index]])) + { + break; + } + } + } + + void SurfacePointList::EnumeratePoints( + AZStd::function + pointCallback) const + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to query a SurfacePointList that's still under construction."); + + for (size_t inputIndex = 0; (inputIndex < m_inputPositionSize); inputIndex++) + { + size_t surfacePointStartIndex = GetSurfacePointStartIndexFromInPositionIndex(inputIndex); + for (size_t index = surfacePointStartIndex; (index < (surfacePointStartIndex + m_numSurfacePointsPerInput[inputIndex])); + index++) + { + if (!pointCallback( + inputIndex, m_surfacePositionList[m_sortedSurfacePointIndices[index]], + m_surfaceNormalList[m_sortedSurfacePointIndices[index]], m_surfaceWeightsList[m_sortedSurfacePointIndices[index]])) + { + break; + } + } + } + } + + AzFramework::SurfaceData::SurfacePoint SurfacePointList::GetHighestSurfacePoint([[maybe_unused]] size_t inputPositionIndex) const + { + AZ_Assert(!m_listIsBeingConstructed, "Trying to query a SurfacePointList that's still under construction."); + + if (m_numSurfacePointsPerInput[inputPositionIndex] == 0) + { + return {}; + } + + size_t surfacePointStartIndex = GetSurfacePointStartIndexFromInPositionIndex(inputPositionIndex); + AzFramework::SurfaceData::SurfacePoint point; + point.m_position = m_surfacePositionList[m_sortedSurfacePointIndices[surfacePointStartIndex]]; + point.m_normal = m_surfaceNormalList[m_sortedSurfacePointIndices[surfacePointStartIndex]]; + point.m_surfaceTags = m_surfaceWeightsList[m_sortedSurfacePointIndices[surfacePointStartIndex]].GetSurfaceTagWeightList(); + + return point; + } + +} diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp index a507391ae6..187ace1cdb 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp @@ -213,7 +213,7 @@ namespace UnitTest // Query every point in our world at 1 meter intervals. for ([[maybe_unused]] auto _ : state) { - SurfaceData::SurfacePointLists points; + SurfaceData::SurfacePointList points; AZ::Aabb inRegion = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f), AZ::Vector3(worldSize)); AZ::Vector2 stepSize(1.0f); @@ -248,7 +248,7 @@ namespace UnitTest } } - SurfaceData::SurfacePointLists points; + SurfaceData::SurfacePointList points; SurfaceData::SurfaceDataSystemRequestBus::Broadcast( &SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePointsFromList, queryPositions, filterTags, points); diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp index aca5a53f4d..8724227960 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp @@ -135,14 +135,17 @@ namespace UnitTest // Call GetSurfacePoints and verify the results SurfaceData::SurfacePointList pointList; + pointList.StartListConstruction(AZStd::span(&queryPoint, 1), 1, {}); SurfaceData::SurfaceDataProviderRequestBus::Event(providerHandle, &SurfaceData::SurfaceDataProviderRequestBus::Events::GetSurfacePoints, queryPoint, pointList); + pointList.EndListConstruction(); + if (pointOnProvider) { ASSERT_EQ(pointList.GetSize(), 1); - pointList.EnumeratePoints( - [this, expectedOutput]( - const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + pointList.EnumeratePoints([this, expectedOutput]( + [[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, + const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { EXPECT_TRUE(SurfacePointsAreEqual(position, normal, masks, expectedOutput)); return true; @@ -185,12 +188,14 @@ namespace UnitTest // Call ModifySurfacePoints and verify the results // Add the surface point with a different entity ID than the entity doing the modification, so that the point doesn't get // filtered out. - SurfaceData::SurfacePointList pointList = { input }; + SurfaceData::SurfacePointList pointList; + pointList.StartListConstruction(AZStd::span(&input, 1)); SurfaceData::SurfaceDataModifierRequestBus::Event(modifierHandle, &SurfaceData::SurfaceDataModifierRequestBus::Events::ModifySurfacePoints, pointList); + pointList.EndListConstruction(); ASSERT_EQ(pointList.GetSize(), 1); - pointList.EnumeratePoints( - [this, expectedOutput]( - const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + pointList.EnumeratePoints([this, expectedOutput]( + [[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, + const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { EXPECT_TRUE(SurfacePointsAreEqual(position, normal, masks, expectedOutput)); return true; diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp index 8500e557bf..9f9a30e21d 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp @@ -114,6 +114,14 @@ class MockSurfaceProvider if (m_providerType == ProviderType::SURFACE_PROVIDER) { + // If the mock provider is generating points, examine the size of the points lists we've added to the mock provider + // to determine the maximum number of points that we will output from a single input position. + registryEntry.m_maxPointsCreatedPerInput = 1; + for (auto& entry : m_GetSurfacePoints) + { + registryEntry.m_maxPointsCreatedPerInput = AZ::GetMax(registryEntry.m_maxPointsCreatedPerInput, entry.second.size()); + } + SurfaceData::SurfaceDataSystemRequestBus::BroadcastResult(m_providerHandle, &SurfaceData::SurfaceDataSystemRequestBus::Events::RegisterSurfaceDataProvider, registryEntry); SurfaceData::SurfaceDataProviderRequestBus::Handler::BusConnect(m_providerHandle); } @@ -152,7 +160,7 @@ class MockSurfaceProvider for (auto& point : surfacePoints->second) { SurfaceData::SurfaceTagWeights weights(point.m_surfaceTags); - surfacePointList.AddSurfacePoint(m_id, point.m_position, point.m_normal, weights); + surfacePointList.AddSurfacePoint(m_id, inPosition, point.m_position, point.m_normal, weights); } } } @@ -208,19 +216,22 @@ public: } void CompareSurfacePointListWithGetSurfacePoints( - const AZStd::vector& queryPositions, SurfaceData::SurfacePointLists& surfacePointLists, + const AZStd::vector& queryPositions, SurfaceData::SurfacePointList& surfacePointLists, const SurfaceData::SurfaceTagVector& testTags) { AZStd::vector singleQueryResults; + SurfaceData::SurfacePointList tempSingleQueryPointList; - for (auto& queryPosition : queryPositions) + for (size_t inputIndex = 0; inputIndex < queryPositions.size(); inputIndex++) { - SurfaceData::SurfacePointList tempSingleQueryPointList; + tempSingleQueryPointList.Clear(); + singleQueryResults.clear(); + SurfaceData::SurfaceDataSystemRequestBus::Broadcast( - &SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, queryPosition, testTags, tempSingleQueryPointList); - tempSingleQueryPointList.EnumeratePoints( - [&singleQueryResults]( - const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + &SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, queryPositions[inputIndex], testTags, tempSingleQueryPointList); + tempSingleQueryPointList.EnumeratePoints([&singleQueryResults]( + [[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, + const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { AzFramework::SurfaceData::SurfacePoint point; point.m_position = position; @@ -229,28 +240,23 @@ public: singleQueryResults.emplace_back(AZStd::move(point)); return true; }); - } - // Verify that each point in each list is equal. - AzFramework::SurfaceData::SurfacePoint* singleQueryPoint = singleQueryResults.begin(); - for (size_t listIndex = 0; listIndex < surfacePointLists.size(); listIndex++) - { - auto& surfacePointList = surfacePointLists[listIndex]; - surfacePointList.EnumeratePoints( - [&singleQueryPoint, singleQueryResults]( + size_t resultIndex = 0; + surfacePointLists.EnumeratePoints( + inputIndex, + [&resultIndex, singleQueryResults]( const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { - EXPECT_NE(singleQueryPoint, singleQueryResults.end()); + EXPECT_NE(resultIndex, singleQueryResults.size()); - EXPECT_EQ(position, singleQueryPoint->m_position); - EXPECT_EQ(normal, singleQueryPoint->m_normal); - EXPECT_TRUE(masks.SurfaceWeightsAreEqual(singleQueryPoint->m_surfaceTags)); - ++singleQueryPoint; + EXPECT_EQ(position, singleQueryResults[resultIndex].m_position); + EXPECT_EQ(normal, singleQueryResults[resultIndex].m_normal); + EXPECT_TRUE(masks.SurfaceWeightsAreEqual(singleQueryResults[resultIndex].m_surfaceTags)); + ++resultIndex; return true; }); + EXPECT_EQ(resultIndex, singleQueryResults.size()); } - - EXPECT_EQ(singleQueryPoint, singleQueryResults.end()); } @@ -484,7 +490,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion) // Query for all the surface points from (0, 0, 16) - (4, 4, 16) with a step size of 1. // Note that the Z range is deliberately chosen to be outside the surface provider range to demonstrate // that it is ignored when selecting points. - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(1.0f, 1.0f); AZ::Aabb regionBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f, 0.0f, 16.0f), AZ::Vector3(4.0f, 4.0f, 16.0f)); SurfaceData::SurfaceTagVector testTags = providerTags; @@ -496,21 +502,17 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion) // We expect every entry in the output list to have two surface points, at heights 0 and 4, sorted in // decreasing height order. The masks list should be the same size as the set of masks the provider owns. // We *could* check every mask as well for completeness, but that seems like overkill. - for (auto& pointList : availablePointsPerPosition) - { - EXPECT_EQ(pointList.GetSize(), 2); - float expectedZ = 4.0f; - pointList.EnumeratePoints( - [providerTags, - &expectedZ](const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, - const SurfaceData::SurfaceTagWeights& masks) -> bool - { - EXPECT_EQ(position.GetZ(), expectedZ); - EXPECT_EQ(masks.GetSize(), providerTags.size()); - expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; - return true; - }); - } + float expectedZ = 4.0f; + availablePointsPerPosition.EnumeratePoints( + [availablePointsPerPosition, providerTags, &expectedZ](size_t inPositionIndex, const AZ::Vector3& position, + [[maybe_unused]] const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_EQ(availablePointsPerPosition.GetSize(inPositionIndex), 2); + EXPECT_EQ(position.GetZ(), expectedZ); + EXPECT_EQ(masks.GetSize(), providerTags.size()); + expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; + return true; + }); } TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingMasks) @@ -525,7 +527,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingMas // Query for all the surface points from (0, 0, 0) - (4, 4, 4) with a step size of 1. // We only include a surface tag that does NOT exist in the surface provider. - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(1.0f, 1.0f); AZ::Aabb regionBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f), AZ::Vector3(4.0f)); SurfaceData::SurfaceTagVector testTags = { SurfaceData::SurfaceTag(m_testSurfaceNoMatchCrc) }; @@ -536,10 +538,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingMas // We expect every entry in the output list to have no surface points, since the requested mask doesn't match // any of the masks from our mock surface provider. - for (auto& queryPosition : availablePointsPerPosition) - { - EXPECT_TRUE(queryPosition.IsEmpty()); - } + EXPECT_TRUE(availablePointsPerPosition.IsEmpty()); } TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingRegion) @@ -553,7 +552,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingReg AZ::Vector3(0.0f), AZ::Vector3(8.0f), AZ::Vector3(0.25f, 0.25f, 4.0f)); // Query for all the surface points from (16, 16) - (20, 20) with a step size of 1. - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(1.0f, 1.0f); AZ::Aabb regionBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(16.0f), AZ::Vector3(20.0f)); SurfaceData::SurfaceTagVector testTags = providerTags; @@ -564,10 +563,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingReg // We expect every entry in the output list to have no surface points, since the input points don't overlap with // our surface provider. - for (auto& pointList : availablePointsPerPosition) - { - EXPECT_TRUE(pointList.IsEmpty()); - } + EXPECT_TRUE(availablePointsPerPosition.IsEmpty()); } TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_ProviderModifierMasksCombine) @@ -603,7 +599,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_ProviderModif for (auto& tagTest : tagTests) { - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(1.0f, 1.0f); AZ::Aabb regionBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f), AZ::Vector3(4.0f)); SurfaceData::SurfaceTagVector testTags = tagTest; @@ -614,20 +610,17 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_ProviderModif // We expect every entry in the output list to have two surface points (with heights 0 and 4), // and each point should have both the "test_surface1" and "test_surface2" tag. - for (auto& pointList : availablePointsPerPosition) - { - EXPECT_EQ(pointList.GetSize(), 2); - float expectedZ = 4.0f; - pointList.EnumeratePoints( - [&expectedZ](const AZ::Vector3& position, - [[maybe_unused]] const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool - { - EXPECT_EQ(position.GetZ(), expectedZ); - EXPECT_EQ(masks.GetSize(), 2); - expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; - return true; - }); - } + float expectedZ = 4.0f; + availablePointsPerPosition.EnumeratePoints( + [availablePointsPerPosition, &expectedZ](size_t inPositionIndex, const AZ::Vector3& position, + [[maybe_unused]] const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_EQ(availablePointsPerPosition.GetSize(inPositionIndex), 2); + EXPECT_EQ(position.GetZ(), expectedZ); + EXPECT_EQ(masks.GetSize(), 2); + expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; + return true; + }); } } @@ -653,7 +646,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_SimilarPoints // Query for all the surface points from (0, 0) - (4, 4) with a step size of 1. - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(1.0f, 1.0f); AZ::Aabb regionBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f), AZ::Vector3(4.0f)); SurfaceData::SurfaceTagVector testTags = { SurfaceData::SurfaceTag(m_testSurface1Crc), SurfaceData::SurfaceTag(m_testSurface2Crc) }; @@ -664,23 +657,21 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_SimilarPoints // We expect every entry in the output list to have two surface points, not four. The two points // should have both surface tags on them. - for (auto& pointList : availablePointsPerPosition) - { - EXPECT_EQ(pointList.GetSize(), 2); - float expectedZ = 4.0f; - pointList.EnumeratePoints( - [&expectedZ]( - const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, - const SurfaceData::SurfaceTagWeights& masks) -> bool - { - // Similar points get merged, but there's no guarantee which value will be kept, so we set our comparison tolerance - // high enough to allow both x.0 and x.0005 to pass. - EXPECT_NEAR(position.GetZ(), expectedZ, 0.001f); - EXPECT_EQ(masks.GetSize(), 2); - expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; - return true; - }); - } + float expectedZ = 4.0f; + availablePointsPerPosition.EnumeratePoints( + [availablePointsPerPosition, &expectedZ]( + size_t inPositionIndex, const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_EQ(availablePointsPerPosition.GetSize(inPositionIndex), 2); + + // Similar points get merged, but there's no guarantee which value will be kept, so we set our comparison tolerance + // high enough to allow both x.0 and x.0005 to pass. + EXPECT_NEAR(position.GetZ(), expectedZ, 0.001f); + EXPECT_EQ(masks.GetSize(), 2); + expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; + return true; + }); } TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_DissimilarPointsDoNotMergeTogether) @@ -704,7 +695,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_DissimilarPoi // Query for all the surface points from (0, 0) - (4, 4) with a step size of 1. - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(1.0f, 1.0f); AZ::Aabb regionBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f), AZ::Vector3(4.0f)); SurfaceData::SurfaceTagVector testTags = { SurfaceData::SurfaceTag(m_testSurface1Crc), SurfaceData::SurfaceTag(m_testSurface2Crc) }; @@ -715,17 +706,14 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_DissimilarPoi // We expect every entry in the output list to have four surface points with one tag each, // because the points are far enough apart that they won't merge. - for (auto& pointList : availablePointsPerPosition) - { - EXPECT_EQ(pointList.GetSize(), 4); - pointList.EnumeratePoints( - []([[maybe_unused]] const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, - const SurfaceData::SurfaceTagWeights& masks) -> bool - { - EXPECT_EQ(masks.GetSize(), 1); - return true; - }); - } + availablePointsPerPosition.EnumeratePoints( + [availablePointsPerPosition](size_t inPositionIndex, [[maybe_unused]] const AZ::Vector3& position, + [[maybe_unused]] const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_EQ(availablePointsPerPosition.GetSize(inPositionIndex), 4); + EXPECT_EQ(masks.GetSize(), 1); + return true; + }); } TEST_F(SurfaceDataTestApp, SurfaceData_VerifyGetSurfacePointsFromRegionAndGetSurfacePointsMatch) @@ -741,7 +729,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_VerifyGetSurfacePointsFromRegionAndGetSur AZ::Vector3(0.25f, 0.25f, 4.0f)); // Query for all the surface points from (0, 0, 16) - (4, 4, 16) with a step size of 1. - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(1.0f, 1.0f); AZ::Aabb regionBounds = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0f, 0.0f, 16.0f), AZ::Vector3(4.0f, 4.0f, 16.0f)); @@ -775,7 +763,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_VerifyGetSurfacePointsFromListAndGetSurfa AZ::Vector3(0.25f, 0.25f, 4.0f)); // Query for all the surface points from (0, 0, 16) - (4, 4, 16) with a step size of 1. - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZStd::vector queryPositions; for (float y = 0.0f; y < 4.0f; y += 1.0f) { @@ -789,8 +777,6 @@ TEST_F(SurfaceDataTestApp, SurfaceData_VerifyGetSurfacePointsFromListAndGetSurfa &SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePointsFromList, queryPositions, providerTags, availablePointsPerPosition); - EXPECT_EQ(availablePointsPerPosition.size(), 16); - // For each point entry returned from GetSurfacePointsFromList, call GetSurfacePoints and verify the results match. CompareSurfacePointListWithGetSurfacePoints(queryPositions, availablePointsPerPosition, providerTags); } diff --git a/Gems/SurfaceData/Code/surfacedata_files.cmake b/Gems/SurfaceData/Code/surfacedata_files.cmake index 8e7435abb3..7db0d869dd 100644 --- a/Gems/SurfaceData/Code/surfacedata_files.cmake +++ b/Gems/SurfaceData/Code/surfacedata_files.cmake @@ -18,10 +18,12 @@ set(FILES Include/SurfaceData/SurfaceDataTagProviderRequestBus.h Include/SurfaceData/SurfaceDataProviderRequestBus.h Include/SurfaceData/SurfaceDataModifierRequestBus.h + Include/SurfaceData/SurfacePointList.h Include/SurfaceData/SurfaceTag.h Include/SurfaceData/Utility/SurfaceDataUtility.h Source/SurfaceDataSystemComponent.cpp Source/SurfaceDataTypes.cpp + Source/SurfacePointList.cpp Source/SurfaceTag.cpp Source/Components/SurfaceDataColliderComponent.cpp Source/Components/SurfaceDataShapeComponent.cpp diff --git a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp index 3092f7c000..cf149c0f3c 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp +++ b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp @@ -165,7 +165,7 @@ namespace Terrain const AZ::Crc32 terrainTag = isHole ? Constants::s_terrainHoleTagCrc : Constants::s_terrainTagCrc; weights.AddSurfaceTagWeight(terrainTag, 1.0f); - surfacePointList.AddSurfacePoint(GetEntityId(), terrainSurfacePoint.m_position, terrainSurfacePoint.m_normal, weights); + surfacePointList.AddSurfacePoint(GetEntityId(), inPosition, terrainSurfacePoint.m_position, terrainSurfacePoint.m_normal, weights); } AZ::Aabb TerrainSurfaceDataSystemComponent::GetSurfaceAabb() const @@ -192,6 +192,7 @@ namespace Terrain registryEntry.m_entityId = GetEntityId(); registryEntry.m_bounds = GetSurfaceAabb(); registryEntry.m_tags = GetSurfaceTags(); + registryEntry.m_maxPointsCreatedPerInput = 1; m_terrainBounds = registryEntry.m_bounds; m_terrainBoundsIsValid = m_terrainBounds.IsValid(); diff --git a/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp b/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp index f4ef3bd6f1..a0982591a2 100644 --- a/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp +++ b/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp @@ -1099,7 +1099,7 @@ namespace Vegetation // 0 = lower left corner, 0.5 = center const float texelOffset = (sectorPointSnapMode == SnapMode::Center) ? 0.5f : 0.0f; - SurfaceData::SurfacePointLists availablePointsPerPosition; + SurfaceData::SurfacePointList availablePointsPerPosition; AZ::Vector2 stepSize(vegStep, vegStep); AZ::Vector3 regionOffset(texelOffset * vegStep, texelOffset * vegStep, 0.0f); AZ::Aabb regionBounds = sectorInfo.m_bounds; @@ -1120,27 +1120,20 @@ namespace Vegetation SurfaceData::SurfaceTagVector(), availablePointsPerPosition); - AZ_Assert(availablePointsPerPosition.size() == (sectorDensity * sectorDensity), - "Veg sector ended up with unexpected density (%d points created, %d expected)", availablePointsPerPosition.size(), - (sectorDensity * sectorDensity)); - uint claimIndex = 0; - for (auto& availablePoints : availablePointsPerPosition) - { - availablePoints.EnumeratePoints( - [this, §orInfo, - &claimIndex](const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool - { - sectorInfo.m_baseContext.m_availablePoints.push_back(); - ClaimPoint& claimPoint = sectorInfo.m_baseContext.m_availablePoints.back(); - claimPoint.m_handle = CreateClaimHandle(sectorInfo, ++claimIndex); - claimPoint.m_position = position; - claimPoint.m_normal = normal; - claimPoint.m_masks = masks; - sectorInfo.m_baseContext.m_masks.AddSurfaceTagWeights(masks); - return true; - }); - } + availablePointsPerPosition.EnumeratePoints([this, §orInfo, &claimIndex] + ([[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, + const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + sectorInfo.m_baseContext.m_availablePoints.push_back(); + ClaimPoint& claimPoint = sectorInfo.m_baseContext.m_availablePoints.back(); + claimPoint.m_handle = CreateClaimHandle(sectorInfo, ++claimIndex); + claimPoint.m_position = position; + claimPoint.m_normal = normal; + claimPoint.m_masks = masks; + sectorInfo.m_baseContext.m_masks.AddSurfaceTagWeights(masks); + return true; + }); } void AreaSystemComponent::VegetationThreadTasks::UpdateSectorCallbacks(SectorInfo& sectorInfo) diff --git a/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp b/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp index ed81ebae87..efc8f0bdf9 100644 --- a/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp +++ b/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp @@ -328,7 +328,8 @@ namespace Vegetation AZ::Vector3 originalInstanceDataPosition = instanceData.m_position; m_points.EnumeratePoints( [&instanceData, originalInstanceDataPosition, &closestPointDistanceSq]( - const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + [[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, + const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { float distanceSq = position.GetDistanceSq(originalInstanceDataPosition); if (distanceSq < closestPointDistanceSq) diff --git a/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp b/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp index aa6c3a8440..986506f573 100644 --- a/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp +++ b/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp @@ -220,7 +220,7 @@ namespace Vegetation float instanceZ = instanceData.m_position.GetZ(); m_points.EnumeratePoints( [instanceZ, lowerZDistanceRange, upperZDistanceRange, &passesFilter]( - const AZ::Vector3& position, + [[maybe_unused]] size_t inPositionIndex, const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, [[maybe_unused]] const SurfaceData::SurfaceTagWeights& masks) -> bool { float pointZ = position.GetZ(); diff --git a/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.h b/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.h index 0a83ddeea3..9c43c91c69 100644 --- a/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.h +++ b/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include diff --git a/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp b/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp index abb577acfc..0bb8c57c44 100644 --- a/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp +++ b/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp @@ -862,7 +862,8 @@ void DebugComponent::PrepareNextReport() SurfaceData::SurfacePointList points; SurfaceData::SurfaceDataSystemRequestBus::Broadcast(&SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, pos, SurfaceData::SurfaceTagVector(), points); - timing.m_worldPosition = points.IsEmpty() ? pos : points.GetHighestSurfacePoint().m_position; + constexpr size_t inPositionIndex = 0; + timing.m_worldPosition = points.IsEmpty(inPositionIndex) ? pos : points.GetHighestSurfacePoint(inPositionIndex).m_position; return timing; }, [](const SectorTracker& sectorTracker, SectorTiming& sectorTiming) diff --git a/Gems/Vegetation/Code/Tests/VegetationMocks.h b/Gems/Vegetation/Code/Tests/VegetationMocks.h index 33a320b03e..dc09d13d92 100644 --- a/Gems/Vegetation/Code/Tests/VegetationMocks.h +++ b/Gems/Vegetation/Code/Tests/VegetationMocks.h @@ -333,18 +333,21 @@ namespace UnitTest void GetSurfacePoints([[maybe_unused]] const AZ::Vector3& inPosition, [[maybe_unused]] const SurfaceData::SurfaceTagVector& masks, SurfaceData::SurfacePointList& surfacePointList) const override { ++m_count; - surfacePointList.AddSurfacePoint(AZ::EntityId(), m_outPosition, m_outNormal, m_outMasks); + surfacePointList.Clear(); + surfacePointList.StartListConstruction(AZStd::span(&inPosition, 1), 1, {}); + surfacePointList.AddSurfacePoint(AZ::EntityId(), inPosition, m_outPosition, m_outNormal, m_outMasks); + surfacePointList.EndListConstruction(); } void GetSurfacePointsFromRegion([[maybe_unused]] const AZ::Aabb& inRegion, [[maybe_unused]] const AZ::Vector2 stepSize, [[maybe_unused]] const SurfaceData::SurfaceTagVector& desiredTags, - [[maybe_unused]] SurfaceData::SurfacePointLists& surfacePointListPerPosition) const override + [[maybe_unused]] SurfaceData::SurfacePointList& surfacePointListPerPosition) const override { } void GetSurfacePointsFromList( [[maybe_unused]] AZStd::span inPositions, [[maybe_unused]] const SurfaceData::SurfaceTagVector& desiredTags, - [[maybe_unused]] SurfaceData::SurfacePointLists& surfacePointLists) const override + [[maybe_unused]] SurfaceData::SurfacePointList& surfacePointLists) const override { }