The initialization path for both the capture and replay layers were
modifying a global `static` structure containing `VkApplicationInfo` to
update it when `Vulkan_Debug_ReplaceAppInfo` is enabled (the default),
and also always to pass through the most accurate `apiVersion` from the
caller instead of defaulting to a minimum.
Worse, code after `vkCreateInstance()` was reading back the value for
`apiVersion` passed through this global constant (rather than reading
the local copy).
The end result is that, at least on Android where 3 instances are
created at roughly the same time _on three different threads_ (as seen
in the logs captured in #3903) they all clobber this global state and
because `vkCreateInstance()` could easily take some time (4-12ms) those
threads would be reading back each others' `apiVersion`.
In turn when the last write to `renderdocAppInfo.apiVersion` is lower
than what an app originally set and expected based on the queried Vulkan
instance and physical device versions, all kinds of weird things happen
when expected promoted-to-core extensions are no longer treated as
available by RenderDoc [^1].
[^1]: A good example is an app enabling Ray Tracing extensions on a
Vulkan 1.2+ device: the buffer_device_address extension is in core
here and not explicitly enabled by at least our application, but if an
instance initializer for Vulkan 1.1 "won" the race condition (that's
the version `android framework` uses as of writing, even on Android 16)
RenderDoc doesn't load the extension function pointers. Ray Tracing
(currently) forces RenderDoc to query BDA for every buffer and crashes
at the first occurrence on a NULL PFN. Similar NULL pointers occurred
in our app when relying on `vkWaitSemaphores()` from the promoted
`VK_KHR_timeline_semaphore` extension for example.
The most trivial and correct solution is to no longer update this global
state, but instead keep it as `const` data and copy it to the stack for
modification and passing through to `vkCreateInstance()` without ever
leaking `apiVersion` outside of the calling function.
For empty draws, VulkanDebugManager::PatchLineStripIndexBuffer() can end up
with an empty list of patched indices, which it then tries to put into a 0
length index buffer. The only caller of this method only uses the index buffer
if the returned indexCount is > 0, so this is wasted work.
Creating a 0 length buffer violates VUID-VkBufferCreateInfo-size-00912 and
fails on KosmicKrisp. Other mesa drivers silently round the size up to 4096
bytes, the minium allocation size for kernel mode drivers.
This is triggered by check_empty_draw_overlays() in the VK_Indirect test.
* This is mostly useful for some of the tests that want to be able to run and
accumulate multiple errors, but while debugging we want an exception to catch.
* This includes:
- Incorrect comparisons
- Missing type annotations where it can't be inferred
- Variable shadowing and bad use
- Unused imports
- asserts for non-None on types
* Passes clean on strict pyright checking except for
`reportMissingParameterType`, `reportUnusedVariable`, and
`reportOptionalMemberAccess` which are all too spammy and low value to be
worth addressing.
* These come from long ago when using PyCharm with not as good type checking and
with renderdoc module stubs that were incomplete so needed help identifying
types.
* assert is handled specially by the type checker so we don't want to have
duplicate self.check / assert
* The original reason for TestCase.check was to print the assertion message but
this can be handled by checking for the AssertionError exception
* This helps with type checkers to know that things can be None or not (both to
silence previous warnings about "unnecessary" checks that are actually
necessary, and to ensure those checks aren't omitted by accident)
Jake Turner