Add new sample for shader debugprintf

SaschaWillems commented 8 months ago

Description

This PR adds a new sample showing how to use debugprintf in shaders. It also includes a tutorial.

Note: This also does some changes to the framework required for samples that do custom instance creation.

Tested on Windows 11 with an NVIDIA RTX 4070

Fixes #926 Fixes #985

General Checklist:

Please ensure the following points are checked:

[x] My code follows the coding style
[x] I have reviewed file licenses
[x] I have commented any added functions (in line with Doxygen)
[x] I have commented any code that could be hard to understand
[x] 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
[x] My changes do not add any regressions
[x] I have tested every sample to ensure everything runs correctly
[x] This PR describes the scope and expected impact of the changes I am making

Note: The Samples CI runs a number of checks including:
- [x] I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
- [x] 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:

[x] I have tested the sample on at least one compliant Vulkan implementation
[x] If the sample is vendor-specific, I have tagged it appropriately
[x] I have stated on what implementation the sample has been tested so that others can test on different implementations and platforms
[x] Any dependent assets have been merged and published in downstream modules
[x] For new samples, I have added a paragraph with a summary to the appropriate chapter in the samples readme
[x] 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
[x] For new samples, I have added a link to the Antora navigation so that the sample will be listed at the Vulkan documentation site

SaschaWillems commented 8 months ago

Thanks for noticing. The framework seems to add VK_KHR_surface explicitly and via glfw (which looks wrong on e.g. Windows as it shows up twice). I guess on your platform the implicit addition via glfw doesn't happen, hence it's missing. Will fix that.

SaschaWillems commented 8 months ago

I have added a tutorial for this sample. It's now ready for review.

SaschaWillems commented 8 months ago

Great description on how to use it.

For some unknown reason, this sample "runs" incredibly slow on my machine (multiple seconds per frame). But when the Vulkan Configurator is running as well, I don't get any output anymore and the speed is as expected. Any idea on that?

That seems to be a bug with the validation layers. I noticed this too, but couldn't reproduce :(

gary-sweet commented 8 months ago

This no longer crashes for me, but I'm still seeing:

[error] [framework/platform/platform.cpp:175] Error Message: Couldn't request feature from device as VK_KHR_get_physical_device_properties2 isn't enabled!
[error] [framework/platform/platform.cpp:176] Failed when running application shader_debugprintf
    71:05:42.547 nxserverlib: Vulkan Display(0x7e5c000ee0) unregistered

SaschaWillems commented 8 months ago

Thank you very much for testing :)

I guess that error is caused by #938

Currently the framework will always enable all features of an extension at the time it's requested. I'll see if I can do a workaround for that.

asuessenbach commented 8 months ago

I think, @gary-sweet's issue here is, that VK_KHR_get_physical_device_properties2 is not enabled (is it supported, at all?), even though it has been added in ShaderDebugPrintf::create_instance! Should have failed there, then.

gary-sweet commented 8 months ago

I think, @gary-sweet's issue here is, that VK_KHR_get_physical_device_properties2 is not enabled (is it supported, at all?), even though it has been added in ShaderDebugPrintf::create_instance! Should have failed there, then.

We do support VK_KHR_get_physical_device_properties2, so it's not obvious what the issue is. I'll do some degugging.

gary-sweet commented 8 months ago

I'll do some debugging.

It looks like this sample is creating its own VkInstance which it then constructs the Instance() object from. This appears to bypass a bunch of the extension enabling that happens in the Instance main constructor (all the enable_extension() stuff). The end result is that the Instance object doesn't think the extension is enabled, even though it actually is in the contained VkInstance.

SaschaWillems commented 8 months ago

Yeah, it's similar to the profiles example. The framework assumes some stuff, even if you create your own instance, which in turn causes problems like these. It also tries to enable all features of an ext structure (see #928). But since I don't want to change anything about the framework in this PR (would be out of it's scope) I'll see if I can workaround this.

SaschaWillems commented 8 months ago

This is kinda hard to debug, and I feel like this is platform-specific. The only place where this error is thrown is in (hpp_)phyiscal_device.h and at least on Windows I never even enter that code when running my sample. So we probably do something different/specific when running on your platform that causes the assert to trigger. I'll somehow try to reproduce this on Windows.

I guess it's caused by this part of device creation: https://github.com/KhronosGroup/Vulkan-Samples/blob/main/framework/core/device.cpp#L109

It'll implicitly enable some extensions for all samples if they're supported.

Sometimes I wish our framework wouldn't do so much implicit stuff. Often feels like the exact opposite of Vulkan :/

SaschaWillems commented 8 months ago

Sorry, this currently fails after merging main. Lots of framework changes and at least something missing for me to continue working on this, see #985,

SaschaWillems commented 8 months ago

Welp, probably happened while merging with main. Thanks for letting me know, will fix.

SaschaWillems commented 8 months ago

Can you check again? Afair the list in that cmake file is purely optional. I'm actually not sure why that list of samples in there even exist. CMake should do a folder traversal and adds all samples it can find, no matter if the sample is explicitly added in that list or not.

gary-sweet commented 8 months ago

Can you check again? Afair the list in that cmake file is purely optional. I'm actually not sure why that list of samples in there even exist. CMake should do a folder traversal and adds all samples it can find, no matter if the sample is explicitly added in that list or not.

Well that's odd then, because the sample didn't exist for me unless I hacked it into that file.

SaschaWillems commented 8 months ago

That's interesting. I just reran cmake and deleted the cache beforehand and the sample is somehow picked up. Will have to investigate.

SaschaWillems commented 8 months ago

Added it to the CMake file anyway. Can't hurt ;)

gary-sweet commented 8 months ago

That's interesting. I just reran cmake and deleted the cache beforehand and the sample is somehow picked up. Will have to investigate.

Hmm. So I trashed my build folders completely and rebuilt and then it appeared, without the entry in cmake. It's possible that the cmake generate hadn't run, which is what I was seeing.

KhronosGroup / Vulkan-Samples