Merge branch 'fix_big_object_navmesh_update' into 'master'

Fix overwhelming recast mesh and navmesh updates for big rotating objects (#6060)

Closes #6060

See merge request OpenMW/openmw!911

apps/openmw/mwworld/scene.cpp

@@ -500,7 +500,7 @@ namespace MWWorld
     {
         const auto navigator = MWBase::Environment::get().getWorld()->getNavigator();
         const auto player = MWBase::Environment::get().getWorld()->getPlayerPtr();
-        navigator->update(player.getRefData().getPosition().asVec3());
+        navigator->updatePlayerPosition(player.getRefData().getPosition().asVec3());
 
         if (!mCurrentCell || !mCurrentCell->isExterior())
             return;

apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp

@@ -54,7 +54,7 @@ namespace
     {
         TileCachedRecastMeshManager manager(mSettings);
         std::size_t calls = 0;
-        manager.forEachTilePosition([&] (const TilePosition&) { ++calls; });
+        manager.forEachTile([&] (const TilePosition&, const CachedRecastMeshManager&) { ++calls; });
         EXPECT_EQ(calls, 0);
     }
 
@@ -73,6 +73,16 @@ namespace
         EXPECT_FALSE(manager.addObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground));
     }
 
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, add_object_should_add_tiles)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        const btBoxShape boxShape(btVector3(20, 20, 100));
+        ASSERT_TRUE(manager.addObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground));
+        for (int x = -1; x < 1; ++x)
+            for (int y = -1; y < 1; ++y)
+                ASSERT_TRUE(manager.hasTile(TilePosition(x, y)));
+    }
+
     TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, update_object_for_changed_object_should_return_changed_tiles)
     {
         TileCachedRecastMeshManager manager(mSettings);
@@ -238,4 +248,79 @@ namespace
         manager.removeObject(ObjectId(&manager));
         EXPECT_EQ(manager.getRevision(), beforeRemoveRevision);
     }
+
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, add_water_for_new_water_should_return_true)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        const osg::Vec2i cellPosition(0, 0);
+        const int cellSize = 8192;
+        EXPECT_TRUE(manager.addWater(cellPosition, cellSize, btTransform::getIdentity()));
+    }
+
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, add_water_for_not_max_int_should_add_new_tiles)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        const osg::Vec2i cellPosition(0, 0);
+        const int cellSize = 8192;
+        ASSERT_TRUE(manager.addWater(cellPosition, cellSize, btTransform::getIdentity()));
+        for (int x = -6; x < 6; ++x)
+            for (int y = -6; y < 6; ++y)
+                ASSERT_TRUE(manager.hasTile(TilePosition(x, y)));
+    }
+
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, add_water_for_max_int_should_not_add_new_tiles)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        const btBoxShape boxShape(btVector3(20, 20, 100));
+        ASSERT_TRUE(manager.addObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground));
+        const osg::Vec2i cellPosition(0, 0);
+        const int cellSize = std::numeric_limits<int>::max();
+        ASSERT_TRUE(manager.addWater(cellPosition, cellSize, btTransform::getIdentity()));
+        for (int x = -6; x < 6; ++x)
+            for (int y = -6; y < 6; ++y)
+                ASSERT_EQ(manager.hasTile(TilePosition(x, y)), -1 <= x && x <= 0 && -1 <= y && y <= 0);
+    }
+
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, remove_water_for_absent_cell_should_return_nullopt)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        EXPECT_EQ(manager.removeWater(osg::Vec2i(0, 0)), std::nullopt);
+    }
+
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, remove_water_for_existing_cell_should_return_removed_water)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        const osg::Vec2i cellPosition(0, 0);
+        const int cellSize = 8192;
+        ASSERT_TRUE(manager.addWater(cellPosition, cellSize, btTransform::getIdentity()));
+        const auto result = manager.removeWater(cellPosition);
+        ASSERT_TRUE(result.has_value());
+        EXPECT_EQ(result->mCellSize, cellSize);
+    }
+
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, remove_water_for_existing_cell_should_remove_empty_tiles)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        const osg::Vec2i cellPosition(0, 0);
+        const int cellSize = 8192;
+        ASSERT_TRUE(manager.addWater(cellPosition, cellSize, btTransform::getIdentity()));
+        ASSERT_TRUE(manager.removeWater(cellPosition));
+        for (int x = -6; x < 6; ++x)
+            for (int y = -6; y < 6; ++y)
+                ASSERT_FALSE(manager.hasTile(TilePosition(x, y)));
+    }
+
+    TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, remove_water_for_existing_cell_should_leave_not_empty_tiles)
+    {
+        TileCachedRecastMeshManager manager(mSettings);
+        const btBoxShape boxShape(btVector3(20, 20, 100));
+        ASSERT_TRUE(manager.addObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground));
+        const osg::Vec2i cellPosition(0, 0);
+        const int cellSize = 8192;
+        ASSERT_TRUE(manager.addWater(cellPosition, cellSize, btTransform::getIdentity()));
+        ASSERT_TRUE(manager.removeWater(cellPosition));
+        for (int x = -6; x < 6; ++x)
+            for (int y = -6; y < 6; ++y)
+                ASSERT_EQ(manager.hasTile(TilePosition(x, y)), -1 <= x && x <= 0 && -1 <= y && y <= 0);
+    }
 }

