Merge branch 'navmesh_fix_remove_tiles' into 'master'

Fix removing navmesh tiles at high player speed

See merge request OpenMW/openmw!1151

components/detournavigator/asyncnavmeshupdater.cpp

@@ -8,6 +8,8 @@
 #include <components/misc/thread.hpp>
 #include <components/loadinglistener/loadinglistener.hpp>
 
+#include <DetourNavMesh.h>
+
 #include <osg/Stats>
 
 #include <algorithm>
@@ -48,7 +50,7 @@ namespace
 
     auto getPriority(const Job& job) noexcept
     {
-        return std::make_tuple(job.mProcessTime, job.mTryNumber, job.mChangeType, job.mDistanceToPlayer, job.mDistanceToOrigin);
+        return std::make_tuple(job.mProcessTime, job.mChangeType, job.mTryNumber, job.mDistanceToPlayer, job.mDistanceToOrigin);
     }
 
     struct LessByJobPriority
@@ -102,7 +104,6 @@ namespace DetourNavigator
     {
         mShouldStop = true;
         std::unique_lock<std::mutex> lock(mMutex);
-        mThreadsQueues.clear();
         mWaiting.clear();
         mHasJob.notify_all();
         lock.unlock();
@@ -124,11 +125,19 @@ namespace DetourNavigator
         if (!playerTileChanged && changedTiles.empty())
             return;
 
+        const dtNavMeshParams params = *navMeshCacheItem->lockConst()->getImpl().getParams();
+
         const std::lock_guard<std::mutex> lock(mMutex);
 
         if (playerTileChanged)
+        {
             for (JobIt job : mWaiting)
+            {
                 job->mDistanceToPlayer = getManhattanDistance(job->mChangedTile, playerTile);
+                if (!shouldAddTile(job->mChangedTile, playerTile, std::min(mSettings.get().mMaxTilesNumber, params.maxTiles)))
+                    job->mChangeType = ChangeType::remove;
+            }
+        }
 
         for (const auto& [changedTile, changeType] : changedTiles)
         {
@@ -235,13 +244,20 @@ namespace DetourNavigator
     void AsyncNavMeshUpdater::reportStats(unsigned int frameNumber, osg::Stats& stats) const
     {
         std::size_t jobs = 0;
+        std::size_t waiting = 0;
+        std::size_t pushed = 0;
 
         {
             const std::lock_guard<std::mutex> lock(mMutex);
             jobs = mJobs.size();
+            waiting = mWaiting.size();
+            pushed = mPushed.size();
         }
 
-        stats.setAttribute(frameNumber, "NavMesh UpdateJobs", jobs);
+        stats.setAttribute(frameNumber, "NavMesh Jobs", jobs);
+        stats.setAttribute(frameNumber, "NavMesh Waiting", waiting);
+        stats.setAttribute(frameNumber, "NavMesh Pushed", pushed);
+        stats.setAttribute(frameNumber, "NavMesh Processing", mProcessingTiles.lockConst()->size());
 
         mNavMeshTilesCache.reportStats(frameNumber, stats);
     }
@@ -340,64 +356,38 @@ namespace DetourNavigator
     {
         std::unique_lock<std::mutex> lock(mMutex);
 
-        const auto threadId = std::this_thread::get_id();
+        bool shouldStop = false;
-        auto& threadQueue = mThreadsQueues[threadId];
+        const auto hasJob = [&]
-
-        while (true)
         {
-            bool shouldStop = false;
+            shouldStop = mShouldStop;
+            return shouldStop
+                || (!mWaiting.empty() && mWaiting.front()->mProcessTime <= std::chrono::steady_clock::now());
+        };
 
-            const auto hasJob = [&] {
+        if (!mHasJob.wait_for(lock, std::chrono::milliseconds(10), hasJob))
-                shouldStop = mShouldStop;
+        {
-                return shouldStop
+            if (mJobs.empty())
-                    || (!mWaiting.empty() && mWaiting.front()->mProcessTime <= std::chrono::steady_clock::now())
+                mDone.notify_all();
-                    || !threadQueue.empty();
+            return mJobs.end();
-            };
-
-            if (!mHasJob.wait_for(lock, std::chrono::milliseconds(10), hasJob))
-            {
-                if (mJobs.empty())
-                    mDone.notify_all();
-                return mJobs.end();
-            }
-
-            if (shouldStop)
-                return mJobs.end();
-
-            Log(Debug::Debug) << "Got " << mJobs.size() << " navigator jobs and "
-                << threadQueue.size() << " thread jobs by thread=" << std::this_thread::get_id();
-
-            const JobIt job = threadQueue.empty()
-                ? getJob(mWaiting, true)
-                : getJob(threadQueue, false);
-
-            if (job == mJobs.end())
-                continue;
-
-            const auto owner = lockTile(job->mAgentHalfExtents, job->mChangedTile);
-
-            if (owner == threadId)
-            {
-                mPushed.erase(getAgentAndTile(*job));
-                return job;
-            }
-
-            postThreadJob(job, mThreadsQueues[owner]);
         }
-    }
 
-    JobIt AsyncNavMeshUpdater::getJob(std::deque<JobIt>& jobs, bool changeLastUpdate)
+        if (shouldStop)
-    {
-        const auto now = std::chrono::steady_clock::now();
-        JobIt job = jobs.front();
-
-        if (job->mProcessTime > now)
             return mJobs.end();
 
-        jobs.pop_front();
+        const JobIt job = mWaiting.front();
 
-        if (changeLastUpdate && job->mChangeType == ChangeType::update)
+        mWaiting.pop_front();
-            mLastUpdates[getAgentAndTile(*job)] = now;
+
+        if (!lockTile(job->mAgentHalfExtents, job->mChangedTile))
+        {
+            ++job->mTryNumber;
+            insertPrioritizedJob(job, mWaiting);
+            return mJobs.end();
+        }
+
+        if (job->mChangeType == ChangeType::update)
+            mLastUpdates[getAgentAndTile(*job)] = std::chrono::steady_clock::now();
+        mPushed.erase(getAgentAndTile(*job));
 
         return job;
     }
@@ -435,7 +425,7 @@ namespace DetourNavigator
         if (mPushed.emplace(job->mAgentHalfExtents, job->mChangedTile).second)
         {
             ++job->mTryNumber;
-            mWaiting.push_back(job);
+            insertPrioritizedJob(job, mWaiting);
             mHasJob.notify_all();
             return;
         }
@@ -443,26 +433,11 @@ namespace DetourNavigator
         mJobs.erase(job);
     }
 
-    void AsyncNavMeshUpdater::postThreadJob(JobIt job, std::deque<JobIt>& queue)
+    bool AsyncNavMeshUpdater::lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile)
-    {
-        queue.push_back(job);
-        mHasJob.notify_all();
-    }
-
-    std::thread::id AsyncNavMeshUpdater::lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile)
     {
         if (mSettings.get().mAsyncNavMeshUpdaterThreads <= 1)
-            return std::this_thread::get_id();
+            return true;
-
+        return mProcessingTiles.lock()->emplace(agentHalfExtents, changedTile).second;
-        auto locked = mProcessingTiles.lock();
-        const auto tile = locked->find(std::make_tuple(agentHalfExtents, changedTile));
-        if (tile == locked->end())
-        {
-            const auto threadId = std::this_thread::get_id();
-            locked->emplace(std::tie(agentHalfExtents, changedTile), threadId);
-            return threadId;
-        }
-        return tile->second;
     }
 
     void AsyncNavMeshUpdater::unlockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile)

