Loader settings bug fixes and expanded test coverage #1593
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change-Id: I1f498c6cbeb2dd6dbfd5fe870d768d3569379ecd
Change-Id: I8b278f8425ac463817d952113d20c6cf92c8d727
The issue was that copy of the VK_INSTANCE_LAYERS would be 'iterated' to find the next path, but wouldn't be reset back to the beginning of the stack allocation when going to the next iteration of the loop. By creating a separate char* to hold the "current path" pointer, the start of the allocation isn't lost. Change-Id: Ic3f0b818bda2f0a017bc30cb1f315b245008e9f7
Change-Id: I6fb421a851a44705c508db66f90e65e2b5b524f0
This acts as filler and allows more code paths to be executed than before, which exposed a bug in vkEnumerateInstanceExtensionProperties returning VK_INCOMPLETE when it shouldn't have.
The logic for whether to return VK_INCOMPLETE is straightforward - if not enough space was available to write out all available instance layers, write what there is space for and return VK_INCOMPLETE. The previous logic did not take into account the way the loader settings file affects which layers are available. The revised logic now correctly handles that situation.
Adds another layer so that the correct behavior occurs even when more layers are present, testing the implementation more thoroughly.
Allows capturing the output of loader_log and comparing it with expected values using fputs/fputc on *nix, and OutputDebugString on Windows. This interception is necessary due to the differing behavior between the debug log (through debug utils messenger) and the actual stderr output.
This was disabled due to issues with address sanitizer, which appear to no longer exist. The effects of intercepting readdir are an important part of the test framework shim, so disabling that makes some tests unreliable. It would be better to disable address sanitizer in CI where it is found to be an issue rather than disable readdir interception.
|
CI Vulkan-Loader build queued with queue ID 301850. |
|
Mingw is a known CI job failure - fixing that is lower priority than getting these changes in. |
|
CI Vulkan-Loader build # 2777 running. |
|
CI Vulkan-Loader build # 2777 passed. |
|
I am still getting RenderDoc to run automatically depiste not being enabled (loader log.txt):
|
|
CI Vulkan-Loader build queued with queue ID 302495. |
|
CI Vulkan-Loader build # 2778 running. |
|
CI Vulkan-Loader build # 2778 passed. |
|
CI Vulkan-Loader build queued with queue ID 302774. |
|
CI Vulkan-Loader build # 2779 running. |
|
CI Vulkan-Loader build # 2779 failed. |
If a loader settings file was present, the logging settings of the file were used irregardless of whether the settings file actually specified. The loader will now check to see if the settings stderr_log value is at least non-zero before deciding to skip a message, allowing for the VK_LOADER_DEBUG filter to be applied in case if it is zero.
Adds important cases which were ignored before. * Layers set to "off" in the loader setttings file only check if their names match, as the layer entry is missing other info * Check that lib_path is not nullptr before strcmp * Make it clear that Meta layers only check if the name of the layers match
charles-lunarg
force-pushed
the
expanded_loader_settings_tests
branch
from
5e442a5
to
dd0973c
Compare
|
CI Vulkan-Loader build queued with queue ID 302973. |
|
CI Vulkan-Loader build # 2780 running. |
Makes sure that layers set to "off" do not get enabled when VK_INSTANCE_LAYERS is set to activate them.
|
CI Vulkan-Loader build # 2780 passed. |
Checks that env-var enabled implicit layers aren't turned on by being in the settings layer by accident.
Log the following message for each enabled layer. "This layer was enabled because Env Var <name> was set to Value <value>"
charles-lunarg
force-pushed
the
expanded_loader_settings_tests
branch
from
dd0973c
to
d8fe584
Compare
|
CI Vulkan-Loader build queued with queue ID 303023. |
|
CI Vulkan-Loader build # 2781 running. |
|
CI Vulkan-Loader build # 2781 passed. |
|
Merging because all CI jobs passed (save for the known mingw failure case) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix VK_INSTANCE_LAYERS bug when using vk_loader_settings.json - stack allocated variable would be indexed past the end of the array if more than 1 layer was present in the settings file, causing garbage to be read as a string.
Make vkEnumerateInstanceExtensionProperties return VK_INCOMPLETE correctly - When some layers are turned "off" in the settings file, the count of layers would be correct but VK_INCOMPLETE would be returned by mistake.
Intercept loader_log in Test Framework - allows seeing the output of VK_LOADER_DEBUG & the loader settings file log settings output directly, rather than going through the Debug Utils Messenger.
Re-enable readdir interception on MacOS which was disabled so address sanitizer could work, which no longer seems to be needed.