Skip to content

Commit 5f413e7

Browse files
psi29apsi29a
committed
Merge branch 'navigator_debug_mesh' into 'master'
Make navigator debug meshes generation safer See merge request OpenMW/openmw!4602
2 parents 9570b29 + ada48d9 commit 5f413e7

File tree

12 files changed

+187
-34
lines changed

12 files changed

+187
-34
lines changed

‎apps/components_tests/detournavigator/asyncnavmeshupdater.cpp

+102
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
#include <components/detournavigator/makenavmesh.hpp>
66
#include <components/detournavigator/navmeshdbutils.hpp>
77
#include <components/detournavigator/serialization.hpp>
8+
#include <components/files/conversion.hpp>
89
#include <components/loadinglistener/loadinglistener.hpp>
10+
#include <components/testing/util.hpp>
911

1012
#include <BulletCollision/CollisionShapes/btBoxShape.h>
1113

@@ -372,6 +374,106 @@ namespace
372374
}
373375
}
374376

377+
TEST_F(DetourNavigatorAsyncNavMeshUpdaterTest, should_write_debug_recast_mesh)
378+
{
379+
mRecastMeshManager.setWorldspace(mWorldspace, nullptr);
380+
addHeightFieldPlane(mRecastMeshManager);
381+
mSettings.mEnableWriteRecastMeshToFile = true;
382+
const std::filesystem::path dir = TestingOpenMW::outputDirPath("DetourNavigatorAsyncNavMeshUpdaterTest");
383+
mSettings.mRecastMeshPathPrefix = Files::pathToUnicodeString(dir) + "/";
384+
Log(Debug::Verbose) << mSettings.mRecastMeshPathPrefix;
385+
AsyncNavMeshUpdater updater(mSettings, mRecastMeshManager, mOffMeshConnectionsManager, nullptr);
386+
const auto navMeshCacheItem = std::make_shared<GuardedNavMeshCacheItem>(1, mSettings);
387+
const std::map<TilePosition, ChangeType> changedTiles{ { TilePosition{ 0, 0 }, ChangeType::add } };
388+
updater.post(mAgentBounds, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles);
389+
updater.wait(WaitConditionType::allJobsDone, &mListener);
390+
EXPECT_TRUE(std::filesystem::exists(dir / "0.0.recastmesh.obj"));
391+
}
392+
393+
TEST_F(DetourNavigatorAsyncNavMeshUpdaterTest, should_write_debug_recast_mesh_with_revision)
394+
{
395+
mRecastMeshManager.setWorldspace(mWorldspace, nullptr);
396+
addHeightFieldPlane(mRecastMeshManager);
397+
mSettings.mEnableWriteRecastMeshToFile = true;
398+
mSettings.mEnableRecastMeshFileNameRevision = true;
399+
const std::filesystem::path dir = TestingOpenMW::outputDirPath("DetourNavigatorAsyncNavMeshUpdaterTest");
400+
mSettings.mRecastMeshPathPrefix = Files::pathToUnicodeString(dir) + "/";
401+
Log(Debug::Verbose) << mSettings.mRecastMeshPathPrefix;
402+
AsyncNavMeshUpdater updater(mSettings, mRecastMeshManager, mOffMeshConnectionsManager, nullptr);
403+
const auto navMeshCacheItem = std::make_shared<GuardedNavMeshCacheItem>(1, mSettings);
404+
const std::map<TilePosition, ChangeType> changedTiles{ { TilePosition{ 0, 0 }, ChangeType::add } };
405+
updater.post(mAgentBounds, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles);
406+
updater.wait(WaitConditionType::allJobsDone, &mListener);
407+
EXPECT_TRUE(std::filesystem::exists(dir / "0.0.recastmesh.1.2.obj"));
408+
}
409+
410+
TEST_F(DetourNavigatorAsyncNavMeshUpdaterTest, writing_recast_mesh_to_absent_file_should_not_fail_tile_generation)
411+
{
412+
mRecastMeshManager.setWorldspace(mWorldspace, nullptr);
413+
addHeightFieldPlane(mRecastMeshManager);
414+
mSettings.mEnableWriteRecastMeshToFile = true;
415+
const std::filesystem::path dir = TestingOpenMW::outputDir() / "absent";
416+
mSettings.mRecastMeshPathPrefix = Files::pathToUnicodeString(dir) + "/";
417+
Log(Debug::Verbose) << mSettings.mRecastMeshPathPrefix;
418+
AsyncNavMeshUpdater updater(mSettings, mRecastMeshManager, mOffMeshConnectionsManager, nullptr);
419+
const auto navMeshCacheItem = std::make_shared<GuardedNavMeshCacheItem>(1, mSettings);
420+
const std::map<TilePosition, ChangeType> changedTiles{ { TilePosition{ 0, 0 }, ChangeType::add } };
421+
updater.post(mAgentBounds, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles);
422+
updater.wait(WaitConditionType::allJobsDone, &mListener);
423+
EXPECT_NE(navMeshCacheItem->lockConst()->getImpl().getTileRefAt(0, 0, 0), 0u);
424+
EXPECT_FALSE(std::filesystem::exists(dir));
425+
}
426+
427+
TEST_F(DetourNavigatorAsyncNavMeshUpdaterTest, should_write_debug_navmesh)
428+
{
429+
mRecastMeshManager.setWorldspace(mWorldspace, nullptr);
430+
addHeightFieldPlane(mRecastMeshManager);
431+
mSettings.mEnableWriteNavMeshToFile = true;
432+
const std::filesystem::path dir = TestingOpenMW::outputDirPath("DetourNavigatorAsyncNavMeshUpdaterTest");
433+
mSettings.mNavMeshPathPrefix = Files::pathToUnicodeString(dir) + "/";
434+
Log(Debug::Verbose) << mSettings.mRecastMeshPathPrefix;
435+
AsyncNavMeshUpdater updater(mSettings, mRecastMeshManager, mOffMeshConnectionsManager, nullptr);
436+
const auto navMeshCacheItem = std::make_shared<GuardedNavMeshCacheItem>(1, mSettings);
437+
const std::map<TilePosition, ChangeType> changedTiles{ { TilePosition{ 0, 0 }, ChangeType::add } };
438+
updater.post(mAgentBounds, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles);
439+
updater.wait(WaitConditionType::allJobsDone, &mListener);
440+
EXPECT_TRUE(std::filesystem::exists(dir / "all_tiles_navmesh.bin"));
441+
}
442+
443+
TEST_F(DetourNavigatorAsyncNavMeshUpdaterTest, should_write_debug_navmesh_with_revision)
444+
{
445+
mRecastMeshManager.setWorldspace(mWorldspace, nullptr);
446+
addHeightFieldPlane(mRecastMeshManager);
447+
mSettings.mEnableWriteNavMeshToFile = true;
448+
mSettings.mEnableNavMeshFileNameRevision = true;
449+
const std::filesystem::path dir = TestingOpenMW::outputDirPath("DetourNavigatorAsyncNavMeshUpdaterTest");
450+
mSettings.mNavMeshPathPrefix = Files::pathToUnicodeString(dir) + "/";
451+
Log(Debug::Verbose) << mSettings.mRecastMeshPathPrefix;
452+
AsyncNavMeshUpdater updater(mSettings, mRecastMeshManager, mOffMeshConnectionsManager, nullptr);
453+
const auto navMeshCacheItem = std::make_shared<GuardedNavMeshCacheItem>(1, mSettings);
454+
const std::map<TilePosition, ChangeType> changedTiles{ { TilePosition{ 0, 0 }, ChangeType::add } };
455+
updater.post(mAgentBounds, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles);
456+
updater.wait(WaitConditionType::allJobsDone, &mListener);
457+
EXPECT_TRUE(std::filesystem::exists(dir / "all_tiles_navmesh.1.1.bin"));
458+
}
459+
460+
TEST_F(DetourNavigatorAsyncNavMeshUpdaterTest, writing_navmesh_to_absent_file_should_not_fail_tile_generation)
461+
{
462+
mRecastMeshManager.setWorldspace(mWorldspace, nullptr);
463+
addHeightFieldPlane(mRecastMeshManager);
464+
mSettings.mEnableWriteNavMeshToFile = true;
465+
const std::filesystem::path dir = TestingOpenMW::outputDir() / "absent";
466+
mSettings.mNavMeshPathPrefix = Files::pathToUnicodeString(dir) + "/";
467+
Log(Debug::Verbose) << mSettings.mRecastMeshPathPrefix;
468+
AsyncNavMeshUpdater updater(mSettings, mRecastMeshManager, mOffMeshConnectionsManager, nullptr);
469+
const auto navMeshCacheItem = std::make_shared<GuardedNavMeshCacheItem>(1, mSettings);
470+
const std::map<TilePosition, ChangeType> changedTiles{ { TilePosition{ 0, 0 }, ChangeType::add } };
471+
updater.post(mAgentBounds, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles);
472+
updater.wait(WaitConditionType::allJobsDone, &mListener);
473+
EXPECT_NE(navMeshCacheItem->lockConst()->getImpl().getTileRefAt(0, 0, 0), 0u);
474+
EXPECT_FALSE(std::filesystem::exists(dir));
475+
}
476+
375477
struct DetourNavigatorSpatialJobQueueTest : Test
376478
{
377479
const AgentBounds mAgentBounds{ CollisionShapeType::Aabb, osg::Vec3f(1, 1, 1) };

‎apps/components_tests/files/hash.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ namespace
3030

3131
TEST(FilesGetHash, shouldClearErrors)
3232
{
33-
const auto fileName = temporaryFilePath("fileName");
33+
const auto fileName = outputFilePath("fileName");
3434
std::string content;
3535
std::fill_n(std::back_inserter(content), 1, 'a');
3636
std::istringstream stream(content);
@@ -41,7 +41,7 @@ namespace
4141

4242
TEST_P(FilesGetHash, shouldReturnHashForStringStream)
4343
{
44-
const auto fileName = temporaryFilePath("fileName");
44+
const auto fileName = outputFilePath("fileName");
4545
std::string content;
4646
std::fill_n(std::back_inserter(content), GetParam().mSize, 'a');
4747
std::istringstream stream(content);

‎apps/components_tests/main.cpp

+6-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include <components/misc/strings/conversion.hpp>
33
#include <components/settings/parser.hpp>
44
#include <components/settings/values.hpp>
5+
#include <components/testing/util.hpp>
56

67
#include <gtest/gtest.h>
78

@@ -24,5 +25,9 @@ int main(int argc, char** argv)
2425
Settings::StaticValues::init();
2526

2627
testing::InitGoogleTest(&argc, argv);
27-
return RUN_ALL_TESTS();
28+
29+
const int result = RUN_ALL_TESTS();
30+
if (result == 0)
31+
std::filesystem::remove_all(TestingOpenMW::outputDir());
32+
return result;
2833
}

‎apps/components_tests/shader/shadermanager.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace
1616
ShaderManager mManager;
1717
ShaderManager::DefineMap mDefines;
1818

19-
ShaderManagerTest() { mManager.setShaderPath("tests_output"); }
19+
ShaderManagerTest() { mManager.setShaderPath(TestingOpenMW::outputDir()); }
2020

2121
template <class F>
2222
void withShaderFile(const std::string& content, F&& f)

‎apps/opencs_tests/main.cpp

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <components/debug/debugging.hpp>
2+
#include <components/testing/util.hpp>
23

34
#include <gtest/gtest.h>
45

@@ -7,5 +8,9 @@ int main(int argc, char* argv[])
78
Log::sMinDebugLevel = Debug::getDebugLevel();
89

910
testing::InitGoogleTest(&argc, argv);
10-
return RUN_ALL_TESTS();
11+
12+
const int result = RUN_ALL_TESTS();
13+
if (result == 0)
14+
std::filesystem::remove_all(TestingOpenMW::outputDir());
15+
return result;
1116
}

‎apps/openmw_tests/main.cpp

+6-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include <components/misc/strings/conversion.hpp>
33
#include <components/settings/parser.hpp>
44
#include <components/settings/values.hpp>
5+
#include <components/testing/util.hpp>
56

67
#include <gtest/gtest.h>
78

@@ -24,5 +25,9 @@ int main(int argc, char* argv[])
2425
Settings::StaticValues::init();
2526

2627
testing::InitGoogleTest(&argc, argv);
27-
return RUN_ALL_TESTS();
28+
29+
const int result = RUN_ALL_TESTS();
30+
if (result == 0)
31+
std::filesystem::remove_all(TestingOpenMW::outputDir());
32+
return result;
2833
}

