Faile to load a gltf file which can be loaded successfully after "pbr work"

[piaoger]

Bevy version

94c4184

Operating system & version

MacOS 10.14.6

What you did

Try to load attached gltf file with tweaked example load_gltf but failed.
I tried it a couple of days before and It can be loaded after the recently change for "PBR work".

I just changed the below line in the example

commands.spawn_scene(asset_server.load("models/xiaorentou-gltf/1.gltf#Scene0"));

What you expected to happen

The attached gltf file should be loaded as before.

What actually happened

example load_gltf crashes. Maybe there are something wrong in latest change for "pbr textures" by mtsr

Additional information

I have attached my gltf file and snapshot as below:

xiaorentou-2.zip

[bjorn3]

Whatvis the exact message with which it crashed? And preferably can you get a backtrace? (RUST_BACKTRACE=1)

[mockersf]

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Compilation("glslang_shader_parse:\nInfo log:\nERROR: 0:335: \'assign\' :  l-value required \"anon@7\" (can\'t modify a uniform)\nERROR: 0:335: \'\' : compilation terminated \nERROR: 2 compilation errors.  No code generated.\n\n\nDebug log:\n\n")', crates/bevy_render/src/pipeline/pipeline_compiler.rs:161:22

backtrace doesn't have anything really interesting

[jakobhellermann]

There's a PR up to fix it: #1771

[piaoger]

There's a PR up to fix it: #1771

fixed above issue but found a new one:

thread 'main' panicked at 'Attribute Vertex_Tangent is required by shader, but not supplied by mesh. Either remove the attribute from the shader or supply the attribute (Vertex_Tangent) to the mesh.', crates/bevy_render/src/pipeline/pipeline_compiler.rs:231:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[jakobhellermann]

That's because we now support normal maps and require the Vertex_Tangent attributes if the normal map texture is set.

As a workaround until we can generate the vertices on the fly (#121) you can load the file into blender, then export as gltf and check the Geometry/Tangents box.

[piaoger]

That's because we now support normal maps and require the Vertex_Tangent attributes if the normal map texture is set.

As a workaround until we can generate the vertices on the fly (#121) you can load the file into blender, then export as gltf and check the Geometry/Tangents box.

OK，that's the result. Maybe it can be beautiful as above if HDR is used :)

[inodentry]

Could we sneak in some quick workaround to at least not crash (like maybe render without normal maps if there are no tangents) into 0.5? It can see this becoming a common issue / pain point for new bevy users. Try to load a model ... bevy barfs and crashes.

[piaoger]

Could we sneak in some quick workaround to at least not crash (like maybe render without normal maps if there are no tangents) into 0.5? It can see this becoming a common issue / pain point for new bevy users. Try to load a model ... bevy barfs and crashes.

Yes, my above gltf file is exported from Substance Painter directly. Its' based on an sample file though some tweaks.

[mockersf]

There is no simple and quick fix for this... It's an issue we already have in 0.4 and 0.3 with attribute Vertex_Uv.

Comments on #1010 may be relevant

[jakobhellermann]

There is no simple and quick fix for this... It's an issue we already have in 0.4 and 0.3 with attribute Vertex_Uv.

Wouldn't a quick fix be to not load the normal maps if the texture doesn't contain the tangent attributes?

[mockersf]

Currently, the gltf loader:

1. load the materials, keep handles
2. load the meshes and store handle to the materials in the meshes, and keep handles

For the quick fix, it's not a small refactor of the loader, we would need to:

1. load the materials and keep them around
2. load the meshes, and modify materials to remove normal maps when no tangents, keep the meshes
3. store the materials, keep the handles
4. modify the meshes to have the materials handle

I think your pr (#1795) is the right way forward as soon as it's available on crates.io, feature or fork

[inodentry]

yes #1795 is the proper solution

I simply asked if anyone who is more familiar with the code base could think of a quick fix, because it would have been nice to avoid the breakage in the release. But oh well, if there is no quick fix, doesn't really matter. As you said, previous bevy releases were broken in many ways, too.

[rparrett]

Confirmed that this was fixed by #1795