components/detournavigator/cachedrecastmeshmanager.cpp

@@ -66,4 +66,9 @@ namespace DetourNavigator
     {
         mImpl.reportNavMeshChange(recastMeshVersion, navMeshVersion);
     }
+
+    Version CachedRecastMeshManager::getVersion() const
+    {
+        return mImpl.getVersion();
+    }
 }

components/detournavigator/cachedrecastmeshmanager.hpp

@@ -28,6 +28,8 @@ namespace DetourNavigator
 
         void reportNavMeshChange(Version recastMeshVersion, Version navMeshVersion);
 
+        Version getVersion() const;
+
     private:
         RecastMeshManager mImpl;
         std::shared_ptr<RecastMesh> mCached;

components/detournavigator/navigator.hpp

@@ -155,11 +155,17 @@ namespace DetourNavigator
         virtual void removePathgrid(const ESM::Pathgrid& pathgrid) = 0;
 
         /**
-         * @brief update start background navmesh update using current scene state.
+         * @brief update starts background navmesh update using current scene state.
          * @param playerPosition setup initial point to order build tiles of navmesh.
          */
         virtual void update(const osg::Vec3f& playerPosition) = 0;
 
+        /**
+         * @brief updatePlayerPosition starts background navmesh update using current scene state only when player position has been changed.
+         * @param playerPosition setup initial point to order build tiles of navmesh.
+         */
+        virtual void updatePlayerPosition(const osg::Vec3f& playerPosition) = 0;
+
         /**
          * @brief disable navigator updates
          */

components/detournavigator/navigatorimpl.cpp

@@ -146,6 +146,15 @@ namespace DetourNavigator
             mNavMeshManager.update(playerPosition, v.first);
     }
 
+    void NavigatorImpl::updatePlayerPosition(const osg::Vec3f& playerPosition)
+    {
+        const TilePosition tilePosition = getTilePosition(mSettings, toNavMeshCoordinates(mSettings, playerPosition));
+        if (mLastPlayerPosition.has_value() && *mLastPlayerPosition == tilePosition)
+            return;
+        update(playerPosition);
+        mLastPlayerPosition = tilePosition;
+    }
+
     void NavigatorImpl::setUpdatesEnabled(bool enabled)
     {
         mUpdatesEnabled = enabled;

components/detournavigator/navigatorimpl.hpp

@@ -46,6 +46,8 @@ namespace DetourNavigator
 
         void update(const osg::Vec3f& playerPosition) override;
 
+        void updatePlayerPosition(const osg::Vec3f& playerPosition) override;
+
         void setUpdatesEnabled(bool enabled) override;
 
         void wait(Loading::Listener& listener, WaitConditionType waitConditionType) override;
@@ -66,6 +68,7 @@ namespace DetourNavigator
         Settings mSettings;
         NavMeshManager mNavMeshManager;
         bool mUpdatesEnabled;
+        std::optional<TilePosition> mLastPlayerPosition;
         std::map<osg::Vec3f, std::size_t> mAgents;
         std::unordered_map<ObjectId, ObjectId> mAvoidIds;
         std::unordered_map<ObjectId, ObjectId> mWaterIds;

components/detournavigator/navigatorstub.hpp

@@ -71,6 +71,8 @@ namespace DetourNavigator
 
         void update(const osg::Vec3f& /*playerPosition*/) override {}
 
+        void updatePlayerPosition(const osg::Vec3f& /*playerPosition*/) override {};
+
         void setUpdatesEnabled(bool /*enabled*/) override {}
 
         void wait(Loading::Listener& /*listener*/, WaitConditionType /*waitConditionType*/) override {}

components/detournavigator/navmeshmanager.cpp

@@ -171,7 +171,7 @@ namespace DetourNavigator
                     }
             }
             const auto maxTiles = std::min(mSettings.mMaxTilesNumber, navMesh.getParams()->maxTiles);
