Add check for valid indices in submesh (#667)

Signed-off-by: Ian Chen <[email protected]>

Author: iche033

graphics/include/gz/common/SubMesh.hh

@@ -405,6 +405,10 @@ namespace gz
       /// the primitive type is not TRIANGLES, or there are no triangles.
       public: double Volume() const;
 
+      /// \brief Verify that all indices point to a valid vertex in the submesh
+      /// \return True if all values of indices are valid, false otherwise.
+      public: bool HasValidIndices() const;
+
       /// \brief Private data pointer.
       GZ_UTILS_IMPL_PTR(dataPtr)
     };

graphics/src/SubMesh.cc

@@ -625,10 +625,12 @@ struct Neighbors
 void SubMesh::RecalculateNormals()
 {
   if (this->dataPtr->primitiveType != SubMesh::TRIANGLES
-      || this->dataPtr->indices.empty()
       || this->dataPtr->indices.size() % 3u != 0)
     return;
 
+  if (!this->HasValidIndices())
+    return;
+
   // Reset all the normals
   for (auto &n : this->dataPtr->normals)
     n.Set(0, 0, 0);
@@ -743,6 +745,9 @@ std::string SubMesh::Name() const
 //////////////////////////////////////////////////
 double SubMesh::Volume() const
 {
+  if (!this->HasValidIndices())
+    return 0.0;
+
   double volume = 0.0;
   if (this->dataPtr->primitiveType == SubMesh::TRIANGLES)
   {
@@ -774,6 +779,20 @@ double SubMesh::Volume() const
   return volume;
 }
 
+//////////////////////////////////////////////////
+bool SubMesh::HasValidIndices() const
+{
+  if (this->dataPtr->indices.empty())
+    return false;
+
+  for (unsigned int idx = 0u; idx < this->dataPtr->indices.size(); ++idx)
+  {
+    if (this->dataPtr->indices[idx] >= this->dataPtr->vertices.size())
+      return false;
+  }
+  return true;
+}
+
 //////////////////////////////////////////////////
 NodeAssignment::NodeAssignment()
   : vertexIndex(0), nodeIndex(0), weight(0.0)

graphics/src/SubMesh_TEST.cc

@@ -598,3 +598,37 @@ TEST_F(SubMeshTest, NormalsRecalculation)
   // because of neighbour vertex
   ASSERT_NE(submesh->Normal(0), submesh->Normal(1));
 }
+
+/////////////////////////////////////////////////
+TEST_F(SubMeshTest, HasValidIndices)
+{
+  // Check empty submesh
+  auto submesh = std::make_shared<common::SubMesh>();
+  EXPECT_FALSE(submesh->HasValidIndices());
+
+  // Check box submesh
+  const common::Mesh *unitBox =
+    common::MeshManager::Instance()->MeshByName("unit_box");
+  ASSERT_TRUE(unitBox != nullptr);
+  auto unitBoxSubmesh = unitBox->SubMeshByIndex(0).lock();
+
+  // Submesh has valid indices and we should be able to calculate normals
+  // and volume
+  EXPECT_TRUE(unitBoxSubmesh->HasValidIndices());
+  unitBoxSubmesh->RecalculateNormals();
+  ASSERT_EQ(unitBoxSubmesh->VertexCount(), unitBoxSubmesh->NormalCount());
+  EXPECT_EQ(24u, unitBoxSubmesh->NormalCount());
+  EXPECT_DOUBLE_EQ(1.0, unitBoxSubmesh->Volume());
+
+  // Add some invalid indices
+  unitBoxSubmesh->AddIndex(100);
+  unitBoxSubmesh->AddIndex(101);
+  unitBoxSubmesh->AddIndex(102);
+
+  // Submesh has invalid indices so it is no longer able to update normals or
+  // compute volume.
+  EXPECT_FALSE(unitBoxSubmesh->HasValidIndices());
+  unitBoxSubmesh->RecalculateNormals();
+  EXPECT_EQ(24u, unitBoxSubmesh->NormalCount());
+  EXPECT_DOUBLE_EQ(0.0, unitBoxSubmesh->Volume());
+}