Vulkan-Samples
Vulkan-Samples copied to clipboard
WIP: Offline Shaders (Without Variants)
Description
As discussed on the 27/02/2024 working group call. The current offline shader approaches are overly complex due to the need to support for variants. Removing variants and simplifying the performance samples use of shaders may reduce the complexity of our shader compilation system.
This PR achieves offline compilation through a CMake helper. It also runs spirv-val to validate the spirv after it is created. For now the .spv files are not added to the repository, this may change if we agree on where they should be stored (build dir or next to source).
The PR enables offline shaders for existing ApiVulkanSamples by altering load_shader
After a configure and build the following commands have been tested on desktop
<path/to>/vulkan_samples batch -C api
<path/to>/vulkan_samples batch -C extensions
<path/to>/vulkan_samples batch -C general
<path/to>/vulkan_samples batch -C tooling
Performance samples work by attempting to find a precompiled shader before loading the source in ShaderSource. Variants have been removed by reworking shaders in samples
<path/to>/vulkan_samples batch -C performance
Todo:
- [ ] constant_data
- [ ] msaa
Fixes #
General Checklist:
Please ensure the following points are checked:
- [ ] My code follows the coding style
- [ ] I have reviewed file licenses
- [ ] 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
- [ ] My changes do not add any new validation layer errors or warnings
- [ ] I have used existing framework/helper functions where possible
- [ ] 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
Note: The Samples CI runs a number of checks including:
- [ ] I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
- [ ] My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions
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 readme of the folder that the sample belongs to e.g. api samples readme
- [ ] For new samples, I have added a tutorial README.md file to guide users through what they need to know to implement code using this feature. For example, see conditional_rendering
- [ ] For new samples, I have added a link to the Antora navigation so that the sample will be listed at the Vulkan documentation site
I put all my suggestions into https://github.com/tomadamatkinson/Vulkan-Samples/pull/3
Thanks @jherico, if you are open to co-authoring I can create a list of todos and we can split up the remaining work on this PR.
As stated on your created PR I'm happy to merge this after I've given it a spin, it looks to have cleaned up some of the more messy cmake!
@tomadamatkinson Sure. Let me know anything you need.
A quick note that I started adding HLSL shaders for some of the samples. We decided to do that first before adding a way to compile those in an automated way. Just wanted to let you know and it would be great if we could defer this until HLSL shaders are in.
@SaschaWillems Does that imply are you adding the SHADER_FILES_HLSL parameter to the CMake macros? Even if they don't get precompiled in your branch right now, having the CMake code be able to differentiate between HLSL and GLSL will save the work of a lot of tiny changes in this eventual PR.
Right now it mostly means I have started evaluating how to get them in. I just started on that branch like a few minutes ago, so anything in it is very much non-final. But we decided to do it similar to how HLSL support was added to my samples.
There will also be a way to compile the shaders in that branch.
But it's very very early...
I'm confused. This branch already supports HLSL shaders. The difficulty with previous approaches was supporting variants. Which we decided not to go forward with. This branch also removes variants from shaders with only a handful of samples left to patch.
Are you stating that you are also not happy with the approach of this revised branch which follows the guidance given in the working group? If so which parts of this branch are not solved how you imagined they would be solved?
I started adding actual HLSL shaders the way we decided during our calls.
I'm NOT going to add offline shader compilation. So imo your PR is still valid and what I'm doing now won't have much effect on your PR aside from maybe some minor things. Your PR is in mind the second step after we added actual shaders and need an easy way to compile them for other maintainers.
Afair we wanted to first add in HLSL shaders and a way of loading SPIR-V generated from them (which is now possible thanks to Mobica's PR) and then add offline compilation for GLSL and HLSL at a later point.
That also matches the proprosal for getting HLSL support into our samples I did some time ago.
But a general plea: Can we lower the pace a bit. Right now things are moving so fast, and even just starting to make some code public starts of discussions. I'm doing all of this in my spare time after a hard work week and tbh. it really stresses me out not being able to work on anything without having to discuss it or trying to clear up confusion :(