KhronosGroup / KhronosGroup/Vulkan-Samples
New layer_settings sample has a few validation warnings
- Dominant language
- C++
- Stars
- 5.4k
- Forks
- 835
- Avg merge
- 2h 16m
- Merged PRs (30d)
- 1
Description
Interesting new sample #1419.
However, there are a few issues:
1. The sample does not currently enable the VVL by default, which is counter-intuitive since it depends on it for demonstration purposes. I realize that the VVL can be optionally enabled for the whole project, but for samples that require it I think it should be enabled under program control (e.g. _shader_debugprintf_).
2. The sample attempts to set multiple layer setting "enables" which will not work since the VVL requires a single "enables" setting specified by a vector of options. Otherwise the VVL will use the first "enables" setting and ignore any subsequent attempts to set other "enables". However, this may be moot as per item 3 here below...
3. The "enables" and "disables" mechanisms are now deprecated by the VVL. I think it should be using the modern mechanisms like `validate_best_practices (VK_LAYER_VALIDATE_BEST_PRACTICES=1)` and `printf_enable (VK_LAYER_PRINTF_ENABLE=1)`. Also, the `VK_VALIDATION_FEATURE_DISABLE_ALL_EXT` disables option is completely deprecated with no replacement. Perhaps the example should illustrate a different option by disabling something else, e.g. `check_shaders (VK_LAYER_CHECK_SHADERS=0)`
4. It also appears that when restoring cached messages to the UI log (after changing check boxes), the recent messages are _appended_ to `log_text_` when they should simply be restored (i.e. `log_text_ = state.recent_messages`). Without this change, unique messages are duplicated in the UI. This solution works when checkboxes are selected separately, but may not be correct if you want to allow multiple check boxes to be active at the same time. Anyways, it's up to you how you want to handle this situation.
See the following for updated VVL settings: https://github.com/KhronosGroup/VulkanSC-ValidationLayers/blob/sc_main/docs/updating_from_VK_EXT_validation_features.md
@gpx1000 I can provide a small PR to address these things if you want. Please advise.
Contributor guide
Research direction
Start with the new layer_settings sample (#1419) and compare its validation-layer settings with docs/updating_from_VK_EXT_validation_features.md. Inspect the code that restores cached messages to log_text_ from state.recent_messages. Done when the sample enables VVL as needed, uses current settings mechanisms, removes deprecated options, and does not duplicate restored messages.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- computer-graphics
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100