godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.27k stars 21.04k forks source link

GLTF mesh primitive texCoord property ignored #80858

Open MAG-MichaelK opened 1 year ago

MAG-MichaelK commented 1 year ago

Godot version

4.1.1-stable

System information

Windows 10

Issue description

According to the GLTF 2.0 specification on materials, individual textures can specify a UV layer to use by providing a texCoord property. Godot currently does not read this property and always assumes UV layer 1 should be used. This results in incorrect rendering when importing GLTF models that have textures that use any UV channel besides 1.

The below image shows the SheenChair GLTF sample model after being imported into Godot 4.1.1-stable:

image

Notice that the AO texture is using the wrong UV layer and causing odd shadows on the chair. Below is the expected result:

image

Steps to reproduce

Minimal reproduction project

GLTFTexCoordMinimalReproduction.zip

Calinou commented 1 year ago

BaseMaterial3D already allows you to specify which UV channel emission and AO should be read on (emission_on_uv2, ao_on_uv2). This should be a matter of tweaking the glTF importer code to set this BaseMaterial3D property based on glTF data.

MAG-MichaelK commented 1 year ago

I have also come across issues with models exported from Unreal using UV2 for albedo textures. My PR shows an example of this.

aaronfranke commented 1 week ago

@MAG-MichaelK Here is what the chair looks like when imported with PR #96748.

Screenshot 2024-10-09 at 9 08 48 PM

PR #96748 handles the UV maps entirely within the glTF import code, remapping them to Godot's UV maps as needed. In cases where it can't perform the mapping, it will show a warning. Your test file does not show a warning, so it's mapped correctly without any compromises. Is this sufficient for your needs?

Your PR #80522 breaks compatibility and changes a core engine feature for the sake of glTF, which is a harder sell. Also, PR #80522 is something @lyuma considers an enhancement rather than a bugfix, and needs justification for all of the complexity that comes with it, while PR #96748 is more of a bugfix from the glTF perspective, since it's reading the texCoord property that's a part of the glTF spec and mapping it to Godot's existing mesh data as best as possible (but I'm still labeling it as an enhancement because UV2 support for glTF is a new feature from Godot's perspective).