diff --git a/apps/openmw/mwrender/renderingmanager.cpp b/apps/openmw/mwrender/renderingmanager.cpp index f54e423f1..c93707374 100644 --- a/apps/openmw/mwrender/renderingmanager.cpp +++ b/apps/openmw/mwrender/renderingmanager.cpp @@ -1446,7 +1446,7 @@ namespace MWRender { try { - const auto locked = it->second.lockConst(); + const auto locked = it->second->lockConst(); mNavMesh->update(locked->getValue(), mNavMeshNumber, locked->getGeneration(), locked->getNavMeshRevision(), mNavigator.getSettings()); } diff --git a/apps/openmw_test_suite/detournavigator/navigator.cpp b/apps/openmw_test_suite/detournavigator/navigator.cpp index 2bf7bf643..5f5433b05 100644 --- a/apps/openmw_test_suite/detournavigator/navigator.cpp +++ b/apps/openmw_test_suite/detournavigator/navigator.cpp @@ -80,14 +80,6 @@ namespace EXPECT_THROW(mNavigator->findPath(mAgentHalfExtents, mStepSize, mStart, mEnd, Flag_walk, mOut), NavigatorException); } - TEST_F(DetourNavigatorNavigatorTest, find_path_for_removed_agent_should_return_empty) - { - mNavigator->addAgent(mAgentHalfExtents); - mNavigator->removeAgent(mAgentHalfExtents); - mNavigator->findPath(mAgentHalfExtents, mStepSize, mStart, mEnd, Flag_walk, mOut); - EXPECT_EQ(mPath, std::deque()); - } - TEST_F(DetourNavigatorNavigatorTest, add_agent_should_count_each_agent) { mNavigator->addAgent(mAgentHalfExtents); diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index ee3b6d77a..e9172f041 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -129,12 +129,17 @@ namespace DetourNavigator const auto firstStart = setFirstStart(start); + const auto navMeshCacheItem = job.mNavMeshCacheItem.lock(); + + if (!navMeshCacheItem) + return true; + const auto recastMesh = mRecastMeshManager.get().getMesh(job.mChangedTile); const auto playerTile = *mPlayerTile.lockConst(); const auto offMeshConnections = mOffMeshConnectionsManager.get().get(job.mChangedTile); const auto status = updateNavMesh(job.mAgentHalfExtents, recastMesh.get(), job.mChangedTile, playerTile, - offMeshConnections, mSettings, job.mNavMeshCacheItem, mNavMeshTilesCache); + offMeshConnections, mSettings, navMeshCacheItem, mNavMeshTilesCache); const auto finish = std::chrono::steady_clock::now(); @@ -143,7 +148,7 @@ namespace DetourNavigator using FloatMs = std::chrono::duration; { - const auto locked = job.mNavMeshCacheItem.lockConst(); + const auto locked = navMeshCacheItem->lockConst(); log("cache updated for agent=", job.mAgentHalfExtents, " status=", status, " generation=", locked->getGeneration(), " revision=", locked->getNavMeshRevision(), @@ -194,7 +199,8 @@ namespace DetourNavigator writeToFile(*recastMesh, mSettings.get().mRecastMeshPathPrefix + std::to_string(job.mChangedTile.x()) + "_" + std::to_string(job.mChangedTile.y()) + "_", recastMeshRevision); if (mSettings.get().mEnableWriteNavMeshToFile) - writeToFile(job.mNavMeshCacheItem.lockConst()->getValue(), mSettings.get().mNavMeshPathPrefix, navMeshRevision); + if (const auto shared = job.mNavMeshCacheItem.lock()) + writeToFile(shared->lockConst()->getValue(), mSettings.get().mNavMeshPathPrefix, navMeshRevision); } std::chrono::steady_clock::time_point AsyncNavMeshUpdater::setFirstStart(const std::chrono::steady_clock::time_point& value) diff --git a/components/detournavigator/asyncnavmeshupdater.hpp b/components/detournavigator/asyncnavmeshupdater.hpp index 98359964d..55e502b45 100644 --- a/components/detournavigator/asyncnavmeshupdater.hpp +++ b/components/detournavigator/asyncnavmeshupdater.hpp @@ -48,7 +48,7 @@ namespace DetourNavigator struct Job { osg::Vec3f mAgentHalfExtents; - SharedNavMeshCacheItem mNavMeshCacheItem; + std::weak_ptr mNavMeshCacheItem; TilePosition mChangedTile; unsigned mTryNumber; ChangeType mChangeType; diff --git a/components/detournavigator/makenavmesh.cpp b/components/detournavigator/makenavmesh.cpp index 5ac28127e..5d2de481e 100644 --- a/components/detournavigator/makenavmesh.cpp +++ b/components/detournavigator/makenavmesh.cpp @@ -521,7 +521,7 @@ namespace UpdateNavMeshStatus replaceTile(const SharedNavMeshCacheItem& navMeshCacheItem, const TilePosition& changedTile, T&& navMeshData) { - const auto locked = navMeshCacheItem.lock(); + const auto locked = navMeshCacheItem->lock(); auto& navMesh = locked->getValue(); const int layer = 0; const auto tileRef = navMesh.getTileRefAt(changedTile.x(), changedTile.y(), layer); @@ -591,14 +591,14 @@ namespace DetourNavigator " playerTile=", playerTile, " changedTileDistance=", getDistance(changedTile, playerTile)); - const auto params = *navMeshCacheItem.lockConst()->getValue().getParams(); + const auto params = *navMeshCacheItem->lockConst()->getValue().getParams(); const osg::Vec3f origin(params.orig[0], params.orig[1], params.orig[2]); const auto x = changedTile.x(); const auto y = changedTile.y(); const auto removeTile = [&] { - const auto locked = navMeshCacheItem.lock(); + const auto locked = navMeshCacheItem->lock(); auto& navMesh = locked->getValue(); const auto tileRef = navMesh.getTileRefAt(x, y, 0); const auto removed = dtStatusSucceed(navMesh.removeTile(tileRef, nullptr, nullptr)); diff --git a/components/detournavigator/navigator.hpp b/components/detournavigator/navigator.hpp index a146fe0fb..cc62d6164 100644 --- a/components/detournavigator/navigator.hpp +++ b/components/detournavigator/navigator.hpp @@ -174,7 +174,7 @@ namespace DetourNavigator if (!navMesh) return out; const auto settings = getSettings(); - return findSmoothPath(navMesh.lock()->getValue(), toNavMeshCoordinates(settings, agentHalfExtents), + return findSmoothPath(navMesh->lockConst()->getValue(), toNavMeshCoordinates(settings, agentHalfExtents), toNavMeshCoordinates(settings, stepSize), toNavMeshCoordinates(settings, start), toNavMeshCoordinates(settings, end), includeFlags, settings, out); } diff --git a/components/detournavigator/navigatorimpl.cpp b/components/detournavigator/navigatorimpl.cpp index 1b83769f4..2e16b6d04 100644 --- a/components/detournavigator/navigatorimpl.cpp +++ b/components/detournavigator/navigatorimpl.cpp @@ -21,10 +21,10 @@ namespace DetourNavigator void NavigatorImpl::removeAgent(const osg::Vec3f& agentHalfExtents) { const auto it = mAgents.find(agentHalfExtents); - if (it == mAgents.end() || --it->second) + if (it == mAgents.end()) return; - mAgents.erase(it); - mNavMeshManager.reset(agentHalfExtents); + if (it->second > 0) + --it->second; } bool NavigatorImpl::addObject(const ObjectId id, const btCollisionShape& shape, const btTransform& transform) @@ -113,6 +113,7 @@ namespace DetourNavigator void NavigatorImpl::update(const osg::Vec3f& playerPosition) { + removeUnusedNavMeshes(); for (const auto& v : mAgents) mNavMeshManager.update(playerPosition, v.first); } @@ -156,4 +157,15 @@ namespace DetourNavigator inserted.first->second = updateId; } } + + void NavigatorImpl::removeUnusedNavMeshes() + { + for (auto it = mAgents.begin(); it != mAgents.end();) + { + if (it->second == 0 && mNavMeshManager.reset(it->first)) + it = mAgents.erase(it); + else + ++it; + } + } } diff --git a/components/detournavigator/navigatorimpl.hpp b/components/detournavigator/navigatorimpl.hpp index 975055984..8156f6655 100644 --- a/components/detournavigator/navigatorimpl.hpp +++ b/components/detournavigator/navigatorimpl.hpp @@ -58,6 +58,7 @@ namespace DetourNavigator void updateAvoidShapeId(const ObjectId id, const ObjectId avoidId); void updateWaterShapeId(const ObjectId id, const ObjectId waterId); void updateId(const ObjectId id, const ObjectId waterId, std::unordered_map& ids); + void removeUnusedNavMeshes(); }; } diff --git a/components/detournavigator/navmeshcacheitem.hpp b/components/detournavigator/navmeshcacheitem.hpp index e64b9d138..e6ca60ef7 100644 --- a/components/detournavigator/navmeshcacheitem.hpp +++ b/components/detournavigator/navmeshcacheitem.hpp @@ -64,7 +64,8 @@ namespace DetourNavigator std::map> mUsedTiles; }; - using SharedNavMeshCacheItem = Misc::SharedGuarded; + using GuardedNavMeshCacheItem = Misc::ScopeGuarded; + using SharedNavMeshCacheItem = std::shared_ptr; } #endif diff --git a/components/detournavigator/navmeshmanager.cpp b/components/detournavigator/navmeshmanager.cpp index 217c17e41..03a8b055f 100644 --- a/components/detournavigator/navmeshmanager.cpp +++ b/components/detournavigator/navmeshmanager.cpp @@ -16,6 +16,20 @@ namespace { return current == add ? current : ChangeType::mixed; } + + /// Safely reset shared_ptr with definite underlying object destrutor call. + /// Assuming there is another thread holding copy of this shared_ptr or weak_ptr to this shared_ptr. + template + bool resetIfUnique(std::shared_ptr& ptr) + { + const std::weak_ptr weak = std::move(ptr); + if (auto shared = weak.lock()) + { + ptr = std::move(shared); + return false; + } + return true; + } } namespace DetourNavigator @@ -79,13 +93,19 @@ namespace DetourNavigator if (cached != mCache.end()) return; mCache.insert(std::make_pair(agentHalfExtents, - std::make_shared(makeEmptyNavMesh(mSettings), ++mGenerationCounter))); + std::make_shared(makeEmptyNavMesh(mSettings), ++mGenerationCounter))); log("cache add for agent=", agentHalfExtents); } - void NavMeshManager::reset(const osg::Vec3f& agentHalfExtents) + bool NavMeshManager::reset(const osg::Vec3f& agentHalfExtents) { + const auto it = mCache.find(agentHalfExtents); + if (it == mCache.end()) + return true; + if (!resetIfUnique(it->second)) + return false; mCache.erase(agentHalfExtents); + return true; } void NavMeshManager::addOffMeshConnection(const ObjectId id, const osg::Vec3f& start, const osg::Vec3f& end) @@ -139,7 +159,7 @@ namespace DetourNavigator } const auto changedTiles = mChangedTiles.find(agentHalfExtents); { - const auto locked = cached.lock(); + const auto locked = cached->lockConst(); const auto& navMesh = locked->getValue(); if (changedTiles != mChangedTiles.end()) { diff --git a/components/detournavigator/navmeshmanager.hpp b/components/detournavigator/navmeshmanager.hpp index ae006da73..cee33c63d 100644 --- a/components/detournavigator/navmeshmanager.hpp +++ b/components/detournavigator/navmeshmanager.hpp @@ -36,7 +36,7 @@ namespace DetourNavigator bool removeWater(const osg::Vec2i& cellPosition); - void reset(const osg::Vec3f& agentHalfExtents); + bool reset(const osg::Vec3f& agentHalfExtents); void addOffMeshConnection(const ObjectId id, const osg::Vec3f& start, const osg::Vec3f& end); diff --git a/components/misc/guarded.hpp b/components/misc/guarded.hpp index 114a77b10..559476867 100644 --- a/components/misc/guarded.hpp +++ b/components/misc/guarded.hpp @@ -83,38 +83,6 @@ namespace Misc std::mutex mMutex; T mValue; }; - - template - class SharedGuarded - { - public: - SharedGuarded() - : mMutex(std::make_shared()), mValue() - {} - - SharedGuarded(std::shared_ptr value) - : mMutex(std::make_shared()), mValue(std::move(value)) - {} - - Locked lock() const - { - return Locked(*mMutex, *mValue); - } - - Locked lockConst() const - { - return Locked(*mMutex, *mValue); - } - - operator bool() const - { - return static_cast(mValue); - } - - private: - std::shared_ptr mMutex; - std::shared_ptr mValue; - }; } #endif