Merge pull request #2244 from elsid/navmesh_cache_item_refactor

Make NavMeshCacheItem consistent

apps/openmw/mwrender/renderingmanager.cpp

@@ -1447,7 +1447,7 @@ namespace MWRender
             try
             {
                 const auto locked = it->second->lockConst();
-                mNavMesh->update(locked->getValue(), mNavMeshNumber, locked->getGeneration(),
+                mNavMesh->update(locked->getImpl(), mNavMeshNumber, locked->getGeneration(),
                                  locked->getNavMeshRevision(), mNavigator.getSettings());
             }
             catch (const std::exception& e)

components/detournavigator/asyncnavmeshupdater.cpp

@@ -200,7 +200,7 @@ namespace DetourNavigator
                         + "_" + std::to_string(job.mChangedTile.y()) + "_", recastMeshRevision);
         if (mSettings.get().mEnableWriteNavMeshToFile)
             if (const auto shared = job.mNavMeshCacheItem.lock())
-                writeToFile(shared->lockConst()->getValue(), mSettings.get().mNavMeshPathPrefix, navMeshRevision);
+                writeToFile(shared->lockConst()->getImpl(), mSettings.get().mNavMeshPathPrefix, navMeshRevision);
     }
 
     std::chrono::steady_clock::time_point AsyncNavMeshUpdater::setFirstStart(const std::chrono::steady_clock::time_point& value)

components/detournavigator/makenavmesh.cpp

@@ -25,8 +25,6 @@ namespace
 {
     using namespace DetourNavigator;
 
-    static const int doNotTransferOwnership = 0;
-
     void initPolyMeshDetail(rcPolyMeshDetail& value)
     {
         value.meshes = nullptr;
@@ -441,56 +439,7 @@ namespace
         return NavMeshData(navMeshData, navMeshDataSize);
     }
 
-    class UpdateNavMeshStatusBuilder
-    {
-    public:
-        UpdateNavMeshStatusBuilder() = default;
 
-        UpdateNavMeshStatusBuilder removed(bool value)
-        {
-            if (value)
-                set(UpdateNavMeshStatus::removed);
-            else
-                unset(UpdateNavMeshStatus::removed);
-            return *this;
-        }
-
-        UpdateNavMeshStatusBuilder added(bool value)
-        {
-            if (value)
-                set(UpdateNavMeshStatus::added);
-            else
-                unset(UpdateNavMeshStatus::added);
-            return *this;
-        }
-
-        UpdateNavMeshStatusBuilder failed(bool value)
-        {
-            if (value)
-                set(UpdateNavMeshStatus::failed);
-            else
-                unset(UpdateNavMeshStatus::failed);
-            return *this;
-        }
-
-        UpdateNavMeshStatus getResult() const
-        {
-            return mResult;
-        }
-
-    private:
-        UpdateNavMeshStatus mResult = UpdateNavMeshStatus::ignored;
-
-        void set(UpdateNavMeshStatus value)
-        {
-            mResult = static_cast<UpdateNavMeshStatus>(static_cast<unsigned>(mResult) | static_cast<unsigned>(value));
-        }
-
-        void unset(UpdateNavMeshStatus value)
-        {
-            mResult = static_cast<UpdateNavMeshStatus>(static_cast<unsigned>(mResult) & ~static_cast<unsigned>(value));
-        }
-    };
 
     template <class T>
     unsigned long getMinValuableBitsNumber(const T value)
@@ -500,49 +449,6 @@ namespace
             ++power;
         return power;
     }
