Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I think there is an error in file GltfLoad.cpp #64

Closed
vrazumov opened this issue Jan 21, 2024 · 2 comments
Closed

I think there is an error in file GltfLoad.cpp #64

vrazumov opened this issue Jan 21, 2024 · 2 comments
Labels
Bug Bug that needs to be fixed Data Issues related to data (structures, formats, ...)

Comments

@vrazumov
Copy link

I think there is an error in file GltfLoad.cpp
Available:

347 if (mat.pbrData.baseColorTexture && images[mat.pbrData.baseColorTexture->textureIndex])
348 matProgram.setTexture(Texture2D::create(*images[mat.pbrData.baseColorTexture->textureIndex]), MaterialTexture::BaseColor);

...

Right:

add const std::vectorfastgltf::Texture& textures as parameter in void loadMaterials

347 if (mat.pbrData.baseColorTexture && textures[mat.pbrData.baseColorTexture->textureIndex].imageIndex)
{
auto imageIndex = textures[mat.pbrData.baseColorTexture->textureIndex].imageIndex.value();
matProgram.setTexture(Texture2D::create(*images[imageIndex]), MaterialTexture::BaseColor);
}

if (mat.emissiveTexture && textures[mat.emissiveTexture->textureIndex].imageIndex)
{
  auto imageIndex = textures[mat.emissiveTexture->textureIndex].imageIndex.value();
  matProgram.setTexture(Texture2D::create(*images[imageIndex]), MaterialTexture::Emissive);
}  

if (mat.occlusionTexture && textures[mat.occlusionTexture->textureIndex].imageIndex) { // Ambient occlusion

  const Image ambientOcclusionImg = extractAmbientOcclusionImage(*images[mat.occlusionTexture->textureIndex]);
  matProgram.setTexture(Texture2D::create(ambientOcclusionImg), MaterialTexture::Ambient);
}

if (mat.normalTexture && textures[mat.normalTexture->textureIndex].imageIndex)
{
  auto imageIndex = textures[mat.normalTexture->textureIndex].imageIndex.value();
  matProgram.setTexture(Texture2D::create(*images[imageIndex]), MaterialTexture::Normal);
}

if (mat.pbrData.metallicRoughnessTexture && textures[mat.pbrData.metallicRoughnessTexture->textureIndex].imageIndex) {
  const auto [metalnessImg, roughnessImg] = extractMetalnessRoughnessImages(*images[mat.pbrData.metallicRoughnessTexture->textureIndex]);
  matProgram.setTexture(Texture2D::create(metalnessImg), MaterialTexture::Metallic);
  matProgram.setTexture(Texture2D::create(roughnessImg), MaterialTexture::Roughness);
}

At least it works correctly for me.

@Razakhel
Copy link
Owner

Razakhel commented Jan 21, 2024

Very true, I misinterpreted how textures & images work within the format. This also fixes the Sponza test model (although not entirely as some triangles are much too stretched for yet unknown reasons), with which most textures were wrong; that explains a lot. I'll unit test this and push a commit soon, thanks a lot!

@Razakhel Razakhel added Bug Bug that needs to be fixed Data Issues related to data (structures, formats, ...) labels Jan 21, 2024
Razakhel added a commit that referenced this issue Jan 21, 2024
- The texture index was wrongly assumed to be the image index, while the latter is actually the textures' source field

- Fixes issue #64
@Razakhel
Copy link
Owner

Fixed by commit db53b4a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug that needs to be fixed Data Issues related to data (structures, formats, ...)
Projects
None yet
Development

No branches or pull requests

2 participants