Skip to content

S
swiftshader
Commit 243997ac by Nicolas Capens Committed by Nicolas Capens
Options

Don't access sampler handles for sampled image descriptors

The Vulkan 1.1 spec states that: "For VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, or VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT, only the imageView and imageLayout members of each element of VkWriteDescriptorSet::pImageInfo are accessed." Also remove the zeroing of sampler descriptor data in SampledImageDescriptor when no sampler handle is provided. This was intended to detect bugs but we didn't do it consistently for all descriptor types (VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER also uses SampledImageDescriptor and has no sampler data). We now pass a null pointer to the sampling routine when no sampler descriptor is available (specifically for OpImageFetch) so it is less likely to still access it unintentionally. Bug: b/139401791 Change-Id: Id1394493e88b7465fe3628c4c7d410cd3eaa3ccf Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/35388 Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarChris Forbes <chrisforbes@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
parent dbd02755
Showing with 12 additions and 18 deletions
src/Vulkan/VkDescriptorSetLayout.cpp
......@@ -158,7 +158,7 @@ void DescriptorSetLayout::initialize(DescriptorSet* descriptorSet)
for(uint32_t j = 0; j < bindings[i].descriptorCount; j++)
{
SampledImageDescriptor* imageSamplerDescriptor = reinterpret_cast<SampledImageDescriptor*>(mem);
imageSamplerDescriptor->updateSampler(vk::Cast(bindings[i].pImmutableSamplers[j]));
imageSamplerDescriptor->updateSampler(bindings[i].pImmutableSamplers[j]);
mem += typeSize;
}
}
......@@ -255,18 +255,9 @@ uint8_t* DescriptorSetLayout::getOffsetPointer(DescriptorSet *descriptorSet, uin
return &descriptorSet->data[byteOffset];
}
void SampledImageDescriptor::updateSampler(const vk::Sampler *newSampler)
void SampledImageDescriptor::updateSampler(const VkSampler newSampler)
{
if(newSampler)
{
memcpy(reinterpret_cast<void*>(&sampler), newSampler, sizeof(sampler));
}
else
{
// Descriptor ID's start at 1, allowing to detect descriptor update
// bugs by checking for 0. Also avoid reading random values.
memset(reinterpret_cast<void*>(&sampler), 0, sizeof(sampler));
}
memcpy(reinterpret_cast<void*>(&sampler), vk::Cast(newSampler), sizeof(sampler));
}
void DescriptorSetLayout::WriteDescriptorSet(Device* device, DescriptorSet *dstSet, VkDescriptorUpdateTemplateEntry const &entry, char const *src)
......@@ -292,7 +283,7 @@ void DescriptorSetLayout::WriteDescriptorSet(Device* device, DescriptorSet *dstS
// descriptorCount of zero, must all either use immutable samplers or must all not use immutable samplers."
if (!binding.pImmutableSamplers)
{
imageSampler[i].updateSampler(vk::Cast(update->sampler));
imageSampler[i].updateSampler(update->sampler);
}
imageSampler[i].device = device;
}
......@@ -347,11 +338,14 @@ void DescriptorSetLayout::WriteDescriptorSet(Device* device, DescriptorSet *dstS
sw::Texture *texture = &imageSampler[i].texture;
// "All consecutive bindings updated via a single VkWriteDescriptorSet structure, except those with a
// descriptorCount of zero, must all either use immutable samplers or must all not use immutable samplers."
if(!binding.pImmutableSamplers)
if(entry.descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER)
{
imageSampler[i].updateSampler(vk::Cast(update->sampler));
// "All consecutive bindings updated via a single VkWriteDescriptorSet structure, except those with a
// descriptorCount of zero, must all either use immutable samplers or must all not use immutable samplers."
if(!binding.pImmutableSamplers)
{
imageSampler[i].updateSampler(update->sampler);
}
}
imageSampler[i].imageViewId = imageView->id;
......
src/Vulkan/VkDescriptorSetLayout.hpp
......@@ -32,7 +32,7 @@ struct alignas(16) SampledImageDescriptor
{
~SampledImageDescriptor() = delete;
void updateSampler(const vk::Sampler *sampler);
void updateSampler(VkSampler sampler);
// TODO(b/129523279): Minimize to the data actually needed.
vk::Sampler sampler;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment