From 7838ced4b8547be15ddeaa76fbf3ff6d900a7b72 Mon Sep 17 00:00:00 2001
From: Ian Chen <ichen@openrobotics.org>
Date: Thu, 5 Oct 2023 09:50:31 -0700
Subject: [PATCH 1/2] Prevent loading lightmaps if mesh is a glb file that has
 an occlusion-metallic-roughness texture (#538)

* Prevent loading lightmaps if mesh is a glb file


---------

Signed-off-by: Ian Chen <ichen@openrobotics.org>
---
 graphics/src/AssimpLoader.cc      | 24 +++++++++++++++---------
 graphics/src/AssimpLoader_TEST.cc | 10 ++++++++--
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/graphics/src/AssimpLoader.cc b/graphics/src/AssimpLoader.cc
index eeac1bee3..ba2c57655 100644
--- 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
index 87e6b8210..f171d427e 100644
--- 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();

From d9d1722d3251ded4c716a1d30a146d17eca4534c Mon Sep 17 00:00:00 2001
From: Steve Peters <scpeters@openrobotics.org>
Date: Sat, 7 Oct 2023 22:31:37 -0700
Subject: [PATCH 2/2] EnumIface: suppress deprecation warning (#540)

The EnumIterator inherits from std::iterator, which has
been deprecated in c++17. There is a fix in gz-common6
but it breaks API, so it can't be merged to Garden. So
just suppress the deprecation warning in Garden.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
---
 include/gz/common/EnumIface.hh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/gz/common/EnumIface.hh b/include/gz/common/EnumIface.hh
index 2b0fe1dd8..83a03e22a 100644
--- a/include/gz/common/EnumIface.hh
+++ b/include/gz/common/EnumIface.hh
@@ -141,6 +141,7 @@ namespace gz
     ///   std::cout << "Type++ Name[" << myTypeIface.Str(*i) << "]\n";
     /// }
     /// \verbatim
+    GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
     template<typename Enum>
     class EnumIterator
     : std::iterator<std::bidirectional_iterator_tag, Enum>
@@ -218,6 +219,7 @@ namespace gz
       /// member value ever used.
       private: Enum c;
     };
+    GZ_UTILS_WARN_RESUME__DEPRECATED_DECLARATION
 
     /// \brief Equality operator
     /// \param[in] _e1 First iterator