-
-    dtStatus addTile(dtNavMesh& navMesh, const NavMeshData& navMeshData)
-    {
-        const dtTileRef lastRef = 0;
-        dtTileRef* const result = nullptr;
-        return navMesh.addTile(navMeshData.mValue.get(), navMeshData.mSize,
-                               doNotTransferOwnership, lastRef, result);
-    }
-
-    dtStatus addTile(dtNavMesh& navMesh, const NavMeshTilesCache::Value& cachedNavMeshData)
-    {
-        const dtTileRef lastRef = 0;
-        dtTileRef* const result = nullptr;
-        return navMesh.addTile(cachedNavMeshData.get().mValue, cachedNavMeshData.get().mSize,
-                               doNotTransferOwnership, lastRef, result);
-    }
-
-    template <class T>
-    UpdateNavMeshStatus replaceTile(const SharedNavMeshCacheItem& navMeshCacheItem,
-        const TilePosition& changedTile, T&& navMeshData)
-    {
-        const auto locked = navMeshCacheItem->lock();
-        auto& navMesh = locked->getValue();
-        const int layer = 0;
-        const auto tileRef = navMesh.getTileRefAt(changedTile.x(), changedTile.y(), layer);
-        unsigned char** const data = nullptr;
-        int* const dataSize = nullptr;
-        const auto removed = dtStatusSucceed(navMesh.removeTile(tileRef, data, dataSize));
-        const auto addStatus = addTile(navMesh, navMeshData);
-
-        if (dtStatusSucceed(addStatus))
-        {
-            locked->setUsedTile(changedTile, std::forward<T>(navMeshData));
-            return UpdateNavMeshStatusBuilder().added(true).removed(removed).getResult();
-        }
-        else
-        {
-            if (removed)
-                locked->removeUsedTile(changedTile);
-            log("failed to add tile with status=", WriteDtStatus {addStatus});
-            return UpdateNavMeshStatusBuilder().removed(removed).failed((addStatus & DT_OUT_OF_MEMORY) != 0).getResult();
-        }
-    }
 }
 
 namespace DetourNavigator
@@ -591,26 +497,13 @@ namespace DetourNavigator
             " playerTile=", playerTile,
             " changedTileDistance=", getDistance(changedTile, playerTile));
 
-        const auto params = *navMeshCacheItem->lockConst()->getValue().getParams();
+        const auto params = *navMeshCacheItem->lockConst()->getImpl().getParams();
         const osg::Vec3f origin(params.orig[0], params.orig[1], params.orig[2]);
 
-        const auto x = changedTile.x();
-        const auto y = changedTile.y();
-
-        const auto removeTile = [&] {
-            const auto locked = navMeshCacheItem->lock();
-            auto& navMesh = locked->getValue();
-            const auto tileRef = navMesh.getTileRefAt(x, y, 0);
-            const auto removed = dtStatusSucceed(navMesh.removeTile(tileRef, nullptr, nullptr));
-            if (removed)
-                locked->removeUsedTile(changedTile);
-            return UpdateNavMeshStatusBuilder().removed(removed).getResult();
-        };
-
         if (!recastMesh)
         {
             log("ignore add tile: recastMesh is null");
-            return removeTile();
+            return navMeshCacheItem->lock()->removeTile(changedTile);
         }
 
         auto recastMeshBounds = recastMesh->getBounds();
@@ -625,13 +518,13 @@ namespace DetourNavigator
         if (isEmpty(recastMeshBounds))
         {
             log("ignore add tile: recastMesh is empty");
-            return removeTile();
+            return navMeshCacheItem->lock()->removeTile(changedTile);
         }
 
         if (!shouldAddTile(changedTile, playerTile, std::min(settings.mMaxTilesNumber, params.maxTiles)))
         {
             log("ignore add tile: too far from player");
-            return removeTile();
+            return navMeshCacheItem->lock()->removeTile(changedTile);
         }
 
         auto cachedNavMeshData = navMeshTilesCache.get(agentHalfExtents, changedTile, *recastMesh, offMeshConnections);
@@ -648,7 +541,7 @@ namespace DetourNavigator
             if (!navMeshData.mValue)
             {
                 log("ignore add tile: NavMeshData is null");
-                return removeTile();
+                return navMeshCacheItem->lock()->removeTile(changedTile);
             }
 
             try
@@ -665,10 +558,10 @@ namespace DetourNavigator
             if (!cachedNavMeshData)
             {
                 log("cache overflow");
-                return replaceTile(navMeshCacheItem, changedTile, std::move(navMeshData));
+                return navMeshCacheItem->lock()->updateTile(changedTile, std::move(navMeshData));
             }
         }
 
-        return replaceTile(navMeshCacheItem, changedTile, std::move(cachedNavMeshData));
+        return navMeshCacheItem->lock()->updateTile(changedTile, std::move(cachedNavMeshData));
     }
 }

