search

KhronosGroup / Vulkan-Samples

One stop solution for all Vulkan samples
Apache License 2.0
4.19k stars 626 forks source link

shader_debugprintf fails starting with Vulkan SDK 1.3.290 #1132

Closed SRSaunders closed 2 weeks ago

SRSaunders commented 3 weeks ago

The _shaderdebugprintf Vulkan API version performance workaround that I submitted for the Vulkan Validation Layer defect https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7562 no longer works when using SDK 1.3.290.

In PR #1084 I changed the API version from Vulkan 1.1 to Vulkan 1.0 which worked around the performance problem for prior SDKs as documented in the VVL defect above. However, something changed in SDK 1.3.290 that correctly enforces the API 1.1 version for the debugPrintfEXT layer feature even without fixing the performance issue. Note the fix for the VVL performance issue is expected in the next Vulkan SDK following 1.3.290.

To fix this regression, I can submit a change that detects the SDK instance version at runtime and sets the API version appropriately for this sample: a) Set API 1.2 for SDKs <= 1.3.290 to avoid the performance defect and satisfy the Vulkan API version test in SDK 1.3.290 b) Set API 1.1 for SDKs > 1.3.290 to leverage the VVL fix https://github.com/KhronosGroup/Vulkan-ValidationLayers/pull/8323 (which I have verified as working with this sample via a private build)

The only concern I have is that while setting API 1.2 for SDKs <= 1.3.290 works around the performance defect, it rules out running the sample on earlier Vulkan 1.1 implementations. However, given that the VVL performance issue made the sample almost unusable with API 1.1 prior to the coming fix, I am not sure this really matters.

@SaschaWillems since you submitted the original VVL defect, do you have any recommendations here? Do you agree with my proposed solution or can you suggest any alternatives? I guess we could just set the API version back to 1.1 for all SDK versions and just live with the performance problem for SDKs <= 1.3.290. Your thoughts?

SaschaWillems commented 3 weeks ago

This was my VVL issues: https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7562

Probably best to raise this problem there.

I have no alternatives, haven't looked at debugprintf since I wrote that sample/reviewed your PR.

SRSaunders commented 3 weeks ago

The VVL issue you raised is fixed and will be shipped with the next SDK. No need to raise it again.

I am talking about the _shaderdebugprintf sample and the regression I caused by changing the API version to 1.0 in the sample (which worked for SDKs leading up to 290). I would like to know whether you would prefer my proposed fix supporting older and current SDKs as suggested above, or a simple revert to Vulkan API 1.1 in the sample. I will push a change based on your feedback.

SaschaWillems commented 3 weeks ago

I'd prefer reverting to api version 1.1. Older SDKs won't be a concern in a few months.

SRSaunders commented 3 weeks ago

Thanks @SaschaWillems. Fix submitted as linked above.