From 0b644a897e123370ea489cf43664a079fe532733 Mon Sep 17 00:00:00 2001 From: elsid Date: Thu, 3 Feb 2022 22:24:26 +0100 Subject: [PATCH] Explicitly bind TileCachedRecastMeshManager with mutex --- .../tilecachedrecastmeshmanager.cpp | 71 +++++++++---------- .../tilecachedrecastmeshmanager.hpp | 32 +++++---- 2 files changed, 54 insertions(+), 49 deletions(-) diff --git a/components/detournavigator/tilecachedrecastmeshmanager.cpp b/components/detournavigator/tilecachedrecastmeshmanager.cpp index c7bd3bc867..e7e46e96df 100644 --- a/components/detournavigator/tilecachedrecastmeshmanager.cpp +++ b/components/detournavigator/tilecachedrecastmeshmanager.cpp @@ -43,7 +43,7 @@ namespace DetourNavigator if (mBounds != infiniteTileBounds) { - const std::lock_guard lock(mMutex); + const auto locked = mWorldspaceTiles.lock(); for (auto& object : mObjects) { const ObjectId id = object.first; @@ -58,7 +58,7 @@ namespace DetourNavigator if (it == data.mTiles.end()) return; data.mTiles.erase(it); - if (removeTile(id, position, mTiles)) + if (removeTile(id, position, locked->mTiles)) changedTiles.emplace_back(position, ChangeType::remove); }; getTilesPositions(getIntersection(mRange, objectRange), onOldTilePosition); @@ -67,7 +67,7 @@ namespace DetourNavigator { if (data.mTiles.find(position) != data.mTiles.end()) return; - if (addTile(id, data.mShape, data.mTransform, data.mAreaType, position, mTiles)) + if (addTile(id, data.mShape, data.mTransform, data.mAreaType, position, locked->mTiles)) { data.mTiles.insert(position); changedTiles.emplace_back(position, ChangeType::add); @@ -91,17 +91,16 @@ namespace DetourNavigator std::string TileCachedRecastMeshManager::getWorldspace() const { - const std::lock_guard lock(mMutex); - return mWorldspace; + return mWorldspaceTiles.lockConst()->mWorldspace; } void TileCachedRecastMeshManager::setWorldspace(std::string_view worldspace) { - const std::lock_guard lock(mMutex); - if (mWorldspace == worldspace) + const auto locked = mWorldspaceTiles.lock(); + if (locked->mWorldspace == worldspace) return; - mTiles.clear(); - mWorldspace = worldspace; + locked->mTiles.clear(); + locked->mWorldspace = worldspace; } std::optional TileCachedRecastMeshManager::removeObject(const ObjectId id) @@ -111,10 +110,10 @@ namespace DetourNavigator return std::nullopt; std::optional result; { - const std::lock_guard lock(mMutex); + const auto locked = mWorldspaceTiles.lock(); for (const auto& tilePosition : object->second.mTiles) { - const auto removed = removeTile(id, tilePosition, mTiles); + const auto removed = removeTile(id, tilePosition, locked->mTiles); if (removed && !result) result = removed; } @@ -138,8 +137,8 @@ namespace DetourNavigator if (cellSize == std::numeric_limits::max()) { - const std::lock_guard lock(mMutex); - for (auto& tile : mTiles) + const auto locked = mWorldspaceTiles.lock(); + for (auto& tile : locked->mTiles) { if (tile.second->addWater(cellPosition, cellSize, level)) { @@ -154,12 +153,12 @@ namespace DetourNavigator getTilesPositions(makeTilesPositionsRange(cellSize, shift, mSettings), [&] (const TilePosition& tilePosition) { - const std::lock_guard lock(mMutex); - auto tile = mTiles.find(tilePosition); - if (tile == mTiles.end()) + const auto locked = mWorldspaceTiles.lock(); + auto tile = locked->mTiles.find(tilePosition); + if (tile == locked->mTiles.end()) { const TileBounds tileBounds = makeRealTileBoundsWithBorder(mSettings, tilePosition); - tile = mTiles.emplace_hint(tile, tilePosition, + tile = locked->mTiles.emplace_hint(tile, tilePosition, std::make_shared(tileBounds, mTilesGeneration)); } if (tile->second->addWater(cellPosition, cellSize, level)) @@ -184,14 +183,14 @@ namespace DetourNavigator std::optional result; for (const auto& tilePosition : object->second) { - const std::lock_guard lock(mMutex); - const auto tile = mTiles.find(tilePosition); - if (tile == mTiles.end()) + const auto locked = mWorldspaceTiles.lock(); + const auto tile = locked->mTiles.find(tilePosition); + if (tile == locked->mTiles.end()) continue; const auto tileResult = tile->second->removeWater(cellPosition); if (tile->second->isEmpty()) { - mTiles.erase(tile); + locked->mTiles.erase(tile); ++mTilesGeneration; } if (tileResult && !result) @@ -219,12 +218,12 @@ namespace DetourNavigator getTilesPositions(makeTilesPositionsRange(cellSize, shift, mSettings), [&] (const TilePosition& tilePosition) { - const std::lock_guard lock(mMutex); - auto tile = mTiles.find(tilePosition); - if (tile == mTiles.end()) + const auto locked = mWorldspaceTiles.lock(); + auto tile = locked->mTiles.find(tilePosition); + if (tile == locked->mTiles.end()) { const TileBounds tileBounds = makeRealTileBoundsWithBorder(mSettings, tilePosition); - tile = mTiles.emplace_hint(tile, tilePosition, + tile = locked->mTiles.emplace_hint(tile, tilePosition, std::make_shared(tileBounds, mTilesGeneration)); } if (tile->second->addHeightfield(cellPosition, cellSize, shape)) @@ -248,14 +247,14 @@ namespace DetourNavigator std::optional result; for (const auto& tilePosition : object->second) { - const std::lock_guard lock(mMutex); - const auto tile = mTiles.find(tilePosition); - if (tile == mTiles.end()) + const auto locked = mWorldspaceTiles.lock(); + const auto tile = locked->mTiles.find(tilePosition); + if (tile == locked->mTiles.end()) continue; const auto tileResult = tile->second->removeHeightfield(cellPosition); if (tile->second->isEmpty()) { - mTiles.erase(tile); + locked->mTiles.erase(tile); ++mTilesGeneration; } if (tileResult && !result) @@ -295,9 +294,9 @@ namespace DetourNavigator void TileCachedRecastMeshManager::reportNavMeshChange(const TilePosition& tilePosition, Version recastMeshVersion, Version navMeshVersion) const { - const std::lock_guard lock(mMutex); - const auto it = mTiles.find(tilePosition); - if (it == mTiles.end()) + const auto locked = mWorldspaceTiles.lockConst(); + const auto it = locked->mTiles.find(tilePosition); + if (it == locked->mTiles.end()) return; it->second->reportNavMeshChange(recastMeshVersion, navMeshVersion); } @@ -341,11 +340,11 @@ namespace DetourNavigator std::shared_ptr TileCachedRecastMeshManager::getManager(std::string_view worldspace, const TilePosition& tilePosition) const { - const std::lock_guard lock(mMutex); - if (mWorldspace != worldspace) + const auto locked = mWorldspaceTiles.lockConst(); + if (locked->mWorldspace != worldspace) return nullptr; - const auto it = mTiles.find(tilePosition); - if (it == mTiles.end()) + const auto it = locked->mTiles.find(tilePosition); + if (it == locked->mTiles.end()) return nullptr; return it->second; } diff --git a/components/detournavigator/tilecachedrecastmeshmanager.hpp b/components/detournavigator/tilecachedrecastmeshmanager.hpp index 906355524e..5e168efc16 100644 --- a/components/detournavigator/tilecachedrecastmeshmanager.hpp +++ b/components/detournavigator/tilecachedrecastmeshmanager.hpp @@ -9,6 +9,8 @@ #include "heightfieldshape.hpp" #include "changetype.hpp" +#include + #include #include #include @@ -42,11 +44,11 @@ namespace DetourNavigator std::set tilesPositions; if (range.mBegin != range.mEnd) { - const std::lock_guard lock(mMutex); + const auto locked = mWorldspaceTiles.lock(); getTilesPositions(range, [&] (const TilePosition& tilePosition) { - if (addTile(id, shape, transform, areaType, tilePosition, mTiles)) + if (addTile(id, shape, transform, areaType, tilePosition, locked->mTiles)) tilesPositions.insert(tilePosition); }); } @@ -67,31 +69,31 @@ namespace DetourNavigator bool changed = false; std::set newTiles; { + const TilesPositionsRange objectRange = makeTilesPositionsRange(shape.getShape(), transform, mSettings); + const TilesPositionsRange range = getIntersection(mRange, objectRange); + const auto locked = mWorldspaceTiles.lock(); const auto onTilePosition = [&] (const TilePosition& tilePosition) { if (data.mTiles.find(tilePosition) != data.mTiles.end()) { newTiles.insert(tilePosition); - if (updateTile(id, transform, areaType, tilePosition, mTiles)) + if (updateTile(id, transform, areaType, tilePosition, locked->mTiles)) { onChangedTile(tilePosition, ChangeType::update); changed = true; } } - else if (addTile(id, shape, transform, areaType, tilePosition, mTiles)) + else if (addTile(id, shape, transform, areaType, tilePosition, locked->mTiles)) { newTiles.insert(tilePosition); onChangedTile(tilePosition, ChangeType::add); changed = true; } }; - const TilesPositionsRange objectRange = makeTilesPositionsRange(shape.getShape(), transform, mSettings); - const TilesPositionsRange range = getIntersection(mRange, objectRange); - const std::lock_guard lock(mMutex); getTilesPositions(range, onTilePosition); for (const auto& tile : data.mTiles) { - if (newTiles.find(tile) == newTiles.end() && removeTile(id, tile, mTiles)) + if (newTiles.find(tile) == newTiles.end() && removeTile(id, tile, locked->mTiles)) { onChangedTile(tile, ChangeType::remove); changed = true; @@ -125,8 +127,8 @@ namespace DetourNavigator template void forEachTile(Function&& function) const { - const std::lock_guard lock(mMutex); - for (auto& [tilePosition, recastMeshManager] : mTiles) + const auto& locked = mWorldspaceTiles.lockConst(); + for (const auto& [tilePosition, recastMeshManager] : locked->mTiles) function(tilePosition, *recastMeshManager); } @@ -145,12 +147,16 @@ namespace DetourNavigator std::set mTiles; }; + struct WorldspaceTiles + { + std::string mWorldspace; + TilesMap mTiles; + }; + const RecastSettings& mSettings; - mutable std::mutex mMutex; TileBounds mBounds; TilesPositionsRange mRange; - std::string mWorldspace; - TilesMap mTiles; + Misc::ScopeGuarded mWorldspaceTiles; std::unordered_map mObjects; std::map> mWaterTilesPositions; std::map> mHeightfieldTilesPositions;