From c5df93bbfa33b75ca6867ef6bb9263e1017ba253 Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 3 Mar 2026 16:58:02 +0000 Subject: [PATCH] Check for samplers potentially being garbage in descriptor updates * If immutable samplers are used the sampler parameter of a descriptor update could be garbage. Checked for elsewhere but not handled in descriptor update template unwrap. --- .../vulkan/wrappers/vk_descriptor_funcs.cpp | 26 ++++++++- util/test/demos/vk/vk_parameter_zoo.cpp | 56 +++++++++++++------ 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp index 12f8f8c26..a5c2c896a 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp @@ -3149,6 +3149,29 @@ void WrappedVulkan::vkUpdateDescriptorSetWithTemplate( byte *dst = memory + entry.offset; const byte *src = (const byte *)pData + entry.offset; + bool hasImmutable = false; + + if(IsCaptureMode(m_State)) + { + VkResourceRecord *record = GetRecord(descriptorSet); + RDCASSERT(record->descInfo && record->descInfo->layout); + const DescSetLayout &layout = *record->descInfo->layout; + + RDCASSERT(entry.dstBinding < record->descInfo->data.binds.size()); + const DescSetLayout::Binding *layoutBinding = &layout.bindings[entry.dstBinding]; + + hasImmutable = layoutBinding->immutableSampler != NULL; + } + else + { + const DescSetLayout &layout = + m_CreationInfo.m_DescSetLayout[m_DescriptorSetState[GetResID(descriptorSet)].layout]; + + const DescSetLayout::Binding *layoutBinding = &layout.bindings[entry.dstBinding]; + + hasImmutable = layoutBinding->immutableSampler != NULL; + } + if(entry.descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER || entry.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER) { @@ -3171,7 +3194,8 @@ void WrappedVulkan::vkUpdateDescriptorSetWithTemplate( entry.descriptorType == VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT) { bool hasSampler = (entry.descriptorType == VK_DESCRIPTOR_TYPE_SAMPLER || - entry.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER); + entry.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER) && + !hasImmutable; bool hasImage = (entry.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER || entry.descriptorType == VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE || entry.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE || diff --git a/util/test/demos/vk/vk_parameter_zoo.cpp b/util/test/demos/vk/vk_parameter_zoo.cpp index 9547ca0a3..189a519cf 100644 --- a/util/test/demos/vk/vk_parameter_zoo.cpp +++ b/util/test/demos/vk/vk_parameter_zoo.cpp @@ -1284,6 +1284,7 @@ void main() }; VkDescriptorUpdateTemplateKHR inlinetempl = VK_NULL_HANDLE; + VkDescriptorUpdateTemplateKHR immuttempl = VK_NULL_HANDLE; VkDescriptorSet inlineuboset = allocateDescriptorSet(inlineubosetlayout); VkDescriptorSet inlineuboset_templ = allocateDescriptorSet(inlineubosetlayout); VkDescriptorSet inlineuboset_templ_dyn = allocateDescriptorSet(inlineubosetlayout); @@ -1393,16 +1394,34 @@ void main() setName(mutableSampler, "mutableSampler"); + VkDescriptorImageInfo immutUpdate = + vkh::DescriptorImageInfo(validImgView, VK_IMAGE_LAYOUT_GENERAL, mutableSampler); + // try writing a different sampler to the immutable sampler, it should not be applied vkh::updateDescriptorSets( - device, - { - vkh::WriteDescriptorSet( - immutdescset, 0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, - { - vkh::DescriptorImageInfo(validImgView, VK_IMAGE_LAYOUT_GENERAL, mutableSampler), - }), - }); + device, { + vkh::WriteDescriptorSet( + immutdescset, 0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, {immutUpdate}), + }); + + if(KHR_descriptor_update_template) + { + std::vector entries = { + {0, 0, 1, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 0, sizeof(VkDescriptorImageInfo)}, + }; + + VkDescriptorUpdateTemplateCreateInfoKHR createInfo = {}; + createInfo.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_UPDATE_TEMPLATE_CREATE_INFO_KHR; + createInfo.descriptorUpdateEntryCount = (uint32_t)entries.size(); + createInfo.pDescriptorUpdateEntries = entries.data(); + createInfo.templateType = VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_DESCRIPTOR_SET_KHR; + createInfo.descriptorSetLayout = immutsetlayout; + createInfo.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS; + + vkCreateDescriptorUpdateTemplateKHR(device, &createInfo, NULL, &immuttempl); + + vkUpdateDescriptorSetWithTemplateKHR(device, immutdescset, immuttempl, &immutUpdate); + } refdatastruct resetrefdata = {}; resetrefdata.sampler.sampler = resetrefdata.combined.sampler = validSampler; @@ -1671,16 +1690,19 @@ void main() Submit(0, 4, {cmd}); } + immutUpdate.sampler = invalidSampler; + // try writing with an invalid sampler to the immutable, it should be ignored vkh::updateDescriptorSets( - device, - { - vkh::WriteDescriptorSet( - immutdescset, 0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, - { - vkh::DescriptorImageInfo(validImgView, VK_IMAGE_LAYOUT_GENERAL, invalidSampler), - }), - }); + device, { + vkh::WriteDescriptorSet( + immutdescset, 0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, {immutUpdate}), + }); + + if(KHR_descriptor_update_template) + { + vkUpdateDescriptorSetWithTemplateKHR(device, immutdescset, immuttempl, &immutUpdate); + } // do a bunch of spinning on fences/semaphores that should not be serialised exhaustively VkResult status = VK_SUCCESS; @@ -2069,6 +2091,8 @@ void main() { vkDestroyDescriptorUpdateTemplateKHR(device, reftempl, NULL); + vkDestroyDescriptorUpdateTemplateKHR(device, immuttempl, NULL); + if(hasExt(VK_EXT_INLINE_UNIFORM_BLOCK_EXTENSION_NAME)) vkDestroyDescriptorUpdateTemplateKHR(device, inlinetempl, NULL); }