-            mRecastMeshManager.forEachTilePosition([&] (const TilePosition& tile)
+            mRecastMeshManager.forEachTile([&] (const TilePosition& tile, CachedRecastMeshManager& recastMeshManager)
             {
                 if (tilesToPost.count(tile))
                     return;
@@ -181,6 +181,8 @@ namespace DetourNavigator
                     tilesToPost.insert(std::make_pair(tile, ChangeType::add));
                 else if (!shouldAdd && presentInNavMesh)
                     tilesToPost.insert(std::make_pair(tile, ChangeType::mixed));
+                else
+                    recastMeshManager.reportNavMeshChange(recastMeshManager.getVersion(), Version {0, 0});
             });
         }
         mAsyncNavMeshUpdater.post(agentHalfExtents, cached, playerTile, tilesToPost);
@@ -214,8 +216,8 @@ namespace DetourNavigator
     RecastMeshTiles NavMeshManager::getRecastMeshTiles()
     {
         std::vector<TilePosition> tiles;
-        mRecastMeshManager.forEachTilePosition(
+        mRecastMeshManager.forEachTile(
-            [&tiles] (const TilePosition& tile) { tiles.push_back(tile); });
+            [&tiles] (const TilePosition& tile, const CachedRecastMeshManager&) { tiles.push_back(tile); });
         RecastMeshTiles result;
         std::transform(tiles.begin(), tiles.end(), std::inserter(result, result.end()),
             [this] (const TilePosition& tile) { return std::make_pair(tile, mRecastMeshManager.getMesh(tile)); });

components/detournavigator/oscillatingrecastmeshobject.cpp

@@ -1,9 +1,23 @@
 #include "oscillatingrecastmeshobject.hpp"
+#include "tilebounds.hpp"
 
 #include <components/bullethelpers/aabb.hpp>
 
+#include <algorithm>
+
 namespace DetourNavigator
 {
+    namespace
+    {
+        void limitBy(btAABB& aabb, const TileBounds& bounds)
+        {
+            aabb.m_min.setX(std::max(aabb.m_min.x(), static_cast<btScalar>(bounds.mMin.x())));
+            aabb.m_min.setY(std::max(aabb.m_min.y(), static_cast<btScalar>(bounds.mMin.y())));
+            aabb.m_max.setX(std::min(aabb.m_max.x(), static_cast<btScalar>(bounds.mMax.x())));
+            aabb.m_max.setY(std::min(aabb.m_max.y(), static_cast<btScalar>(bounds.mMax.y())));
+        }
+    }
+
     OscillatingRecastMeshObject::OscillatingRecastMeshObject(RecastMeshObject&& impl, std::size_t lastChangeRevision)
         : mImpl(std::move(impl))
         , mLastChangeRevision(lastChangeRevision)
@@ -19,7 +33,7 @@ namespace DetourNavigator
     }
 
     bool OscillatingRecastMeshObject::update(const btTransform& transform, const AreaType areaType,
-                                             std::size_t lastChangeRevision)
+                                             std::size_t lastChangeRevision, const TileBounds& bounds)
     {
         const btTransform oldTransform = mImpl.getTransform();
         if (!mImpl.update(transform, areaType))
@@ -37,6 +51,7 @@ namespace DetourNavigator
         }
         const btAABB currentAabb = mAabb;
         mAabb.merge(BulletHelpers::getAabb(mImpl.getShape(), transform));
+        limitBy(mAabb, bounds);
         return currentAabb != mAabb;
     }
 }

components/detournavigator/oscillatingrecastmeshobject.hpp

@@ -3,6 +3,7 @@
 
 #include "areatype.hpp"
 #include "recastmeshobject.hpp"