‎components/detournavigator/asyncnavmeshupdater.cpp

+34-11
Original file line numberDiff line numberDiff line change
@@ -453,9 +453,9 @@ namespace DetourNavigator
453453
Misc::setCurrentThreadIdlePriority();
454454
while (!mShouldStop)
455455
{
456-
try
456+
if (JobIt job = getNextJob(); job != mJobs.end())
457457
{
458-
if (JobIt job = getNextJob(); job != mJobs.end())
458+
try
459459
{
460460
const JobStatus status = processJob(*job);
461461
Log(Debug::Debug) << "Processed job " << job->mId << " with status=" << status
@@ -480,20 +480,29 @@ namespace DetourNavigator
480480
}
481481
}
482482
}
483-
else
484-
cleanupLastUpdates();
483+
catch (const std::exception& e)
484+
{
485+
Log(Debug::Warning) << "Failed to process navmesh job " << job->mId
486+
<< " for worldspace=" << job->mWorldspace << " agent=" << job->mAgentBounds
487+
<< " changedTile=(" << job->mChangedTile << ")"
488+
<< " changeType=" << job->mChangeType
489+
<< " by thread=" << std::this_thread::get_id() << ": " << e.what();
490+
unlockTile(job->mId, job->mAgentBounds, job->mChangedTile);
491+
removeJob(job);
492+
}
485493
}
486-
catch (const std::exception& e)
494+
else
487495
{
488-
Log(Debug::Error) << "AsyncNavMeshUpdater::process exception: " << e.what();
496+
cleanupLastUpdates();
489497
}
490498
}
491499
Log(Debug::Debug) << "Stop navigator jobs processing by thread=" << std::this_thread::get_id();
492500
}
493501

