From 9463fe2f5484b5545271965d414550332d58e1ad Mon Sep 17 00:00:00 2001 From: Alexander William Clegg Date: Fri, 20 Dec 2019 09:32:46 -0800 Subject: [PATCH 1/3] Refactor PhysicsTest.cpp to extract world/scene settup from individual tests to avoid code duplication. --- src/tests/PhysicsTest.cpp | 77 +++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/src/tests/PhysicsTest.cpp b/src/tests/PhysicsTest.cpp index cec8c2cae4..cf9a9009ad 100644 --- a/src/tests/PhysicsTest.cpp +++ b/src/tests/PhysicsTest.cpp @@ -26,44 +26,62 @@ const std::string physicsConfigFile = Cr::Utility::Directory::join(SCENE_DATASETS, "../default.phys_scene_config.json"); -TEST(PhysicsTest, JoinCompound) { - LOG(INFO) << "Starting physics test"; - // must create a context and renderer first - esp::gfx::WindowlessContext::uptr context_ = - esp::gfx::WindowlessContext::create_unique(0); - std::shared_ptr renderer_ = esp::gfx::Renderer::create(); +class PhysicsTestWorld { + public: + PhysicsTestWorld(const std::string sceneFile) { + sceneFile_ = sceneFile; + + context_ = esp::gfx::WindowlessContext::create_unique(0); + renderer_ = esp::gfx::Renderer::create(); + + sceneID_ = sceneManager_.initSceneGraph(); + auto& sceneGraph = sceneManager_.getSceneGraph(sceneID_); + esp::scene::SceneNode* navSceneNode = + &sceneGraph.getRootNode().createChild(); + auto& drawables = sceneManager_.getSceneGraph(sceneID_).getDrawables(); + const esp::assets::AssetInfo info = + esp::assets::AssetInfo::fromPath(sceneFile); + + resourceManager_.loadScene(info, physicsManager_, navSceneNode, &drawables, + physicsConfigFile); + }; // must declare these in this order due to avoid deallocation errors - ResourceManager resourceManager; + esp::gfx::WindowlessContext::uptr context_; + esp::gfx::Renderer::ptr renderer_; + + ResourceManager resourceManager_; SceneManager sceneManager_; - std::shared_ptr physicsManager_; + PhysicsManager::ptr physicsManager_; + + std::string sceneFile_; + + int sceneID_; +}; + +TEST(PhysicsTest, JoinCompound) { + LOG(INFO) << "Starting physics test: JoinCompound"; std::string sceneFile = Cr::Utility::Directory::join(dataDir, "test_assets/scenes/plane.glb"); std::string objectFile = Cr::Utility::Directory::join( dataDir, "test_assets/objects/nested_box.glb"); - int sceneID = sceneManager_.initSceneGraph(); - auto& sceneGraph = sceneManager_.getSceneGraph(sceneID); - esp::scene::SceneNode* navSceneNode = &sceneGraph.getRootNode().createChild(); - auto& drawables = sceneManager_.getSceneGraph(sceneID).getDrawables(); - const esp::assets::AssetInfo info = - esp::assets::AssetInfo::fromPath(sceneFile); - - resourceManager.loadScene(info, physicsManager_, navSceneNode, nullptr, - physicsConfigFile); + PhysicsTestWorld physicsTestWorld(sceneFile); - if (physicsManager_->getPhysicsSimulationLibrary() != + if (physicsTestWorld.physicsManager_->getPhysicsSimulationLibrary() != PhysicsManager::PhysicsSimulationLibrary::NONE) { // if we have a simulation implementation then test a joined vs. unjoined // object esp::assets::PhysicsObjectAttributes physicsObjectAttributes; physicsObjectAttributes.setString("renderMeshHandle", objectFile); - resourceManager.loadObject(physicsObjectAttributes, objectFile); + physicsTestWorld.resourceManager_.loadObject(physicsObjectAttributes, + objectFile); // get a reference to the stored template to edit esp::assets::PhysicsObjectAttributes& objectTemplate = - resourceManager.getPhysicsObjectAttributes(objectFile); + physicsTestWorld.resourceManager_.getPhysicsObjectAttributes( + objectFile); for (int i = 0; i < 2; i++) { // mark the object not joined @@ -73,30 +91,33 @@ TEST(PhysicsTest, JoinCompound) { objectTemplate.setBool("joinCollisionMeshes", true); } - physicsManager_->reset(); + physicsTestWorld.physicsManager_->reset(); std::vector objectIds; // add and simulate the object int num_objects = 7; for (int o = 0; o < num_objects; o++) { - int objectId = physicsManager_->addObject(objectFile, nullptr); + int objectId = + physicsTestWorld.physicsManager_->addObject(objectFile, nullptr); objectIds.push_back(o); Magnum::Matrix4 R{ Magnum::Matrix4::rotationX(Magnum::Math::Rad(-1.56)) * Magnum::Matrix4::rotationY(Magnum::Math::Rad(-0.25))}; float boxHeight = 2.0 + (o * 2); Magnum::Vector3 initialPosition{0.0, boxHeight, 0.0}; - physicsManager_->setRotation( + physicsTestWorld.physicsManager_->setRotation( objectId, Magnum::Quaternion::fromMatrix(R.rotationNormalized())); - physicsManager_->setTranslation(objectId, initialPosition); + physicsTestWorld.physicsManager_->setTranslation(objectId, + initialPosition); } float timeToSim = 10.0; - while (physicsManager_->getWorldTime() < timeToSim) { - physicsManager_->stepPhysics(0.1); + while (physicsTestWorld.physicsManager_->getWorldTime() < timeToSim) { + physicsTestWorld.physicsManager_->stepPhysics(0.1); } - int numActiveObjects = physicsManager_->checkActiveObjects(); + int numActiveObjects = + physicsTestWorld.physicsManager_->checkActiveObjects(); LOG(INFO) << " Number of active objects: " << numActiveObjects; if (i == 1) { @@ -105,7 +126,7 @@ TEST(PhysicsTest, JoinCompound) { } for (int o : objectIds) { - physicsManager_->removeObject(o); + physicsTestWorld.physicsManager_->removeObject(o); } } } From b3dcd9fdedc7590df5b983c9eae933a1258914de Mon Sep 17 00:00:00 2001 From: Alexander William Clegg Date: Fri, 20 Dec 2019 11:02:52 -0800 Subject: [PATCH 2/3] Refactor PhysicsTestWorld to gtest test fixture PhysicsManagerTest --- src/tests/PhysicsTest.cpp | 52 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/src/tests/PhysicsTest.cpp b/src/tests/PhysicsTest.cpp index cf9a9009ad..7ec373de60 100644 --- a/src/tests/PhysicsTest.cpp +++ b/src/tests/PhysicsTest.cpp @@ -26,25 +26,28 @@ const std::string physicsConfigFile = Cr::Utility::Directory::join(SCENE_DATASETS, "../default.phys_scene_config.json"); -class PhysicsTestWorld { - public: - PhysicsTestWorld(const std::string sceneFile) { - sceneFile_ = sceneFile; - +class PhysicsManagerTest : public testing::Test { + protected: + void SetUp() override { context_ = esp::gfx::WindowlessContext::create_unique(0); renderer_ = esp::gfx::Renderer::create(); sceneID_ = sceneManager_.initSceneGraph(); + }; + + void initScene(const std::string sceneFile) { + sceneFile_ = sceneFile; + + const esp::assets::AssetInfo info = + esp::assets::AssetInfo::fromPath(sceneFile); + auto& sceneGraph = sceneManager_.getSceneGraph(sceneID_); esp::scene::SceneNode* navSceneNode = &sceneGraph.getRootNode().createChild(); auto& drawables = sceneManager_.getSceneGraph(sceneID_).getDrawables(); - const esp::assets::AssetInfo info = - esp::assets::AssetInfo::fromPath(sceneFile); - resourceManager_.loadScene(info, physicsManager_, navSceneNode, &drawables, physicsConfigFile); - }; + } // must declare these in this order due to avoid deallocation errors esp::gfx::WindowlessContext::uptr context_; @@ -59,7 +62,7 @@ class PhysicsTestWorld { int sceneID_; }; -TEST(PhysicsTest, JoinCompound) { +TEST_F(PhysicsManagerTest, JoinCompound) { LOG(INFO) << "Starting physics test: JoinCompound"; std::string sceneFile = @@ -67,21 +70,19 @@ TEST(PhysicsTest, JoinCompound) { std::string objectFile = Cr::Utility::Directory::join( dataDir, "test_assets/objects/nested_box.glb"); - PhysicsTestWorld physicsTestWorld(sceneFile); + initScene(sceneFile); - if (physicsTestWorld.physicsManager_->getPhysicsSimulationLibrary() != + if (physicsManager_->getPhysicsSimulationLibrary() != PhysicsManager::PhysicsSimulationLibrary::NONE) { // if we have a simulation implementation then test a joined vs. unjoined // object esp::assets::PhysicsObjectAttributes physicsObjectAttributes; physicsObjectAttributes.setString("renderMeshHandle", objectFile); - physicsTestWorld.resourceManager_.loadObject(physicsObjectAttributes, - objectFile); + resourceManager_.loadObject(physicsObjectAttributes, objectFile); // get a reference to the stored template to edit esp::assets::PhysicsObjectAttributes& objectTemplate = - physicsTestWorld.resourceManager_.getPhysicsObjectAttributes( - objectFile); + resourceManager_.getPhysicsObjectAttributes(objectFile); for (int i = 0; i < 2; i++) { // mark the object not joined @@ -91,33 +92,30 @@ TEST(PhysicsTest, JoinCompound) { objectTemplate.setBool("joinCollisionMeshes", true); } - physicsTestWorld.physicsManager_->reset(); + physicsManager_->reset(); std::vector objectIds; // add and simulate the object int num_objects = 7; for (int o = 0; o < num_objects; o++) { - int objectId = - physicsTestWorld.physicsManager_->addObject(objectFile, nullptr); + int objectId = physicsManager_->addObject(objectFile, nullptr); objectIds.push_back(o); Magnum::Matrix4 R{ Magnum::Matrix4::rotationX(Magnum::Math::Rad(-1.56)) * Magnum::Matrix4::rotationY(Magnum::Math::Rad(-0.25))}; float boxHeight = 2.0 + (o * 2); Magnum::Vector3 initialPosition{0.0, boxHeight, 0.0}; - physicsTestWorld.physicsManager_->setRotation( + physicsManager_->setRotation( objectId, Magnum::Quaternion::fromMatrix(R.rotationNormalized())); - physicsTestWorld.physicsManager_->setTranslation(objectId, - initialPosition); + physicsManager_->setTranslation(objectId, initialPosition); } float timeToSim = 10.0; - while (physicsTestWorld.physicsManager_->getWorldTime() < timeToSim) { - physicsTestWorld.physicsManager_->stepPhysics(0.1); + while (physicsManager_->getWorldTime() < timeToSim) { + physicsManager_->stepPhysics(0.1); } - int numActiveObjects = - physicsTestWorld.physicsManager_->checkActiveObjects(); + int numActiveObjects = physicsManager_->checkActiveObjects(); LOG(INFO) << " Number of active objects: " << numActiveObjects; if (i == 1) { @@ -126,7 +124,7 @@ TEST(PhysicsTest, JoinCompound) { } for (int o : objectIds) { - physicsTestWorld.physicsManager_->removeObject(o); + physicsManager_->removeObject(o); } } } From d7d6bfaee955b62aeea22bb80619c96487e8a574 Mon Sep 17 00:00:00 2001 From: Alexander William Clegg Date: Mon, 6 Jan 2020 11:16:44 -0800 Subject: [PATCH 3/3] Removed unused variable sceneFile_ --- src/tests/PhysicsTest.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/tests/PhysicsTest.cpp b/src/tests/PhysicsTest.cpp index 7ec373de60..83a26b6898 100644 --- a/src/tests/PhysicsTest.cpp +++ b/src/tests/PhysicsTest.cpp @@ -36,8 +36,6 @@ class PhysicsManagerTest : public testing::Test { }; void initScene(const std::string sceneFile) { - sceneFile_ = sceneFile; - const esp::assets::AssetInfo info = esp::assets::AssetInfo::fromPath(sceneFile); @@ -57,8 +55,6 @@ class PhysicsManagerTest : public testing::Test { SceneManager sceneManager_; PhysicsManager::ptr physicsManager_; - std::string sceneFile_; - int sceneID_; };