+#include "tilebounds.hpp"
 
 #include <LinearMath/btTransform.h>
 #include <BulletCollision/Gimpact/btBoxCollision.h>
@@ -15,7 +16,8 @@ namespace DetourNavigator
             explicit OscillatingRecastMeshObject(RecastMeshObject&& impl, std::size_t lastChangeRevision);
             explicit OscillatingRecastMeshObject(const RecastMeshObject& impl, std::size_t lastChangeRevision);
 
-            bool update(const btTransform& transform, const AreaType areaType, std::size_t lastChangeRevision);
+            bool update(const btTransform& transform, const AreaType areaType, std::size_t lastChangeRevision,
+                        const TileBounds& bounds);
 
             const RecastMeshObject& getImpl() const { return mImpl; }
 

components/detournavigator/recastmeshmanager.cpp

@@ -5,19 +5,19 @@ namespace DetourNavigator
     RecastMeshManager::RecastMeshManager(const Settings& settings, const TileBounds& bounds, std::size_t generation)
         : mGeneration(generation)
         , mMeshBuilder(settings, bounds)
+        , mTileBounds(bounds)
     {
     }
 
     bool RecastMeshManager::addObject(const ObjectId id, const btCollisionShape& shape, const btTransform& transform,
                                       const AreaType areaType)
     {
+        const auto object = mObjects.lower_bound(id);
+        if (object != mObjects.end() && object->first == id)
+            return false;
         const auto iterator = mObjectsOrder.emplace(mObjectsOrder.end(),
             OscillatingRecastMeshObject(RecastMeshObject(shape, transform, areaType), mRevision + 1));
-        if (!mObjects.emplace(id, iterator).second)
+        mObjects.emplace_hint(object, id, iterator);
-        {
-            mObjectsOrder.erase(iterator);
-            return false;
-        }
         ++mRevision;
         return true;
     }
@@ -29,7 +29,7 @@ namespace DetourNavigator
             return false;
         const std::size_t lastChangeRevision = mLastNavMeshReportedChange.has_value()
                 ? mLastNavMeshReportedChange->mRevision : mRevision;
-        if (!object->second->update(transform, areaType, lastChangeRevision))
+        if (!object->second->update(transform, areaType, lastChangeRevision, mTileBounds))
             return false;
         ++mRevision;
         return true;
@@ -95,6 +95,11 @@ namespace DetourNavigator
             mLastNavMeshReportedChange = mLastNavMeshReport;
     }
 
