port: 5 to main 2022-11-16 (#481)

Signed-off-by: Michael Carroll <[email protected]>

Changelog.md

@@ -4,6 +4,33 @@
 
 ## Gazebo Common 5.x
 
+## Gazebo Common 5.3.0 (2022-11-14)
+
+1. Expose Vertex & Index raw ptrs for efficient reading
+    * [Pull request #474](https://github.com/gazebosim/ign-common/pull/474)
+
+## Gazebo Common 5.2.2 (2022-10-26)
+
+1. [Backport] Avoid Io.hh header name clash (#471) 
+    * [Pull request #472](https://github.com/gazebosim/gz-common/pull/472)
+
+## Gazebo Common 5.2.1 (2022-10-19)
+
+1. Fix arm builds and tests
+    * [Pull request #462](https://github.com/gazebosim/gz-common/pull/462)
+    * [Pull request #463](https://github.com/gazebosim/gz-common/pull/463)
+
+## Gazebo Common 5.2.0 (2022-10-18)
+
+1. Add CSV data parsing
+    * [Pull request #402](https://github.com/gazebosim/gz-common/pull/402)
+
+1. Skip CSV header when reading DataFrame.
+    * [Pull request #435](https://github.com/gazebosim/gz-common/pull/435)
+
+1. Adds an API to retrieve keys.
+    * [Pull request #446](https://github.com/gazebosim/gz-common/pull/446)
+
 ## Gazebo Common 5.1.0 (2022-10-13)
 
 1. 4 ➡️  5

graphics/include/gz/common/SubMesh.hh

@@ -157,6 +157,12 @@ namespace gz
       /// \sa bool HasVertex(const unsigned int) const
       public: gz::math::Vector3d Vertex(const unsigned int _index) const;
 
+      /// \brief Get the raw vertex pointer. This is unsafe, it is the
+      /// caller's responsability to ensure it's not indexed out of bounds.
+      /// The valid range is [0; VertexCount())
+      /// \return Raw vertices
+      public: const gz::math::Vector3d* VertexPtr() const;
+
       /// \brief Set a vertex
       /// \param[in] _index Index of the vertex
       /// \param[in] _v The new vertex coordinate
@@ -217,6 +223,12 @@ namespace gz
       /// \return The index, or -1 if the _index is out of bounds.
       public: int Index(const unsigned int _index) const;
 
+      /// \brief Get the raw index pointer. This is unsafe, it is the
+      /// caller's responsability to ensure it's not indexed out of bounds.
+      /// The valid range is [0; IndexCount())
+      /// \return Raw indices
+      public: const unsigned int* IndexPtr() const;
+
       /// \brief Set an index
       /// \param[in] _index Index of the indices
       /// \param[in] _i The new index value to set to

graphics/src/AssimpLoader.cc

@@ -689,12 +689,19 @@ Mesh *AssimpLoader::Load(const std::string &_filename)
     {
       auto& animChan = anim->mChannels[chanIdx];
       auto chanName = ToString(animChan->mNodeName);
-      for (unsigned keyIdx = 0; keyIdx < animChan->mNumPositionKeys; ++keyIdx)
+      auto numKeys = std::max(
+          animChan->mNumPositionKeys, animChan->mNumRotationKeys);
+      // Position and rotation arrays might be different lengths,
+      // iterate over the maximum of the two, safely access by checking
+      // number of keys
+      for (unsigned keyIdx = 0; keyIdx < numKeys; ++keyIdx)
       {
         // Note, Scaling keys are not supported right now
         // Compute the position into a math pose
-        auto& posKey = animChan->mPositionKeys[keyIdx];
-        auto& quatKey = animChan->mRotationKeys[keyIdx];
+        auto& posKey = animChan->mPositionKeys[
+          std::min(keyIdx, animChan->mNumPositionKeys - 1)];
+        auto& quatKey = animChan->mRotationKeys[
+          std::min(keyIdx, animChan->mNumRotationKeys - 1)];
         math::Vector3d pos(posKey.mValue.x, posKey.mValue.y, posKey.mValue.z);
         math::Quaterniond quat(quatKey.mValue.w, quatKey.mValue.x,
             quatKey.mValue.y, quatKey.mValue.z);

graphics/src/Mesh_TEST.cc

@@ -95,18 +95,21 @@ TEST_F(MeshTest, Mesh)
   EXPECT_EQ(submesh.lock()->Vertex(0), v0 * scale);
   EXPECT_EQ(submesh.lock()->Normal(0), n0);
   EXPECT_EQ(submesh.lock()->TexCoord(0), uv0);
+  EXPECT_EQ(submesh.lock()->VertexPtr()[0], v0 * scale);
 
   mesh->Scale(scale);
   EXPECT_EQ(submesh.lock()->Vertex(0), v0 * scale * scale);
   EXPECT_EQ(submesh.lock()->Normal(0), n0);
   EXPECT_EQ(submesh.lock()->TexCoord(0), uv0);
+  EXPECT_EQ(submesh.lock()->VertexPtr()[0], v0 * scale * scale);
 
   // translate
   math::Vector3d t0(2, 3, -12);
   mesh->Translate(t0);
   EXPECT_EQ(submesh.lock()->Vertex(0), v0 * scale * scale + t0);
   EXPECT_EQ(submesh.lock()->Normal(0), n0);
   EXPECT_EQ(submesh.lock()->TexCoord(0), uv0);
+  EXPECT_EQ(submesh.lock()->VertexPtr()[0], v0 * scale * scale + t0);
 
   // center
   math::Vector3d c0(0.1, 3, 1);
@@ -151,4 +154,5 @@ TEST_F(MeshTest, Mesh)
   EXPECT_TRUE(math::equal(vertices[1], submesh.lock()->Vertex(0).Y()));
   EXPECT_TRUE(math::equal(vertices[2], submesh.lock()->Vertex(0).Z()));
   EXPECT_EQ(indices[0], submesh.lock()->Index(0));
+  EXPECT_EQ(indices[0], submesh.lock()->IndexPtr()[0]);
 }

graphics/src/SubMesh.cc

@@ -173,6 +173,12 @@ gz::math::Vector3d SubMesh::Vertex(const unsigned int _index) const
   return this->dataPtr->vertices[_index];
 }
 
+//////////////////////////////////////////////////
+const gz::math::Vector3d* SubMesh::VertexPtr() const
+{
+  return this->dataPtr->vertices.data();
+}
+
 //////////////////////////////////////////////////
 bool SubMesh::HasVertex(const unsigned int _index) const
 {
@@ -349,6 +355,12 @@ int SubMesh::Index(const unsigned int _index) const
   return this->dataPtr->indices[_index];
 }
 
+//////////////////////////////////////////////////
+const unsigned int* SubMesh::IndexPtr() const
+{
+  return this->dataPtr->indices.data();
+}
+
 //////////////////////////////////////////////////
 void SubMesh::SetIndex(const unsigned int _index, const unsigned int _i)
 {

graphics/src/SubMesh_TEST.cc

@@ -72,18 +72,21 @@ TEST_F(SubMeshTest, SubMesh)
   EXPECT_EQ(submesh->Vertex(0u), v0);
   EXPECT_TRUE(submesh->HasVertex(v0));
   EXPECT_EQ(submesh->IndexOfVertex(v0), 0);
+  EXPECT_EQ(submesh->VertexPtr()[0], v0);
 
   submesh->AddVertex(v1);
   EXPECT_EQ(submesh->VertexCount(), 2u);
   EXPECT_EQ(submesh->Vertex(1u), v1);
   EXPECT_TRUE(submesh->HasVertex(v1));
   EXPECT_EQ(submesh->IndexOfVertex(v1), 1);
+  EXPECT_EQ(submesh->VertexPtr()[1], v1);
 
   submesh->AddVertex(v2.X(), v2.Y(), v2.Z());
   EXPECT_EQ(submesh->VertexCount(), 3u);
   EXPECT_EQ(submesh->Vertex(2u), v2);
   EXPECT_TRUE(submesh->HasVertex(v2));
   EXPECT_EQ(submesh->IndexOfVertex(v2), 2);
+  EXPECT_EQ(submesh->VertexPtr()[2], v2);
 
   // max / min
   math::Vector3d max(2, 3, 3);
@@ -127,14 +130,17 @@ TEST_F(SubMeshTest, SubMesh)
   submesh->AddIndex(0u);
   EXPECT_EQ(submesh->IndexCount(), 1u);
   EXPECT_EQ(submesh->Index(0), 0);
+  EXPECT_EQ(submesh->IndexPtr()[0], 0u);
 
   submesh->AddIndex(2u);
   EXPECT_EQ(submesh->IndexCount(), 2u);
   EXPECT_EQ(submesh->Index(1u), 2);
+  EXPECT_EQ(submesh->IndexPtr()[1], 2u);
 
   submesh->AddIndex(1u);
   EXPECT_EQ(submesh->IndexCount(), 3u);
   EXPECT_EQ(submesh->Index(2u), 1);
+  EXPECT_EQ(submesh->IndexPtr()[2], 1u);
 
   // add node assignment
   submesh->AddNodeAssignment(1u, 0u, 0.5f);
@@ -153,10 +159,16 @@ TEST_F(SubMeshTest, SubMesh)
   // test directly setting values and failure cases
   submesh->SetVertex(2u, v0);
   EXPECT_EQ(submesh->Vertex(2u), v0);
+  EXPECT_EQ(submesh->VertexPtr()[2u], v0);
   submesh->SetVertex(2u, v2);
   EXPECT_EQ(submesh->Vertex(2u), v2);
+  EXPECT_EQ(submesh->VertexPtr()[2u], v2);
+
+  // Failure case: write out of bounds should be ignored
   submesh->SetVertex(3u, math::Vector3d(0.9, 2, 4));
   EXPECT_EQ(submesh->Vertex(3u), math::Vector3d::Zero);
+  // Out-of-bounds read of a raw pointer is UB. We can't test it.
+  // EXPECT_EQ(submesh->VertexPtr()[3u], math::Vector3d::Zero);
   EXPECT_FALSE(submesh->HasVertex(math::Vector3d(0.9, 2, 4)));
   EXPECT_EQ(submesh->IndexOfVertex(math::Vector3d(0.9, 2, 4)), -1);
 
@@ -191,6 +203,10 @@ TEST_F(SubMeshTest, SubMesh)
   EXPECT_EQ(submesh->Vertex(1), v1 * scale);
   EXPECT_EQ(submesh->Vertex(2), v2 * scale);
 
+  EXPECT_EQ(submesh->VertexPtr()[0], v0 * scale);
+  EXPECT_EQ(submesh->VertexPtr()[1], v1 * scale);
+  EXPECT_EQ(submesh->VertexPtr()[2], v2 * scale);
+
   EXPECT_EQ(submesh->Normal(0), n0);
   EXPECT_EQ(submesh->Normal(1), n1);
   EXPECT_EQ(submesh->Normal(2), n2);
@@ -206,6 +222,10 @@ TEST_F(SubMeshTest, SubMesh)
   EXPECT_EQ(submesh->Vertex(1), v1);
   EXPECT_EQ(submesh->Vertex(2), v2);
 
+  EXPECT_EQ(submesh->VertexPtr()[0], v0);
+  EXPECT_EQ(submesh->VertexPtr()[1], v1);
+  EXPECT_EQ(submesh->VertexPtr()[2], v2);
+
   EXPECT_EQ(submesh->Normal(0), n0);
   EXPECT_EQ(submesh->Normal(1), n1);
   EXPECT_EQ(submesh->Normal(2), n2);
@@ -222,6 +242,10 @@ TEST_F(SubMeshTest, SubMesh)
   EXPECT_EQ(submesh->Vertex(1), v1 + t0);
   EXPECT_EQ(submesh->Vertex(2), v2 + t0);
 
+  EXPECT_EQ(submesh->VertexPtr()[0], v0 + t0);
+  EXPECT_EQ(submesh->VertexPtr()[1], v1 + t0);
+  EXPECT_EQ(submesh->VertexPtr()[2], v2 + t0);
+
   EXPECT_EQ(submesh->Normal(0), n0);
   EXPECT_EQ(submesh->Normal(1), n1);
   EXPECT_EQ(submesh->Normal(2), n2);
@@ -239,6 +263,10 @@ TEST_F(SubMeshTest, SubMesh)
   EXPECT_EQ(submesh->Vertex(1), v1 + t0 + t);
   EXPECT_EQ(submesh->Vertex(2), v2 + t0 + t);
 
+  EXPECT_EQ(submesh->VertexPtr()[0], v0 + t0 + t);
+  EXPECT_EQ(submesh->VertexPtr()[1], v1 + t0 + t);
+  EXPECT_EQ(submesh->VertexPtr()[2], v2 + t0 + t);
+
   // copy constructor
   common::SubMeshPtr submeshCopy(new common::SubMesh(*(submesh.get())));
   ASSERT_NE(nullptr, submeshCopy);
@@ -254,13 +282,19 @@ TEST_F(SubMeshTest, SubMesh)
       submesh->NodeAssignmentsCount());
 
   for (unsigned int i = 0; i < submeshCopy->VertexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Vertex(i), submesh->Vertex(i));
+    EXPECT_EQ(submeshCopy->VertexPtr()[i], submesh->VertexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->NormalCount(); ++i)
     EXPECT_EQ(submeshCopy->Normal(i), submesh->Normal(i));
   for (unsigned int i = 0; i < submeshCopy->TexCoordCount(); ++i)
     EXPECT_EQ(submeshCopy->TexCoord(i), submesh->TexCoord(i));
   for (unsigned int i = 0; i < submeshCopy->IndexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Index(i), submesh->Index(i));
+    EXPECT_EQ(submeshCopy->IndexPtr()[i], submesh->IndexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->NodeAssignmentsCount(); ++i)
   {
     common::NodeAssignment nodeCopy =
@@ -286,21 +320,37 @@ TEST_F(SubMeshTest, SubMesh)
     EXPECT_DOUBLE_EQ(vertices[i*3], submesh->Vertex(i).X());
     EXPECT_DOUBLE_EQ(vertices[i*3+1], submesh->Vertex(i).Y());
     EXPECT_DOUBLE_EQ(vertices[i*3+2], submesh->Vertex(i).Z());
+
+    EXPECT_DOUBLE_EQ(vertices[i*3], submesh->VertexPtr()[i].X());
+    EXPECT_DOUBLE_EQ(vertices[i*3+1], submesh->VertexPtr()[i].Y());
+    EXPECT_DOUBLE_EQ(vertices[i*3+2], submesh->VertexPtr()[i].Z());
   }
   for (unsigned int i = 0; i < submeshCopy->IndexCount(); ++i)
+  {
     EXPECT_EQ(indices[i], submesh->Index(i));
-
+    EXPECT_EQ(indices[i], submesh->IndexPtr()[i]);
+  }
 
   // recalculate normal and verify they are different
   submesh->RecalculateNormals();
+
+  EXPECT_NE(submeshCopy->VertexPtr(), submesh->VertexPtr());
+  EXPECT_NE(submeshCopy->IndexPtr(), submesh->IndexPtr());
+
   for (unsigned int i = 0; i < submeshCopy->NormalCount(); ++i)
     EXPECT_NE(submeshCopy->Normal(i), submesh->Normal(i));
   for (unsigned int i = 0; i < submeshCopy->VertexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Vertex(i), submesh->Vertex(i));
+    EXPECT_EQ(submeshCopy->VertexPtr()[i], submesh->VertexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->TexCoordCount(); ++i)
     EXPECT_EQ(submeshCopy->TexCoord(i), submesh->TexCoord(i));
   for (unsigned int i = 0; i < submeshCopy->IndexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Index(i), submesh->Index(i));
+    EXPECT_EQ(submeshCopy->IndexPtr()[i], submesh->IndexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->NodeAssignmentsCount(); ++i)
   {
     common::NodeAssignment nodeCopy =
@@ -314,11 +364,17 @@ TEST_F(SubMeshTest, SubMesh)
   for (unsigned int i = 0; i < submeshCopy->NormalCount(); ++i)
     EXPECT_EQ(submeshCopy->Normal(i), submesh->Normal(i));
   for (unsigned int i = 0; i < submeshCopy->VertexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Vertex(i), submesh->Vertex(i));
+    EXPECT_EQ(submeshCopy->VertexPtr()[i], submesh->VertexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->TexCoordCount(); ++i)
     EXPECT_EQ(submeshCopy->TexCoord(i), submesh->TexCoord(i));
   for (unsigned int i = 0; i < submeshCopy->IndexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Index(i), submesh->Index(i));
+    EXPECT_EQ(submeshCopy->IndexPtr()[i], submesh->IndexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->NodeAssignmentsCount(); ++i)
   {
     common::NodeAssignment nodeCopy =
@@ -335,9 +391,15 @@ TEST_F(SubMeshTest, SubMesh)
   for (unsigned int i = 0; i < submeshCopy->NormalCount(); ++i)
     EXPECT_EQ(submeshCopy->Normal(i), submesh->Normal(i));
   for (unsigned int i = 0; i < submeshCopy->VertexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Vertex(i), submesh->Vertex(i));
+    EXPECT_EQ(submeshCopy->VertexPtr()[i], submesh->VertexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->IndexCount(); ++i)
+  {
     EXPECT_EQ(submeshCopy->Index(i), submesh->Index(i));
+    EXPECT_EQ(submeshCopy->IndexPtr()[i], submesh->IndexPtr()[i]);
+  }
   for (unsigned int i = 0; i < submeshCopy->NodeAssignmentsCount(); ++i)
   {
     common::NodeAssignment nodeCopy =

io/include/gz/common/DataFrame.hh

@@ -68,6 +68,18 @@ namespace gz
         return this->storage.at(_key);
       }
 
+      /// \brief Retrieve all keys
+      /// \return A vector with keys
+      public: const std::vector<K> Keys() const
+      {
+        std::vector<K> keyList;
+        for (auto &[k, _]: this->storage)
+        {
+          keyList.push_back(k);
+        }
+        return keyList;
+      }
+
       /// \brief Data frame storage
       private: std::unordered_map<K, V> storage;
     };

io/src/CSVStreams.cc

@@ -43,7 +43,7 @@ std::istream &ExtractCSVToken(
   char character;
   if (_stream.peek(), !_stream.fail() && _stream.eof())
   {
-    _token = {CSVToken::TERMINATOR, EOF};
+    _token = {CSVToken::TERMINATOR, static_cast<char>(EOF)};
   }
   else if (_stream.get(character))
   {
@@ -121,7 +121,8 @@ std::istream &ParseCSVRow(
             state = FIELD_START;
             break;
           case CSVToken::TERMINATOR:
-            if (token.character != EOF || !_row.empty() || text.tellp() > 0)
+            if (token.character != static_cast<char>(EOF) || !_row.empty() ||
+                text.tellp() > 0)
             {
               _row.push_back(text.str());
               state = RECORD_END;
@@ -140,7 +141,8 @@ std::istream &ParseCSVRow(
           state = FIELD_END;
           break;
         }
-        if (token.type != CSVToken::TERMINATOR || token.character != EOF)
+        if (token.type != CSVToken::TERMINATOR ||
+            token.character != static_cast<char>(EOF))
         {
           text << token.character;
           break;

io/src/CSVStreams_TEST.cc

@@ -53,7 +53,7 @@ TEST(CSVStreams, CanExtractCSVTokens)
 
   EXPECT_TRUE(ExtractCSVToken(sstream, token, CSVDialect::Unix));
   EXPECT_EQ(token.type, CSVToken::TERMINATOR);
-  EXPECT_EQ(token.character, EOF);
+  EXPECT_EQ(token.character, static_cast<char>(EOF));
 }
 
 /////////////////////////////////////////////////