Prevent material destruction if setting the same material to a submesh (#1168)

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

Author: iche033

include/gz/rendering/base/BaseMesh.hh

@@ -308,6 +308,13 @@ namespace gz
         subMesh->SetMaterial(_material, false);
       }
 
+      // If the same material is being set, return early and don't try
+      // to destroy the material. We still need call SetMaterial on the
+      // submeshes in case the user changed some material properties
+      // before setting it back.
+      if (!_unique && _material == this->material)
+        return;
+
       if (this->material && this->ownsMaterial)
         this->Scene()->DestroyMaterial(this->material);
 
@@ -441,6 +448,13 @@ namespace gz
 
       this->SetMaterialImpl(_material);
 
+      // If the same material is being set, return early and don't try
+      // to destroy the material. We still need to call SetMaterialImpl
+      // above in case the user changed some material properties
+      // before setting it back.
+      if (!_unique && _material == origMaterial)
+        return;
+
       if (origMaterial && origUnique)
         this->Scene()->DestroyMaterial(origMaterial);
 

test/common_test/Mesh_TEST.cc

@@ -81,6 +81,11 @@ TEST_F(MeshTest, MeshSubMesh)
   submesh->SetMaterial(matClone, true);
   EXPECT_NE(matClone, submesh->Material());
 
+  // Setting the same material but do not clone
+  MaterialPtr newMatClone = submesh->Material();
+  submesh->SetMaterial(newMatClone, false);
+  EXPECT_EQ(newMatClone, submesh->Material());
+
   // Clean up
   engine->DestroyScene(scene);
 }
@@ -309,6 +314,11 @@ TEST_F(MeshTest, MeshClone)
         false);
   }
 
+  // Setting the same material but do not clone
+  MaterialPtr newMatClone = clonedMesh->Material();
+  clonedMesh->SetMaterial(newMatClone, false);
+  EXPECT_EQ(newMatClone, clonedMesh->Material());
+
   // Clean up
   engine->DestroyScene(scene);
 }