gfx-rs / gfx

[maintenance mode] A low-overhead Vulkan-like GPU API for Rust.
http://gfx-rs.github.io/
Apache License 2.0
5.35k stars 551 forks source link

Remove dependance on glsl_to_spirv #2546

Open grandafrato opened 5 years ago

grandafrato commented 5 years ago

In the quad example you use glsl_to_spirv, however that is now deprecated and now says to use shaderc-rs instead.

Michael-Lfx commented 5 years ago

glsl-to-spirv is also used in vulkan backend.

grandafrato commented 5 years ago

Really this isn't just about the examples, but all of the gfx-rs code that depends on the deprecated library. (Sorry about the many name changes, I didn't make it clear at first that I meant the whole library.)

grovesNL commented 5 years ago

I think it should be removed from the Vulkan backend already, see https://github.com/gfx-rs/gfx/pull/2533

kvark commented 5 years ago

To clarify: GLSL is in now way a part of gfx-hal API, so the issue is rather harmless. It just affects the way people start using the project, since they replicate the examples, and we shouldn't promote a deprecated library there. So, a documentation issue for the most part.

ghost commented 5 years ago

I don't think shaderc-rs is a very good alternative. It's kind of a pain to build on Windows, requiring a C++ compiler (e.g. usually Visual Studio, which is an ordeal to install), Python and CMake in the path. glsl_to_spirv used glslang instead and included a pre-built binary for Windows.

I'm sure it's not a problem for experienced developers, but it won't make for a good experience for anyone looking to learn Rust or gfx-hal and doesn't have these already installed.

kvark commented 5 years ago

Good point! Maybe we can just undeprecate in then?

On Jan 3, 2019, at 12:03, Aleksi Juvani notifications@github.com wrote:

I don't think shaderc-rs is a very good alternative. It's kind of a pain to build on Windows, requiring a C++ compiler (e.g. usually Visual Studio, which is an ordeal to install), Python and CMake in the path. glsl_to_spirv used glslang instead and included a pre-built binary for Windows.

I'm sure it's not a problem for experienced developers, but it won't make for a good experience for anyone looking to learn Rust or gfx-hal and doesn't have these already installed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

Michael-Lfx commented 5 years ago

I don't think shaderc-rs is a very good alternative. It's kind of a pain to build on Windows, requiring a C++ compiler (e.g. usually Visual Studio, which is an ordeal to install), Python and CMake in the path. glsl_to_spirv used glslang instead and included a pre-built binary for Windows.

I'm sure it's not a problem for experienced developers, but it won't make for a good experience for anyone looking to learn Rust or gfx-hal and doesn't have these already installed.

IIRC shaderc is based on glslang, spriv-cross and other KHR projects. And yes it requires a complex environment to build which is difficult for beginners. Here we just need glslang to convert glsl to Vulkan compatible spirv binary. I think it's good to make it simple. Because TimNN/glslang-sys stays in May 10, 2015, I have a proposal that gfx should hold a glslang-rs crate to follow the upstream. @kvark

kvark commented 5 years ago

I have a proposal that gfx should hold a glslang-rs crate to follow the upstream

I don't think we, the gfx-rs team, should be owning it. As I mentioned above, our entry point is SPIR-V and we care about all the conversions from it. The real reason for us to depend on glslang (in examples only!) is about documentation, since just providing raw SPV shaders would not be very beginner friendly or readable.

Edit: that is to say, if somebody within gfx-rs is willing to take that role, I'm fine with it. Just saying I personally can't allocate my time on it.

Michael-Lfx commented 5 years ago

Got it. We have to rely on glslang or something like it to do the realtime shader conversion actually. So we are seeking a update-to-date glslang-sys crate just like Josh maintains the spirv-cross-sys crate.

grovesNL commented 5 years ago

I started work on a wrapper for glslang a few months ago so we could use it from Rust directly, instead of having to call into a separate process through glslang's CLI. I think it would be slightly more performant and we could think about the API design a bit more and map types directly.

Unfortunately as far as I could tell glslang is focused on CLI usage more than usage as a library. If I recall correctly, there seemed to be quite a bit logic remapping the CLI options to the C++ options, but it's too bad because the CLI seems to have a pretty good API.

So to experiment with it, I changed its main function slightly (and renamed it from main to anything else) so I can invoke it easily from Rust and build a wrapper around it, like I did with SPIRV-Cross. After I did all of this I noticed efforts like shaderc-rs etc. and decided it wasn't worth it for me to maintain it because I'm busy enough already 😆 If there's demand for this and somebody willing to maintain it, I'd be happy to assist with setting it up the same way as spirv_cross.

grovesNL commented 5 years ago

Also I think the only reason glsl_to_spirv might avoid the compiler on Windows is because it distributes a pre-built binary on Windows, but I don't think this is a very good solution. I haven't had many issues with people building spirv_cross through cc-rs on most platforms (besides occasionally needing to configure CI or upgrade gcc/clang on some platforms), so maybe people are hitting a different problem with shaderc, or there is a bug somewhere?

ghost commented 5 years ago

I didn't realize that most of the backends already depend on spirv_cross. I suppose that the build environment complexity is a moot point if to build the backends you need to invoke a C++ compiler anyway.

ghost commented 5 years ago

Unless you're using the Vulkan backend I suppose, which does not depend on spirv_cross.

kvark commented 5 years ago

Sounds like shaderrc is the winner for now.

On Jan 7, 2019, at 06:37, Aleksi Juvani notifications@github.com wrote:

Unless you're using the Vulkan backend I suppose, which does not depend on spirv_cross.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Lokathor commented 5 years ago

I'll note here that Rust on windows already demands VS Studio for the msvc toolchain, so it's reasonably likely to already be on the dev's machine. And if you're using the gnu toolchain, well you only do that for compatibility with other gnu libs, which strongly implies you've got a gnu compiler installed.

There might be any other concern, but "needs a c++ compiler" isn't a big one if you're already doing rust at all.

ghost commented 5 years ago

Interesting. I suppose they need that for the linker. I had no idea.

breakin commented 5 years ago

I'm just now starting looking at Rust and gfx-rs but I am experienced windows programmer. Whatever you end up doing please make sure that Cargo build/run just works for the examples without any manual steps. Having MSVC tollchain installed is ok, but no manual steps like "place dlls here and there" or "build this solution and then do x and z" or "use mingw" or such. Thanks!

ghost commented 5 years ago

We could be able to avoid the need to install CMake if we wrapped glslang like @grovesNL explained above, but instead of using CMake to generate the glslang project files, manually listed the files required for the build. This way we would only require MSVC to be installed, and everything should work out of the box.

breakin commented 5 years ago

Cmake might be ok though... But building LLVM or clang or such will take a huge amount of time so prebuilt is also nice of course if that is involved in any way...