From d1a294664e3540501de4917d0c2070f6c8220be7 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 27 Aug 2022 01:26:23 +0200 Subject: [PATCH 1/7] Do not try to update tile to post changedTiles contains unique set of positions and tilesToPost is empty initially therefore it's not possible to add the same position twice. --- components/detournavigator/navmeshmanager.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/components/detournavigator/navmeshmanager.cpp b/components/detournavigator/navmeshmanager.cpp index 126b5a1da5..07035b17cc 100644 --- a/components/detournavigator/navmeshmanager.cpp +++ b/components/detournavigator/navmeshmanager.cpp @@ -177,16 +177,8 @@ namespace DetourNavigator const auto locked = cached->lockConst(); const auto& navMesh = locked->getImpl(); for (const auto& [tilePosition, changeType] : changedTiles) - { if (navMesh.getTileAt(tilePosition.x(), tilePosition.y(), 0)) - { - auto tileToPost = tilesToPost.find(tilePosition); - if (tileToPost == tilesToPost.end()) - tilesToPost.emplace(tilePosition, changeType); - else - tileToPost->second = addChangeType(tileToPost->second, changeType); - } - } + tilesToPost.emplace(tilePosition, changeType); const auto maxTiles = std::min(mSettings.mMaxTilesNumber, navMesh.getParams()->maxTiles); mRecastMeshManager.forEachTile([&] (const TilePosition& tile, CachedRecastMeshManager& recastMeshManager) { From 914edd11947457e07a411a5754cef60d30b766d6 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 27 Aug 2022 01:54:20 +0200 Subject: [PATCH 2/7] Make frequently called oneliners inline --- components/detournavigator/cachedrecastmeshmanager.cpp | 5 ----- components/detournavigator/cachedrecastmeshmanager.hpp | 2 +- components/detournavigator/recastmeshmanager.cpp | 5 ----- components/detournavigator/recastmeshmanager.hpp | 2 +- 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/components/detournavigator/cachedrecastmeshmanager.cpp b/components/detournavigator/cachedrecastmeshmanager.cpp index 61c2a36345..65f99fae1e 100644 --- a/components/detournavigator/cachedrecastmeshmanager.cpp +++ b/components/detournavigator/cachedrecastmeshmanager.cpp @@ -97,9 +97,4 @@ namespace DetourNavigator { mImpl.reportNavMeshChange(recastMeshVersion, navMeshVersion); } - - Version CachedRecastMeshManager::getVersion() const - { - return mImpl.getVersion(); - } } diff --git a/components/detournavigator/cachedrecastmeshmanager.hpp b/components/detournavigator/cachedrecastmeshmanager.hpp index 1fcb8e5a28..df4a5b3319 100644 --- a/components/detournavigator/cachedrecastmeshmanager.hpp +++ b/components/detournavigator/cachedrecastmeshmanager.hpp @@ -41,7 +41,7 @@ namespace DetourNavigator void reportNavMeshChange(const Version& recastMeshVersion, const Version& navMeshVersion); - Version getVersion() const; + Version getVersion() const { return mImpl.getVersion(); } private: RecastMeshManager mImpl; diff --git a/components/detournavigator/recastmeshmanager.cpp b/components/detournavigator/recastmeshmanager.cpp index 8eb8b462a9..b0ac337da5 100644 --- a/components/detournavigator/recastmeshmanager.cpp +++ b/components/detournavigator/recastmeshmanager.cpp @@ -164,9 +164,4 @@ namespace DetourNavigator || mLastNavMeshReportedChange->mNavMeshVersion < mLastNavMeshReport->mNavMeshVersion) mLastNavMeshReportedChange = mLastNavMeshReport; } - - Version RecastMeshManager::getVersion() const - { - return Version {mGeneration, mRevision}; - } } diff --git a/components/detournavigator/recastmeshmanager.hpp b/components/detournavigator/recastmeshmanager.hpp index 8618d749e6..ad27933807 100644 --- a/components/detournavigator/recastmeshmanager.hpp +++ b/components/detournavigator/recastmeshmanager.hpp @@ -55,7 +55,7 @@ namespace DetourNavigator void reportNavMeshChange(const Version& recastMeshVersion, const Version& navMeshVersion); - Version getVersion() const; + Version getVersion() const { return Version {mGeneration, mRevision}; } private: struct Report From 204ab6fea3fc1c5956c825d4987c7ac35f3fc83d Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 30 Aug 2022 00:11:34 +0200 Subject: [PATCH 3/7] Use version instead of generation and revision for recast mesh --- .../detournavigator/navmeshtilescache.cpp | 8 ++-- apps/openmw/mwrender/recastmesh.cpp | 8 ++-- apps/openmw/mwrender/recastmesh.hpp | 4 +- .../detournavigator/navmeshtilescache.cpp | 29 ++++++------- .../detournavigator/recastmeshbuilder.cpp | 43 +++++++++---------- .../detournavigator/asyncnavmeshupdater.cpp | 4 +- components/detournavigator/recastmesh.cpp | 5 +-- components/detournavigator/recastmesh.hpp | 16 ++----- .../detournavigator/recastmeshbuilder.cpp | 4 +- .../detournavigator/recastmeshbuilder.hpp | 2 +- .../detournavigator/recastmeshmanager.cpp | 2 +- 11 files changed, 56 insertions(+), 69 deletions(-) diff --git a/apps/benchmarks/detournavigator/navmeshtilescache.cpp b/apps/benchmarks/detournavigator/navmeshtilescache.cpp index a0c204dc6c..79726955c9 100644 --- a/apps/benchmarks/detournavigator/navmeshtilescache.cpp +++ b/apps/benchmarks/detournavigator/navmeshtilescache.cpp @@ -140,12 +140,14 @@ namespace const CollisionShapeType agentShapeType = CollisionShapeType::Aabb; const osg::Vec3f agentHalfExtents = generateAgentHalfExtents(0.5, 1.5, random); const TilePosition tilePosition = generateVec2i(10000, random); - const std::size_t generation = std::uniform_int_distribution(0, 100)(random); - const std::size_t revision = std::uniform_int_distribution(0, 10000)(random); + const Version version { + .mGeneration = std::uniform_int_distribution(0, 100)(random), + .mRevision = std::uniform_int_distribution(0, 10000)(random), + }; Mesh mesh = generateMesh(triangles, random); std::vector water; generateWater(std::back_inserter(water), 1, random); - RecastMesh recastMesh(generation, revision, std::move(mesh), std::move(water), + RecastMesh recastMesh(version, std::move(mesh), std::move(water), {generateHeightfield(random)}, {generateFlatHeightfield(random)}, {}); return Key {AgentBounds {agentShapeType, agentHalfExtents}, tilePosition, std::move(recastMesh)}; } diff --git a/apps/openmw/mwrender/recastmesh.cpp b/apps/openmw/mwrender/recastmesh.cpp index 06884f1332..e5f5438dcb 100644 --- a/apps/openmw/mwrender/recastmesh.cpp +++ b/apps/openmw/mwrender/recastmesh.cpp @@ -53,8 +53,7 @@ namespace MWRender continue; } - if (it->second.mGeneration != tile->second->getGeneration() - || it->second.mRevision != tile->second->getRevision()) + if (it->second.mVersion != tile->second->getVersion()) { const auto group = SceneUtil::createRecastMeshGroup(*tile->second, settings.mRecast); MWBase::Environment::get().getResourceSystem()->getSceneManager()->recreateShaders(group, "debug"); @@ -62,8 +61,7 @@ namespace MWRender mRootNode->removeChild(it->second.mValue); mRootNode->addChild(group); it->second.mValue = group; - it->second.mGeneration = tile->second->getGeneration(); - it->second.mRevision = tile->second->getRevision(); + it->second.mVersion = tile->second->getVersion(); } ++it; @@ -76,7 +74,7 @@ namespace MWRender const auto group = SceneUtil::createRecastMeshGroup(*tile.second, settings.mRecast); MWBase::Environment::get().getResourceSystem()->getSceneManager()->recreateShaders(group, "debug"); group->setNodeMask(Mask_Debug); - mGroups.emplace(tile.first, Group {tile.second->getGeneration(), tile.second->getRevision(), group}); + mGroups.emplace(tile.first, Group {tile.second->getVersion(), group}); mRootNode->addChild(group); } } diff --git a/apps/openmw/mwrender/recastmesh.hpp b/apps/openmw/mwrender/recastmesh.hpp index 194ec04a62..0f0b34423a 100644 --- a/apps/openmw/mwrender/recastmesh.hpp +++ b/apps/openmw/mwrender/recastmesh.hpp @@ -2,6 +2,7 @@ #define OPENMW_MWRENDER_RECASTMESH_H #include +#include #include @@ -44,8 +45,7 @@ namespace MWRender private: struct Group { - std::size_t mGeneration; - std::size_t mRevision; + DetourNavigator::Version mVersion; osg::ref_ptr mValue; }; diff --git a/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp b/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp index 0180373e10..bc31b5be79 100644 --- a/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp +++ b/apps/openmw_test_suite/detournavigator/navmeshtilescache.cpp @@ -97,14 +97,13 @@ namespace { const AgentBounds mAgentBounds {CollisionShapeType::Aabb, {1, 2, 3}}; const TilePosition mTilePosition {0, 0}; - const std::size_t mGeneration = 0; - const std::size_t mRevision = 0; + const Version mVersion {0, 0}; const Mesh mMesh {makeMesh()}; const std::vector mWater {}; const std::vector mHeightfields {}; const std::vector mFlatHeightfields {}; const std::vector mSources {}; - const RecastMesh mRecastMesh {mGeneration, mRevision, mMesh, mWater, mHeightfields, mFlatHeightfields, mSources}; + const RecastMesh mRecastMesh {mVersion, mMesh, mWater, mHeightfields, mFlatHeightfields, mSources}; std::unique_ptr mPreparedNavMeshData {makePeparedNavMeshData(3)}; const std::size_t mRecastMeshSize = sizeof(mRecastMesh) + getSize(mRecastMesh); @@ -192,7 +191,7 @@ namespace const std::size_t maxSize = 1; NavMeshTilesCache cache(maxSize); const std::vector water(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh unexistentRecastMesh(mGeneration, mRevision, mMesh, water, mHeightfields, mFlatHeightfields, mSources); + const RecastMesh unexistentRecastMesh(mVersion, mMesh, water, mHeightfields, mFlatHeightfields, mSources); cache.set(mAgentBounds, mTilePosition, mRecastMesh, std::move(mPreparedNavMeshData)); EXPECT_FALSE(cache.get(mAgentBounds, mTilePosition, unexistentRecastMesh)); @@ -204,7 +203,7 @@ namespace NavMeshTilesCache cache(maxSize); const std::vector water(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh anotherRecastMesh(mGeneration, mRevision, mMesh, water, mHeightfields, mFlatHeightfields, mSources); + const RecastMesh anotherRecastMesh(mVersion, mMesh, water, mHeightfields, mFlatHeightfields, mSources); auto anotherPreparedNavMeshData = makePeparedNavMeshData(3); const auto copy = clone(*anotherPreparedNavMeshData); @@ -222,7 +221,7 @@ namespace NavMeshTilesCache cache(maxSize); const std::vector water(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh anotherRecastMesh(mGeneration, mRevision, mMesh, water, mHeightfields, mFlatHeightfields, mSources); + const RecastMesh anotherRecastMesh(mVersion, mMesh, water, mHeightfields, mFlatHeightfields, mSources); auto anotherPreparedNavMeshData = makePeparedNavMeshData(3); const auto value = cache.set(mAgentBounds, mTilePosition, mRecastMesh, @@ -238,12 +237,12 @@ namespace const auto copy = clone(*mPreparedNavMeshData); const std::vector leastRecentlySetWater(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh leastRecentlySetRecastMesh(mGeneration, mRevision, mMesh, leastRecentlySetWater, + const RecastMesh leastRecentlySetRecastMesh(mVersion, mMesh, leastRecentlySetWater, mHeightfields, mFlatHeightfields, mSources); auto leastRecentlySetData = makePeparedNavMeshData(3); const std::vector mostRecentlySetWater(1, CellWater {osg::Vec2i(), Water {2, 0.0f}}); - const RecastMesh mostRecentlySetRecastMesh(mGeneration, mRevision, mMesh, mostRecentlySetWater, + const RecastMesh mostRecentlySetRecastMesh(mVersion, mMesh, mostRecentlySetWater, mHeightfields, mFlatHeightfields, mSources); auto mostRecentlySetData = makePeparedNavMeshData(3); @@ -266,13 +265,13 @@ namespace NavMeshTilesCache cache(maxSize); const std::vector leastRecentlyUsedWater(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh leastRecentlyUsedRecastMesh(mGeneration, mRevision, mMesh, leastRecentlyUsedWater, + const RecastMesh leastRecentlyUsedRecastMesh(mVersion, mMesh, leastRecentlyUsedWater, mHeightfields, mFlatHeightfields, mSources); auto leastRecentlyUsedData = makePeparedNavMeshData(3); const auto leastRecentlyUsedCopy = clone(*leastRecentlyUsedData); const std::vector mostRecentlyUsedWater(1, CellWater {osg::Vec2i(), Water {2, 0.0f}}); - const RecastMesh mostRecentlyUsedRecastMesh(mGeneration, mRevision, mMesh, mostRecentlyUsedWater, + const RecastMesh mostRecentlyUsedRecastMesh(mVersion, mMesh, mostRecentlyUsedWater, mHeightfields, mFlatHeightfields, mSources); auto mostRecentlyUsedData = makePeparedNavMeshData(3); const auto mostRecentlyUsedCopy = clone(*mostRecentlyUsedData); @@ -307,7 +306,7 @@ namespace NavMeshTilesCache cache(maxSize); const std::vector water(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh tooLargeRecastMesh(mGeneration, mRevision, mMesh, water, + const RecastMesh tooLargeRecastMesh(mVersion, mMesh, water, mHeightfields, mFlatHeightfields, mSources); auto tooLargeData = makePeparedNavMeshData(10); @@ -322,12 +321,12 @@ namespace NavMeshTilesCache cache(maxSize); const std::vector anotherWater(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh anotherRecastMesh(mGeneration, mRevision, mMesh, anotherWater, + const RecastMesh anotherRecastMesh(mVersion, mMesh, anotherWater, mHeightfields, mFlatHeightfields, mSources); auto anotherData = makePeparedNavMeshData(3); const std::vector tooLargeWater(1, CellWater {osg::Vec2i(), Water {2, 0.0f}}); - const RecastMesh tooLargeRecastMesh(mGeneration, mRevision, mMesh, tooLargeWater, + const RecastMesh tooLargeRecastMesh(mVersion, mMesh, tooLargeWater, mHeightfields, mFlatHeightfields, mSources); auto tooLargeData = makePeparedNavMeshData(10); @@ -348,7 +347,7 @@ namespace NavMeshTilesCache cache(maxSize); const std::vector water(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh anotherRecastMesh(mGeneration, mRevision, mMesh, water, mHeightfields, mFlatHeightfields, mSources); + const RecastMesh anotherRecastMesh(mVersion, mMesh, water, mHeightfields, mFlatHeightfields, mSources); auto anotherData = makePeparedNavMeshData(3); const auto firstCopy = cache.set(mAgentBounds, mTilePosition, mRecastMesh, std::move(mPreparedNavMeshData)); @@ -367,7 +366,7 @@ namespace NavMeshTilesCache cache(maxSize); const std::vector water(1, CellWater {osg::Vec2i(), Water {1, 0.0f}}); - const RecastMesh anotherRecastMesh(mGeneration, mRevision, mMesh, water, mHeightfields, mFlatHeightfields, mSources); + const RecastMesh anotherRecastMesh(mVersion, mMesh, water, mHeightfields, mFlatHeightfields, mSources); auto anotherData = makePeparedNavMeshData(3); cache.set(mAgentBounds, mTilePosition, mRecastMesh, std::move(mPreparedNavMeshData)); diff --git a/apps/openmw_test_suite/detournavigator/recastmeshbuilder.cpp b/apps/openmw_test_suite/detournavigator/recastmeshbuilder.cpp index dcdca1ee72..4018b02de0 100644 --- a/apps/openmw_test_suite/detournavigator/recastmeshbuilder.cpp +++ b/apps/openmw_test_suite/detournavigator/recastmeshbuilder.cpp @@ -58,8 +58,7 @@ namespace struct DetourNavigatorRecastMeshBuilderTest : Test { TileBounds mBounds; - const std::size_t mGeneration = 0; - const std::size_t mRevision = 0; + const Version mVersion {0, 0}; const osg::ref_ptr mSource {nullptr}; const ObjectTransform mObjectTransform {ESM::Position {{0, 0, 0}, {0, 0, 0}}, 0.0f}; @@ -75,7 +74,7 @@ namespace TEST_F(DetourNavigatorRecastMeshBuilderTest, create_for_empty_should_return_empty) { RecastMeshBuilder builder(mBounds); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector()); EXPECT_EQ(recastMesh->getMesh().getIndices(), std::vector()); EXPECT_EQ(recastMesh->getMesh().getAreaTypes(), std::vector()); @@ -90,7 +89,7 @@ namespace RecastMeshBuilder builder(mBounds); builder.addObject(static_cast(shape), btTransform::getIdentity(), AreaType_ground, mSource, mObjectTransform); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -1, -1, 0, -1, 1, 0, @@ -111,7 +110,7 @@ namespace btTransform(btMatrix3x3::getIdentity().scaled(btVector3(1, 2, 3)), btVector3(1, 2, 3)), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ 0, 0, 3, 0, 4, 3, @@ -128,7 +127,7 @@ namespace RecastMeshBuilder builder(mBounds); builder.addObject(static_cast(shape), btTransform::getIdentity(), AreaType_ground, mSource, mObjectTransform); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -0.5, -0.5, 0, -0.5, 0.5, 0, @@ -145,7 +144,7 @@ namespace RecastMeshBuilder builder(mBounds); builder.addObject(static_cast(shape), btTransform::getIdentity(), AreaType_ground, mSource, mObjectTransform); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -1, -1, -2, -1, -1, 2, @@ -192,7 +191,7 @@ namespace btTransform::getIdentity(), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -1, -1, -2, -1, -1, 0, @@ -239,7 +238,7 @@ namespace btTransform(btMatrix3x3::getIdentity().scaled(btVector3(1, 2, 3)), btVector3(1, 2, 3)), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ 0, 0, 3, 0, 4, 3, @@ -263,7 +262,7 @@ namespace btTransform(btMatrix3x3::getIdentity().scaled(btVector3(1, 2, 3)), btVector3(1, 2, 3)), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ 1, 2, 12, 1, 10, 12, @@ -285,7 +284,7 @@ namespace btTransform::getIdentity(), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -3, -3, 0, -3, -2, 0, @@ -312,7 +311,7 @@ namespace btTransform::getIdentity(), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -3, -3, 0, -3, -2, 0, @@ -337,7 +336,7 @@ namespace static_cast(-osg::PI_4))), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_THAT(recastMesh->getMesh().getVertices(), Pointwise(FloatNear(1e-5f), std::vector({ 0, -4.24264049530029296875f, 4.44089209850062616169452667236328125e-16f, 0, -3.535533905029296875f, -0.707106769084930419921875f, @@ -362,7 +361,7 @@ namespace static_cast(osg::PI_4))), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_THAT(recastMesh->getMesh().getVertices(), Pointwise(FloatNear(1e-5f), std::vector({ -4.24264049530029296875f, 0, 4.44089209850062616169452667236328125e-16f, -3.535533905029296875f, 0, -0.707106769084930419921875f, @@ -387,7 +386,7 @@ namespace static_cast(osg::PI_4))), AreaType_ground, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_THAT(recastMesh->getMesh().getVertices(), Pointwise(FloatNear(1e-5f), std::vector({ -1.41421353816986083984375f, -1.1102230246251565404236316680908203125e-16f, 0, 1.1102230246251565404236316680908203125e-16f, -1.41421353816986083984375f, 0, @@ -416,7 +415,7 @@ namespace btTransform::getIdentity(), AreaType_null, mSource, mObjectTransform ); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -3, -3, 0, -3, -2, 0, @@ -433,7 +432,7 @@ namespace { RecastMeshBuilder builder(mBounds); builder.addWater(osg::Vec2i(1, 2), Water {1000, 300.0f}); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getWater(), std::vector({ CellWater {osg::Vec2i(1, 2), Water {1000, 300.0f}} })); @@ -448,7 +447,7 @@ namespace RecastMeshBuilder builder(mBounds); builder.addObject(static_cast(shape), btTransform::getIdentity(), AreaType_ground, mSource, mObjectTransform); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getMesh().getVertices(), std::vector({ -1, -1, 0, -1, 1, 0, @@ -467,7 +466,7 @@ namespace mBounds.mMin = osg::Vec2f(100, 100); RecastMeshBuilder builder(mBounds); builder.addHeightfield(cellPosition, cellSize, height); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); EXPECT_EQ(recastMesh->getFlatHeightfields(), std::vector({ FlatHeightfield {cellPosition, cellSize, height}, })); @@ -487,7 +486,7 @@ namespace const float maxHeight = 8; RecastMeshBuilder builder(mBounds); builder.addHeightfield(cellPosition, cellSize, heights.data(), size, minHeight, maxHeight); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); Heightfield expected; expected.mCellPosition = cellPosition; expected.mCellSize = cellSize; @@ -519,7 +518,7 @@ namespace const float maxHeight = 8; RecastMeshBuilder builder(maxCellTileBounds(cellPosition, cellSize)); builder.addHeightfield(cellPosition, cellSize, heights.data(), size, minHeight, maxHeight); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); Heightfield expected; expected.mCellPosition = cellPosition; expected.mCellSize = cellSize; @@ -552,7 +551,7 @@ namespace mBounds.mMin = osg::Vec2f(750, 750); RecastMeshBuilder builder(mBounds); builder.addHeightfield(cellPosition, cellSize, heights.data(), size, minHeight, maxHeight); - const auto recastMesh = std::move(builder).create(mGeneration, mRevision); + const auto recastMesh = std::move(builder).create(mVersion); Heightfield expected; expected.mCellPosition = cellPosition; expected.mCellSize = cellSize; diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index ea69a9b32e..1ece1131c7 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -513,9 +513,7 @@ namespace DetourNavigator const Job& job, const GuardedNavMeshCacheItem& navMeshCacheItem, const RecastMesh& recastMesh) { const Version navMeshVersion = navMeshCacheItem.lockConst()->getVersion(); - mRecastMeshManager.get().reportNavMeshChange(job.mChangedTile, - Version {recastMesh.getGeneration(), recastMesh.getRevision()}, - navMeshVersion); + mRecastMeshManager.get().reportNavMeshChange(job.mChangedTile, recastMesh.getVersion(), navMeshVersion); if (status == UpdateNavMeshStatus::removed || status == UpdateNavMeshStatus::lost) { diff --git a/components/detournavigator/recastmesh.cpp b/components/detournavigator/recastmesh.cpp index 16220d74f1..9f344d0918 100644 --- a/components/detournavigator/recastmesh.cpp +++ b/components/detournavigator/recastmesh.cpp @@ -18,11 +18,10 @@ namespace DetourNavigator mAreaTypes = std::move(areaTypes); } - RecastMesh::RecastMesh(std::size_t generation, std::size_t revision, Mesh mesh, std::vector water, + RecastMesh::RecastMesh(const Version& version, Mesh mesh, std::vector water, std::vector heightfields, std::vector flatHeightfields, std::vector meshSources) - : mGeneration(generation) - , mRevision(revision) + : mVersion(version) , mMesh(std::move(mesh)) , mWater(std::move(water)) , mHeightfields(std::move(heightfields)) diff --git a/components/detournavigator/recastmesh.hpp b/components/detournavigator/recastmesh.hpp index ebe372a015..1ad1348136 100644 --- a/components/detournavigator/recastmesh.hpp +++ b/components/detournavigator/recastmesh.hpp @@ -3,6 +3,7 @@ #include "areatype.hpp" #include "objecttransform.hpp" +#include "version.hpp" #include #include @@ -129,19 +130,11 @@ namespace DetourNavigator class RecastMesh { public: - RecastMesh(std::size_t generation, std::size_t revision, Mesh mesh, std::vector water, + explicit RecastMesh(const Version& version, Mesh mesh, std::vector water, std::vector heightfields, std::vector flatHeightfields, std::vector sources); - std::size_t getGeneration() const - { - return mGeneration; - } - - std::size_t getRevision() const - { - return mRevision; - } + const Version& getVersion() const noexcept { return mVersion; } const Mesh& getMesh() const noexcept { return mMesh; } @@ -163,8 +156,7 @@ namespace DetourNavigator const std::vector& getMeshSources() const noexcept { return mMeshSources; } private: - std::size_t mGeneration; - std::size_t mRevision; + Version mVersion; Mesh mMesh; std::vector mWater; std::vector mHeightfields; diff --git a/components/detournavigator/recastmeshbuilder.cpp b/components/detournavigator/recastmeshbuilder.cpp index 7f3557f6d3..ff216b0b0e 100644 --- a/components/detournavigator/recastmeshbuilder.cpp +++ b/components/detournavigator/recastmeshbuilder.cpp @@ -272,13 +272,13 @@ namespace DetourNavigator mHeightfields.push_back(std::move(heightfield)); } - std::shared_ptr RecastMeshBuilder::create(std::size_t generation, std::size_t revision) && + std::shared_ptr RecastMeshBuilder::create(const Version& version) && { mTriangles.erase(std::remove_if(mTriangles.begin(), mTriangles.end(), isNan), mTriangles.end()); std::sort(mTriangles.begin(), mTriangles.end()); std::sort(mWater.begin(), mWater.end()); Mesh mesh = makeMesh(std::move(mTriangles)); - return std::make_shared(generation, revision, std::move(mesh), std::move(mWater), + return std::make_shared(version, std::move(mesh), std::move(mWater), std::move(mHeightfields), std::move(mFlatHeightfields), std::move(mSources)); } diff --git a/components/detournavigator/recastmeshbuilder.hpp b/components/detournavigator/recastmeshbuilder.hpp index d0848c2a45..de5cd03967 100644 --- a/components/detournavigator/recastmeshbuilder.hpp +++ b/components/detournavigator/recastmeshbuilder.hpp @@ -58,7 +58,7 @@ namespace DetourNavigator void addHeightfield(const osg::Vec2i& cellPosition, int cellSize, const float* heights, std::size_t size, float minHeight, float maxHeight); - std::shared_ptr create(std::size_t generation, std::size_t revision) &&; + std::shared_ptr create(const Version& version) &&; private: const TileBounds mBounds; diff --git a/components/detournavigator/recastmeshmanager.cpp b/components/detournavigator/recastmeshmanager.cpp index b0ac337da5..a883f7839b 100644 --- a/components/detournavigator/recastmeshmanager.cpp +++ b/components/detournavigator/recastmeshmanager.cpp @@ -143,7 +143,7 @@ namespace DetourNavigator } for (const auto& [instance, objectTransform, shape, transform, areaType] : objects) builder.addObject(shape, transform, areaType, instance->getSource(), objectTransform); - return std::move(builder).create(mGeneration, revision); + return std::move(builder).create(Version {.mGeneration = mGeneration, .mRevision = revision}); } bool RecastMeshManager::isEmpty() const From de80b86cc12ccc1fc3e329f9a842f1369c4fd887 Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 2 Sep 2022 20:26:57 +0200 Subject: [PATCH 4/7] Use proper type to cast enum value --- components/detournavigator/asyncnavmeshupdater.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index 1ece1131c7..5740b9b4e4 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -125,7 +125,7 @@ namespace DetourNavigator case JobStatus::Fail: return stream << "JobStatus::Fail"; case JobStatus::MemoryCacheMiss: return stream << "JobStatus::MemoryCacheMiss"; } - return stream << "JobStatus::" << static_cast>(value); + return stream << "JobStatus::" << static_cast>(value); } Job::Job(const AgentBounds& agentBounds, std::weak_ptr navMeshCacheItem, From 98ddc31902de840deac76d42a57c964450592b90 Mon Sep 17 00:00:00 2001 From: elsid Date: Thu, 1 Sep 2022 23:47:19 +0200 Subject: [PATCH 5/7] Fix calculating min distance to nearest absent tile Tile can be present in either mPushed (waiting in a queue), mProcessingTiles ( being processed or waiting in db queue), mPresentTiles (added to navmesh). It's not enough to walk over mPushed tiles to find all not present. Need also to check mProcessingTiles. Otherwise if all tiles are in mProcessingTiles only waitUntilJobsDoneForNotPresentTiles may return too early because there are none in mPushed and therefore none tiles are considered to be absent on navmesh which is not true. --- .../detournavigator/asyncnavmeshupdater.cpp | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index 5740b9b4e4..913a8919ea 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -21,6 +21,8 @@ #include #include #include +#include +#include namespace DetourNavigator { @@ -31,15 +33,22 @@ namespace DetourNavigator return std::abs(lhs.x() - rhs.x()) + std::abs(lhs.y() - rhs.y()); } - int getMinDistanceTo(const TilePosition& position, int maxDistance, - const std::set>& pushedTiles, - const std::set>& presentTiles) + bool isAbsentTileTooClose(const TilePosition& position, int distance, + const std::set>& pushedTiles, + const std::set>& presentTiles, + const Misc::ScopeGuarded>>& processingTiles) { - int result = maxDistance; - for (const auto& [agentBounds, tile] : pushedTiles) - if (presentTiles.find(std::tie(agentBounds, tile)) == presentTiles.end()) - result = std::min(result, getManhattanDistance(position, tile)); - return result; + const auto isAbsentAndCloserThan = [&] (const std::tuple& v) + { + return presentTiles.find(v) == presentTiles.end() + && getManhattanDistance(position, std::get<1>(v)) < distance; + }; + if (std::any_of(pushedTiles.begin(), pushedTiles.end(), isAbsentAndCloserThan)) + return true; + if (const auto locked = processingTiles.lockConst(); + std::any_of(locked->begin(), locked->end(), isAbsentAndCloserThan)) + return true; + return false; } auto getPriority(const Job& job) noexcept @@ -248,27 +257,22 @@ namespace DetourNavigator void AsyncNavMeshUpdater::waitUntilJobsDoneForNotPresentTiles(Loading::Listener& listener) { const std::size_t initialJobsLeft = getTotalJobs(); - std::size_t maxProgress = initialJobsLeft + mThreads.size(); + std::size_t maxProgress = initialJobsLeft; std::size_t prevJobsLeft = initialJobsLeft; std::size_t jobsDone = 0; std::size_t jobsLeft = 0; const int maxDistanceToPlayer = mSettings.get().mWaitUntilMinDistanceToPlayer; const TilePosition playerPosition = *mPlayerTile.lockConst(); - int minDistanceToPlayer = 0; const auto isDone = [&] { jobsLeft = mJobs.size(); if (jobsLeft == 0) - { - minDistanceToPlayer = 0; return true; - } - minDistanceToPlayer = getMinDistanceTo(playerPosition, maxDistanceToPlayer, mPushed, mPresentTiles); - return minDistanceToPlayer >= maxDistanceToPlayer; + return !isAbsentTileTooClose(playerPosition, maxDistanceToPlayer, mPushed, mPresentTiles, mProcessingTiles); }; std::unique_lock lock(mMutex); - if (getMinDistanceTo(playerPosition, maxDistanceToPlayer, mPushed, mPresentTiles) >= maxDistanceToPlayer - || (mJobs.empty() && mProcessingTiles.lockConst()->empty())) + if (!isAbsentTileTooClose(playerPosition, maxDistanceToPlayer, mPushed, mPresentTiles, mProcessingTiles) + || mJobs.empty()) return; Loading::ScopedLoad load(&listener); listener.setLabel("#{Navigation:BuildingNavigationMesh}"); @@ -277,7 +281,7 @@ namespace DetourNavigator { if (maxProgress < jobsLeft) { - maxProgress = jobsLeft + mThreads.size(); + maxProgress = jobsLeft; listener.setProgressRange(maxProgress); listener.setProgress(jobsDone); } @@ -289,12 +293,6 @@ namespace DetourNavigator listener.increaseProgress(newJobsDone); } } - lock.unlock(); - if (minDistanceToPlayer < maxDistanceToPlayer) - { - mProcessingTiles.wait(mProcessed, [] (const auto& v) { return v.empty(); }); - listener.setProgress(maxProgress); - } } void AsyncNavMeshUpdater::waitUntilAllJobsDone() From 955db8f8257b3c92ba07afeba7f66ca4f2e7c20a Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 4 Sep 2022 20:06:52 +0200 Subject: [PATCH 6/7] Call Navigator::setWorldspace once per changing cell --- apps/openmw/mwworld/scene.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/openmw/mwworld/scene.cpp b/apps/openmw/mwworld/scene.cpp index 489f80ed20..29c8b6f76b 100644 --- a/apps/openmw/mwworld/scene.cpp +++ b/apps/openmw/mwworld/scene.cpp @@ -391,8 +391,6 @@ namespace MWWorld const int cellX = cell->getCell()->getGridX(); const int cellY = cell->getCell()->getGridY(); - mNavigator.setWorldspace(cell->getCell()->mCellId.mWorldspace); - if (cell->getCell()->isExterior()) { osg::ref_ptr land = mRendering.getLandManager()->getLand(cellX, cellY); @@ -541,6 +539,7 @@ namespace MWWorld unloadCell (cell); } + mNavigator.setWorldspace(mWorld.getExterior(playerCellX, playerCellY)->getCell()->mCellId.mWorldspace); mNavigator.updateBounds(pos); mCurrentGridCenter = osg::Vec2i(playerCellX, playerCellY); @@ -659,7 +658,10 @@ namespace MWWorld loadingListener->setLabel("Testing exterior cells ("+std::to_string(i)+"/"+std::to_string(cells.getExtSize())+")..."); CellStore *cell = mWorld.getExterior(it->mData.mX, it->mData.mY); - loadCell(cell, nullptr, false, osg::Vec3f(it->mData.mX + 0.5f, it->mData.mY + 0.5f, 0) * Constants::CellSizeInUnits); + mNavigator.setWorldspace(cell->getCell()->mCellId.mWorldspace); + const osg::Vec3f position = osg::Vec3f(it->mData.mX + 0.5f, it->mData.mY + 0.5f, 0) * Constants::CellSizeInUnits; + mNavigator.updateBounds(position); + loadCell(cell, nullptr, false, position); auto iter = mActiveCells.begin(); while (iter != mActiveCells.end()) @@ -704,8 +706,10 @@ namespace MWWorld loadingListener->setLabel("Testing interior cells ("+std::to_string(i)+"/"+std::to_string(cells.getIntSize())+")..."); CellStore *cell = mWorld.getInterior(it->mName); + mNavigator.setWorldspace(cell->getCell()->mCellId.mWorldspace); ESM::Position position; mWorld.findInteriorPosition(it->mName, position); + mNavigator.updateBounds(position.asVec3()); loadCell(cell, nullptr, false, position.asVec3()); auto iter = mActiveCells.begin(); @@ -844,6 +848,7 @@ namespace MWWorld loadingListener->setProgressRange(cell->count()); + mNavigator.setWorldspace(cell->getCell()->mCellId.mWorldspace); mNavigator.updateBounds(position.asVec3()); // Load cell. From 180d609e0d78f51809ad0d3d55c4151fcade02f8 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 5 Sep 2022 11:28:09 +0200 Subject: [PATCH 7/7] Check "wait until min distance to player" only for requiredTilesPresent wait condition allJobsDone should wait even if "wait until min distance to player" is 0. --- apps/openmw_test_suite/detournavigator/navigator.cpp | 3 +++ components/detournavigator/asyncnavmeshupdater.cpp | 4 +++- components/detournavigator/navigatorimpl.cpp | 3 +-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/openmw_test_suite/detournavigator/navigator.cpp b/apps/openmw_test_suite/detournavigator/navigator.cpp index 57dd50739e..765f27e88a 100644 --- a/apps/openmw_test_suite/detournavigator/navigator.cpp +++ b/apps/openmw_test_suite/detournavigator/navigator.cpp @@ -190,6 +190,9 @@ namespace TEST_F(DetourNavigatorNavigatorTest, add_object_should_change_navmesh) { + mSettings.mWaitUntilMinDistanceToPlayer = 0; + mNavigator.reset(new NavigatorImpl(mSettings, std::make_unique(":memory:", std::numeric_limits::max()))); + const std::array heightfieldData {{ 0, 0, 0, 0, 0, 0, -25, -25, -25, -25, diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index 913a8919ea..28f8666943 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -256,12 +256,14 @@ namespace DetourNavigator void AsyncNavMeshUpdater::waitUntilJobsDoneForNotPresentTiles(Loading::Listener& listener) { + const int maxDistanceToPlayer = mSettings.get().mWaitUntilMinDistanceToPlayer; + if (maxDistanceToPlayer <= 0) + return; const std::size_t initialJobsLeft = getTotalJobs(); std::size_t maxProgress = initialJobsLeft; std::size_t prevJobsLeft = initialJobsLeft; std::size_t jobsDone = 0; std::size_t jobsLeft = 0; - const int maxDistanceToPlayer = mSettings.get().mWaitUntilMinDistanceToPlayer; const TilePosition playerPosition = *mPlayerTile.lockConst(); const auto isDone = [&] { diff --git a/components/detournavigator/navigatorimpl.cpp b/components/detournavigator/navigatorimpl.cpp index 95dc057adb..6a194a7615 100644 --- a/components/detournavigator/navigatorimpl.cpp +++ b/components/detournavigator/navigatorimpl.cpp @@ -154,8 +154,7 @@ namespace DetourNavigator void NavigatorImpl::wait(Loading::Listener& listener, WaitConditionType waitConditionType) { - if (mSettings.mWaitUntilMinDistanceToPlayer > 0) - mNavMeshManager.wait(listener, waitConditionType); + mNavMeshManager.wait(listener, waitConditionType); } SharedNavMeshCacheItem NavigatorImpl::getNavMesh(const AgentBounds& agentBounds) const