Make shader files part of their corresponding build projects, rebuild on demand

[jherico]

I may have gotten a little carried away.

• Removes all shader handling (source and SPV) from sample_helper.cmake and moved to compile_shaders. This means compile_shaders can be used with anything whether it goes through add_sample or add_library.
• Refactored compile_shaders in terms of filter_shaders and compile_shader macro, reducing duplicated code
• If building is possible, then the generated SPV files will be added to the project and automatically recompiled on project build when the SPV is out of date relative to its source file
• Removed add_custom_target(vkb__shaders) and pushed the imgui shaders to be part of framework, since that's where the GUI code lives for now.

TODO:

The ${CMAKE_COMMAND} -E copy "${SHADER_FILE_SPV}" "${STORED_SHADER_FILE_SPV}" should probably be factored out of the existing custom commands and into its own. It has a different OUTPUT and DEPENDS than the other portions of the actual compile commands. It should probably also be encased in some kind of CMake flag set at the command line, since most people using this repository don't want that copy to happen. If they have a different SDK version it could produce different SPIR-V and thus make a bunch of shader files suddenly look like they've changed from the perspective of source control.

It should also be possible to be more precise in the DEPENDS for the command, at least for GLSL. If a shader uses any include files, we want changes to them to also trigger rebuild of a given SPV. This comment on reddit could serve as a starting point for that, unless there's similar functionality in glslc.

[tomadamatkinson]

Thanks @jherico for creating the PR, I'm happy to co-author this piece of work so will likely merge your changes after testing them

There is still this open question on where and how do we save the spv. As you pointed out there could be slight changes between the spv that versions of tools generate. We should probably test this theory at some point

Will take a look later today!

[tomadamatkinson]

Great simplifications, much easier to read

Ran locally and this solution only generates a handful for shaders for me when building the entire project. This is likely due to the target_sources approach. Any ideas how this can be fixed?

[tomadamatkinson]

If it helps, these are the shaders that where generated when running the following command. This is only a small portion of the shaders

cmake --build build --target vulkan_samples -- -j 12

[jherico]

Thanks for taking a look.

I tried building it locally (both through Visual Studio, and through the command line) and it generated 245 shaders. This seems to correspond with the number of *.spv.rule files I have in my build folder.

However during one of the builds I noticed this...

50>Compiling HLSL hlsl_shaders/hlsl_shader.frag to SPIR-V
50>The process cannot access the file because it is being used by another process. C:/Users/bdavi/kgit/Vulkan-Samples/build/shaders/hlsl_shaders/hlsl_shader.frag.spv
50>
50>C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'C:\Users\bdavi\kgit\Vulkan-Samples\build\CMakeFiles\e3a936efc0019db87ed5504855c4d53a\hlsl_shader.frag.spv.rule' exited with code 1.
50>Done building project "hlsl_shaders.vcxproj" -- FAILED.

Because multiple sample files can reference the same shader, and because the individual samples now build the shaders for themselves, there can be a race condition where two samples (probably the C++ and C variants of specific example) both try to build a particular sample concurrently.

If your issues are related to the race condition, I can try to look for some kind of serialization mechanism, to ensure that CMake doesn't try to build the same output multiple times concurrently. But I'm not confident that CMake alone can do that and while I'm sure Python could, I don't think requiring python for shader compilation would be reasonable, since it's never been before. So if that doesn't work, I can revert the code back to having the vkb__shaders target be the owner of all the shader SPV outputs and have the vulkan_samples target depend on it, thus

If you repeat the build command does it produce more shaders? If not, then it seems like the race condition isn't the issue and there's something else different about your environment compared to mine. I'll try to investigate more tomorrow.

for clarity I ran the following commands to test locally:

$ find . -name *.spv -delete

$ find . -name *.spv | wc -l
0

$ cmake --build . --config relwithdebinfo --parallel 12
MSBuild version 17.8.5+b5265ef37 for .NET Framework
   ...
  <snip>
   ...
  Compiling GLSL 16bit_arithmetic/visualize.vert to SPIR-V
  Compiling GLSL 16bit_arithmetic/visualize.frag to SPIR-V
  Compiling GLSL 16bit_arithmetic/compute_buffer.comp to SPIR-V
  16bit_arithmetic.vcxproj -> C:\Users\bdavi\kgit\Vulkan-Samples\build\samples\performance\16bit_arithmetic\16bit_arithmetic.dir\RelWithDebInfo\16bit_arithmetic.lib

$ find . -name *.spv | wc -l
245

$ find . -name *.spv.rule | wc -l
245

This is all running on Windows 10, Visual Studio 2022 and git bash. I'll try to run it again on an ubuntu and mac environment later today.

[jherico]

OK, so testing on Mac I discovered that ninja builds will fail in the presence of a Vulkan SDK, because you get output like this:

-- Build files have been written to: D:/a/Vulkan-Samples/Vulkan-Samples/build/windows
ninja: error: build.ninja:8819: multiple rules generate shaders/hlsl_shaders/hlsl_shader.vert.spv

This applies both to the original code on your branch and my changes.

I've updated my code to consolidate all of the spir-v files as sources of the restored vkb__shaders target, so the projects now only reference the source files, not the SPIR-V files (which can't be edited anyway) and instead just have a depdendency on vkb__shaders.

Running the build on my mac I can verify that there are 248 spv files in my build directory and that marking a shader dirty causes a rebuild of that shader and ONLY that shader.

bdavis@brads-mbp Vulkan-Samples % cmake --build build
[0/2] Re-checking globbed directories...
ninja: no work to do.
bdavis@brads-mbp Vulkan-Samples % touch shaders/afbc/base.frag
bdavis@brads-mbp Vulkan-Samples % cmake --build build         
[0/2] Re-checking globbed directories...
[1/1] Compiling GLSL afbc/base.frag to SPIR-V
bdavis@brads-mbp Vulkan-Samples % find . | grep spv  | wc -l                   
    1668

[jherico]

I've also verified this builds on my Github codespaces Ubuntu environment and generates (nearly) the same number of SPV files. I'm not quite clear yet on why there's a different number on Ubuntu vs Mac vs PC, even if it's a range of 5 out of 250.

Also, the Github diff view seems to have completely lost the thread of where the changes should line up. Sorry.

Sorry if this approach seems overly complex now, but something similar to it will still need to happen to avoid the Ninja build failures on the original branch.

Cheers.

[tomadamatkinson]

Nice solution! Thanks a lot. Will merge as is and asses the main branch for some tasks to split up (if you haven't already solved them already)

Your platform specific investigation might be an early warning that this is going to behave differently on multiple platforms... Hopefully we can find something that works 99% of the time

[jherico]

Thanks