494502
JobStatus AsyncNavMeshUpdater::processJob(Job& job)
495503
{
496-
Log(Debug::Debug) << "Processing job " << job.mId << " for agent=(" << job.mAgentBounds << ")"
504+
Log(Debug::Debug) << "Processing job " << job.mId << " for worldspace=" << job.mWorldspace
505+
<< " agent=" << job.mAgentBounds << ""
497506
<< " changedTile=(" << job.mChangedTile << ")"
498507
<< " changeType=" << job.mChangeType << " by thread=" << std::this_thread::get_id();
499508

@@ -543,7 +552,14 @@ namespace DetourNavigator
543552
return JobStatus::Done;
544553
}
545554

546-
writeDebugRecastMesh(mSettings, job.mChangedTile, *recastMesh);
555+
try
556+
{
557+
writeDebugRecastMesh(mSettings, job.mChangedTile, *recastMesh);
558+
}
559+
catch (const std::exception& e)
560+
{
561+
Log(Debug::Warning) << "Failed to write debug recast mesh: " << e.what();
562+
}
547563

548564
NavMeshTilesCache::Value cachedNavMeshData
549565
= mNavMeshTilesCache.get(job.mAgentBounds, job.mChangedTile, *recastMesh);
@@ -666,12 +682,19 @@ namespace DetourNavigator
666682
mPresentTiles.insert(std::make_tuple(job.mAgentBounds, job.mChangedTile));
667683
}
668684