components/detournavigator/asyncnavmeshupdater.hpp

@@ -60,7 +60,7 @@ namespace DetourNavigator
         const TilePosition mChangedTile;
         const std::chrono::steady_clock::time_point mProcessTime;
         unsigned mTryNumber = 0;
-        const ChangeType mChangeType;
+        ChangeType mChangeType;
         int mDistanceToPlayer;
         const int mDistanceToOrigin;
 
@@ -99,10 +99,9 @@ namespace DetourNavigator
         std::set<std::tuple<osg::Vec3f, TilePosition>> mPushed;
         Misc::ScopeGuarded<TilePosition> mPlayerTile;
         NavMeshTilesCache mNavMeshTilesCache;
-        Misc::ScopeGuarded<std::map<std::tuple<osg::Vec3f, TilePosition>, std::thread::id>> mProcessingTiles;
+        Misc::ScopeGuarded<std::set<std::tuple<osg::Vec3f, TilePosition>>> mProcessingTiles;
         std::map<std::tuple<osg::Vec3f, TilePosition>, std::chrono::steady_clock::time_point> mLastUpdates;
         std::set<std::tuple<osg::Vec3f, TilePosition>> mPresentTiles;
-        std::map<std::thread::id, std::deque<JobIt>> mThreadsQueues;
         std::vector<std::thread> mThreads;
 
         void process() noexcept;
@@ -119,7 +118,7 @@ namespace DetourNavigator
 
         void repost(JobIt job);
 
-        std::thread::id lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile);
+        bool lockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile);
 
         void unlockTile(const osg::Vec3f& agentHalfExtents, const TilePosition& changedTile);
 

components/resource/stats.cpp

@@ -390,7 +390,10 @@ void StatsHandler::setUpScene(osgViewer::ViewerBase *viewer)
             "Land",
             "Composite",
             "",
-            "NavMesh UpdateJobs",
+            "NavMesh Jobs",
+            "NavMesh Waiting",
+            "NavMesh Pushed",
+            "NavMesh Processing",
             "NavMesh CacheSize",
             "NavMesh UsedTiles",
             "NavMesh CachedTiles",