Merge branch 'fix_navigator_update_for_big_object' into 'master'

Limit NavMeshManager update range by player tile and max tiles (#7149)

See merge request OpenMW/openmw!2618

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;
+        const TilesPositionsRange range{
-        bounds.mMin = osg::Vec2f(182, 182);
+            .mBegin = TilePosition(0, 0),
-        bounds.mMax = osg::Vec2f(1000, 1000);
+            .mEnd = TilePosition(1, 1),
-        manager.setBounds(bounds, nullptr);
+        };
+        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;
+        const TilesPositionsRange range{
-        bounds.mMin = osg::Vec2f(-1000, -1000);
+            .mBegin = TilePosition(-1, -1),
-        bounds.mMax = osg::Vec2f(1000, 1000);
+            .mEnd = TilePosition(2, 2),
-        manager.setBounds(bounds, nullptr);
+        };
+        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;
+        const TilesPositionsRange range{
-        bounds.mMin = osg::Vec2f(182, 182);
+            .mBegin = TilePosition(0, 0),
-        bounds.mMax = osg::Vec2f(1000, 1000);
+            .mEnd = TilePosition(1, 1),
-        manager.setBounds(bounds, nullptr);
+        };
+        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;
+        const TilesPositionsRange range{
-        bounds.mMin = osg::Vec2f(-1000, -1000);
+            .mBegin = TilePosition(-1, -1),
-        bounds.mMax = osg::Vec2f(1000, 1000);
+            .mEnd = TilePosition(1, 1),
-        manager.setBounds(bounds, nullptr);
+        };
+        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;
+        const TilesPositionsRange range1{
-        bounds.mMin = osg::Vec2f(182, 0);
+            .mBegin = TilePosition(0, 0),
-        bounds.mMax = osg::Vec2f(1000, 1000);
+            .mEnd = TilePosition(1, 1),
-        manager.setBounds(bounds, nullptr);
+        };
+        manager.setRange(range1, nullptr);
         manager.addObject(ObjectId(&boxShape), shape, btTransform::getIdentity(), AreaType::AreaType_ground, nullptr);
-        bounds.mMin = osg::Vec2f(-1000, -1000);
+        const TilesPositionsRange range2{
-        bounds.mMax = osg::Vec2f(0, -182);
+            .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(
+            const int radius = static_cast<int>(std::ceil(std::sqrt(static_cast<float>(maxTiles) / osg::PIf) + 1));
-                settings, std::ceil(std::sqrt(static_cast<float>(maxTiles) / osg::PIf) + 1) * getTileSize(settings));
+            return TilesPositionsRange{
-            TileBounds result;
+                .mBegin = center - TilePosition(radius, radius),
-            result.mMin = center - osg::Vec2f(radius, radius);
+                .mEnd = center + TilePosition(radius + 1, radius + 1),
-            result.mMax = center + osg::Vec2f(radius, radius);
+            };
-            return result;
+        }
+
+        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(
+        const TilePosition playerTile = toNavMeshTilePosition(mSettings.mRecast, playerPosition);
-            mSettings.mRecast, osg::Vec2f(playerPosition.x(), playerPosition.y()), mSettings.mMaxTilesNumber);
+        const TilesPositionsRange range = makeRange(playerTile, mSettings.mMaxTilesNumber);
-        mRecastMeshManager.setBounds(bounds, getImpl(guard));
+        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
+        const TilePosition playerTile = toNavMeshTilePosition(mSettings.mRecast, playerPosition);
-            = getTilePosition(mSettings.mRecast, toNavMeshCoordinates(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(),
+        const TilesPositionsRange infiniteRange{
-                                                 -std::numeric_limits<float>::max()),
+            .mBegin = TilePosition(std::numeric_limits<int>::min(), std::numeric_limits<int>::min()),
-            osg::Vec2f(std::numeric_limits<float>::max(), std::numeric_limits<float>::max()) };
+            .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(infiniteRange)
-        , mRange(makeTilesPositionsRange(mBounds.mMin, mBounds.mMax, mSettings))
     {
     }
 
-    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 (mRange != infiniteRange)
-        if (mBounds != infiniteTileBounds)
         {
             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 = range;
-        mRange = newRange;
     }
 
-    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;