Merge gz-common5 ➡️ main (#541)

Signed-off-by: Michael Carroll <[email protected]>
gazebosim · Oct 9, 2023 · 669c4ec · 669c4ec
2 parents e5465f1 + 8c1213e
commit 669c4ec
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 11 deletions.
diff --git a/graphics/src/AssimpLoader.cc b/graphics/src/AssimpLoader.cc
@@ -425,6 +425,21 @@ MaterialPtr AssimpLoader::Implementation::CreateMaterial(
           this->GenerateTextureName(_scene, assimpMat, "Roughness"));
       pbr.SetRoughnessMap(texName, texData);
     }
+    // Load lightmap only if it is not a glb/glTF mesh that contains a
+    // MetallicRoughness texture
+    // It was found that lightmap field just stores the entire MetallicRoughness
+    // texture. Issues were also reported in assimp:
+    // https://github.com/assimp/assimp/issues/3120
+    // https://github.com/assimp/assimp/issues/4637
+    unsigned int uvIdx = 0;
+    ret = assimpMat->GetTexture(
+        aiTextureType_LIGHTMAP, 0, &texturePath, NULL, &uvIdx);
+    if (ret == AI_SUCCESS)
+    {
+      auto [texName, texData] = this->LoadTexture(_scene, texturePath,
+          this->GenerateTextureName(_scene, assimpMat, "Lightmap"));
+      pbr.SetLightMap(texName, uvIdx, texData);
+    }
   }
 #endif
   ret = assimpMat->GetTexture(aiTextureType_NORMALS, 0, &texturePath);
@@ -442,15 +457,6 @@ MaterialPtr AssimpLoader::Implementation::CreateMaterial(
         this->GenerateTextureName(_scene, assimpMat, "Emissive"));
     pbr.SetEmissiveMap(texName, texData);
   }
-  unsigned int uvIdx = 0;
-  ret = assimpMat->GetTexture(
-      aiTextureType_LIGHTMAP, 0, &texturePath, NULL, &uvIdx);
-  if (ret == AI_SUCCESS)
-  {
-    auto [texName, texData] = this->LoadTexture(_scene, texturePath,
-        this->GenerateTextureName(_scene, assimpMat, "Lightmap"));
-    pbr.SetLightMap(texName, uvIdx, texData);
-  }
 #ifndef GZ_ASSIMP_PRE_5_2_0
   float value;
   ret = assimpMat->Get(AI_MATKEY_METALLIC_FACTOR, value);

diff --git a/graphics/src/AssimpLoader_TEST.cc b/graphics/src/AssimpLoader_TEST.cc
@@ -708,10 +708,16 @@ TEST_F(AssimpLoader, LoadGlbPbrAsset)
   // Check pixel values to test metallicroughness texture splitting
   EXPECT_FLOAT_EQ(pbr->MetalnessMapData()->Pixel(256, 256).R(), 0.0);
   EXPECT_FLOAT_EQ(pbr->RoughnessMapData()->Pixel(256, 256).R(), 124.0 / 255.0);
+
   // Bug in assimp 5.0.x that doesn't parse coordinate sets properly
-  EXPECT_EQ(pbr->LightMapTexCoordSet(), 1);
+  // \todo(iche033) Lightmaps are disabled for glb meshes
+  // due to upstream bug
+  // EXPECT_EQ(pbr->LightMapTexCoordSet(), 1);
 #endif
-  EXPECT_NE(pbr->LightMapData(), nullptr);
+
+  // \todo(iche033) Lightmaps are disabled for glb meshes
+  // due to upstream bug
+  // EXPECT_NE(pbr->LightMapData(), nullptr);
 
   // Mesh has 3 animations
   auto skel = mesh->MeshSkeleton();