Skip to content

Commit 9760732

Browse files
spencer-lunargspencer-lunarg
committed
layers: Add new CombineImageSampler Heap VU
1 parent 39c951c commit 9760732

2 files changed

Lines changed: 112 additions & 23 deletions

File tree

‎layers/core_checks/cc_spirv.cpp‎

Lines changed: 62 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3237,8 +3237,6 @@ bool CoreChecks::ValidateShaderDescriptorSetAndBindingMappingInfo(const spirv::M
32373237
VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_ARRAY_EXT,
32383238
VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_SHADER_RECORD_INDEX_EXT})) {
32393239
const uint32_t base_opcode = resource_variable.base_type.Opcode();
3240-
// TODO - Seem we are missing the image portion of combined image samplers
3241-
const bool is_sampler = resource_variable.is_combined_image_sampler;
32423240

32433241
struct MappingSourceInfo {
32443242
uint32_t offset = 0;
@@ -3248,10 +3246,12 @@ bool CoreChecks::ValidateShaderDescriptorSetAndBindingMappingInfo(const spirv::M
32483246
Field align_field = Field::Empty;
32493247
} info;
32503248

3251-
if (is_sampler) {
3252-
info.vuid = "VUID-VkDescriptorSetAndBindingMappingEXT-source-11254";
3253-
info.align = phys_dev_ext_props.descriptor_heap_props.samplerDescriptorAlignment;
3254-
info.align_field = Field::samplerDescriptorAlignment;
3249+
if (base_opcode == spv::OpTypeSampledImage) {
3250+
// VUID being added in
3251+
// https://gitlab.khronos.org/vulkan/vulkan/-/merge_requests/8242
3252+
info.vuid = "UNASSIGNED-VkDescriptorSetAndBindingMappingEXT-combined-image";
3253+
info.align = phys_dev_ext_props.descriptor_heap_props.imageDescriptorAlignment;
3254+
info.align_field = Field::imageDescriptorAlignment;
32553255
} else if (base_opcode == spv::OpTypeImage) {
32563256
info.vuid = "VUID-VkDescriptorSetAndBindingMappingEXT-source-11251";
32573257
info.align = phys_dev_ext_props.descriptor_heap_props.imageDescriptorAlignment;
@@ -3274,45 +3274,86 @@ bool CoreChecks::ValidateShaderDescriptorSetAndBindingMappingInfo(const spirv::M
32743274

32753275
if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_CONSTANT_OFFSET_EXT) {
32763276
const auto& source_data = mapping.sourceData.constantOffset;
3277-
info.offset = is_sampler ? source_data.samplerHeapOffset : source_data.heapOffset;
3278-
info.array_stride = is_sampler ? source_data.samplerHeapArrayStride : source_data.heapArrayStride;
3277+
info.offset = source_data.heapOffset;
3278+
info.array_stride = source_data.heapArrayStride;
32793279
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_PUSH_INDEX_EXT) {
32803280
const auto& source_data = mapping.sourceData.pushIndex;
3281-
info.offset = is_sampler ? source_data.samplerHeapOffset : source_data.heapOffset;
3282-
info.array_stride = is_sampler ? source_data.samplerHeapArrayStride : source_data.heapArrayStride;
3281+
info.offset = source_data.heapOffset;
3282+
info.array_stride = source_data.heapArrayStride;
32833283
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_EXT) {
32843284
const auto& source_data = mapping.sourceData.indirectIndex;
3285-
info.offset = is_sampler ? source_data.samplerHeapOffset : source_data.heapOffset;
3286-
info.array_stride = is_sampler ? source_data.samplerHeapArrayStride : source_data.heapArrayStride;
3285+
info.offset = source_data.heapOffset;
3286+
info.array_stride = source_data.heapArrayStride;
32873287
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_ARRAY_EXT) {
32883288
const auto& source_data = mapping.sourceData.indirectIndexArray;
3289-
info.offset = is_sampler ? source_data.samplerHeapOffset : source_data.heapOffset;
3289+
info.offset = source_data.heapOffset;
32903290
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_SHADER_RECORD_INDEX_EXT) {
32913291
const auto& source_data = mapping.sourceData.shaderRecordIndex;
3292-
info.offset = is_sampler ? source_data.samplerHeapOffset : source_data.heapOffset;
3293-
info.array_stride = is_sampler ? source_data.samplerHeapArrayStride : source_data.heapArrayStride;
3292+
info.offset = source_data.heapOffset;
3293+
info.array_stride = source_data.heapArrayStride;
32943294
}
32953295

32963296
if (!IsIntegerMultipleOf(info.offset, info.align) || !IsIntegerMultipleOf(info.array_stride, info.align)) {
32973297
const Field source_field = vvl::Field_VkDescriptorMappingSourceDataEXT(mapping.source);
3298-
const Field offset_field = is_sampler ? Field::samplerHeapOffset : Field::heapOffset;
3299-
const Field stride_field = is_sampler ? Field::samplerHeapArrayStride : Field::heapArrayStride;
33003298

33013299
std::stringstream ss;
33023300
ss << "(" << string_VkDescriptorMappingSourceEXT(mapping.source) << ") is used to map descriptor "
33033301
<< resource_variable.DescribeDescriptor() << " in " << entrypoint.Describe() << " but it is unaligned.\n"
3304-
<< String(source_field) << "." << String(offset_field) << " (" << info.offset << ") ";
3302+
<< String(source_field) << ".heapOffset (" << info.offset << ") ";
33053303
if (mapping.source != VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_ARRAY_EXT) {
3306-
ss << "and " << String(source_field) << "." << String(stride_field) << " (" << info.array_stride
3307-
<< ") both ";
3304+
ss << "and " << String(source_field) << ".heapArrayStride (" << info.array_stride << ") both ";
33083305
}
3309-
ss << "must be aligned with " << String(info.align_field) << " (" << info.align << ")";
3306+
ss << "must be zero or aligned with " << String(info.align_field) << " (" << info.align << ")";
33103307

33113308
skip |= LogError(
33123309
info.vuid, module_state.handle(),
33133310
loc.pNext(Struct::VkShaderDescriptorSetAndBindingMappingInfoEXT, Field::pMappings, i).dot(Field::source),
33143311
"%s", ss.str().c_str());
33153312
}
3313+
3314+
// Combined Image Sampler is only spot we need to check twice
3315+
if (base_opcode == spv::OpTypeSampledImage) {
3316+
if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_CONSTANT_OFFSET_EXT) {
3317+
const auto& source_data = mapping.sourceData.constantOffset;
3318+
info.offset = source_data.samplerHeapOffset;
3319+
info.array_stride = source_data.samplerHeapArrayStride;
3320+
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_PUSH_INDEX_EXT) {
3321+
const auto& source_data = mapping.sourceData.pushIndex;
3322+
info.offset = source_data.samplerHeapOffset;
3323+
info.array_stride = source_data.samplerHeapArrayStride;
3324+
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_EXT) {
3325+
const auto& source_data = mapping.sourceData.indirectIndex;
3326+
info.offset = source_data.samplerHeapOffset;
3327+
info.array_stride = source_data.samplerHeapArrayStride;
3328+
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_ARRAY_EXT) {
3329+
const auto& source_data = mapping.sourceData.indirectIndexArray;
3330+
info.offset = source_data.samplerHeapOffset;
3331+
} else if (mapping.source == VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_SHADER_RECORD_INDEX_EXT) {
3332+
const auto& source_data = mapping.sourceData.shaderRecordIndex;
3333+
info.offset = source_data.samplerHeapOffset;
3334+
info.array_stride = source_data.samplerHeapArrayStride;
3335+
}
3336+
3337+
info.align = phys_dev_ext_props.descriptor_heap_props.samplerDescriptorAlignment;
3338+
3339+
if (!IsIntegerMultipleOf(info.offset, info.align) || !IsIntegerMultipleOf(info.array_stride, info.align)) {
3340+
const Field source_field = vvl::Field_VkDescriptorMappingSourceDataEXT(mapping.source);
3341+
3342+
std::stringstream ss;
3343+
ss << "(" << string_VkDescriptorMappingSourceEXT(mapping.source) << ") is used to map descriptor "
3344+
<< resource_variable.DescribeDescriptor() << " in " << entrypoint.Describe() << " but it is unaligned.\n"
3345+
<< String(source_field) << ".samplerHeapOffset (" << info.offset << ") ";
3346+
if (mapping.source != VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_ARRAY_EXT) {
3347+
ss << "and " << String(source_field) << ".samplerHeapArrayStride (" << info.array_stride << ") both ";
3348+
}
3349+
ss << "must be zero or aligned with samplerDescriptorAlignment (" << info.align << ")";
3350+
3351+
skip |= LogError("VUID-VkDescriptorSetAndBindingMappingEXT-source-11254", module_state.handle(),
3352+
loc.pNext(Struct::VkShaderDescriptorSetAndBindingMappingInfoEXT, Field::pMappings, i)
3353+
.dot(Field::source),
3354+
"%s", ss.str().c_str());
3355+
}
3356+
}
33163357
} else if (IsValueIn(mapping.source,
33173358
{VK_DESCRIPTOR_MAPPING_SOURCE_PUSH_DATA_EXT, VK_DESCRIPTOR_MAPPING_SOURCE_SHADER_RECORD_DATA_EXT,
33183359
VK_DESCRIPTOR_MAPPING_SOURCE_RESOURCE_HEAP_DATA_EXT})) {

‎tests/unit/descriptor_heap.cpp‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3453,8 +3453,7 @@ TEST_F(NegativeDescriptorHeap, OpTypeSampler) {
34533453
}
34543454
}
34553455

3456-
TEST_F(NegativeDescriptorHeap, OpTypeSampledImage) {
3457-
TEST_DESCRIPTION("Validate that mapping is aligned for OpTypeSampledImage");
3456+
TEST_F(NegativeDescriptorHeap, OpTypeSampledImageAlignedSampler) {
34583457
AddRequiredFeature(vkt::Feature::bufferDeviceAddress);
34593458
RETURN_IF_SKIP(InitBasicDescriptorHeap());
34603459
RETURN_IF_SKIP(CheckSlangSupport());
@@ -3520,6 +3519,55 @@ TEST_F(NegativeDescriptorHeap, OpTypeSampledImage) {
35203519
}
35213520
}
35223521

3522+
TEST_F(NegativeDescriptorHeap, OpTypeSampledImageAlignedImage) {
3523+
AddRequiredFeature(vkt::Feature::bufferDeviceAddress);
3524+
RETURN_IF_SKIP(InitBasicDescriptorHeap());
3525+
3526+
if (heap_props.imageDescriptorAlignment < 2) {
3527+
GTEST_SKIP() << "Cannot be unaligned with imageDescriptorAlignment less than 2";
3528+
}
3529+
3530+
VkDescriptorSetAndBindingMappingEXT mappings[2];
3531+
mappings[0] = MakeSetAndBindingMapping(0, 0);
3532+
mappings[0].source = VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_CONSTANT_OFFSET_EXT;
3533+
mappings[0].sourceData.constantOffset.heapOffset = 0;
3534+
mappings[0].sourceData.constantOffset.heapArrayStride = 0;
3535+
3536+
mappings[1] = MakeSetAndBindingMapping(0, 1);
3537+
mappings[1].source = VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_CONSTANT_OFFSET_EXT;
3538+
3539+
VkShaderDescriptorSetAndBindingMappingInfoEXT mapping_info = vku::InitStructHelper();
3540+
mapping_info.mappingCount = 2;
3541+
mapping_info.pMappings = mappings;
3542+
3543+
char const* cs_source = R"glsl(
3544+
#version 450
3545+
layout(local_size_x = 1) in;
3546+
layout(set = 0, binding = 0) buffer Output { int result[]; };
3547+
layout(set = 0, binding = 1) uniform sampler2D tex;
3548+
void main() {
3549+
vec4 color = textureLod(tex, vec2(gl_GlobalInvocationID.xy), 0);
3550+
result[gl_LocalInvocationIndex] = int(color.r);
3551+
}
3552+
)glsl";
3553+
3554+
VkShaderObj cs_module = VkShaderObj(*m_device, cs_source, VK_SHADER_STAGE_COMPUTE_BIT);
3555+
3556+
VkPipelineCreateFlags2CreateInfo pipeline_create_flags_2_create_info = vku::InitStructHelper();
3557+
pipeline_create_flags_2_create_info.flags = VK_PIPELINE_CREATE_2_DESCRIPTOR_HEAP_BIT_EXT;
3558+
3559+
CreateComputePipelineHelper pipe(*this, &pipeline_create_flags_2_create_info);
3560+
pipe.cp_ci_.layout = VK_NULL_HANDLE;
3561+
pipe.cp_ci_.stage = cs_module.GetStageCreateInfo(&mapping_info);
3562+
3563+
mappings[1].sourceData.constantOffset.heapOffset = 1;
3564+
mappings[1].sourceData.constantOffset.heapArrayStride = 0;
3565+
3566+
m_errorMonitor->SetDesiredError("UNASSIGNED-VkDescriptorSetAndBindingMappingEXT-combined-image");
3567+
pipe.CreateComputePipeline(false);
3568+
m_errorMonitor->VerifyFound();
3569+
}
3570+
35233571
TEST_F(NegativeDescriptorHeap, NoMappingStruct) {
35243572
AddRequiredFeature(vkt::Feature::bufferDeviceAddress);
35253573
AddRequiredFeature(vkt::Feature::runtimeDescriptorArray);

0 commit comments

Comments
 (0)