birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix for ATOM-15595 : OctreeNode silently evicts entries that are larger than the size of the root node (#870)

Browse Source 
- Fixed a bug where an entry would get removed from the octree when being updated if it was too large to be fully contained by the root node (the desired behavior is that it just lives in the root node)
- Added a unit test to ensure that large entries can exist in the root node
- Updated the unit tests to manually count the number of entries instead of relying on GetEntryCount, since GetEntryCount was reporting an unreliable count before this bug was fixed.
This commit is contained in:
Tommy Walton
2021-05-24 10:10:50 -07:00
committed by GitHub
parent 58adcf168f
commit 838202873a
2 changed files with 79 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Framework/AzFramework/AzFramework/Visibility/OctreeSystemComponent.cpp
+2 -1
View File
@@ -123,8 +123,9 @@ namespace AzFramework
OctreeNode* insertCheck = this;
while (insertCheck != nullptr)
{
if (AZ::ShapeIntersection::Contains(insertCheck->m_bounds, boundingVolume))
if (AZ::ShapeIntersection::Contains(insertCheck->m_bounds, boundingVolume) || !insertCheck->m_parent)
{
// Insert here if the entry is fully contained or if we've reached the root node
return insertCheck->Insert(octreeScene, entry);
}
insertCheck = insertCheck->m_parent;
Code/Framework/Tests/OctreeTests.cpp
+77 -18
View File
@@ -14,6 +14,7 @@
#include <AzCore/Console/IConsole.h>
#include <AzCore/Console/Console.h>
#include <AzCore/Name/NameDictionary.h>
#include <AzCore/Console/IConsole.h>
#include <AzFramework/Visibility/OctreeSystemComponent.h>
#include <random>
@@ -94,6 +95,20 @@ namespace UnitTest
AZ::Console* m_console;
};
void ValidateEntryCountEqualsExpectedCount(const IVisibilityScene* visScene, uint32_t expectedEntryCount)
{
// InsertOrUpdateEntry assumes that updating an existing entry won't change the count
// so it doesn't modify the counter used by GetEntryCount.
// If an entry is removed from the octree as an unintended side effect of updating an existing entry,
// GetEntryCount can't be relied upon to report the actual entry count.
// So manually count the entries when using the entry count for validation.
uint32_t manualEntryCount = 0;
visScene->EnumerateNoCull([&manualEntryCount](const AzFramework::IVisibilityScene::NodeData& nodeData) { manualEntryCount += nodeData.m_entries.size(); });
EXPECT_EQ(manualEntryCount, expectedEntryCount);
EXPECT_EQ(visScene->GetEntryCount(), expectedEntryCount);
}
TEST_F(OctreeTests, InsertDeleteSingleEntry)
{
AzFramework::VisibilityEntry visEntry;
@@ -102,11 +117,11 @@ namespace UnitTest
m_octreeScene->InsertOrUpdateEntry(visEntry);
EXPECT_TRUE(visEntry.m_internalNode != nullptr);
EXPECT_TRUE(visEntry.m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 1);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 1);
m_octreeScene->RemoveEntry(visEntry);
EXPECT_TRUE(visEntry.m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 0);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 0);
EXPECT_TRUE(true); //TEST
}
@@ -121,34 +136,34 @@ namespace UnitTest
m_octreeScene->InsertOrUpdateEntry(visEntry[0]);
EXPECT_TRUE(visEntry[0].m_internalNode != nullptr);
EXPECT_TRUE(visEntry[0].m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 1);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 1);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
m_octreeScene->InsertOrUpdateEntry(visEntry[1]); // This should force a split of the root node
EXPECT_TRUE(visEntry[1].m_internalNode != nullptr);
EXPECT_TRUE(visEntry[1].m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 2);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 2);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1 + m_octreeScene->GetChildNodeCount());
m_octreeScene->InsertOrUpdateEntry(visEntry[2]); // This should force a split of the roots +/+/+ child node
EXPECT_TRUE(visEntry[2].m_internalNode != nullptr);
EXPECT_TRUE(visEntry[2].m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 3);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 3);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1 + (2 * m_octreeScene->GetChildNodeCount()));
m_octreeScene->RemoveEntry(visEntry[2]);
EXPECT_TRUE(visEntry[2].m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 2);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 2);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1 + m_octreeScene->GetChildNodeCount());
m_octreeScene->RemoveEntry(visEntry[1]);
EXPECT_TRUE(visEntry[1].m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 1);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 1);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
m_octreeScene->RemoveEntry(visEntry[0]);
EXPECT_TRUE(visEntry[0].m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 0);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 0);
}
TEST_F(OctreeTests, UpdateSingleEntry)
@@ -159,19 +174,19 @@ namespace UnitTest
m_octreeScene->InsertOrUpdateEntry(visEntry);
EXPECT_TRUE(visEntry.m_internalNode != nullptr);
EXPECT_TRUE(visEntry.m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 1);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 1);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
visEntry.m_boundingVolume = AZ::Aabb::CreateFromMinMax(AZ::Vector3(-0.5f), AZ::Vector3(0.5f));
m_octreeScene->InsertOrUpdateEntry(visEntry);
EXPECT_TRUE(visEntry.m_internalNode != nullptr);
EXPECT_TRUE(visEntry.m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 1);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 1);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
m_octreeScene->RemoveEntry(visEntry);
EXPECT_TRUE(visEntry.m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 0);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 0);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
}
@@ -185,19 +200,19 @@ namespace UnitTest
m_octreeScene->InsertOrUpdateEntry(visEntry[0]);
EXPECT_TRUE(visEntry[0].m_internalNode != nullptr);
EXPECT_TRUE(visEntry[0].m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 1);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 1);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
m_octreeScene->InsertOrUpdateEntry(visEntry[1]); // This should force a split of the root node
EXPECT_TRUE(visEntry[1].m_internalNode != nullptr);
EXPECT_TRUE(visEntry[1].m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 2);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 2);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1 + m_octreeScene->GetChildNodeCount());
m_octreeScene->InsertOrUpdateEntry(visEntry[2]); // This should force a split of the roots +/+/+ child node
EXPECT_TRUE(visEntry[2].m_internalNode != nullptr);
EXPECT_TRUE(visEntry[2].m_internalNodeIndex == 0);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 3);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 3);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1 + (2 * m_octreeScene->GetChildNodeCount()));
visEntry[1].m_boundingVolume = AZ::Aabb::CreateFromMinMax(AZ::Vector3(-0.9f), AZ::Vector3(-0.6f));
@@ -206,22 +221,22 @@ namespace UnitTest
m_octreeScene->InsertOrUpdateEntry(visEntry[0]);
m_octreeScene->InsertOrUpdateEntry(visEntry[1]);
m_octreeScene->InsertOrUpdateEntry(visEntry[2]);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 3);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 3);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1 + (2 * m_octreeScene->GetChildNodeCount()));
m_octreeScene->RemoveEntry(visEntry[2]);
EXPECT_TRUE(visEntry[2].m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 2);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 2);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1 + m_octreeScene->GetChildNodeCount());
m_octreeScene->RemoveEntry(visEntry[1]);
EXPECT_TRUE(visEntry[1].m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 1);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 1);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
m_octreeScene->RemoveEntry(visEntry[0]);
EXPECT_TRUE(visEntry[0].m_internalNode == nullptr);
EXPECT_TRUE(m_octreeScene->GetEntryCount() == 0);
ValidateEntryCountEqualsExpectedCount(m_octreeScene, 0);
EXPECT_TRUE(m_octreeScene->GetNodeCount() == 1);
}
@@ -365,4 +380,48 @@ namespace UnitTest
AZ::Frustum bound3 = AZ::Frustum(AZ::ViewFrustumAttributes(frustumTransform, 1.0f, 2.0f * atanf(0.5f), 2.6f, 2.9f));
EnumerateMultipleEntriesHelper(m_octreeScene, bound1, bound2, bound3);
}
TEST_F(OctreeTests, InsertOrUpdateEntry_OverFillRootNodeWithLargeEntries_EntriesAreNotLost)
{
// Validate that the octree works if you exceed the max entry count with large entries,
// which will overfill the root node since they can't be distributed to child nodes
// Get the max extents and entries-per-node for the octree
AZ::IConsole* console = AZ::Interface<AZ::IConsole>::Get();
EXPECT_TRUE(console);
float maxExtents = 0.0f;
AZ::GetValueResult getCvarResult = console->GetCvarValue("bg_octreeMaxWorldExtents", maxExtents);
EXPECT_EQ(getCvarResult, AZ::GetValueResult::Success);
uint32_t maxEntriesPerNode = 0;
getCvarResult = console->GetCvarValue("bg_octreeNodeMaxEntries", maxEntriesPerNode);
EXPECT_EQ(getCvarResult, AZ::GetValueResult::Success);
// Create root entries that would exceed the size of the root node
AZ::Aabb exceedMaxExtents = AZ::Aabb::CreateFromMinMax(AZ::Vector3(-maxExtents - 1.0f), AZ::Vector3(maxExtents + 1.0f));
uint32_t exceedMaxEntriesPerNode = maxEntriesPerNode + 1;
AzFramework::VisibilityEntry visEntry;
visEntry.m_boundingVolume = exceedMaxExtents;
AZStd::vector<AzFramework::VisibilityEntry> visEntries(exceedMaxEntriesPerNode, visEntry);
// Insert them all into the scene
for (AzFramework::VisibilityEntry& entry : visEntries)
{
m_octreeScene->InsertOrUpdateEntry(entry);
}
// Expect all the entries to be in the scene
ValidateEntryCountEqualsExpectedCount(m_octreeScene, visEntries.size());
// Update them, without making any actual changes
for (AzFramework::VisibilityEntry& entry : visEntries)
{
m_octreeScene->InsertOrUpdateEntry(entry);
}
// Expect all the entries to be in the scene
ValidateEntryCountEqualsExpectedCount(m_octreeScene, visEntries.size());
}
}