From 41f80fd35bfc4dab07f6547074be14cbeba5e742 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Tue, 5 Sep 2023 12:01:57 -0700 Subject: [PATCH] Fix arrow visual indexing (#889) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix node indexing --------- Signed-off-by: Ian Chen Co-authored-by: Alejandro Hernández Cordero --- Migration.md | 4 +- include/gz/rendering/base/BaseArrowVisual.hh | 41 +++++--------------- include/gz/rendering/base/BaseAxisVisual.hh | 2 +- test/common_test/ArrowVisual_TEST.cc | 22 +++++++---- 4 files changed, 29 insertions(+), 40 deletions(-) diff --git a/Migration.md b/Migration.md index f3b41e0e9..527776ebb 100644 --- a/Migration.md +++ b/Migration.md @@ -18,7 +18,9 @@ release will remove the deprecated code. BaseStore APIs such as `IterByIndex` may now be different from before since the order of objects stored in maps and vectors are different. The iterators returned may also change or become invalid when objects are added or removed - from the store. + from the store. This impacts users using APIs to access nodes / visuals by + index, e.g. `Node::ChildByIndex` and `Scene::VisualByIndex` may now + return a different node pointer. ## Gazebo Rendering 6.x to 7.x diff --git a/include/gz/rendering/base/BaseArrowVisual.hh b/include/gz/rendering/base/BaseArrowVisual.hh index a928f6f4f..fde45f258 100644 --- a/include/gz/rendering/base/BaseArrowVisual.hh +++ b/include/gz/rendering/base/BaseArrowVisual.hh @@ -103,7 +103,7 @@ namespace gz template VisualPtr BaseArrowVisual::Head() const { - return std::dynamic_pointer_cast(this->ChildByIndex(2)); + return std::dynamic_pointer_cast(this->ChildByIndex(0)); } ////////////////////////////////////////////////// @@ -117,44 +117,28 @@ namespace gz template VisualPtr BaseArrowVisual::Rotation() const { - return std::dynamic_pointer_cast(this->ChildByIndex(0)); + return std::dynamic_pointer_cast(this->ChildByIndex(2)); } ////////////////////////////////////////////////// template void BaseArrowVisual::ShowArrowHead(bool _b) { - NodePtr child = this->ChildByIndex(2); - VisualPtr visual = std::dynamic_pointer_cast(child); - if (visual) - { - visual->SetVisible(_b); - } + this->Head()->SetVisible(_b); } ////////////////////////////////////////////////// template void BaseArrowVisual::ShowArrowShaft(bool _b) { - NodePtr child = this->ChildByIndex(1); - VisualPtr visual = std::dynamic_pointer_cast(child); - if (visual) - { - visual->SetVisible(_b); - } + this->Shaft()->SetVisible(_b); } ////////////////////////////////////////////////// template void BaseArrowVisual::ShowArrowRotation(bool _b) { - NodePtr child = this->ChildByIndex(0); - VisualPtr visual = std::dynamic_pointer_cast(child); - if (visual) - { - visual->SetVisible(_b); - this->rotationVisible = _b; - } + this->Rotation()->SetVisible(_b); } ////////////////////////////////////////////////// @@ -163,16 +147,11 @@ namespace gz { T::SetVisible(_visible); - NodePtr child = this->ChildByIndex(0); - VisualPtr visual = std::dynamic_pointer_cast(child); - if (visual) - { - // Force rotation visual visibility to false - // if the arrow visual is not visible. - // Else, rotation visual's visibility overrides - // its parent's visibility. - visual->SetVisible(this->rotationVisible && _visible); - } + // Force rotation visual visibility to false + // if the arrow visual is not visible. + // Else, rotation visual's visibility overrides + // its parent's visibility. + this->Rotation()->SetVisible(this->rotationVisible && _visible); } ////////////////////////////////////////////////// diff --git a/include/gz/rendering/base/BaseAxisVisual.hh b/include/gz/rendering/base/BaseAxisVisual.hh index 4d0677c2a..eefc46abc 100644 --- a/include/gz/rendering/base/BaseAxisVisual.hh +++ b/include/gz/rendering/base/BaseAxisVisual.hh @@ -126,7 +126,7 @@ namespace gz void BaseAxisVisual::ShowAxisHead(unsigned int _axis, bool _b) { auto arrow = std::dynamic_pointer_cast( - this->ChildByIndex(2u - _axis)); + this->ChildByIndex(_axis)); if (arrow) { arrow->ShowArrowHead(_b); diff --git a/test/common_test/ArrowVisual_TEST.cc b/test/common_test/ArrowVisual_TEST.cc index e22736647..2315979af 100644 --- a/test/common_test/ArrowVisual_TEST.cc +++ b/test/common_test/ArrowVisual_TEST.cc @@ -51,29 +51,37 @@ TEST_F(ArrowVisualTest, ArrowVisual) // check children and geometry EXPECT_EQ(3u, visual->ChildCount()); - NodePtr node = visual->ChildByIndex(0u); + NodePtr node = visual->Rotation(); VisualPtr child = std::dynamic_pointer_cast(node); ASSERT_NE(nullptr, child); EXPECT_EQ(1u, child->GeometryCount()); - EXPECT_EQ(node, visual->Rotation()); + MeshPtr mesh = std::dynamic_pointer_cast(child->GeometryByIndex(0u)); + ASSERT_NE(nullptr, mesh); + MeshDescriptor desc = mesh->Descriptor(); + EXPECT_NE(std::string::npos, desc.meshName.find("rotation")); - node = visual->ChildByIndex(1u); + node = visual->Shaft(); child = std::dynamic_pointer_cast(node); ASSERT_NE(nullptr, child); EXPECT_EQ(1u, child->GeometryCount()); - EXPECT_EQ(node, visual->Shaft()); + mesh = std::dynamic_pointer_cast(child->GeometryByIndex(0u)); + ASSERT_NE(nullptr, mesh); + desc = mesh->Descriptor(); + EXPECT_NE(std::string::npos, desc.meshName.find("cylinder")); - node = visual->ChildByIndex(2u); + node = visual->Head(); child = std::dynamic_pointer_cast(node); ASSERT_NE(nullptr, child); EXPECT_EQ(1u, child->GeometryCount()); - EXPECT_EQ(node, visual->Head()); - // test destroy ArrowVisualPtr visual2 = scene->CreateArrowVisual(); ASSERT_NE(nullptr, visual2); visual2->Destroy(); EXPECT_EQ(0u, visual2->ChildCount()); + mesh = std::dynamic_pointer_cast(child->GeometryByIndex(0u)); + ASSERT_NE(nullptr, mesh); + desc = mesh->Descriptor(); + EXPECT_NE(std::string::npos, desc.meshName.find("cone")); // Clean up engine->DestroyScene(scene);