669-
writeDebugNavMesh(mSettings, navMeshCacheItem, navMeshVersion);
685+
try
686+
{
687+
writeDebugNavMesh(mSettings, navMeshCacheItem, navMeshVersion);
688+
}
689+
catch (const std::exception& e)
690+
{
691+
Log(Debug::Warning) << "Failed to write debug navmesh: " << e.what();
692+
}
670693

671694
return isSuccess(status) ? JobStatus::Done : JobStatus::Fail;
672695
}
673696

674-
JobIt AsyncNavMeshUpdater::getNextJob()
697+
JobIt AsyncNavMeshUpdater::getNextJob() noexcept
675698
{
676699
std::unique_lock<std::mutex> lock(mMutex);
677700

@@ -746,7 +769,7 @@ namespace DetourNavigator
746769
return mJobs.size();
747770
}
748771

749-
void AsyncNavMeshUpdater::cleanupLastUpdates()
772+
void AsyncNavMeshUpdater::cleanupLastUpdates() noexcept
750773
{
751774
const auto now = std::chrono::steady_clock::now();
752775

‎components/detournavigator/asyncnavmeshupdater.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ namespace DetourNavigator
244244
inline JobStatus handleUpdateNavMeshStatus(UpdateNavMeshStatus status, const Job& job,
245245
const GuardedNavMeshCacheItem& navMeshCacheItem, const RecastMesh& recastMesh);
246246

247-
JobIt getNextJob();
247+
inline JobIt getNextJob() noexcept;
248248

249249
void postThreadJob(JobIt job, std::deque<JobIt>& queue);
250250

@@ -254,7 +254,7 @@ namespace DetourNavigator
254254

255255
inline std::size_t getTotalJobs() const;
256256

257-
void cleanupLastUpdates();
257+
inline void cleanupLastUpdates() noexcept;
258258

259259
inline void waitUntilJobsDoneForNotPresentTiles(Loading::Listener* listener);
260260

‎components/detournavigator/makenavmesh.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ namespace DetourNavigator
523523
std::unique_ptr<PreparedNavMeshData> prepareNavMeshTileData(const RecastMesh& recastMesh, ESM::RefId worldspace,
524524
const TilePosition& tilePosition, const AgentBounds& agentBounds, const RecastSettings& settings)
525525
{
526-
RecastContext context(worldspace, tilePosition, agentBounds, settings.mMaxLogLevel);
526+
RecastContext context(worldspace, tilePosition, agentBounds, recastMesh.getVersion(), settings.mMaxLogLevel);
527527

528528
const auto [minZ, maxZ] = getBoundsByZ(recastMesh, agentBounds.mHalfExtents.z(), settings);
529529

‎components/detournavigator/recastcontext.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,20 @@ namespace DetourNavigator
2323
return Debug::Debug;
2424
}
2525

26-
std::string formatPrefix(
27-
ESM::RefId worldspace, const TilePosition& tilePosition, const AgentBounds& agentBounds)
26+
std::string formatPrefix(ESM::RefId worldspace, const TilePosition& tilePosition,
27+
const AgentBounds& agentBounds, const Version& version)
2828
{
2929
std::ostringstream stream;
3030
stream << "Worldspace: " << worldspace << "; tile position: " << tilePosition.x() << ", "
31-
<< tilePosition.y() << "; agent bounds: " << agentBounds << "; ";
31+
<< tilePosition.y() << "; agent bounds: " << agentBounds << "; version: " << version << "; ";
3232
return stream.str();
3333
}
3434
}
3535

3636
RecastContext::RecastContext(ESM::RefId worldspace, const TilePosition& tilePosition,
37-
const AgentBounds& agentBounds, Debug::Level maxLogLevel)
37+
const AgentBounds& agentBounds, const Version& version, Debug::Level maxLogLevel)
3838
: mMaxLogLevel(maxLogLevel)
39-
, mPrefix(formatPrefix(worldspace, tilePosition, agentBounds))
39+
, mPrefix(formatPrefix(worldspace, tilePosition, agentBounds, version))
4040
{
4141
}
4242

‎components/detournavigator/recastcontext.hpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@
1313
namespace DetourNavigator
1414
{
1515
struct AgentBounds;
16+
struct Version;
1617

1718
class RecastContext final : public rcContext
1819
{
1920
public:
2021
explicit RecastContext(ESM::RefId worldspace, const TilePosition& tilePosition, const AgentBounds& agentBounds,
21-
Debug::Level maxLogLevel);
22+
const Version& version, Debug::Level maxLogLevel);
2223

2324
const std::string& getPrefix() const { return mPrefix; }
2425

0 commit comments

Comments
 (0)