Use shaderc instead of glsl-to-spirv for shader compilation

[nlordell]

No description provided.

[nlordell]

Looks like the build timed out 😢

[rukai]

Hi!
This looks really cool, I am interested in this PR, although I will need to a do lot of research before I can think about merging it.
Would you mind resolving the merge conflicts so I can test/review it?

Would also be nice to get any opinions you have on advantages/disadvantages on glsl-to-spirv vs shaderc-rs.

Edit: Just found these issues, looks like tomaka and Gabriel agree that using shaderc-rs is a good option.
#5
#911
Another related issue:
#910

[nlordell]

Cool! I will try to find time today to resolve the merge conflict, if not some time this week. Some advantages off the top of my head:

• No gslangValidator.exe included in the repo
• Another repo (shaderc-rs maintained by Google) maintains the crate - so less work for vulkano
• shaderc is meant to be used as a library as opposed to gslangValidator which must be shelled. This also means that if ever you want to compile a shader at runtime, it can be done with much less of a fuss

[rukai]

Docs

Something I failed to realize before, is that merging this PR means that using vulkano would require setting up cmake, a c++ compiler etc. which can be quite painful depending on the platform.

To merge this PR I will need extensive docs on how to setup for each platform.
Please expand the macOS setup section in the readme.md into a setup section with subsections that describe setup for each platform.
Or if you come up with a better format, feel free to do that instead.

The platforms I know to be currently supported are Windows msvc, windows gnu, macOS and Linux.
I don't know the state of android or iOS?
For Linux, instructions for the latest Ubuntu release would suffice.
If you don't have access to a platform let us know so someone else can provide docs for it before merging.

The changelog entry also needs to be expanded, potentially linking to the new setup instructions in the readme.

If we cant keep a platform working or the steps to setup a platform are way too painful then we will need to abandon this PR.

C++ compiler error

On another note, when I try to compile your branch of vulkano on Arch Linux I get a C++ compiler error so I have filed a bug google/shaderc-rs#25

[antiagainst]

It's nice to see that shaderc-rs is considered for vulkano. Let me know if any change in shaderc-rs is needed. Happy to help. :)

Yes, cmake & C++ compiler is needed. But I think it's the same case for the existing hlsl-to-spirv wrapper around Glslang? But I'm definitely not an expert on vulkano to assert anything.

[antiagainst]

PS: google/shaderc-rs#25 is fixed now. :)

[rukai]

@antiagainst Since it was created hlsl-to-spirv includes a windows .exe in the source, apparently because setting up the build environment was too hard. Hopefully we can just document the setup process well instead.

[rukai]

Sorry for the delay, this all looks good, just two nitpicks.
Once this is rebased and google/shaderc-rs#26 has been addressed then I can merge it. I'm happy to include the steps in the readme, for setting up the shaderc dev environment, after merging this PR.

[rukai]

You might need to also remove the readme I just added /~https://github.com/vulkano-rs/vulkano/blob/master/glsl-to-spirv/readme.md

[nlordell]

So you mentioned that you wanted detailed notes on how to get setup for building shaderc. I added a small paragraph about the required dependencies (CMake and Python).

• Installing both from the Windows installers worked for me (in the spirit of transparency, I used Chocolatey which runs the installers under the hood) on my Windows system. On Windows I tried with both Python 2.7.15 and 3.6.1 and both worked, so it doesn't look like it needs Python 2 or Python 3 specifically.
• Mac ships with Python and installing CMake with Homebrew worked for me there.
• I don't have a Ubuntu system but using Ubuntu on Linux Subsystem for Windows just sudo apt-get install cmake was enough - as Ubuntu also ships with Python. I was too lazy to setup a VM with a proper Ubuntu installation but I don't see why it wouldn't work.

Since there wasn't really any setup that needed to be done for any platform I didn't feel like more instructions were needed in the README for installing CMake and Python (I added links to the project websites 😄)

[rukai]

Thanks for the fixes and writing the readme changes. I'm not sure why but github is listing all sorts of irrelevent changes from previous commits under "files changed" which is concerning. I guess it doesnt matter because its not reverting them its just hard to see the actual changes?!? Maybe you should be using rebase instead of merge in the future?

[HadrienG2]

@rukai Here is a diff from a cleanly rebased (well, cherry-picked) version: master...HadrienG2:shaderc-cleanup .

It shows the ~same thing as the PR diff for me, so I'm not sure if this is a GitHub false positive.

[rukai]

oh huh, must have been a bug with github or a bug with my brain. Files changed looks fine now.

[HadrienG2]

Github sometimes needs a page refresh, might be that :)

[rukai]

Only thing I'm waiting on now is figuring out how to build on windows-gnu. I will look into this in a few days hopefully.

[nlordell]

Its a shaderc-rs issue I think. I added some info on google/shaderc-rs#26

[nlordell]

With google/shaderc-rs#27 I managed to get this building and the tests pass. Here is my full step-by-step for getting tests passing:

New-Item C:\Temp -Type Directory
Push-Location C:\Temp

# Install chocolatey package manager
Invoke-WebRequest ((New-Object System.Net.WebClient).DownloadString("https://chocolatey.org/install.ps1"))

# Install required tools
& choco -y install git cmake python 7z
Set-Alias git "C:\Program Files\Git\bin\git.exe"
Set-Alias 7z "C:\Program Files\7-Zip\7z.exe"
$env:Path += "C:\Program Files\CMake\bin\;C:\Python37\;"

# Install a specific version of MinGW that works with Rust
Invoke-WebRequest https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/x86_64-6.3.0-release-posix-seh-rt_v5-rev2.7z -OutFile mingw-w64.7z
7z x mingw-w64.7z
$env:Path += "$pwd\mingw64\bin\;"

# Install Rust
Invoke-WebRequest https://win.rustup.rs/x86_64 -OutFile rustup-init.exe
& .\rustup-init.exe -y --default-toolchain stable-gnu
$env:Path += "$HOME\.cargo\bin\;"

# Get Vulkano and run tests
& git clone -b shaderc https://github.com/nlordell/vulkano.git
Push-Location vulkano
cargo test

Something that might be worth documenting in the README is that MSYS2 with latest MinGW-w64 toolchain does not work well with Rust and that your best bet is to use the same version Rust uses:

• x86_64: https://downloads.sourceforge.net/mingw-w64/x86_64-6.3.0-release-posix-seh-rt_v5-rev2.7z
• i686: https://downloads.sourceforge.net/mingw-w64/i686-6.3.0-release-posix-dwarf-rt_v5-rev2.7z

[nlordell]

So I did a bit more digging and any single Windows toolchain can support all of the Windows targets (/~https://github.com/rust-lang-nursery/rustup.rs#working-with-rust-on-windows). I think its might be much easier to actually use the MSVC toolchain and target x86_64-pc-windows-gnu instead of getting a GNU toolchain working (given the many pain points).

[rukai]

Thanks for ensuring vulkano continues to run on every platform, I'll merge it now.

[ghost]

Supporting a gnu toolchain is necessary because both Eclipse Corrosion and Intellij Rust only support gnu for debugging. People shouldn't be forced to use VS Code just to debug their code.