From fd64d7e0897fa37581843565a671dad9540fb580 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Wed, 4 Oct 2023 22:50:50 +0000 Subject: [PATCH 1/2] Prevent loading lightmaps if mesh is a glb file Signed-off-by: Ian Chen --- graphics/src/AssimpLoader.cc | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/graphics/src/AssimpLoader.cc b/graphics/src/AssimpLoader.cc index eeac1bee..ba2c5765 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); From e02a4c72565df865d90b5ba42df150829aaea264 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 5 Oct 2023 00:11:19 +0000 Subject: [PATCH 2/2] fix test Signed-off-by: Ian Chen --- graphics/src/AssimpLoader_TEST.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/graphics/src/AssimpLoader_TEST.cc b/graphics/src/AssimpLoader_TEST.cc index 87e6b821..f171d427 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();