Limit NavMeshManager update range by player tile and max tiles

Object AABB may be much larger than area currently covered by navmesh. In this
case all tiles beyond covered range should be ignored. Attempt to iterate over
them will not result in any new tile updates but can take quite a while. At
maximum this can be pow(INT_MAX - INT_MIN, 2) iterations.

Use arbitrary time limit to check for update call to finish in the test.

apps/openmw_test_suite/detournavigator/navigator.cpp

@@ -20,9 +20,12 @@
 #include <gtest/gtest.h>
 
 #include <array>
+#include <condition_variable>
 #include <deque>
 #include <limits>
 #include <memory>
+#include <mutex>
+#include <thread>
 
 MATCHER_P3(Vec3fEq, x, y, z, "")
 {
@@ -1226,4 +1229,33 @@ namespace
             findPath(*mNavigator, agentBounds, mStepSize, mStart, mEnd, Flag_walk, mAreaCosts, mEndTolerance, mOut),
             Status::NavMeshNotFound);
     }
+
+    TEST_F(DetourNavigatorNavigatorTest, update_for_very_big_object_should_be_limited)
+    {
+        CollisionShapeInstance bigBox(std::make_unique<btBoxShape>(btVector3(1e9, 1e9, 1e9)));
+
+        mNavigator->updateBounds(mPlayerPosition, nullptr);
+        mNavigator->addAgent(mAgentBounds);
+        mNavigator->addObject(ObjectId(&bigBox.shape()), ObjectShapes(bigBox.instance(), mObjectTransform),
+            btTransform::getIdentity(), nullptr);
+
+        bool updated = false;
+        std::condition_variable updateFinished;
+        std::mutex mutex;
+
+        std::thread thread([&] {
+            mNavigator->update(mPlayerPosition, nullptr);
+            std::lock_guard lock(mutex);
+            updated = true;
+            updateFinished.notify_all();
+        });
+
+        {
+            std::unique_lock lock(mutex);
+            updateFinished.wait_for(lock, std::chrono::seconds(3), [&] { return updated; });
+            ASSERT_TRUE(updated);
+        }
+
+        thread.join();
+    }
 }

apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp

@@ -1,4 +1,5 @@
 
+#include <components/detournavigator/debug.hpp>
 #include <components/detournavigator/settingsutils.hpp>
 #include <components/detournavigator/tilecachedrecastmeshmanager.hpp>
 #include <components/esm/refid.hpp>
@@ -82,10 +83,11 @@ namespace
         TileCachedRecastMeshManager manager(mSettings);
         const btBoxShape boxShape(btVector3(20, 20, 100));
         const CollisionShape shape(mInstance, boxShape, mObjectTransform);
-        TileBounds bounds;
-        bounds.mMin = osg::Vec2f(182, 182);
-        bounds.mMax = osg::Vec2f(1000, 1000);
-        manager.setBounds(bounds, nullptr);
+        const TilesPositionsRange range{
+            .mBegin = TilePosition(0, 0),
+            .mEnd = TilePosition(1, 1),
+        };
+        manager.setRange(range, nullptr);
         manager.addObject(ObjectId(&boxShape), shape, btTransform::getIdentity(), AreaType::AreaType_ground, nullptr);
         EXPECT_THAT(manager.takeChangedTiles(nullptr), ElementsAre(std::pair(TilePosition(0, 0), ChangeType::add)));
     }
@@ -97,10 +99,11 @@ namespace
         const btTransform transform(
             btMatrix3x3::getIdentity(), btVector3(getTileSize(mSettings) / mSettings.mRecastScaleFactor, 0, 0));
         const CollisionShape shape(mInstance, boxShape, mObjectTransform);
