From 7a7c20d49e501f6bd136a033c75cab7c718b017b Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 21 Apr 2023 01:45:21 +0200 Subject: [PATCH] Define single UpdateGuard type in a separate file --- apps/navmeshtool/worldspacedata.cpp | 12 +++---- apps/openmw/mwworld/scene.cpp | 2 +- apps/openmw/mwworld/worldimp.cpp | 2 +- components/CMakeLists.txt | 1 + components/detournavigator/navigator.hpp | 5 ++- components/detournavigator/navigatorimpl.cpp | 30 +++++++++--------- components/detournavigator/navigatorimpl.hpp | 23 ++------------ components/detournavigator/navigatorstub.hpp | 3 +- components/detournavigator/navmeshmanager.cpp | 20 ++++++------ components/detournavigator/navmeshmanager.hpp | 19 ++---------- .../tilecachedrecastmeshmanager.cpp | 3 +- .../tilecachedrecastmeshmanager.hpp | 20 +++++------- components/detournavigator/updateguard.hpp | 31 +++++++++++++++++++ 13 files changed, 83 insertions(+), 88 deletions(-) create mode 100644 components/detournavigator/updateguard.hpp diff --git a/apps/navmeshtool/worldspacedata.cpp b/apps/navmeshtool/worldspacedata.cpp index 12c8c050a2..04347f6c8f 100644 --- a/apps/navmeshtool/worldspacedata.cpp +++ b/apps/navmeshtool/worldspacedata.cpp @@ -279,7 +279,7 @@ namespace NavMeshTool return *it->second; }(); - const TileCachedRecastMeshManager::UpdateGuard guard(navMeshInput.mTileCachedRecastMeshManager); + const auto guard = navMeshInput.mTileCachedRecastMeshManager.makeUpdateGuard(); if (exterior) { @@ -293,15 +293,15 @@ namespace NavMeshTool getAabb(cellPosition, minHeight, maxHeight), navMeshInput.mAabb, navMeshInput.mAabbInitialized); navMeshInput.mTileCachedRecastMeshManager.addHeightfield( - cellPosition, ESM::Land::REAL_SIZE, heightfieldShape, &guard); + cellPosition, ESM::Land::REAL_SIZE, heightfieldShape, guard.get()); - navMeshInput.mTileCachedRecastMeshManager.addWater(cellPosition, ESM::Land::REAL_SIZE, -1, &guard); + navMeshInput.mTileCachedRecastMeshManager.addWater(cellPosition, ESM::Land::REAL_SIZE, -1, guard.get()); } else { if ((cell.mData.mFlags & ESM::Cell::HasWater) != 0) navMeshInput.mTileCachedRecastMeshManager.addWater( - cellPosition, std::numeric_limits::max(), cell.mWater, &guard); + cellPosition, std::numeric_limits::max(), cell.mWater, guard.get()); } forEachObject(cell, esmData, vfs, bulletShapeManager, readers, [&](BulletObject object) { @@ -319,13 +319,13 @@ namespace NavMeshTool object.getObjectTransform()); navMeshInput.mTileCachedRecastMeshManager.addObject( - objectId, shape, transform, DetourNavigator::AreaType_ground, &guard); + objectId, shape, transform, DetourNavigator::AreaType_ground, guard.get()); if (const btCollisionShape* avoid = object.getShapeInstance()->mAvoidCollisionShape.get()) { const CollisionShape avoidShape(object.getShapeInstance(), *avoid, object.getObjectTransform()); navMeshInput.mTileCachedRecastMeshManager.addObject( - objectId, avoidShape, transform, DetourNavigator::AreaType_null, &guard); + objectId, avoidShape, transform, DetourNavigator::AreaType_null, guard.get()); } data.mObjects.emplace_back(std::move(object)); diff --git a/apps/openmw/mwworld/scene.cpp b/apps/openmw/mwworld/scene.cpp index b061b287f1..23e12e53f8 100644 --- a/apps/openmw/mwworld/scene.cpp +++ b/apps/openmw/mwworld/scene.cpp @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/apps/openmw/mwworld/worldimp.cpp b/apps/openmw/mwworld/worldimp.cpp index 82cdd095e2..43297c0eaf 100644 --- a/apps/openmw/mwworld/worldimp.cpp +++ b/apps/openmw/mwworld/worldimp.cpp @@ -45,9 +45,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index fa9fcf5d5d..3f81bdc714 100644 --- a/components/CMakeLists.txt +++ b/components/CMakeLists.txt @@ -398,6 +398,7 @@ add_component_dir(detournavigator tilecachedrecastmeshmanager tileposition tilespositionsrange + updateguard version waitconditiontype ) diff --git a/components/detournavigator/navigator.hpp b/components/detournavigator/navigator.hpp index 3e522b72b6..f2acc8c9d6 100644 --- a/components/detournavigator/navigator.hpp +++ b/components/detournavigator/navigator.hpp @@ -8,6 +8,7 @@ #include "objecttransform.hpp" #include "recastmeshtiles.hpp" #include "sharednavmeshcacheitem.hpp" +#include "updateguard.hpp" #include "waitconditiontype.hpp" #include @@ -58,8 +59,6 @@ namespace DetourNavigator } }; - class UpdateGuard; - /** * @brief Top level interface of detournavigator component. Navigator allows to build a scene with navmesh and find * a path for an agent there. Scene contains agents, geometry objects and water. Agent are distinguished only by @@ -71,7 +70,7 @@ namespace DetourNavigator { virtual ~Navigator() = default; - virtual std::unique_ptr makeUpdateGuard() = 0; + virtual ScopedUpdateGuard makeUpdateGuard() = 0; /** * @brief addAgent should be called for each agent even if all of them has same half extents. diff --git a/components/detournavigator/navigatorimpl.cpp b/components/detournavigator/navigatorimpl.cpp index 9d6084e596..7af425fcfd 100644 --- a/components/detournavigator/navigatorimpl.cpp +++ b/components/detournavigator/navigatorimpl.cpp @@ -35,12 +35,12 @@ namespace DetourNavigator void NavigatorImpl::setWorldspace(std::string_view worldspace, const UpdateGuard* guard) { - mNavMeshManager.setWorldspace(worldspace, getImpl(guard)); + mNavMeshManager.setWorldspace(worldspace, guard); } void NavigatorImpl::updateBounds(const osg::Vec3f& playerPosition, const UpdateGuard* guard) { - mNavMeshManager.updateBounds(playerPosition, getImpl(guard)); + mNavMeshManager.updateBounds(playerPosition, guard); } void NavigatorImpl::addObject( @@ -54,12 +54,12 @@ namespace DetourNavigator { const CollisionShape collisionShape( shapes.mShapeInstance, *shapes.mShapeInstance->mCollisionShape, shapes.mTransform); - bool result = mNavMeshManager.addObject(id, collisionShape, transform, AreaType_ground, getImpl(guard)); + bool result = mNavMeshManager.addObject(id, collisionShape, transform, AreaType_ground, guard); if (const btCollisionShape* const avoidShape = shapes.mShapeInstance->mAvoidCollisionShape.get()) { const ObjectId avoidId(avoidShape); const CollisionShape avoidCollisionShape(shapes.mShapeInstance, *avoidShape, shapes.mTransform); - if (mNavMeshManager.addObject(avoidId, avoidCollisionShape, transform, AreaType_null, getImpl(guard))) + if (mNavMeshManager.addObject(avoidId, avoidCollisionShape, transform, AreaType_null, guard)) { updateAvoidShapeId(id, avoidId, guard); result = true; @@ -83,11 +83,11 @@ namespace DetourNavigator void NavigatorImpl::updateObject( const ObjectId id, const ObjectShapes& shapes, const btTransform& transform, const UpdateGuard* guard) { - mNavMeshManager.updateObject(id, transform, AreaType_ground, getImpl(guard)); + mNavMeshManager.updateObject(id, transform, AreaType_ground, guard); if (const btCollisionShape* const avoidShape = shapes.mShapeInstance->mAvoidCollisionShape.get()) { const ObjectId avoidId(avoidShape); - if (mNavMeshManager.updateObject(avoidId, transform, AreaType_null, getImpl(guard))) + if (mNavMeshManager.updateObject(avoidId, transform, AreaType_null, guard)) updateAvoidShapeId(id, avoidId, guard); } } @@ -100,35 +100,35 @@ namespace DetourNavigator void NavigatorImpl::removeObject(const ObjectId id, const UpdateGuard* guard) { - mNavMeshManager.removeObject(id, getImpl(guard)); + mNavMeshManager.removeObject(id, guard); const auto avoid = mAvoidIds.find(id); if (avoid != mAvoidIds.end()) - mNavMeshManager.removeObject(avoid->second, getImpl(guard)); + mNavMeshManager.removeObject(avoid->second, guard); const auto water = mWaterIds.find(id); if (water != mWaterIds.end()) - mNavMeshManager.removeObject(water->second, getImpl(guard)); + mNavMeshManager.removeObject(water->second, guard); mNavMeshManager.removeOffMeshConnections(id); } void NavigatorImpl::addWater(const osg::Vec2i& cellPosition, int cellSize, float level, const UpdateGuard* guard) { - mNavMeshManager.addWater(cellPosition, cellSize, level, getImpl(guard)); + mNavMeshManager.addWater(cellPosition, cellSize, level, guard); } void NavigatorImpl::removeWater(const osg::Vec2i& cellPosition, const UpdateGuard* guard) { - mNavMeshManager.removeWater(cellPosition, getImpl(guard)); + mNavMeshManager.removeWater(cellPosition, guard); } void NavigatorImpl::addHeightfield( const osg::Vec2i& cellPosition, int cellSize, const HeightfieldShape& shape, const UpdateGuard* guard) { - mNavMeshManager.addHeightfield(cellPosition, cellSize, shape, getImpl(guard)); + mNavMeshManager.addHeightfield(cellPosition, cellSize, shape, guard); } void NavigatorImpl::removeHeightfield(const osg::Vec2i& cellPosition, const UpdateGuard* guard) { - mNavMeshManager.removeHeightfield(cellPosition, getImpl(guard)); + mNavMeshManager.removeHeightfield(cellPosition, guard); } void NavigatorImpl::addPathgrid(const ESM::Cell& cell, const ESM::Pathgrid& pathgrid) @@ -151,7 +151,7 @@ namespace DetourNavigator void NavigatorImpl::update(const osg::Vec3f& playerPosition, const UpdateGuard* guard) { removeUnusedNavMeshes(); - mNavMeshManager.update(playerPosition, getImpl(guard)); + mNavMeshManager.update(playerPosition, guard); } void NavigatorImpl::wait(WaitConditionType waitConditionType, Loading::Listener* listener) @@ -195,7 +195,7 @@ namespace DetourNavigator auto inserted = ids.insert(std::make_pair(id, updateId)); if (!inserted.second) { - mNavMeshManager.removeObject(inserted.first->second, getImpl(guard)); + mNavMeshManager.removeObject(inserted.first->second, guard); inserted.first->second = updateId; } } diff --git a/components/detournavigator/navigatorimpl.hpp b/components/detournavigator/navigatorimpl.hpp index b8d2aa7978..1439a89f28 100644 --- a/components/detournavigator/navigatorimpl.hpp +++ b/components/detournavigator/navigatorimpl.hpp @@ -3,6 +3,7 @@ #include "navigator.hpp" #include "navmeshmanager.hpp" +#include "updateguard.hpp" #include #include @@ -21,10 +22,7 @@ namespace DetourNavigator */ explicit NavigatorImpl(const Settings& settings, std::unique_ptr&& db); - std::unique_ptr makeUpdateGuard() override - { - return std::make_unique(*this); - } + ScopedUpdateGuard makeUpdateGuard() override { return mNavMeshManager.makeUpdateGuard(); } bool addAgent(const AgentBounds& agentBounds) override; @@ -97,23 +95,6 @@ namespace DetourNavigator friend class UpdateGuard; }; - - class UpdateGuard - { - public: - explicit UpdateGuard(NavigatorImpl& navigator) - : mImpl(navigator.mNavMeshManager) - { - } - - private: - NavMeshManager::UpdateGuard mImpl; - - friend inline const NavMeshManager::UpdateGuard* getImpl(const UpdateGuard* guard) - { - return guard == nullptr ? nullptr : &guard->mImpl; - } - }; } #endif diff --git a/components/detournavigator/navigatorstub.hpp b/components/detournavigator/navigatorstub.hpp index 5d55fde8a2..7b93811c15 100644 --- a/components/detournavigator/navigatorstub.hpp +++ b/components/detournavigator/navigatorstub.hpp @@ -4,6 +4,7 @@ #include "navigator.hpp" #include "settings.hpp" #include "stats.hpp" +#include "updateguard.hpp" namespace Loading { @@ -17,7 +18,7 @@ namespace DetourNavigator public: NavigatorStub() = default; - std::unique_ptr makeUpdateGuard() override { return nullptr; } + ScopedUpdateGuard makeUpdateGuard() override { return nullptr; } bool addAgent(const AgentBounds& /*agentBounds*/) override { return true; } diff --git a/components/detournavigator/navmeshmanager.cpp b/components/detournavigator/navmeshmanager.cpp index c7985c49b6..5c93aee5e7 100644 --- a/components/detournavigator/navmeshmanager.cpp +++ b/components/detournavigator/navmeshmanager.cpp @@ -64,7 +64,7 @@ namespace DetourNavigator { if (worldspace == mWorldspace) return; - mRecastMeshManager.setWorldspace(worldspace, getImpl(guard)); + mRecastMeshManager.setWorldspace(worldspace, guard); for (auto& [agent, cache] : mCache) cache = std::make_shared(++mGenerationCounter, mSettings); mWorldspace = worldspace; @@ -74,45 +74,45 @@ namespace DetourNavigator { const TilePosition playerTile = toNavMeshTilePosition(mSettings.mRecast, playerPosition); const TilesPositionsRange range = makeRange(playerTile, mSettings.mMaxTilesNumber); - mRecastMeshManager.setRange(range, getImpl(guard)); + mRecastMeshManager.setRange(range, guard); } bool NavMeshManager::addObject(const ObjectId id, const CollisionShape& shape, const btTransform& transform, const AreaType areaType, const UpdateGuard* guard) { - return mRecastMeshManager.addObject(id, shape, transform, areaType, getImpl(guard)); + return mRecastMeshManager.addObject(id, shape, transform, areaType, guard); } bool NavMeshManager::updateObject( const ObjectId id, const btTransform& transform, const AreaType areaType, const UpdateGuard* guard) { - return mRecastMeshManager.updateObject(id, transform, areaType, getImpl(guard)); + return mRecastMeshManager.updateObject(id, transform, areaType, guard); } void NavMeshManager::removeObject(const ObjectId id, const UpdateGuard* guard) { - mRecastMeshManager.removeObject(id, getImpl(guard)); + mRecastMeshManager.removeObject(id, guard); } void NavMeshManager::addWater(const osg::Vec2i& cellPosition, int cellSize, float level, const UpdateGuard* guard) { - mRecastMeshManager.addWater(cellPosition, cellSize, level, getImpl(guard)); + mRecastMeshManager.addWater(cellPosition, cellSize, level, guard); } void NavMeshManager::removeWater(const osg::Vec2i& cellPosition, const UpdateGuard* guard) { - mRecastMeshManager.removeWater(cellPosition, getImpl(guard)); + mRecastMeshManager.removeWater(cellPosition, guard); } void NavMeshManager::addHeightfield( const osg::Vec2i& cellPosition, int cellSize, const HeightfieldShape& shape, const UpdateGuard* guard) { - mRecastMeshManager.addHeightfield(cellPosition, cellSize, shape, getImpl(guard)); + mRecastMeshManager.addHeightfield(cellPosition, cellSize, shape, guard); } void NavMeshManager::removeHeightfield(const osg::Vec2i& cellPosition, const UpdateGuard* guard) { - mRecastMeshManager.removeHeightfield(cellPosition, getImpl(guard)); + mRecastMeshManager.removeHeightfield(cellPosition, guard); } void NavMeshManager::addAgent(const AgentBounds& agentBounds) @@ -166,7 +166,7 @@ namespace DetourNavigator return; mLastRecastMeshManagerRevision = mRecastMeshManager.getRevision(); mPlayerTile = playerTile; - const auto changedTiles = mRecastMeshManager.takeChangedTiles(getImpl(guard)); + const auto changedTiles = mRecastMeshManager.takeChangedTiles(guard); const TilesPositionsRange range = mRecastMeshManager.getLimitedObjectsRange(); for (const auto& [agentBounds, cached] : mCache) update(agentBounds, playerTile, range, cached, changedTiles); diff --git a/components/detournavigator/navmeshmanager.hpp b/components/detournavigator/navmeshmanager.hpp index 332d9e25f7..b50d6fb9f2 100644 --- a/components/detournavigator/navmeshmanager.hpp +++ b/components/detournavigator/navmeshmanager.hpp @@ -20,25 +20,10 @@ namespace DetourNavigator class NavMeshManager { public: - class UpdateGuard - { - public: - explicit UpdateGuard(NavMeshManager& manager) - : mImpl(manager.mRecastMeshManager) - { - } - - friend const TileCachedRecastMeshManager::UpdateGuard* getImpl(const UpdateGuard* guard) - { - return guard == nullptr ? nullptr : &guard->mImpl; - } - - private: - const TileCachedRecastMeshManager::UpdateGuard mImpl; - }; - explicit NavMeshManager(const Settings& settings, std::unique_ptr&& db); + ScopedUpdateGuard makeUpdateGuard() { return mRecastMeshManager.makeUpdateGuard(); } + void setWorldspace(std::string_view worldspace, const UpdateGuard* guard); void updateBounds(const osg::Vec3f& playerPosition, const UpdateGuard* guard); diff --git a/components/detournavigator/tilecachedrecastmeshmanager.cpp b/components/detournavigator/tilecachedrecastmeshmanager.cpp index 9a62479e7d..1e55719c13 100644 --- a/components/detournavigator/tilecachedrecastmeshmanager.cpp +++ b/components/detournavigator/tilecachedrecastmeshmanager.cpp @@ -3,6 +3,7 @@ #include "gettilespositions.hpp" #include "recastmeshbuilder.hpp" #include "settingsutils.hpp" +#include "updateguard.hpp" #include #include @@ -45,7 +46,7 @@ namespace DetourNavigator class MaybeLockGuard { public: - explicit MaybeLockGuard(Mutex& mutex, const TileCachedRecastMeshManager::UpdateGuard* guard) + explicit MaybeLockGuard(Mutex& mutex, const UpdateGuard* guard) : mImpl(guard == nullptr ? std::optional>(mutex) : std::nullopt) { } diff --git a/components/detournavigator/tilecachedrecastmeshmanager.hpp b/components/detournavigator/tilecachedrecastmeshmanager.hpp index db1f18f4d5..12e8ddee2e 100644 --- a/components/detournavigator/tilecachedrecastmeshmanager.hpp +++ b/components/detournavigator/tilecachedrecastmeshmanager.hpp @@ -10,6 +10,7 @@ #include "recastmesh.hpp" #include "recastmeshobject.hpp" #include "tileposition.hpp" +#include "updateguard.hpp" #include "version.hpp" #include @@ -36,20 +37,14 @@ namespace DetourNavigator class TileCachedRecastMeshManager { public: - class UpdateGuard - { - public: - explicit UpdateGuard(TileCachedRecastMeshManager& manager) - : mImpl(manager.mMutex) - { - } - - private: - const std::lock_guard mImpl; - }; - explicit TileCachedRecastMeshManager(const RecastSettings& settings); + ScopedUpdateGuard makeUpdateGuard() + { + mMutex.lock(); + return ScopedUpdateGuard(&mUpdateGuard); + } + void setRange(const TilesPositionsRange& range, const UpdateGuard* guard); TilesPositionsRange getLimitedObjectsRange() const; @@ -147,6 +142,7 @@ namespace DetourNavigator std::size_t mGeneration = 0; std::size_t mRevision = 0; mutable std::mutex mMutex; + UpdateGuard mUpdateGuard{ mMutex }; inline static IndexPoint makeIndexPoint(const TilePosition& tilePosition); diff --git a/components/detournavigator/updateguard.hpp b/components/detournavigator/updateguard.hpp new file mode 100644 index 0000000000..bb9bdc6fe8 --- /dev/null +++ b/components/detournavigator/updateguard.hpp @@ -0,0 +1,31 @@ +#ifndef OPENMW_COMPONENTS_DETOURNAVIGATOR_UPDATEGUARD_H +#define OPENMW_COMPONENTS_DETOURNAVIGATOR_UPDATEGUARD_H + +#include +#include + +namespace DetourNavigator +{ + class UpdateGuard + { + public: + explicit UpdateGuard(std::mutex& mutex) + : mMutex(mutex) + { + } + + private: + std::mutex& mMutex; + + friend struct UnlockUpdateGuard; + }; + + struct UnlockUpdateGuard + { + void operator()(UpdateGuard* value) const { value->mMutex.unlock(); } + }; + + using ScopedUpdateGuard = std::unique_ptr; +} + +#endif