build: Generate Vulkan headers automatically as part of the build #9217
Conversation
If the glslang tool is available, we can regenerate the Vulkan headers with it during the build, rather than having an out-of-band step to produce Vulkan headers on a maintainer's Windows machine and having everyone else rely on the derived files having been committed to git. On platforms where compiling everything from its preferred from for modification is desired, such as many Linux distributions, the generated headers can be deleted from the source directory to force them to be regenerated. The derived files are still copied into the source directory to be committed, in case there are platforms where the Vulkan driver is desirable but CMake is not used. This could be removed if no such platform exists. glslangValidator(1) is an older name for glslang(1), so look for both names. Signed-off-by: Simon McVittie <smcv@collabora.com>
|
My CMake scripting is probably not the most elegant, I'm better at Autotools and Meson. In principle the D3D shaders could get a similar treatment, but someone who can test in a Windows development environment would have to drive that, and the Linux distributions that strongly prefer the build to be done from first principles aren't going to care about D3D shaders; in the Debian packaging, we currently just delete them, so that they don't have to be reviewed when updating SDL. |
| set(intermediate "${CMAKE_CURRENT_BINARY_DIR}/${dir}/VULKAN_VertexShader.h") | ||
| set(output "${CMAKE_CURRENT_SOURCE_DIR}/${dir}/VULKAN_VertexShader.h") |
There was a problem hiding this comment.
Are the generated headers stable between glslang versions?
If they differ, perhaps we should hide the regeneration behind a SDL_DEVEL CMake option?
There was a problem hiding this comment.
Are the generated headers stable between glslang versions?
Experimentally: not identical, but if they aren't backwards-compatible then I would have assumed that ought to be considered to be a bug in glslang?
(Analogous: C compiler output isn't stable between compilers or versions, but we don't commit generated object code to git and hide regeneration of it behind a special developer option.)
There was a problem hiding this comment.
glslang should always emit valid spirv, that's not my concern here.
I'm worrying about people accidentally committing their regenerated headers when doing git add -A.
This will/might cause back and forth in pull requests.
There was a problem hiding this comment.
I see your point. Perhaps it could be regenerated if either: a special option is set, or the generated "source" file already doesn't exist? Would that be viable?
There was a problem hiding this comment.
That would be nice!
I'm thinking about a SDL_DEVEL option, but I'm bad at naming things so that's up to you :)
|
Keeping vulkan/compile_shaders.bat would be advisable: devs may prefer using it. |
Are there SDL maintainers who might update the precompiled shader headers, but are not building with CMake? My concern about keeping the batch file is that if the batch file and the CMake build system are both responsible for knowing the canonical glslang options to be used, they can get out of sync. |
|
Right now, |
They can build using Visual Studio project files |
|
I don't have any strong opinions here. The original version of the D3D12 renderer I ported had the shader code in comments and just had checked in generated arrays with some comments about how to generate them. At first I did the same in Vulkan, but then @slouken added the build batch files, which were a definite big improvement to development workflow. In principle having it as part of cmake sounds nice, but yeah it does make a new external dependency in glslangValidator and SPIR-V will vary by what version user has on their machine unless we include it or build it as an external dependency at a specific revision. |
|
The batch files are definitely better than having no automatic generation at all, but bringing them into the build system seems nicer (to me, at least) than having an out-of-band step, particularly an out-of-band step that in principle only works on Windows (although, as mentioned above, in practice the batch file is so simple that right now it can also be run as a shell script).
Only if the packager/developer has chosen to install it - if it's missing, the pre-generated version is still available. I would expect that platforms with a packaging system that knows about build-dependencies (dpkg, RPM, etc.) would prefer to delete the generated files and use this as an external dependency, whereas platforms with no good packaging system would prefer to continue using the pre-generated version. |
|
What if we have an optional step that's run if the user has specified an glslang executable to use? That gives us the best of both worlds - packagers can specify their dependency and developers can omit it if they're not iterating on shaders, or set it to a specific version if they are. We could do the same for the D3D shaders - run the generation commands if the user has set the shader generation command, otherwise the pregenerated ones will be used. |
|
BTW, the unrelated CI failures should be fixed now. |
Sure, that sounds like a good route. I'll put that in my list (it's a long list). |
If the glslang tool is available, we can regenerate the Vulkan headers with it during the build, rather than having an out-of-band step to produce Vulkan headers on a maintainer's Windows machine and having everyone else rely on the derived files having been committed to git.
On platforms where compiling everything from its preferred from for modification is desired, such as many Linux distributions, the generated headers can be deleted from the source directory to force them to be regenerated.
The derived files are still copied into the source directory to be committed, in case there are platforms where the Vulkan driver is desirable but CMake is not used. This could be removed if no such platform exists.
glslangValidator(1) is an older name for glslang(1), so look for both names.
glslang-tools version 14.0.0 on Debian produces a slightly different
VULKAN_VertexShader.h. Is this expected? The current headers seem to have been generated with 13.1.1, and the pixel shaders come out identical except for the comment that indicates the version of glslang that was used to generate them.