Consider replace glsl-to-spirv to shaderc-rs

[zimond]

I failed to compile notan on macos due to glsl-to-spirv build script failure. As it is already deprecated, have you considered to replace it with shaderc-rs, which is recommended by the original author?

[Nazariglez]

I'm using macos without issues yet, can you share the error? glsl to spirv dependency is one of my main concerns. I was thinking to move to naga, however is still a wip and the glsl front seems not to be used a lot due that most rust ecosystem uses wgsl. Shaderc is ok to me, I was using glsl to spirv because it's easy.

Let's see if we can solve your issue first.

[zimond]

running: "cmake" "--build" "." "--target" "install" "--config" "Debug" "--parallel" "4"

  --- stderr
  fatal: not a git repository (or any of the parent directories): .git
  Unknown argument --parallel
  Unknown argument 4
  Usage: cmake --build <dir> [options] [-- [native-options]]
  Options:
    <dir>          = Project binary directory to be built.
    --target <tgt> = Build <tgt> instead of default targets.
                     May only be specified once.
    --config <cfg> = For multi-configuration tools, choose <cfg>.
    --clean-first  = Build target 'clean' first, then build.
                     (To clean only, use --target 'clean'.)
    --use-stderr   = Ignored.  Behavior is default in CMake >= 3.0.
    --             = Pass remaining options to the native tool.
  thread 'main' panicked at '
  command did not execute successfully, got: exit status: 1

[Nazariglez]

Not sure what it could be, it sounds like it could be an old version of cmake maybe (Guessing because the --parallel not been an option). It works for me using 3.21.x and 3.23.x in two different machines, just to discard, what version are you using? I think that we need at least 3.12 to use --parallel.

[zimond]

I think it's due to https://github.com/rust-lang/cmake-rs/issues/131, upgrading cmake solves the problem. I'll leave this open for the legacy dependency issue

[jawadsh123]

ran into a similar issue, failed to compile on an M1 mac

running: "cmake" "--build" "." "--target" "install" "--config" "Debug" "--parallel" "10"
.
.
.
--- stderr
  fatal: not a git repository (or any of the parent directories): .git
  make: warning: -j10 forced in submake: resetting jobserver mode.
  In file included from ~/.cargo/registry/src/github.com-1ecc6299db9ec823/glsl-to-spirv-0.1.7/glslang/hlsl/hlslScanContext.cpp:45:
  In file included from ~/.cargo/registry/src/github.com-1ecc6299db9ec823/glsl-to-spirv-0.1.7/glslang/hlsl/../glslang/Include/Types.h:41:
  In file included from ~/.cargo/registry/src/github.com-1ecc6299db9ec823/glsl-to-spirv-0.1.7/glslang/hlsl/../glslang/Include/../Include/Common.h:99:
  ~/.cargo/registry/src/github.com-1ecc6299db9ec823/glsl-to-spirv-0.1.7/glslang/hlsl/../glslang/Include/PoolAlloc.h:314:54: error: 'operator=' is a private member of 'glslang::TPoolAllocator'
      void setAllocator(TPoolAllocator* a) { allocator = *a; }
                                             ~~~~~~~~~ ^ ~~
  ~/.cargo/registry/src/github.com-1ecc6299db9ec823/glsl-to-spirv-0.1.7/glslang/hlsl/../glslang/Include/PoolAlloc.h:244:21: note: declared private here
      TPoolAllocator& operator=(const TPoolAllocator&);  // don't allow assignment operator

it seems setAllocator was removed from glslang in 2019, but glsl-to-spirv hasn't been updated since 2018 😅

[jawadsh123]

tried switching to shaderc-rs, felt quite straightforward couldn't do very extensive testing but some examples seem to work 🙌

one downside is first run takes a sweet while, since shaderc compiles from source not great with rust build systems yet, so dunno if we can speed that up 😄

do you know if things have changed on naga or is shaderc our only options?

[Nazariglez]

Hey! Thanks for looking into this!

It makes sense glsl-to-spirv not working on m1, it has pre-compiled binaries inside of the crate, and I doubt that it's updated. I may check where the codebase lives to do a PR.

About naga, I am not sure if there is any progress on the GLSL frontend. I want to add it to notan, but we'll be "beta-testers" of naga GLSL, which is fine, but I would prefer to have it behind a feature flag, because SPIRV is battle tested.

Then Shader-c, I like it, but as you said the issue is the compilation time, I am afraid it will scare some people that just want to jump on notan to test a bit.

The frictionless path for us is updating glsl-to-spirv with m1 support for now.

And then, I see 2 possible steps for us, I am open to suggestions or ideas.

1. Add shader-c support under feature flag meanwhile or for people that prefer it.
2. Add naga under feature flag allowing the user to use it or fallback to spirv if something goes wrong.

Thoughts?

[jawadsh123]

hey! sorry for coming back so late, been a bit preoccupied 😅

Totally agree shaderc likely shouldn't be made default, first compile is too slow I did try hunting down glsl-to-spirv's home earlier, used to live under https://github.com/vulkano-rs/vulkano, but it's been deleted after deprecation (they moved to shaderc) Bevy used to maintain a fork, maybe we could do something similar, though idk if it's worth it 🤷

As for other options

shaderc Supporting it under a feature flag sounds great imo. Will temporarily make things work for folks on M1, or anyone glsl-to-spirv breaks for. I can setup a quick PR since I already have it working (very minimal change nyways)

naga should be the long term goal? everyone loves it, bevy moved to it too I'm not very experienced in shader translation, but open to playing with it. Though can't commit to anything soonish, feel free to pick it up if you get the time 😄

[Nazariglez]

I am working on the integration with Naga, I am not sure about it yet because the result looks "dirty" compared to shaderc or glsl_to_spirv, so I am worried that there could be some performance penalty, but I did not have a good test yet.

A PR with shaderc under a feature flag would be awesome if you have the time to put it together.

Regarding to the bevy fork of glsl-to-spriv, I think that is a good idea for a middle-term solution, I am going to clone it. I think that I can borrow a m1 to update the crate.

Thanks!

[Nazariglez]

Closed by #165