-        TileBounds bounds;
-        bounds.mMin = osg::Vec2f(-1000, -1000);
-        bounds.mMax = osg::Vec2f(1000, 1000);
-        manager.setBounds(bounds, nullptr);
+        const TilesPositionsRange range{
+            .mBegin = TilePosition(-1, -1),
+            .mEnd = TilePosition(2, 2),
+        };
+        manager.setRange(range, nullptr);
         manager.addObject(ObjectId(&boxShape), shape, transform, AreaType::AreaType_ground, nullptr);
         manager.takeChangedTiles(nullptr);
         EXPECT_TRUE(
@@ -130,10 +133,11 @@ namespace
         TileCachedRecastMeshManager manager(mSettings);
         const btBoxShape boxShape(btVector3(20, 20, 100));
         const CollisionShape shape(mInstance, boxShape, mObjectTransform);
-        TileBounds bounds;
-        bounds.mMin = osg::Vec2f(182, 182);
-        bounds.mMax = osg::Vec2f(1000, 1000);
-        manager.setBounds(bounds, nullptr);
+        const TilesPositionsRange range{
+            .mBegin = TilePosition(0, 0),
+            .mEnd = TilePosition(1, 1),
+        };
+        manager.setRange(range, nullptr);
         manager.addObject(ObjectId(&boxShape), shape, btTransform::getIdentity(), AreaType::AreaType_ground, nullptr);
         manager.takeChangedTiles(nullptr);
         manager.removeObject(ObjectId(&boxShape), nullptr);
@@ -169,10 +173,11 @@ namespace
         get_mesh_for_moved_object_should_return_recast_mesh_for_each_used_tile)
     {
         TileCachedRecastMeshManager manager(mSettings);
-        TileBounds bounds;
-        bounds.mMin = osg::Vec2f(-1000, -1000);
-        bounds.mMax = osg::Vec2f(1000, 1000);
-        manager.setBounds(bounds, nullptr);
+        const TilesPositionsRange range{
+            .mBegin = TilePosition(-1, -1),
+            .mEnd = TilePosition(1, 1),
+        };
+        manager.setRange(range, nullptr);
         manager.setWorldspace(mWorldspace, nullptr);
 
         const btBoxShape boxShape(btVector3(20, 20, 100));
@@ -452,15 +457,18 @@ namespace
         TileCachedRecastMeshManager manager(mSettings);
         const btBoxShape boxShape(btVector3(20, 20, 100));
         const CollisionShape shape(mInstance, boxShape, mObjectTransform);
-        TileBounds bounds;
-        bounds.mMin = osg::Vec2f(182, 0);
-        bounds.mMax = osg::Vec2f(1000, 1000);
-        manager.setBounds(bounds, nullptr);
+        const TilesPositionsRange range1{
+            .mBegin = TilePosition(0, 0),
+            .mEnd = TilePosition(1, 1),
+        };
+        manager.setRange(range1, nullptr);
         manager.addObject(ObjectId(&boxShape), shape, btTransform::getIdentity(), AreaType::AreaType_ground, nullptr);
-        bounds.mMin = osg::Vec2f(-1000, -1000);
-        bounds.mMax = osg::Vec2f(0, -182);
+        const TilesPositionsRange range2{
+            .mBegin = TilePosition(-1, -1),
+            .mEnd = TilePosition(0, 0),
+        };
         manager.takeChangedTiles(nullptr);
-        manager.setBounds(bounds, nullptr);
+        manager.setRange(range2, nullptr);
         EXPECT_THAT(manager.takeChangedTiles(nullptr),
             ElementsAre(
                 std::pair(TilePosition(-1, -1), ChangeType::add), std::pair(TilePosition(0, 0), ChangeType::remove)));

components/detournavigator/navmeshmanager.cpp

@@ -41,14 +41,18 @@ namespace DetourNavigator
 {
     namespace
     {
-        TileBounds makeBounds(const RecastSettings& settings, const osg::Vec2f& center, int maxTiles)
+        TilesPositionsRange makeRange(const TilePosition& center, int maxTiles)
         {
-            const float radius = fromNavMeshCoordinates(
-                settings, std::ceil(std::sqrt(static_cast<float>(maxTiles) / osg::PIf) + 1) * getTileSize(settings));
-            TileBounds result;
-            result.mMin = center - osg::Vec2f(radius, radius);
-            result.mMax = center + osg::Vec2f(radius, radius);
-            return result;
+            const int radius = static_cast<int>(std::ceil(std::sqrt(static_cast<float>(maxTiles) / osg::PIf) + 1));
+            return TilesPositionsRange{
+                .mBegin = center - TilePosition(radius, radius),
+                .mEnd = center + TilePosition(radius + 1, radius + 1),
+            };
+        }
+
+        TilePosition toNavMeshTilePosition(const RecastSettings& settings, const osg::Vec3f& position)
+        {
+            return getTilePosition(settings, toNavMeshCoordinates(settings, position));
         }
     }
 
@@ -72,9 +76,9 @@ namespace DetourNavigator
 
     void NavMeshManager::updateBounds(const osg::Vec3f& playerPosition, const UpdateGuard* guard)
     {
-        const TileBounds bounds = makeBounds(
-            mSettings.mRecast, osg::Vec2f(playerPosition.x(), playerPosition.y()), mSettings.mMaxTilesNumber);
-        mRecastMeshManager.setBounds(bounds, getImpl(guard));
+        const TilePosition playerTile = toNavMeshTilePosition(mSettings.mRecast, playerPosition);
+        const TilesPositionsRange range = makeRange(playerTile, mSettings.mMaxTilesNumber);
+        mRecastMeshManager.setRange(range, getImpl(guard));
     }
 
     bool NavMeshManager::addObject(const ObjectId id, const CollisionShape& shape, const btTransform& transform,
@@ -161,15 +165,14 @@ namespace DetourNavigator
 
     void NavMeshManager::update(const osg::Vec3f& playerPosition, const UpdateGuard* guard)
     {
-        const auto playerTile
-            = getTilePosition(mSettings.mRecast, toNavMeshCoordinates(mSettings.mRecast, playerPosition));
+        const TilePosition playerTile = toNavMeshTilePosition(mSettings.mRecast, playerPosition);
         if (mLastRecastMeshManagerRevision == mRecastMeshManager.getRevision() && mPlayerTile.has_value()
             && *mPlayerTile == playerTile)
             return;
         mLastRecastMeshManagerRevision = mRecastMeshManager.getRevision();
         mPlayerTile = playerTile;
         const auto changedTiles = mRecastMeshManager.takeChangedTiles(getImpl(guard));
-        const TilesPositionsRange range = mRecastMeshManager.getRange();
+        const TilesPositionsRange range = mRecastMeshManager.getLimitedObjectsRange();
         for (const auto& [agentBounds, cached] : mCache)
             update(agentBounds, playerTile, range, cached, changedTiles);
     }
@@ -179,6 +182,7 @@ namespace DetourNavigator
         const std::map<osg::Vec2i, ChangeType>& changedTiles)
     {
         std::map<osg::Vec2i, ChangeType> tilesToPost = changedTiles;
+        std::map<osg::Vec2i, ChangeType> tilesToPost1;
         {
             const auto locked = cached->lockConst();
             const auto& navMesh = locked->getImpl();
@@ -222,7 +226,7 @@ namespace DetourNavigator
     RecastMeshTiles NavMeshManager::getRecastMeshTiles() const
     {
         RecastMeshTiles result;
-        getTilesPositions(mRecastMeshManager.getRange(), [&](const TilePosition& v) {
+        getTilesPositions(mRecastMeshManager.getLimitedObjectsRange(), [&](const TilePosition& v) {
             if (auto mesh = mRecastMeshManager.getCachedMesh(mWorldspace, v))
                 result.emplace(v, std::move(mesh));
         });

components/detournavigator/tilecachedrecastmeshmanager.cpp

@@ -18,9 +18,10 @@ namespace DetourNavigator
 {
     namespace
     {
-        const TileBounds infiniteTileBounds{ osg::Vec2f(-std::numeric_limits<float>::max(),
-                                                 -std::numeric_limits<float>::max()),
-            osg::Vec2f(std::numeric_limits<float>::max(), std::numeric_limits<float>::max()) };
+        const TilesPositionsRange infiniteRange{
+            .mBegin = TilePosition(std::numeric_limits<int>::min(), std::numeric_limits<int>::min()),
+            .mEnd = TilePosition(std::numeric_limits<int>::max(), std::numeric_limits<int>::max()),
+        };
 
         struct AddHeightfield
         {
@@ -57,19 +58,17 @@ namespace DetourNavigator
 
     TileCachedRecastMeshManager::TileCachedRecastMeshManager(const RecastSettings& settings)
         : mSettings(settings)
-        , mBounds(infiniteTileBounds)
-        , mRange(makeTilesPositionsRange(mBounds.mMin, mBounds.mMax, mSettings))
+        , mRange(infiniteRange)
     {
     }
 
-    void TileCachedRecastMeshManager::setBounds(const TileBounds& bounds, const UpdateGuard* guard)
+    void TileCachedRecastMeshManager::setRange(const TilesPositionsRange& range, const UpdateGuard* guard)
     {
-        if (mBounds == bounds)
+        if (mRange == range)
             return;
 
         bool changed = false;
-        const auto newRange = makeTilesPositionsRange(bounds.mMin, bounds.mMax, mSettings);
-        if (mBounds != infiniteTileBounds)
+        if (mRange != infiniteRange)
         {
             for (const auto& [id, data] : mObjects)
             {
@@ -77,14 +76,14 @@ namespace DetourNavigator
                     = makeTilesPositionsRange(data->mObject.getShape(), data->mObject.getTransform(), mSettings);
 
                 getTilesPositions(getIntersection(mRange, objectRange), [&](const TilePosition& v) {
-                    if (!isInTilesPositionsRange(newRange, v))
+                    if (!isInTilesPositionsRange(range, v))
                     {
                         addChangedTile(v, ChangeType::remove);
                         changed = true;
                     }
                 });
 
-                getTilesPositions(getIntersection(newRange, objectRange), [&](const TilePosition& v) {
+                getTilesPositions(getIntersection(range, objectRange), [&](const TilePosition& v) {
                     if (!isInTilesPositionsRange(mRange, v))
                     {
                         addChangedTile(v, ChangeType::add);
@@ -100,17 +99,19 @@ namespace DetourNavigator
             ++mRevision;
         }
 
-        mBounds = bounds;
-        mRange = newRange;
+        mRange = range;
     }
 
-    TilesPositionsRange TileCachedRecastMeshManager::getRange() const
+    TilesPositionsRange TileCachedRecastMeshManager::getLimitedObjectsRange() const
     {
+        if (mObjects.empty())
+            return {};
         const auto bounds = mObjectIndex.bounds();
-        return TilesPositionsRange{
+        const TilesPositionsRange objectsRange{
             .mBegin = makeTilePosition(bounds.min_corner()),
             .mEnd = makeTilePosition(bounds.max_corner()) + TilePosition(1, 1),
         };
+        return getIntersection(mRange, objectsRange);
     }
 
     void TileCachedRecastMeshManager::setWorldspace(const ESM::RefId& worldspace, const UpdateGuard* guard)

components/detournavigator/tilecachedrecastmeshmanager.hpp

@@ -44,9 +44,9 @@ namespace DetourNavigator
 
         explicit TileCachedRecastMeshManager(const RecastSettings& settings);
 
-        void setBounds(const TileBounds& bounds, const UpdateGuard* guard);
+        void setRange(const TilesPositionsRange& range, const UpdateGuard* guard);
 
-        TilesPositionsRange getRange() const;
+        TilesPositionsRange getLimitedObjectsRange() const;
 
         void setWorldspace(const ESM::RefId& worldspace, const UpdateGuard* guard);
 
@@ -126,7 +126,6 @@ namespace DetourNavigator
         using HeightfieldIndexValue = std::pair<IndexBox, std::map<osg::Vec2i, HeightfieldData>::const_iterator>;
 
         const RecastSettings& mSettings;
-        TileBounds mBounds;
         TilesPositionsRange mRange;
         ESM::RefId mWorldspace;
         std::unordered_map<ObjectId, std::unique_ptr<ObjectData>> mObjects;