+    Version RecastMeshManager::getVersion() const
+    {
+        return Version {mGeneration, mRevision};
+    }
+
     void RecastMeshManager::rebuild()
     {
         mMeshBuilder.reset();

components/detournavigator/recastmeshmanager.hpp

@@ -13,7 +13,6 @@
 #include <list>
 #include <map>
 #include <optional>
-#include <unordered_map>
 
 class btCollisionShape;
 
@@ -53,6 +52,8 @@ namespace DetourNavigator
 
         void reportNavMeshChange(Version recastMeshVersion, Version navMeshVersion);
 
+        Version getVersion() const;
+
     private:
         struct Report
         {
@@ -63,8 +64,9 @@ namespace DetourNavigator
         std::size_t mRevision = 0;
         std::size_t mGeneration;
         RecastMeshBuilder mMeshBuilder;
+        TileBounds mTileBounds;
         std::list<OscillatingRecastMeshObject> mObjectsOrder;
-        std::unordered_map<ObjectId, std::list<OscillatingRecastMeshObject>::iterator> mObjects;
+        std::map<ObjectId, std::list<OscillatingRecastMeshObject>::iterator> mObjects;
         std::list<Water> mWaterOrder;
         std::map<osg::Vec2i, std::list<Water>::iterator> mWater;
         std::optional<Report> mLastNavMeshReportedChange;

components/detournavigator/tilecachedrecastmeshmanager.cpp

@@ -3,6 +3,9 @@
 #include "gettilespositions.hpp"
 #include "settingsutils.hpp"
 
+#include <algorithm>
+#include <vector>
+
 namespace DetourNavigator
 {
     TileCachedRecastMeshManager::TileCachedRecastMeshManager(const Settings& settings)
@@ -12,23 +15,22 @@ namespace DetourNavigator
     bool TileCachedRecastMeshManager::addObject(const ObjectId id, const btCollisionShape& shape,
                                                 const btTransform& transform, const AreaType areaType)
     {
-        bool result = false;
+        std::vector<TilePosition> tilesPositions;
-        auto& tilesPositions = mObjectsTilesPositions[id];
         const auto border = getBorderSize(mSettings);
         {
             auto tiles = mTiles.lock();
             getTilesPositions(shape, transform, mSettings, [&] (const TilePosition& tilePosition)
                 {
                     if (addTile(id, shape, transform, areaType, tilePosition, border, tiles.get()))
-                    {
+                        tilesPositions.push_back(tilePosition);
-                        tilesPositions.insert(tilePosition);
-                        result = true;
-                    }
                 });
         }
-        if (result)
+        if (tilesPositions.empty())
+            return false;
+        std::sort(tilesPositions.begin(), tilesPositions.end());
+        mObjectsTilesPositions.insert_or_assign(id, std::move(tilesPositions));
         ++mRevision;
-        return result;
+        return true;
     }
 
     std::optional<RemovedRecastMeshObject> TileCachedRecastMeshManager::removeObject(const ObjectId id)

components/detournavigator/tilecachedrecastmeshmanager.hpp

@@ -9,9 +9,10 @@
 
 #include <components/misc/guarded.hpp>
 
+#include <algorithm>
 #include <map>
 #include <mutex>
-#include <set>
+#include <vector>
 
 namespace DetourNavigator
 {
@@ -33,14 +34,14 @@ namespace DetourNavigator
             auto& currentTiles = object->second;
             const auto border = getBorderSize(mSettings);
             bool changed = false;
-            std::set<TilePosition> newTiles;
+            std::vector<TilePosition> newTiles;
             {
                 auto tiles = mTiles.lock();
                 const auto onTilePosition = [&] (const TilePosition& tilePosition)
                 {
-                    if (currentTiles.count(tilePosition))
+                    if (std::binary_search(currentTiles.begin(), currentTiles.end(), tilePosition))
                     {
-                        newTiles.insert(tilePosition);
+                        newTiles.push_back(tilePosition);
                         if (updateTile(id, transform, areaType, tilePosition, tiles.get()))
                         {
                             onChangedTile(tilePosition);
@@ -49,24 +50,27 @@ namespace DetourNavigator
                     }
                     else if (addTile(id, shape, transform, areaType, tilePosition, border, tiles.get()))
                     {
-                        newTiles.insert(tilePosition);
+                        newTiles.push_back(tilePosition);
                         onChangedTile(tilePosition);
                         changed = true;
                     }
                 };
                 getTilesPositions(shape, transform, mSettings, onTilePosition);
+                std::sort(newTiles.begin(), newTiles.end());
                 for (const auto& tile : currentTiles)
                 {
-                    if (!newTiles.count(tile) && removeTile(id, tile, tiles.get()))
+                    if (!std::binary_search(newTiles.begin(), newTiles.end(), tile) && removeTile(id, tile, tiles.get()))
                     {
                         onChangedTile(tile);
                         changed = true;
                     }
                 }
             }
-            std::swap(currentTiles, newTiles);
             if (changed)
+            {
+                currentTiles = std::move(newTiles);
                 ++mRevision;
+            }
             return changed;
         }
 
@@ -81,10 +85,10 @@ namespace DetourNavigator
         bool hasTile(const TilePosition& tilePosition);
 
         template <class Function>
-        void forEachTilePosition(Function&& function)
+        void forEachTile(Function&& function)
         {
-            for (const auto& tile : *mTiles.lock())
+            for (auto& [tilePosition, recastMeshManager] : *mTiles.lock())
-                function(tile.first);
+                function(tilePosition, recastMeshManager);
         }
 
         std::size_t getRevision() const;
@@ -94,7 +98,7 @@ namespace DetourNavigator
     private:
         const Settings& mSettings;
         Misc::ScopeGuarded<std::map<TilePosition, CachedRecastMeshManager>> mTiles;
-        std::unordered_map<ObjectId, std::set<TilePosition>> mObjectsTilesPositions;
+        std::unordered_map<ObjectId, std::vector<TilePosition>> mObjectsTilesPositions;
         std::map<osg::Vec2i, std::vector<TilePosition>> mWaterTilesPositions;
         std::size_t mRevision = 0;
         std::size_t mTilesGeneration = 0;