diff --git a/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp b/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp index 93fc5fbaf..8b1ee230d 100644 --- a/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp +++ b/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp @@ -55,17 +55,22 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_should_return_cached_value) { - const std::size_t maxSize = 1; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 49; + const std::size_t maxSize = navMeshDataSize + navMeshKeySize; NavMeshTilesCache cache(maxSize); const auto result = cache.set(mAgentHalfExtents, mTilePosition, mRecastMesh, mOffMeshConnections, std::move(mNavMeshData)); + ASSERT_TRUE(result); EXPECT_EQ(result.get(), (NavMeshDataRef {mData, 1})); } TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_existing_element_should_throw_exception) { - const std::size_t maxSize = 2; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 49; + const std::size_t maxSize = 2 * (navMeshDataSize + navMeshKeySize); NavMeshTilesCache cache(maxSize); const auto anotherData = reinterpret_cast(dtAlloc(1, DT_ALLOC_PERM)); NavMeshData anotherNavMeshData {anotherData, 1}; @@ -79,7 +84,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, get_should_return_cached_value) { - const std::size_t maxSize = 1; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 49; + const std::size_t maxSize = navMeshDataSize + navMeshKeySize; NavMeshTilesCache cache(maxSize); cache.set(mAgentHalfExtents, mTilePosition, mRecastMesh, mOffMeshConnections, std::move(mNavMeshData)); @@ -121,7 +128,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_should_replace_unused_value) { - const std::size_t maxSize = 1; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 117; + const std::size_t maxSize = navMeshDataSize + navMeshKeySize; NavMeshTilesCache cache(maxSize); const std::vector water {1, RecastMesh::Water {1, btTransform::getIdentity()}}; @@ -139,7 +148,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_should_not_replace_used_value) { - const std::size_t maxSize = 1; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 49; + const std::size_t maxSize = navMeshDataSize + navMeshKeySize; NavMeshTilesCache cache(maxSize); const std::vector water {1, RecastMesh::Water {1, btTransform::getIdentity()}}; @@ -155,7 +166,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_should_replace_unused_least_recently_set_value) { - const std::size_t maxSize = 2; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 117; + const std::size_t maxSize = 2 * (navMeshDataSize + navMeshKeySize); NavMeshTilesCache cache(maxSize); const std::vector leastRecentlySetWater {1, RecastMesh::Water {1, btTransform::getIdentity()}}; @@ -185,7 +198,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_should_replace_unused_least_recently_used_value) { - const std::size_t maxSize = 2; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 117; + const std::size_t maxSize = 2 * (navMeshDataSize + navMeshKeySize); NavMeshTilesCache cache(maxSize); const std::vector leastRecentlyUsedWater {1, RecastMesh::Water {1, btTransform::getIdentity()}}; @@ -227,7 +242,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_should_not_replace_unused_least_recently_used_value_when_item_does_not_not_fit_cache_max_size) { - const std::size_t maxSize = 1; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 49; + const std::size_t maxSize = 2 * (navMeshDataSize + navMeshKeySize); NavMeshTilesCache cache(maxSize); const std::vector water {1, RecastMesh::Water {1, btTransform::getIdentity()}}; @@ -243,7 +260,10 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, set_should_not_replace_unused_least_recently_used_value_when_item_does_not_not_fit_size_of_unused_items) { - const std::size_t maxSize = 2; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize1 = 49; + const std::size_t navMeshKeySize2 = 117; + const std::size_t maxSize = 2 * navMeshDataSize + navMeshKeySize1 + navMeshKeySize2; NavMeshTilesCache cache(maxSize); const std::vector anotherWater {1, RecastMesh::Water {1, btTransform::getIdentity()}}; @@ -269,7 +289,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, release_used_after_set_then_used_by_get_item_should_left_this_item_available) { - const std::size_t maxSize = 1; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 49; + const std::size_t maxSize = navMeshDataSize + navMeshKeySize; NavMeshTilesCache cache(maxSize); const std::vector water {1, RecastMesh::Water {1, btTransform::getIdentity()}}; @@ -290,7 +312,9 @@ namespace TEST_F(DetourNavigatorNavMeshTilesCacheTest, release_twice_used_item_should_left_this_item_available) { - const std::size_t maxSize = 1; + const std::size_t navMeshDataSize = 1; + const std::size_t navMeshKeySize = 49; + const std::size_t maxSize = navMeshDataSize + navMeshKeySize; NavMeshTilesCache cache(maxSize); const std::vector water {1, RecastMesh::Water {1, btTransform::getIdentity()}}; diff --git a/components/detournavigator/navmeshtilescache.cpp b/components/detournavigator/navmeshtilescache.cpp index 8c5fc6381..484831493 100644 --- a/components/detournavigator/navmeshtilescache.cpp +++ b/components/detournavigator/navmeshtilescache.cpp @@ -1,6 +1,8 @@ #include "navmeshtilescache.hpp" #include "exceptions.hpp" +#include + namespace DetourNavigator { namespace @@ -61,6 +63,7 @@ namespace DetourNavigator if (tileValues == agentValues->second.end()) return Value(); + // TODO: use different function to make key to avoid unnecessary std::string allocation const auto tile = tileValues->second.find(makeNavMeshKey(recastMesh, offMeshConnections)); if (tile == tileValues->second.end()) return Value(); @@ -84,11 +87,17 @@ namespace DetourNavigator if (navMeshSize > mFreeNavMeshDataSize + (mMaxNavMeshDataSize - mUsedNavMeshDataSize)) return Value(); - while (!mFreeItems.empty() && mUsedNavMeshDataSize + navMeshSize > mMaxNavMeshDataSize) + const auto navMeshKey = makeNavMeshKey(recastMesh, offMeshConnections); + const auto itemSize = navMeshSize + navMeshKey.size(); + + if (itemSize > mFreeNavMeshDataSize + (mMaxNavMeshDataSize - mUsedNavMeshDataSize)) + return Value(); + + while (!mFreeItems.empty() && mUsedNavMeshDataSize + itemSize > mMaxNavMeshDataSize) removeLeastRecentlyUsed(); - const auto navMeshKey = makeNavMeshKey(recastMesh, offMeshConnections); const auto iterator = mFreeItems.emplace(mFreeItems.end(), agentHalfExtents, changedTile, navMeshKey); + // TODO: use std::string_view or some alternative to avoid navMeshKey copy into both mFreeItems and mValues const auto emplaced = mValues[agentHalfExtents][changedTile].emplace(navMeshKey, iterator); if (!emplaced.second) @@ -98,8 +107,8 @@ namespace DetourNavigator } iterator->mNavMeshData = std::move(value); - mUsedNavMeshDataSize += navMeshSize; - mFreeNavMeshDataSize += navMeshSize; + mUsedNavMeshDataSize += itemSize; + mFreeNavMeshDataSize += itemSize; acquireItemUnsafe(iterator); @@ -122,7 +131,7 @@ namespace DetourNavigator if (value == tileValues->second.end()) return; - mUsedNavMeshDataSize -= static_cast(item.mNavMeshData.mSize); + mUsedNavMeshDataSize -= static_cast(item.mNavMeshData.mSize) + item.mNavMeshKey.size(); mFreeItems.pop_back(); tileValues->second.erase(value); @@ -142,7 +151,7 @@ namespace DetourNavigator return; mBusyItems.splice(mBusyItems.end(), mFreeItems, iterator); - mFreeNavMeshDataSize -= static_cast(iterator->mNavMeshData.mSize); + mFreeNavMeshDataSize -= static_cast(iterator->mNavMeshData.mSize) + iterator->mNavMeshKey.size(); } void NavMeshTilesCache::releaseItem(ItemIterator iterator) @@ -153,6 +162,6 @@ namespace DetourNavigator const std::lock_guard lock(mMutex); mFreeItems.splice(mFreeItems.begin(), mBusyItems, iterator); - mFreeNavMeshDataSize += static_cast(iterator->mNavMeshData.mSize); + mFreeNavMeshDataSize += static_cast(iterator->mNavMeshData.mSize) + iterator->mNavMeshKey.size(); } }