From 0479ebf5ae9a0e4ddee58c3a3b4277ead38477ca Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 10 Mar 2019 14:36:16 +0300 Subject: [PATCH] Remove unused actors and navmeshes on update When there is only one actor (player) on a scene and it moving to other cell first it will be removed from navigator then added. Remove cause navmesh removing for its half extents. After it is added navmesh for same half extents is created and added. While this all happens there are still jobs for old navmesh are processing. Old navmesh still exists because it is stored by shared pointer. So jobs take tiles from cache and place them into old navmesh. After that other jobs take same tiles from cache (half extents and coordinates are equal) and place them into other navmesh. dtNavMesh changes tile data on add and remove. Adding tile to two dtNavMesh corrupts tile in both nameshes. --- .../detournavigator/navigator.cpp | 8 ------- components/detournavigator/navigatorimpl.cpp | 18 ++++++++++++--- components/detournavigator/navigatorimpl.hpp | 1 + components/detournavigator/navmeshmanager.cpp | 22 ++++++++++++++++++- components/detournavigator/navmeshmanager.hpp | 2 +- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/apps/openmw_test_suite/detournavigator/navigator.cpp b/apps/openmw_test_suite/detournavigator/navigator.cpp index 2bf7bf6438..5f5433b05f 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/navigatorimpl.cpp b/components/detournavigator/navigatorimpl.cpp index 1b83769f4c..2e16b6d04d 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 9750559844..8156f6655e 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/navmeshmanager.cpp b/components/detournavigator/navmeshmanager.cpp index 217c17e410..3e26915657 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 @@ -83,9 +97,15 @@ namespace DetourNavigator 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) diff --git a/components/detournavigator/navmeshmanager.hpp b/components/detournavigator/navmeshmanager.hpp index ae006da735..cee33c63df 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);