gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.19k stars 897 forks source link

[spv-in] SPIRV Parsing Failures for BVE-Reborn and Rend3 #4310

Closed cwfitzgerald closed 11 months ago

cwfitzgerald commented 3 years ago

Here all the current problems preventing bve-reborn and rend3 from getting full shader validation. I have updated both to the latest wgpu master, so I may have issues that were already fixed but not yet pulled all the way through.

Rend3:

This is a bindless based renderer, so uses more advanced features.

BVE-Reborn

More traditional renderer.

Other Info

All other errors I believe to be duplicates of the above.

Systemcluster commented 3 years ago

Two of these shaders (pre_cull.spv and opaque.spv) currently generate the following error:

...
[2021-02-14T17:54:15Z DEBUG naga::front::spv]           Return [1]
[2021-02-14T17:54:15Z DEBUG naga::front::spv::function] FunctionEnd
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidId(24)', examples\convert.rs:95:64

When lookup_function.lookup(fun_id) is called from patch_function_calls it's empty, so the lookup fails. https://github.com/gfx-rs/naga/blob/7a124550d4f75a7089d1fe9d415a87e042fd88a8/src/front/spv/mod.rs#L1615

patch_function_calls is only called here: https://github.com/gfx-rs/naga/blob/7a124550d4f75a7089d1fe9d415a87e042fd88a8/src/front/spv/function.rs#L149

Should it be populated before that line?

blit.spv parses now 🎉

kvark commented 3 years ago

Were you running Naga from latest master, or are these errors from gfx-10 tag that is used by wgpu at the moment? I think the issue about function calls is fixed now in gfx-rs/naga#458 and will be included in gfx-11 tag/train when it ships.

Systemcluster commented 3 years ago

I'm running the latest master, so gfx-rs/naga#458 included. Before gfx-rs/naga#458 it failed inside fun.fill_global_use instead.

kvark commented 3 years ago

Do you by any chance have forward function calling there? I.e. the function being called goes after the function that is the caller?

kvark commented 3 years ago

For now, we require that any functions called from something are defined prior to something. So we don't support forward calls or recursive calls. The spirv frontend should gracefully fail otherwise, and it kinda does, just without a proper error message.

Systemcluster commented 3 years ago

I don't think so, no. The corresponding GLSL for pre_cull.spv is this. It's the bool visible = frustum_contains_sphere(uniforms.frustum, vec4(mesh_center, mesh_radius)); line that seems to cause it there. frustum_contains_sphere is defined prior and not called recursively.

kvark commented 3 years ago

Interesting. So it is defined prior in GLSL, but SPIR-V puts is behind :/ That's unfortunate, and I haven't seen this behavior before. Running on Dota2 shader set didn't produce anything like this. We need some logic to anticipate this and reorder the functions. You can probably work around it by running spirv-opt on the shaders on your side.

kvark commented 3 years ago

Filed gfx-rs/naga#573 for this problem

cwfitzgerald commented 11 months ago

I think this one is actually solved