KhronosGroup / KhronosGroup/Vulkan-Samples

New layer_settings sample has a few validation warnings

Open
#1,568 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.