From a7e6bc12a8591a4c2a021b841909bf7a03035864 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 19 Mar 2021 12:53:22 -0700 Subject: [PATCH] Suggestions to capsule (#200) Signed-off-by: Louise Poubel --- include/ignition/rendering/Capsule.hh | 5 +- .../ignition/rendering/ogre/OgreCapsule.hh | 4 +- ogre/src/OgreCapsule.cc | 85 +++++++++--------- .../ignition/rendering/ogre2/Ogre2Capsule.hh | 8 +- ogre2/src/Ogre2Capsule.cc | 87 +++++++++---------- ogre2/src/Ogre2Marker.cc | 36 +------- src/Capsule_TEST.cc | 6 +- src/Marker_TEST.cc | 1 + 8 files changed, 93 insertions(+), 139 deletions(-) diff --git a/include/ignition/rendering/Capsule.hh b/include/ignition/rendering/Capsule.hh index ddfc2a6cd..00a67051c 100644 --- a/include/ignition/rendering/Capsule.hh +++ b/include/ignition/rendering/Capsule.hh @@ -28,9 +28,8 @@ namespace ignition { inline namespace IGNITION_RENDERING_VERSION_NAMESPACE { - /// \class CapsuleVisual CapsuleVisual.hh ignition/rendering/CapsuleVisual - /// \brief A CapsuleVisual geometry class. The visual appearance is based - /// on the type specified. + /// \class Capsule Capsule.hh ignition/rendering/Capsule + /// \brief Geometry for a capsule shape. class IGNITION_RENDERING_VISIBLE Capsule : public virtual Geometry { diff --git a/ogre/include/ignition/rendering/ogre/OgreCapsule.hh b/ogre/include/ignition/rendering/ogre/OgreCapsule.hh index b38387ec1..9cbb2a2ee 100644 --- a/ogre/include/ignition/rendering/ogre/OgreCapsule.hh +++ b/ogre/include/ignition/rendering/ogre/OgreCapsule.hh @@ -65,8 +65,8 @@ namespace ignition public: virtual void SetMaterial(MaterialPtr _material, bool _unique) override; - /// \brief Create the Capsule geometry in ogre - private: void Create(); + /// \brief Update the capsule geometry in ogre + private: void Update(); /// \brief Capsule should only be created by scene. private: friend class OgreScene; diff --git a/ogre/src/OgreCapsule.cc b/ogre/src/OgreCapsule.cc index 309355a77..1897afda3 100644 --- a/ogre/src/OgreCapsule.cc +++ b/ogre/src/OgreCapsule.cc @@ -52,34 +52,31 @@ OgreCapsule::~OgreCapsule() // no ops } +////////////////////////////////////////////////// +Ogre::MovableObject *OgreCapsule::OgreObject() const +{ + return this->dataPtr->ogreMesh->OgreObject(); +} + ////////////////////////////////////////////////// void OgreCapsule::PreRender() { if (this->capsuleDirty) { - this->Create(); + this->Update(); this->capsuleDirty = false; } } -////////////////////////////////////////////////// -Ogre::MovableObject *OgreCapsule::OgreObject() const -{ - return this->dataPtr->ogreMesh->OgreObject(); -} - ////////////////////////////////////////////////// void OgreCapsule::Init() { - this->Create(); + this->Update(); } ////////////////////////////////////////////////// void OgreCapsule::Destroy() { - if (!this->Scene()) - return; - if (this->dataPtr->ogreMesh) { this->dataPtr->ogreMesh->Destroy(); @@ -94,48 +91,49 @@ void OgreCapsule::Destroy() } ////////////////////////////////////////////////// -void OgreCapsule::Create() +void OgreCapsule::Update() { common::MeshManager *meshMgr = common::MeshManager::Instance(); std::string capsuleMeshName = this->Name() + "_capsule_mesh" + "_" + std::to_string(this->radius) + "_" + std::to_string(this->length); + + // Create new mesh if needed if (!meshMgr->HasMesh(capsuleMeshName)) { meshMgr->CreateCapsule(capsuleMeshName, this->radius, this->length, 32, 32); - MeshDescriptor meshDescriptor; - meshDescriptor.mesh = meshMgr->MeshByName(capsuleMeshName); - if (meshDescriptor.mesh == nullptr) - { - ignerr << "Capsule mesh is unavailable in the Mesh Manager" << std::endl; - return; - } - else + } + + MeshDescriptor meshDescriptor; + meshDescriptor.mesh = meshMgr->MeshByName(capsuleMeshName); + if (meshDescriptor.mesh == nullptr) + { + ignerr << "Capsule mesh is unavailable in the Mesh Manager" << std::endl; + return; + } + + auto visual = std::dynamic_pointer_cast(this->Parent()); + + // clear geom if needed + if (this->dataPtr->ogreMesh) + { + if (visual) { - auto visual = std::dynamic_pointer_cast(this->Parent()); - // clear geom if needed - if (this->dataPtr->ogreMesh) - { - if (visual) - { - visual->RemoveGeometry( - std::dynamic_pointer_cast(shared_from_this())); - } - this->dataPtr->ogreMesh->Destroy(); - } - this->dataPtr->ogreMesh = - std::dynamic_pointer_cast( - this->Scene()->CreateMesh(meshDescriptor)); - if (this->dataPtr->material != nullptr) - { - this->dataPtr->ogreMesh->SetMaterial(this->dataPtr->material, false); - } - if (visual) - { - visual->AddGeometry( - std::dynamic_pointer_cast(shared_from_this())); - } + visual->RemoveGeometry( + std::dynamic_pointer_cast(shared_from_this())); } + this->dataPtr->ogreMesh->Destroy(); + } + this->dataPtr->ogreMesh = std::dynamic_pointer_cast( + this->Scene()->CreateMesh(meshDescriptor)); + if (this->dataPtr->material != nullptr) + { + this->dataPtr->ogreMesh->SetMaterial(this->dataPtr->material, false); + } + if (visual) + { + visual->AddGeometry( + std::dynamic_pointer_cast(shared_from_this())); } } @@ -155,6 +153,7 @@ void OgreCapsule::SetMaterial(MaterialPtr _material, bool _unique) return; } + // Set material for the underlying dynamic renderable this->dataPtr->ogreMesh->SetMaterial(derived, false); this->dataPtr->material = derived; } diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2Capsule.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2Capsule.hh index ebb2d6616..f8755ee98 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2Capsule.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2Capsule.hh @@ -67,12 +67,8 @@ namespace ignition public: virtual void SetMaterial(MaterialPtr _material, bool _unique) override; - /// \brief Set material to capsule geometry. - /// \param[in] _material Ogre material. - protected: virtual void SetMaterialImpl(Ogre2MaterialPtr _material); - - /// \brief Create the Capsule geometry in ogre - private: void Create(); + /// \brief Update the capsule geometry in ogre + private: void Update(); /// \brief Capsule should only be created by scene. private: friend class Ogre2Scene; diff --git a/ogre2/src/Ogre2Capsule.cc b/ogre2/src/Ogre2Capsule.cc index 8a53e0118..cb15acef0 100644 --- a/ogre2/src/Ogre2Capsule.cc +++ b/ogre2/src/Ogre2Capsule.cc @@ -63,7 +63,7 @@ void Ogre2Capsule::PreRender() { if (this->capsuleDirty) { - this->Create(); + this->Update(); this->capsuleDirty = false; } } @@ -71,15 +71,12 @@ void Ogre2Capsule::PreRender() ////////////////////////////////////////////////// void Ogre2Capsule::Init() { - this->Create(); + this->Update(); } ////////////////////////////////////////////////// void Ogre2Capsule::Destroy() { - if (!this->Scene()) - return; - if (this->dataPtr->ogreMesh) { this->dataPtr->ogreMesh->Destroy(); @@ -93,50 +90,50 @@ void Ogre2Capsule::Destroy() } } -//////////////////////////////////////////////// -void Ogre2Capsule::Create() +////////////////////////////////////////////////// +void Ogre2Capsule::Update() { common::MeshManager *meshMgr = common::MeshManager::Instance(); std::string capsuleMeshName = this->Name() + "_capsule_mesh" + "_" + std::to_string(this->radius) + "_" + std::to_string(this->length); + + // Create new mesh if needed if (!meshMgr->HasMesh(capsuleMeshName)) { - meshMgr->CreateCapsule(capsuleMeshName, this->radius, this->length, 12, 32); - MeshDescriptor meshDescriptor; - meshDescriptor.mesh = meshMgr->MeshByName(capsuleMeshName); - if (meshDescriptor.mesh == nullptr) - { - ignerr << "Capsule mesh is unavailable in the Mesh Manager" << std::endl; - return; - } - else + meshMgr->CreateCapsule(capsuleMeshName, this->radius, this->length, 32, 32); + } + + MeshDescriptor meshDescriptor; + meshDescriptor.mesh = meshMgr->MeshByName(capsuleMeshName); + if (meshDescriptor.mesh == nullptr) + { + ignerr << "Capsule mesh is unavailable in the Mesh Manager" << std::endl; + return; + } + + auto visual = std::dynamic_pointer_cast(this->Parent()); + + // clear geom if needed + if (this->dataPtr->ogreMesh) + { + if (visual) { - auto visual = std::dynamic_pointer_cast(this->Parent()); - - // clear geom if needed - if (this->dataPtr->ogreMesh) - { - if (visual) - { - visual->RemoveGeometry( - std::dynamic_pointer_cast(shared_from_this())); - } - this->dataPtr->ogreMesh->Destroy(); - } - this->dataPtr->ogreMesh = - std::dynamic_pointer_cast( - this->Scene()->CreateMesh(meshDescriptor)); - if (this->dataPtr->material != nullptr) - { - this->dataPtr->ogreMesh->SetMaterial(this->dataPtr->material, false); - } - if (visual) - { - visual->AddGeometry( - std::dynamic_pointer_cast(shared_from_this())); - } + visual->RemoveGeometry( + std::dynamic_pointer_cast(shared_from_this())); } + this->dataPtr->ogreMesh->Destroy(); + } + this->dataPtr->ogreMesh = std::dynamic_pointer_cast( + this->Scene()->CreateMesh(meshDescriptor)); + if (this->dataPtr->material != nullptr) + { + this->dataPtr->ogreMesh->SetMaterial(this->dataPtr->material, false); + } + if (visual) + { + visual->AddGeometry( + std::dynamic_pointer_cast(shared_from_this())); } } @@ -157,14 +154,8 @@ void Ogre2Capsule::SetMaterial(MaterialPtr _material, bool _unique) } // Set material for the underlying dynamic renderable - this->dataPtr->ogreMesh->SetMaterial(_material, false); - this->SetMaterialImpl(derived); -} - -////////////////////////////////////////////////// -void Ogre2Capsule::SetMaterialImpl(Ogre2MaterialPtr _material) -{ - this->dataPtr->material = _material; + this->dataPtr->ogreMesh->SetMaterial(derived, false); + this->dataPtr->material = derived; } ////////////////////////////////////////////////// diff --git a/ogre2/src/Ogre2Marker.cc b/ogre2/src/Ogre2Marker.cc index 8cab1570b..705ed38ec 100644 --- a/ogre2/src/Ogre2Marker.cc +++ b/ogre2/src/Ogre2Marker.cc @@ -245,40 +245,8 @@ void Ogre2Marker::SetType(MarkerType _markerType) newMesh = this->scene->CreateBox(); break; case MT_CAPSULE: - { - common::MeshManager *meshMgr = common::MeshManager::Instance(); - std::string capsuleMeshName = std::string("marker_capsule_mesh") - + "_" + std::to_string(0.5) - + "_" + std::to_string(1.0); - if (!meshMgr->HasMesh(capsuleMeshName)) - { - meshMgr->CreateCapsule(capsuleMeshName, 0.5, 1.0, 12, 32); - MeshDescriptor meshDescriptor; - meshDescriptor.mesh = meshMgr->MeshByName(capsuleMeshName); - if (meshDescriptor.mesh == nullptr) - { - ignerr << "Capsule mesh is unavailable in the Mesh Manager" - << std::endl; - return; - } - else - { - this->dataPtr->mesh = - std::dynamic_pointer_cast( - this->Scene()->CreateMesh(meshDescriptor)); - if (this->dataPtr->material != nullptr) - { - this->dataPtr->mesh->SetMaterial(this->dataPtr->material, false); - } - if (visual) - { - visual->AddGeometry( - std::dynamic_pointer_cast(shared_from_this())); - } - } - } - } - return; + newMesh = this->scene->CreateCapsule(); + break; case MT_CYLINDER: newMesh = this->scene->CreateCylinder(); break; diff --git a/src/Capsule_TEST.cc b/src/Capsule_TEST.cc index f43172dfe..99e2b7ad9 100644 --- a/src/Capsule_TEST.cc +++ b/src/Capsule_TEST.cc @@ -75,9 +75,9 @@ void CapsuleTest::Capsule(const std::string &_renderEngine) capsule->SetMaterial(mat); MaterialPtr capsuleMat = capsule->Material(); ASSERT_NE(nullptr, capsuleMat); - EXPECT_EQ(math::Color(0.6, 0.7, 0.8), capsuleMat->Ambient()); - EXPECT_EQ(math::Color(0.3, 0.8, 0.2), capsuleMat->Diffuse()); - EXPECT_EQ(math::Color(0.4, 0.9, 1.0), capsuleMat->Specular()); + EXPECT_EQ(math::Color(0.6f, 0.7f, 0.8f), capsuleMat->Ambient()); + EXPECT_EQ(math::Color(0.3f, 0.8f, 0.2f), capsuleMat->Diffuse()); + EXPECT_EQ(math::Color(0.4f, 0.9f, 1.0f), capsuleMat->Specular()); // Clean up engine->DestroyScene(scene); diff --git a/src/Marker_TEST.cc b/src/Marker_TEST.cc index 213ecc109..9a6861020 100644 --- a/src/Marker_TEST.cc +++ b/src/Marker_TEST.cc @@ -81,6 +81,7 @@ void MarkerTest::Marker(const std::string &_renderEngine) marker->SetType(MarkerType::MT_CYLINDER); EXPECT_EQ(MarkerType::MT_CYLINDER, marker->Type()); + marker->SetType(MarkerType::MT_NONE); EXPECT_EQ(MarkerType::MT_NONE, marker->Type());