components/detournavigator/makenavmesh.hpp

@@ -20,21 +20,6 @@ namespace DetourNavigator
     class RecastMesh;
     struct Settings;
 
-    enum class UpdateNavMeshStatus : unsigned
-    {
-        ignored = 0,
-        removed = 1 << 0,
-        added = 1 << 1,
-        replaced = removed | added,
-        failed = 1 << 2,
-        lost = removed | failed,
-    };
-
-    inline bool isSuccess(UpdateNavMeshStatus value)
-    {
-        return (static_cast<unsigned>(value) & static_cast<unsigned>(UpdateNavMeshStatus::failed)) == 0;
-    }
-
     inline float getLength(const osg::Vec2i& value)
     {
         return std::sqrt(float(osg::square(value.x()) + osg::square(value.y())));

components/detournavigator/navigator.hpp

@@ -174,7 +174,7 @@ namespace DetourNavigator
             if (!navMesh)
                 return out;
             const auto settings = getSettings();
-            return findSmoothPath(navMesh->lockConst()->getValue(), toNavMeshCoordinates(settings, agentHalfExtents),
+            return findSmoothPath(navMesh->lockConst()->getImpl(), toNavMeshCoordinates(settings, agentHalfExtents),
                 toNavMeshCoordinates(settings, stepSize), toNavMeshCoordinates(settings, start),
                 toNavMeshCoordinates(settings, end), includeFlags, settings, out);
         }

components/detournavigator/navmeshcacheitem.hpp

@@ -4,29 +4,113 @@
 #include "sharednavmesh.hpp"
 #include "tileposition.hpp"
 #include "navmeshtilescache.hpp"
+#include "dtstatus.hpp"
 
 #include <components/misc/guarded.hpp>
 
+#include <DetourNavMesh.h>
+
 #include <map>
 
 namespace DetourNavigator
 {
+    enum class UpdateNavMeshStatus : unsigned
+    {
+        ignored = 0,
+        removed = 1 << 0,
+        added = 1 << 1,
+        replaced = removed | added,
+        failed = 1 << 2,
+        lost = removed | failed,
+    };
+
+    inline bool isSuccess(UpdateNavMeshStatus value)
+    {
+        return (static_cast<unsigned>(value) & static_cast<unsigned>(UpdateNavMeshStatus::failed)) == 0;
+    }
+
+    class UpdateNavMeshStatusBuilder
+    {
+    public:
+        UpdateNavMeshStatusBuilder() = default;
+
+        UpdateNavMeshStatusBuilder removed(bool value)
+        {
+            if (value)
+                set(UpdateNavMeshStatus::removed);
+            else
+                unset(UpdateNavMeshStatus::removed);
+            return *this;
+        }
+
+        UpdateNavMeshStatusBuilder added(bool value)
+        {
+            if (value)
+                set(UpdateNavMeshStatus::added);
+            else
+                unset(UpdateNavMeshStatus::added);
+            return *this;
+        }
+
+        UpdateNavMeshStatusBuilder failed(bool value)
+        {
+            if (value)
+                set(UpdateNavMeshStatus::failed);
+            else
+                unset(UpdateNavMeshStatus::failed);
+            return *this;
+        }
+
+        UpdateNavMeshStatus getResult() const
+        {
+            return mResult;
+        }
+
+    private:
+        UpdateNavMeshStatus mResult = UpdateNavMeshStatus::ignored;
+
+        void set(UpdateNavMeshStatus value)
+        {
+            mResult = static_cast<UpdateNavMeshStatus>(static_cast<unsigned>(mResult) | static_cast<unsigned>(value));
+        }
+
+        void unset(UpdateNavMeshStatus value)
+        {
+            mResult = static_cast<UpdateNavMeshStatus>(static_cast<unsigned>(mResult) & ~static_cast<unsigned>(value));
+        }
+    };
+
+    inline unsigned char* getRawData(NavMeshData& navMeshData)
+    {
+        return navMeshData.mValue.get();
+    }
+
+    inline unsigned char* getRawData(NavMeshTilesCache::Value& cachedNavMeshData)
+    {
+        return cachedNavMeshData.get().mValue;
+    }
+
+    inline int getSize(const NavMeshData& navMeshData)
+    {
+        return navMeshData.mSize;
+    }
+
+    inline int getSize(const NavMeshTilesCache::Value& cachedNavMeshData)
+    {
+        return cachedNavMeshData.get().mSize;
+    }
+
     class NavMeshCacheItem
     {
     public:
-        NavMeshCacheItem(const NavMeshPtr& value, std::size_t generation)
+        NavMeshCacheItem(const NavMeshPtr& impl, std::size_t generation)
-            : mValue(value), mGeneration(generation), mNavMeshRevision(0)
+            : mImpl(impl), mGeneration(generation), mNavMeshRevision(0)
         {
         }
 
-        const dtNavMesh& getValue() const
+        const dtNavMesh& getImpl() const
         {
-            return *mValue;
+            return *mImpl;
-        }
-
-        dtNavMesh& getValue()
-        {
-            return *mValue;
         }
 
         std::size_t getGeneration() const
@@ -39,6 +123,38 @@ namespace DetourNavigator
             return mNavMeshRevision;
         }
 
+        template <class T>
+        UpdateNavMeshStatus updateTile(const TilePosition& position, T&& navMeshData)
+        {
+            const auto removed = removeTileImpl(position);
+            const auto addStatus = addTileImpl(getRawData(navMeshData), getSize(navMeshData));
+            if (dtStatusSucceed(addStatus))
+            {
+                setUsedTile(position, std::forward<T>(navMeshData));
+                return UpdateNavMeshStatusBuilder().added(true).removed(removed).getResult();
+            }
+            else
+            {
+                if (removed)
+                    removeUsedTile(position);
+                return UpdateNavMeshStatusBuilder().removed(removed).failed((addStatus & DT_OUT_OF_MEMORY) != 0).getResult();
+            }
+        }
+
+        UpdateNavMeshStatus removeTile(const TilePosition& position)
+        {
+            const auto removed = dtStatusSucceed(removeTileImpl(position));
+            if (removed)
+                removeUsedTile(position);
+            return UpdateNavMeshStatusBuilder().removed(removed).getResult();
+        }
+
+    private:
+        NavMeshPtr mImpl;
+        std::size_t mGeneration;
+        std::size_t mNavMeshRevision;
+        std::map<TilePosition, std::pair<NavMeshTilesCache::Value, NavMeshData>> mUsedTiles;
+
         void setUsedTile(const TilePosition& tilePosition, NavMeshTilesCache::Value value)
         {
             mUsedTiles[tilePosition] = std::make_pair(std::move(value), NavMeshData());
@@ -57,11 +173,22 @@ namespace DetourNavigator
             ++mNavMeshRevision;
         }
 
-    private:
+        dtStatus addTileImpl(unsigned char* data, int size)
-        NavMeshPtr mValue;
+        {
-        std::size_t mGeneration;
+            const int doNotTransferOwnership = 0;
-        std::size_t mNavMeshRevision;
+            const dtTileRef lastRef = 0;
-        std::map<TilePosition, std::pair<NavMeshTilesCache::Value, NavMeshData>> mUsedTiles;
+            dtTileRef* const result = nullptr;
+            return mImpl->addTile(data, size, doNotTransferOwnership, lastRef, result);
+        }
+
+        dtStatus removeTileImpl(const TilePosition& position)
+        {
+            const int layer = 0;
+            const auto tileRef = mImpl->getTileRefAt(position.x(), position.y(), layer);
+            unsigned char** const data = nullptr;
+            int* const dataSize = nullptr;
+            return mImpl->removeTile(tileRef, data, dataSize);
+        }
     };
 
     using GuardedNavMeshCacheItem = Misc::ScopeGuarded<NavMeshCacheItem>;

components/detournavigator/navmeshmanager.cpp

@@ -160,7 +160,7 @@ namespace DetourNavigator
         const auto changedTiles = mChangedTiles.find(agentHalfExtents);
         {
             const auto locked = cached->lockConst();
-            const auto& navMesh = locked->getValue();
+            const auto& navMesh = locked->getImpl();
             if (changedTiles != mChangedTiles.end())
             {
                 for (const auto& tile : changedTiles->second)