From 73734fc04da2901bc78d7c421498ac34ac9089d8 Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 22 Feb 2019 11:34:53 +0300 Subject: [PATCH 1/2] Fix update navmesh for not changed objects When update method is called for not changed object befor this change all object tiles were considered as not object tiles and were removed. Also this marked those tiles as changed. This lead to alternation between remove and add each tile update method was called. Problem was detected by using Animated Containers mod. --- .../tilecachedrecastmeshmanager.cpp | 18 ++++++++++++++++++ .../tilecachedrecastmeshmanager.cpp | 4 +--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp b/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp index 0443fc9ce..b84f7a22e 100644 --- a/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp +++ b/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp @@ -120,4 +120,22 @@ namespace EXPECT_EQ(manager.getMesh(TilePosition(0, -1)), nullptr); EXPECT_EQ(manager.getMesh(TilePosition(0, 0)), nullptr); } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_mesh_for_not_changed_object_after_update_should_return_recast_mesh_for_same_tiles) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_NE(manager.getMesh(TilePosition(-1, -1)), nullptr); + EXPECT_NE(manager.getMesh(TilePosition(-1, 0)), nullptr); + EXPECT_NE(manager.getMesh(TilePosition(0, -1)), nullptr); + EXPECT_NE(manager.getMesh(TilePosition(0, 0)), nullptr); + + manager.updateObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_NE(manager.getMesh(TilePosition(-1, -1)), nullptr); + EXPECT_NE(manager.getMesh(TilePosition(-1, 0)), nullptr); + EXPECT_NE(manager.getMesh(TilePosition(0, -1)), nullptr); + EXPECT_NE(manager.getMesh(TilePosition(0, 0)), nullptr); + } } diff --git a/components/detournavigator/tilecachedrecastmeshmanager.cpp b/components/detournavigator/tilecachedrecastmeshmanager.cpp index 7070603c7..b878c2d3e 100644 --- a/components/detournavigator/tilecachedrecastmeshmanager.cpp +++ b/components/detournavigator/tilecachedrecastmeshmanager.cpp @@ -47,11 +47,9 @@ namespace DetourNavigator { if (currentTiles.count(tilePosition)) { + newTiles.insert(tilePosition); if (updateTile(id, transform, areaType, tilePosition, tiles.get())) - { - newTiles.insert(tilePosition); changedTiles.push_back(tilePosition); - } } else if (addTile(id, shape, transform, areaType, tilePosition, border, tiles.get())) { From 2342a31addabff88cd1238033bc98c6f884abde8 Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 22 Feb 2019 11:35:09 +0300 Subject: [PATCH 2/2] Add more tests for TileCachedRecastMeshManager --- .../tilecachedrecastmeshmanager.cpp | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp b/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp index b84f7a22e..5275d9119 100644 --- a/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp +++ b/apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp @@ -10,6 +10,7 @@ #include #include +#include namespace { @@ -56,6 +57,45 @@ namespace EXPECT_EQ(calls, 0); } + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, add_object_for_new_object_should_return_true) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + EXPECT_TRUE(manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground)); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, add_object_for_existing_object_should_return_false) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_FALSE(manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground)); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, update_object_for_changed_object_should_return_changed_tiles) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + const btTransform transform(btMatrix3x3::getIdentity(), btVector3(getTileSize(mSettings) / mSettings.mRecastScaleFactor, 0, 0)); + manager.addObject(ObjectId(1ul), boxShape, transform, AreaType::AreaType_ground); + EXPECT_THAT( + manager.updateObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground), + ElementsAre(TilePosition(-1, -1), TilePosition(-1, 0), TilePosition(0, -1), TilePosition(0, 0), + TilePosition(1, -1), TilePosition(1, 0)) + ); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, update_object_for_not_changed_object_should_return_empty) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_EQ( + manager.updateObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground), + std::vector() + ); + } + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_mesh_after_add_object_should_return_recast_mesh_for_each_used_tile) { TileCachedRecastMeshManager manager(mSettings); @@ -138,4 +178,62 @@ namespace EXPECT_NE(manager.getMesh(TilePosition(0, -1)), nullptr); EXPECT_NE(manager.getMesh(TilePosition(0, 0)), nullptr); } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_revision_after_add_object_new_should_return_incremented_value) + { + TileCachedRecastMeshManager manager(mSettings); + const auto initialRevision = manager.getRevision(); + const btBoxShape boxShape(btVector3(20, 20, 100)); + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_EQ(manager.getRevision(), initialRevision + 1); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_revision_after_add_object_existing_should_return_same_value) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + const auto beforeAddRevision = manager.getRevision(); + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_EQ(manager.getRevision(), beforeAddRevision); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_revision_after_update_moved_object_should_return_incremented_value) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + const btTransform transform(btMatrix3x3::getIdentity(), btVector3(getTileSize(mSettings) / mSettings.mRecastScaleFactor, 0, 0)); + manager.addObject(ObjectId(1ul), boxShape, transform, AreaType::AreaType_ground); + const auto beforeUpdateRevision = manager.getRevision(); + manager.updateObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_EQ(manager.getRevision(), beforeUpdateRevision + 1); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_revision_after_update_not_changed_object_should_return_same_value) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + const auto beforeUpdateRevision = manager.getRevision(); + manager.updateObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + EXPECT_EQ(manager.getRevision(), beforeUpdateRevision); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_revision_after_remove_existing_object_should_return_incremented_value) + { + TileCachedRecastMeshManager manager(mSettings); + const btBoxShape boxShape(btVector3(20, 20, 100)); + manager.addObject(ObjectId(1ul), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground); + const auto beforeRemoveRevision = manager.getRevision(); + manager.removeObject(ObjectId(1ul)); + EXPECT_EQ(manager.getRevision(), beforeRemoveRevision + 1); + } + + TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_revision_after_remove_absent_object_should_return_same_value) + { + TileCachedRecastMeshManager manager(mSettings); + const auto beforeRemoveRevision = manager.getRevision(); + manager.removeObject(ObjectId(1ul)); + EXPECT_EQ(manager.getRevision(), beforeRemoveRevision); + } }