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

Ensure a GLTF Default material is loaded #1016

Merged
merged 1 commit into from
Dec 9, 2020

Conversation

4-rodrigo-salazar
Copy link
Contributor

@4-rodrigo-salazar 4-rodrigo-salazar commented Dec 6, 2020

GLTF does not require that mesh primitives provide a material. In that case, a default material is supposed to be used.

In the bevy gltf loader, we iterate over all materials (gltf.materials()) but this iterates over all the materials defined in the gltf, not including the default material. This means we never create a StandardMaterial asset for the default gltf material. This causes a panic later on since we do create a PbrBundle referencing the handle for the default material.

Panic:
thread 'Compute Task Pool (3)' panicked at 'called Option::unwrap()on aNonevalue', C:\github\bevy\crates\bevy_render\src\shader\shader_defs.rs:101:52

This changes makes it so that when iterating over all mesh primitives, if we run into a primitive which has a material which isn't yet in the 'load_context', then we at that time create the standard material and add it to the load_context. The default material will be captured here since primitive.material() will return it. Unfortunately, the gltf library can not provide a default implementation function, otherwise we could just append this onto our gltf.materials() iterator.

Here is a glb file which reproducing the issue (provided by Sheepyhead on discord). I have another glb that repros this as well from my own project, but that one is not postable.

man_with_melting_head.zip

Tested loading this glb and also tested FlightHelmet.

@Moxinilian Moxinilian added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Dec 7, 2020
@ColdIce1605
Copy link
Contributor

oh I just realized that this is the issue I'm having as described in this pr

@cart
Copy link
Member

cart commented Dec 9, 2020

Good call. The GLTF default material thing is a little weird, but looking at the gltf lib implementation I think this approach is probably the right call.

@cart cart merged commit 19c4f33 into bevyengine:master Dec 9, 2020
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants