Optimize SurfaceData bulk queries (#7593)

* Add comparison operators to SurfaceTagWeight.

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

* Changed AddSurfaceTagWeight to always combine weights.
This simplifies the API a bit and defines the behavior if someone ever tries to add a duplicate tag.

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

* Added benchmarks for measuring the performance-critical APIs.

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

* Changed SurfaceTagWeights to a fixed_vector.

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

* Add inPosition to AddSurfacePoint.
This will be used to detect which input the surface point is associated with.

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

* Add inPositionIndex to the appropriate APIs.

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

* Switched Gradient Surface benchmarks to use actual surface components.
The gradient unit tests and benchmarks were previously using a mock surface data system, which led to misleading benchmark results. Now, the actual SurfaceData system gets constructed, and the tests use a mock provider, but the benchmarks use actual shape providers for more realistic benchmarking.

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

* Fixed unit tests to have better query ranges.
Half of each previous range was querying outside the surface provider's data.

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

* First attempt at removing SurfacePointLists.
This currently runs significantly slower than the previous code but passes the unit tests.

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

* Another attempt at optimization.
This one runs faster than the previous, but still slow.

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

* Fix the cmake dependency so that the gradient tests rebuild SurfaceData.dll when run.

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

* Switch SurfaceAltitudeGradient over to the new bulk API.
Also, optimized the non-bulk API by having it reuse the SurfacePointList to avoid the repeated allocation / deallocation cost.

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

* Switched to using an indirect index so that all allocations are consecutive in our reserved buffer.

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

* Switched back to SurfaceTagWeight again.

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

* Added runtime dependency to LmbrCentral for unit tests.

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

* Switched code over to use the full EnumeratePoints in most cases.

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

* Added knowledge of max surface point creation into the system.

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

* Add generic GetSurfacePointsFromList API implementation for surface providers.

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

* Fixed implementation to use the correct maximum number of input points based on the surface providers being queried.

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

* Fix out-of-bounds references on empty lists.

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

* Fix memory allocation that caused benchmark runs to crash.

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

* Starting to clean up the API.

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

* Move SurfacePointList into separate files for easier maintainability.

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

* Fixed bug where too many points were filtered out due to using the position Z as a part of the AABB check.

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

* Made FilterPoints an internal part of SurfacePointList so we can choose when and how to perform the filtering.

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

* Final cleanup / comments.

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

* Added includes for non-unity builds.

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

* Changed how unit tests initialize the mock lists to try and fix the linux errors.

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

* Fixed compile error.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
This commit is contained in:
Mike Balfour
2022-02-14 16:27:53 -06:00
committed by GitHub
parent d2cdb511d0
commit 8b82041361
32 changed files with 970 additions and 514 deletions
@@ -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<decltype(m_registrationMutex)> 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<const AZ::Vector3>(&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<size_t>(ceil(inRegion.GetXExtent() / stepSize.GetX())) *
aznumeric_cast<size_t>(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<const AZ::Vector3> inPositions, const SurfaceTagVector& desiredTags, SurfacePointLists& surfacePointLists) const
AZStd::span<const AZ::Vector3> inPositions,
const SurfaceTagVector& desiredTags,
SurfacePointList& surfacePointLists) const
{
AZ::Aabb inBounds = AZ::Aabb::CreateNull();
for (auto& position : inPositions)
{
inBounds.AddPoint(position);
}
GetSurfacePointsFromListInternal(inPositions, inBounds, desiredTags, surfacePointLists);
}
void SurfaceDataSystemComponent::GetSurfacePointsFromListInternal(
AZStd::span<const AZ::Vector3> inPositions, const AZ::Aabb& inPositionBounds,
const SurfaceTagVector& desiredTags, SurfacePointList& surfacePointLists) const
{
AZStd::shared_lock<decltype(m_registrationMutex)> registrationLock(m_registrationMutex);
const size_t totalQueryPositions = inPositions.size();
surfacePointLists.clear();
surfacePointLists.resize(totalQueryPositions);
for (auto& surfacePointList : surfacePointLists)
{
surfacePointList.ReserveSpace(m_registeredSurfaceDataProviders.size());
}
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<const SurfaceTag> 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<decltype(m_registrationMutex)> 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<decltype(m_registrationMutex)> 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<decltype(m_registrationMutex)> 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<decltype(m_registrationMutex)> registrationLock(m_registrationMutex);
auto entryItr = m_registeredSurfaceDataModifiers.find(handle);
if (entryItr != m_registeredSurfaceDataModifiers.end())