Fix arrow visual indexing (#889)

* fix node indexing --------- Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]>
gazebosim · Sep 5, 2023 · 41f80fd · 41f80fd
1 parent 077e1e7
commit 41f80fd
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 40 deletions.
diff --git 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
@@ -103,7 +103,7 @@ namespace gz
     template <class T>
     VisualPtr BaseArrowVisual<T>::Head() const
     {
-      return std::dynamic_pointer_cast<Visual>(this->ChildByIndex(2));
+      return std::dynamic_pointer_cast<Visual>(this->ChildByIndex(0));
     }
 
     //////////////////////////////////////////////////
@@ -117,44 +117,28 @@ namespace gz
     template <class T>
     VisualPtr BaseArrowVisual<T>::Rotation() const
     {
-      return std::dynamic_pointer_cast<Visual>(this->ChildByIndex(0));
+      return std::dynamic_pointer_cast<Visual>(this->ChildByIndex(2));
     }
 
     //////////////////////////////////////////////////
     template <class T>
     void BaseArrowVisual<T>::ShowArrowHead(bool _b)
     {
-      NodePtr child = this->ChildByIndex(2);
-      VisualPtr visual = std::dynamic_pointer_cast<Visual>(child);
-      if (visual)
-      {
-        visual->SetVisible(_b);
-      }
+      this->Head()->SetVisible(_b);
     }
 
     //////////////////////////////////////////////////
     template <class T>
     void BaseArrowVisual<T>::ShowArrowShaft(bool _b)
     {
-      NodePtr child = this->ChildByIndex(1);
-      VisualPtr visual = std::dynamic_pointer_cast<Visual>(child);
-      if (visual)
-      {
-        visual->SetVisible(_b);
-      }
+      this->Shaft()->SetVisible(_b);
     }
 
     //////////////////////////////////////////////////
     template <class T>
     void BaseArrowVisual<T>::ShowArrowRotation(bool _b)
     {
-      NodePtr child = this->ChildByIndex(0);
-      VisualPtr visual = std::dynamic_pointer_cast<Visual>(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<Visual>(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
@@ -126,7 +126,7 @@ namespace gz
     void BaseAxisVisual<T>::ShowAxisHead(unsigned int _axis, bool _b)
     {
       auto arrow = std::dynamic_pointer_cast<rendering::ArrowVisual>(
-            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
@@ -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<Visual>(node);
   ASSERT_NE(nullptr, child);
   EXPECT_EQ(1u, child->GeometryCount());
-  EXPECT_EQ(node, visual->Rotation());
+  MeshPtr mesh = std::dynamic_pointer_cast<Mesh>(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<Visual>(node);
   ASSERT_NE(nullptr, child);
   EXPECT_EQ(1u, child->GeometryCount());
-  EXPECT_EQ(node, visual->Shaft());
+  mesh = std::dynamic_pointer_cast<Mesh>(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<Visual>(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<Mesh>(child->GeometryByIndex(0u));
+  ASSERT_NE(nullptr, mesh);
+  desc = mesh->Descriptor();
+  EXPECT_NE(std::string::npos, desc.meshName.find("cone"));
 
   // Clean up
   engine->DestroyScene(scene);