From db53b4ad8ffd6b60f2d3a5e9106d7c2152b6a0b6 Mon Sep 17 00:00:00 2001
From: Razakhel <romain.milbert@gmail.com>
Date: Sun, 21 Jan 2024 15:10:44 +0100
Subject: [PATCH] [Data/GltfLoad] Fixed textures & images pairing

- The texture index was wrongly assumed to be the image index, while the latter is actually the textures' source field

- Fixes issue #64
---
 src/RaZ/Data/GltfLoad.cpp                     | 52 ++++++++++++++-----
 src/RaZ/Render/ShaderProgram.cpp              |  2 +-
 .../meshes/\303\247\303\273b\303\250.gltf"    | 16 +++---
 3 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/src/RaZ/Data/GltfLoad.cpp b/src/RaZ/Data/GltfLoad.cpp
index 00ed13f7..564ff01e 100644
--- a/src/RaZ/Data/GltfLoad.cpp
+++ b/src/RaZ/Data/GltfLoad.cpp
@@ -325,7 +325,28 @@ std::pair<Image, Image> extractMetalnessRoughnessImages(const Image& metalRoughI
   return { std::move(metalnessImg), std::move(roughnessImg) };
 }
 
-void loadMaterials(const std::vector<fastgltf::Material>& materials, const std::vector<std::optional<Image>>& images, MeshRenderer& meshRenderer) {
+template <template <typename> typename OptionalT, typename TextureInfoT, typename FuncT>
+void loadTexture(const OptionalT<TextureInfoT>& textureInfo,
+                 const std::vector<fastgltf::Texture>& textures,
+                 const std::vector<std::optional<Image>>& images,
+                 const FuncT& callback) {
+  static_assert(std::is_base_of_v<fastgltf::TextureInfo, TextureInfoT>);
+
+  if (!textureInfo.has_value())
+    return;
+
+  const fastgltf::Optional<std::size_t>& imgIndex = textures[textureInfo->textureIndex].imageIndex;
+
+  if (!imgIndex.has_value() || !images[*imgIndex].has_value())
+    return;
+
+  callback(*images[*imgIndex]);
+}
+
+void loadMaterials(const std::vector<fastgltf::Material>& materials,
+                   const std::vector<fastgltf::Texture>& textures,
+                   const std::vector<std::optional<Image>>& images,
+                   MeshRenderer& meshRenderer) {
   Logger::debug("[GltfLoad] Loading " + std::to_string(materials.size()) + " material(s)...");
 
   meshRenderer.getMaterials().clear();
@@ -343,25 +364,28 @@ void loadMaterials(const std::vector<fastgltf::Material>& materials, const std::
     matProgram.setAttribute(mat.pbrData.metallicFactor, MaterialAttribute::Metallic);
     matProgram.setAttribute(mat.pbrData.roughnessFactor, MaterialAttribute::Roughness);
 
-    if (mat.pbrData.baseColorTexture && images[mat.pbrData.baseColorTexture->textureIndex])
-      matProgram.setTexture(Texture2D::create(*images[mat.pbrData.baseColorTexture->textureIndex]), MaterialTexture::BaseColor);
+    loadTexture(mat.pbrData.baseColorTexture, textures, images, [&matProgram] (const Image& img) {
+      matProgram.setTexture(Texture2D::create(img), MaterialTexture::BaseColor);
+    });
 
-    if (mat.emissiveTexture && images[mat.emissiveTexture->textureIndex])
-      matProgram.setTexture(Texture2D::create(*images[mat.emissiveTexture->textureIndex]), MaterialTexture::Emissive);
+    loadTexture(mat.emissiveTexture, textures, images, [&matProgram] (const Image& img) {
+      matProgram.setTexture(Texture2D::create(img), MaterialTexture::Emissive);
+    });
 
-    if (mat.occlusionTexture && images[mat.occlusionTexture->textureIndex]) { // Ambient occlusion
-      const Image ambientOcclusionImg = extractAmbientOcclusionImage(*images[mat.occlusionTexture->textureIndex]);
+    loadTexture(mat.occlusionTexture, textures, images, [&matProgram] (const Image& img) {
+      const Image ambientOcclusionImg = extractAmbientOcclusionImage(img);
       matProgram.setTexture(Texture2D::create(ambientOcclusionImg), MaterialTexture::Ambient);
-    }
+    });
 
-    if (mat.normalTexture && images[mat.normalTexture->textureIndex])
-      matProgram.setTexture(Texture2D::create(*images[mat.normalTexture->textureIndex]), MaterialTexture::Normal);
+    loadTexture(mat.normalTexture, textures, images, [&matProgram] (const Image& img) {
+      matProgram.setTexture(Texture2D::create(img), MaterialTexture::Normal);
+    });
 
-    if (mat.pbrData.metallicRoughnessTexture && images[mat.pbrData.metallicRoughnessTexture->textureIndex]) {
-      const auto [metalnessImg, roughnessImg] = extractMetalnessRoughnessImages(*images[mat.pbrData.metallicRoughnessTexture->textureIndex]);
+    loadTexture(mat.pbrData.metallicRoughnessTexture, textures, images, [&matProgram] (const Image& img) {
+      const auto [metalnessImg, roughnessImg] = extractMetalnessRoughnessImages(img);
       matProgram.setTexture(Texture2D::create(metalnessImg), MaterialTexture::Metallic);
       matProgram.setTexture(Texture2D::create(roughnessImg), MaterialTexture::Roughness);
-    }
+    });
 
     loadedMat.loadType(MaterialType::COOK_TORRANCE);
   }
@@ -407,7 +431,7 @@ std::pair<Mesh, MeshRenderer> load(const FilePath& filePath) {
   auto [mesh, meshRenderer] = loadMeshes(asset->meshes, asset->buffers, asset->bufferViews, asset->accessors, transforms);
 
   const std::vector<std::optional<Image>> images = loadImages(asset->images, asset->buffers, asset->bufferViews, parentPath);
-  loadMaterials(asset->materials, images, meshRenderer);
+  loadMaterials(asset->materials, asset->textures, images, meshRenderer);
 
   Logger::debug("[GltfLoad] Loaded glTF file (" + std::to_string(mesh.getSubmeshes().size()) + " submesh(es), "
                                                 + std::to_string(mesh.recoverVertexCount()) + " vertices, "
diff --git a/src/RaZ/Render/ShaderProgram.cpp b/src/RaZ/Render/ShaderProgram.cpp
index b6c7448a..17443f25 100644
--- a/src/RaZ/Render/ShaderProgram.cpp
+++ b/src/RaZ/Render/ShaderProgram.cpp
@@ -130,7 +130,7 @@ void ShaderProgram::setImageTexture(TexturePtr texture, const std::string& unifo
     throw std::runtime_error("Error: Using image textures requires OpenGL 4.2+ or OpenGL ES 3.1+");
   }
 
-  if (texture->getColorspace() == Raz::TextureColorspace::INVALID || texture->getColorspace() == Raz::TextureColorspace::DEPTH)
+  if (texture->getColorspace() == TextureColorspace::INVALID || texture->getColorspace() == TextureColorspace::DEPTH)
     throw std::invalid_argument("Error: The given image texture's colorspace is invalid");
 
   auto imgTextureIt = std::find_if(m_imageTextures.begin(), m_imageTextures.end(), [&uniformName] (const auto& element) {
diff --git "a/tests/assets/meshes/\303\247\303\273b\303\250.gltf" "b/tests/assets/meshes/\303\247\303\273b\303\250.gltf"
index 5137c39c..99bddc68 100644
--- "a/tests/assets/meshes/\303\247\303\273b\303\250.gltf"
+++ "b/tests/assets/meshes/\303\247\303\273b\303\250.gltf"
@@ -146,19 +146,19 @@
           0.99
         ],
         "baseColorTexture": {
-          "index": 0
+          "index": 2
         },
         "metallicFactor": 0.5,
         "roughnessFactor": 0.25,
         "metallicRoughnessTexture": {
-          "index": 0
+          "index": 2
         }
       },
       "normalTexture": {
-        "index": 2
+        "index": 1
       },
       "occlusionTexture": {
-        "index": 0
+        "index": 2
       },
       "emissiveFactor": [
         0.75,
@@ -166,7 +166,7 @@
         0.75
       ],
       "emissiveTexture": {
-        "index": 1
+        "index": 0
       }
     }
   ],
@@ -302,15 +302,15 @@
   "textures": [
     {
       "sampler": 0,
-      "source": 0
+      "source": 1
     },
     {
       "sampler": 0,
-      "source": 1
+      "source": 2
     },
     {
       "sampler": 0,
-      "source": 2
+      "source": 0
     }
   ]
 }