diff --git a/Code/Framework/AzFramework/AzFramework/Visibility/OctreeSystemComponent.cpp b/Code/Framework/AzFramework/AzFramework/Visibility/OctreeSystemComponent.cpp index cdb7ce0dd9..9ca17a6736 100644 --- a/Code/Framework/AzFramework/AzFramework/Visibility/OctreeSystemComponent.cpp +++ b/Code/Framework/AzFramework/AzFramework/Visibility/OctreeSystemComponent.cpp @@ -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; diff --git a/Code/Framework/Tests/OctreeTests.cpp b/Code/Framework/Tests/OctreeTests.cpp index 29c7ef3ff3..27c4fdf0eb 100644 --- a/Code/Framework/Tests/OctreeTests.cpp +++ b/Code/Framework/Tests/OctreeTests.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -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::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 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()); + } }