vk: offset descriptor template entries by their element counts - #197
Merged
jmacnak merged 3 commits intoOct 2, 2026
Merged
Conversation
calcLinearizedDescriptorUpdateTemplateInfo() and the three descriptor type helpers it calls do not use any decoder state. Make them file scope functions so that vk_decoder_global_state_unittest.cpp, which includes the source file, can test the template layout without a VkDecoderGlobalState. No functional change. Test: Vulkan_unittests --gtest_filter=VkDecoderGlobalState* Assisted-by: Claude Code:Claude-Opus-5.5
calcLinearizedDescriptorUpdateTemplateInfo() sizes the linearized buffers
by the sum of descriptorCount over the entries, and the guest sends every
element of every entry in entry order (vkUpdateDescriptorSetWithTemplateSized
and Sized2), but the per-entry offsets advanced by one element per entry.
An entry that follows an array entry therefore read the array's second
element instead of its own, so the host wrote the wrong image, buffer or
buffer view into every binding after the first array of a template.
Advance the image info, buffer info and buffer view cursors by the entry's
descriptorCount, as the inline uniform block path already does.
Test: Vulkan_unittests --gtest_filter=VkDecoderGlobalState*; the new
VkDecoderGlobalStateDescriptorUpdateTemplateTest fails on entries
1, 3 and 5 without the fix and passes with it.
Assisted-by: Claude Code:Claude-Opus-5.5
jmacnak
requested changes
Oct 1, 2026
jmacnak
left a comment
jmacnak
left a comment
Member
There was a problem hiding this comment.
Overall LGTM, one minor readability nit about the unit test.
Describe each template entry together with its expected offset and stride,
written as the previous entries' descriptorCount times their descriptor
sizes, so every expectation sits next to the entry it belongs to.
Test: Vulkan_unittests --gtest_filter=VkDecoderGlobalState*; the test still
fails on entries 1, 3 and 5 without the offset fix.
Assisted-by: Claude Code:Claude-Opus-5.5
Contributor
Author
Done — switched to a TestEntryInfo table as suggested. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
calcLinearizedDescriptorUpdateTemplateInfo()sizes the linearized buffers by the sum ofdescriptorCount, and the guest sends every element of every entry in entry order, but the per-entry offsets advanced by only one element per entry. Any image, buffer or buffer view entry that follows an entry withdescriptorCount > 1therefore reads the wrong elements, and the host writes the wrong descriptors. We hit this with a game on an x86_64 android-34 guest, where it showed up as a GPU fault.The first commit only moves the function and the three type helpers it uses to file scope (no functional change), so that
vk_decoder_global_state_unittest.cppcan call it. The second commit advances the offsets bydescriptorCount, as the inline uniform block path already does, and adds a test.Testing: on Windows, I compiled
vk_decoder_global_state_unittest.cppfrom this branch with clang-cl and the Android Emulator's CMake flags. I linked it intoVulkan_unittestswith that build's other objects, plus this branch'svk_emulated_physical_device_memory.cppandgoldfish_vk_supported_extensions.cpp. Then I ran--gtest_filter=VkDecoderGlobalState*: 7/7 pass. Without the fix, the new test fails on entries 1, 3 and 5. I have not run a full CMake or Bazel build ofmain. The test is only in the CMakeVulkan_unitteststarget; Bazel has no target for this file.This change was developed with AI assistance (Claude Code; see the
Assisted-bytrailer). The tests above were run on my machine, and I am responsible for the change.