-
Notifications
You must be signed in to change notification settings - Fork 180
Replace the has been deprecated glsl-to-spirv with shaderc-rs crate #44
Conversation
…aderc-rs``` crate
I support this change, but we need to explain how to build shaderc in the examples readme. You can link to or copy the relevant text from the shaderc readme https://github.com/google/shaderc-rs |
oh... and I think we should consider using https://github.com/Ralith/vk-shader-macros instead of shaderc-rs. Currently gfx hal 0.2 takes spirv as &[u8] which is wrong and will be fixed in 0.3 |
@rukai When build wgpu-rs examples, shaderc will be compiled automaticly. |
shaderc-rs handles compilation but your environment has to be specifically setup for it to work, depending on your OS you may need to setup a C++ compiler + ninja or maybe just install the shaderc library via your package manager, if such a package is available. |
shaderc compile result CompilationArtifact can output [u32] spirv stream :). |
That's a red flag for me. We don't want to require any more steps than there is. We'd even like to reduce the preparation by, say, eliminating the "--features " parameter. Could you explain to me one thing. So glsl-to-spirv is deprecated, ok. Is it not doing it's job? In my book, as long as it's working for our needs, we should keep it (given that it doesn't require that much of an environment setup). Later, we'd replace it by something more Rust-ic. |
Yeah it is bad. I see shaderc-rs as the lesser evil though. I think in this case grenlight was prompted to make this pr because glsl-to-spirv was miscompiling his spirv? This has been discussed many times before: The other option is resuming maintence of glsl-to-spirv under the gfx org. I can help set that up, but I am not interested in long-term maintenence, I have enough projects already. Additionally only trusted people could supply prs to update the windows exe. |
I agree with your opinion. Although wgpu users may already have C++ and Pathon environments, but it is possible to generate additional configurations, that is not friendly enough to some users |
You could remove the binary from |
Not sure if it matters but as of today there is a path of making glslang, which glsl-to-spirv uses to work on mobile devices, like iOS. I know that MoltenVK has managed to do that. Abandoning that means effort has to be put in for making sure that shaderc and it’s rust wrapper does so as well. I’m not against the move but there’s an uncertainty that shaderc can compiled for and run on iOS while glslang has been proven to do - at least the MoltenVK guys managed to do so with their build scripts at least. Obviously I’m ignoring any issues with the macro itself and only discussing the internal libs here so if there’s an issue with the macro that’s a different topic. My plan is eventually make sure the examples run on iOS as well because I see them as testcases for wgpu working as expected :-). Edit: Both libs aren’t written in Rust which means that arguments against Shaderc and setting up build environments are the same for glslang. Edit2: Gah writing on mobile, shaderc doesn’t need to set up a build environment(?) the Rust wrapper seems to manage to set things up, not sure if it’s building from scratch or I happen to have every dependency or if it’s using precompiled libs. |
I feel like we could skip the whole glsl-to-spirv -> shaderc transition and instead focus on generating SPIRV in Rust based off GLSL, which becomes more and more of an option. Is there an urgency here? I mean, wgpu-rs doesn't even have GLSL anywhere in its API. So as long as our examples (that use glsl-to-spirv) still work, we are good. |
glsl-to-spirv or shaderc they can all do their work right now, and compared to other enhancements (the use of external texture data :) ), this may not be so urgent, after all, on mobile devices, users can precompile shaders into SPIRV files for use. |
Closing this as stale. The mid-term plan for the shaders here is using WGSL language and Naga for translating them into SPIR-V. |
Since
glsl-to-spirv
has been deprecated, wgpu-rs/issues/36 is related to glsl spec, and glslc is still iterating fast, useshaderc-rs
to follow up on future updates fromglslc
.Extra information
compile
shaderc-rs
need to installPathon
3, and maybe need to remove local computer’sCMakeCache.txt
file to let cmake use latest Pathon version.If the following error message appears when compiling on macOS:
Make sure have XCode installed as well as command line tools:
xcode-select --install