WIP: Redesign functionality of GLSLcompiler class

[Szymon-Rajczakowski-Mobica]

Description

Redesign functionality of GLSLcompiler class. Change name of this class to ShaderCompiler. Add support for HLSL, GLSL and SPV files.

General Checklist:

Please ensure the following points are checked:

• [ ] My code follows the coding style
• [ ] I have reviewed file licenses
• [ ] I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
• [ ] I have commented any added functions (in line with Doxygen)
• [ ] I have commented any code that could be hard to understand
• [ ] My changes do not add any new compiler warnings
• [x] My changes do not add any new validation layer errors or warnings
• [x] I have used existing framework/helper functions where possible
• [ ] My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions
• [ ] My changes do not add any regressions
• [ ] I have tested every sample to ensure everything runs correctly
• [ ] This PR describes the scope and expected impact of the changes I am making

Sample Checklist

If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist:

• [ ] I have tested the sample on at least one compliant Vulkan implementation
• [ ] If the sample is vendor-specific, I have tagged it appropriately
• [ ] I have stated on what implementation the sample has been tested so that others can test on different implementations and platforms
• [ ] Any dependent assets have been merged and published in downstream modules
• [ ] For new samples, I have added a paragraph with a summary to the appropriate chapter in the samples readme

[SaschaWillems]

Not sure if this should be a separate sample. Imo it would be better if this was an addition to the existing basic raytracing sample, e.g. maybe a compile time define so people can toggle between GLSL and HLSL.

[CLAassistant]

All committers have signed the CLA.

[SaschaWillems]

I did a first quick review and added a few comments.

One thing I'm not sure about is the direction of this PR.

On one hand it seems to add HLSL support to the framework's compiler, but only through glslang, which is very very limited. And on the other hand it also adds direct loading of SPIR-V for e.g. the basic ray tracing sample.

Tbh. I think we should discuss a different approach that better fits our framework strategy.

We've been talking about HLSL support for quite some time and one option would be adding direct SPIR-V loading to the shader compiler (as a derived class maybe) so we can add HLSL to all samples with offline compiled SPIR-V files via DXC. The goal is also to not have to change every sample but to support shader language selection at framework level instead. And imo we should move into that direction.

I appreciate the work put in this PR, but it feels a bit like a stop gap towards our actual goal and we might revert a large part of that PR in the future.

I'll wait for other opinions, and I'd happily discuss this in the next samples call :)

[Patryk-Jastrzebski-Mobica]

Hi @SaschaWillems, We have discussed this problem and think that the best solution will be to extend HLSL support (add raytracing support) in the glslang repository. At the moment @Dawid-Lorenz-Mobica is investigating how to add raytracing support for the HLSL language in the glslang repository.

[SaschaWillems]

Tbh I'm not sure if that's the right approach. HLSL in glslang is very limited and the actual reference compiler is DXC. So if we want to add HLSL support we should either integrate DXC or offline compile HLSL to SPIR-V and just use that.

[SaschaWillems]

@Szymon-Rajczakowski-Mobica: Can you solve the merge conflict? We talked about this on our last call and would like to merge this as early as possible, so we can start working on HLSL shaders :)

[SaschaWillems]

I gave this a try and added/updated a few comments (see above) that needs to be resolved.

Several build checks also failed, e.g. Ubuntu and Android builds. These need to be fixed too.

One thing that should be changed is that right now instead of the actual human readable HLSL files, the SPV files are added to the project:

That doesn't make a lot of sense, I'd expect the hlsl source files in here.

We should also discuss folder structure. Right now (for the one sample were HLSL was added), GLSL and HLSL files are in the same folder, with the HLSL ones being prefixed. I'd prefer to separate them via folders (similar to what I do in my samples). I propose we do this with sub folders like this:

• \shaders
  • \sample
    • \glsl <-- GLSL shaders here
    • \hlsl <-- HLSL shaders and compiled SPV in here

[SaschaWillems]

Besides that: is there any means to switch between HSLS and GLSL now?

I was wondering the same. I don't see a way to select HLSL or GLSL. We should do that as a command line switch.

[pawel-jastrzebski-mobica]

Besides that: is there any means to switch between HSLS and GLSL now?

I was wondering the same. I don't see a way to select HLSL or GLSL. We should do that as a command line switch.

@SaschaWillems: I really don't know how to command-line switch should be used in context of this change. Since the PR adds the ability compile HLSL to SPV for both off-line and on-line use-case. Please explain the functionality that could be switched using command-line.

[SaschaWillems]

Selecting between running a sample with the GLSL based shader or the HLSL based shader.

[SaschaWillems]

Also odd, I get three out-of-place hlsl related entries in the MSVC project browser now:

Probably left overs. Would be good if this PR did not include any HLSL shaders at all, but just the code required to load SPIR-V. We'll add the HLSL in separate PRs.

[pawel-jastrzebski-mobica]

@SaschaWillems : do you want to remove the whole off-line shader compilation functionality from the PR?

[SaschaWillems]

Nope, having a way to compile the HLSL shaders vie the build system is fine. Problem is where those projects are put in the project structure. They appear at the top level, but should instead be part of the sample itself.

[Seweryn-Zielas-Mobica]

I changed way of using dxc for offline compilation. I had problem with add_custom_target/add_custom_command that created targets in top level view of Visual Studio solution, I was only found way how to move it into other folders, but couldn't move it under other targets like specific samples. From what I red and understood, this is most probably not possible. Instead of add_custom_target/add_custom_command is used execute_process which seems to work as good or maybe even better than previous solution, as now code for compiling shaders seems more straightforward and less bulky.

The difference in how it works now is rather not significant, before HLSL shaders were compiled at the start of executing build command, e.g. "cmake --build build/linux --config Release --target vulkan_samples -j$(nproc)" Now they are compiled after running command for generating project, e.g. "cmake -G "Unix Makefiles" -Bbuild/linux -DCMAKE_BUILD_TYPE=Release"

I also updated cmake to only compile shaders if dxc executable is found, by searching for it under VULKAN_SDK path.

There is still glslang used for compilation HLSL shaders at sample runtime, and rather big modification of hpp_hello_triangle.cpp which I'm not sure if is needed.

There is still some work left to do, mostly stylistic like naming scheme and directories/folders paths, but core functionality of this PR that is compiling HLSL shaders seems to be done.

[Seweryn-Zielas-Mobica]

@SaschaWillems As we discussed that we should only add offline compilation for HLSL, I've created created new PR that only do this, as it seemed easier and less misguiding when looking at this PR title. New PR: #990

[marty-johnson59]

Closing this as we're going a different direction