Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OOM in Vulkan FetchShaderFeedback for descriptor binding's which use VariableDescriptorCount feature #3105

Merged
merged 2 commits into from
Nov 1, 2023
+82 −14
Merged

Fix OOM in Vulkan FetchShaderFeedback for descriptor binding's which use VariableDescriptorCount feature #3105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d37026d
Test for very large variable sized descriptor binding
Zorro666 Oct 31, 2023
9204451
Handle variable sized descriptor sets in FetchShaderFeedback
Zorro666 Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions renderdoc/driver/vulkan/vk_shader_feedback.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,9 @@ bool VulkanReplay::FetchShaderFeedback(uint32_t eventId)
if(pipeLayouts[i] == ResourceId())
continue;

const rdcarray<VulkanStatePipeline::DescriptorAndOffsets> &descSets =
(result.compute ? state.compute.descSets : state.graphics.descSets);

rdcspv::Binding key;

for(size_t set = 0; set < pipeInfo.descSetLayouts.size(); set++)
Expand All @@ -1599,11 +1602,26 @@ bool VulkanReplay::FetchShaderFeedback(uint32_t eventId)
if(bindData.descriptorCount > 1 &&
bindData.layoutDescType != VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK)
{
uint32_t descriptorCount = bindData.descriptorCount;
if(bindData.variableSize)
{
if(set < descSets.size())
{
ResourceId descSet = descSets[set].descSet;
if(descSet != ResourceId())
{
auto it = m_pDriver->m_DescriptorSetState.find(descSet);
if(it != m_pDriver->m_DescriptorSetState.end())
descriptorCount = it->second.data.variableDescriptorCount;
}
}
}

key.binding = (uint32_t)binding;

offsetMap[key] = {feedbackStorageSize, bindData.descriptorCount};
offsetMap[key] = {feedbackStorageSize, descriptorCount};

feedbackStorageSize += bindData.descriptorCount * sizeof(uint32_t);
feedbackStorageSize += descriptorCount * sizeof(uint32_t);
}
}
}
Expand Down
68 changes: 58 additions & 10 deletions util/test/demos/vk/vk_descriptor_index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,14 @@

#endif

#define DESC_ARRAY3_SIZE 3

#define BUFIDX 15
#define INDEX3 4
#define INDEX1 49
#define INDEX2 381
#define NONUNIFORMIDX 20
#define TEX3_INDEX 1

RD_TEST(VK_Descriptor_Indexing, VulkanGraphicsTest)
{
Expand Down Expand Up @@ -93,6 +96,7 @@ layout(push_constant) uniform PushData
uint bufidx;
uint idx1;
uint idx2;
uint idx3;
} push;

struct tex_ref
Expand Down Expand Up @@ -123,8 +127,11 @@ void main()
outbuf[push.bufidx].outrefs[3].binding = 2;
outbuf[push.bufidx].outrefs[3].idx = push.idx2+5;

outbuf[push.bufidx].outrefs[4].binding = 3;
outbuf[push.bufidx].outrefs[4].idx = push.idx3;

// terminator
outbuf[push.bufidx].outrefs[4].binding = 100;
outbuf[push.bufidx].outrefs[5].binding = 100;
}

)EOSHADER";
Expand Down Expand Up @@ -152,6 +159,7 @@ layout(binding = 0, std430) buffer inbuftype {

layout(binding = 1) uniform sampler2D tex1[)EOSHADER" STRINGIZE(DESC_ARRAY1_SIZE) R"EOSHADER(];
layout(binding = 2) uniform sampler2D tex2[];
layout(binding = 3) uniform sampler2D tex3[)EOSHADER" STRINGIZE(DESC_ARRAY3_SIZE) R"EOSHADER(];

void add_color(sampler2D tex)
{
Expand Down Expand Up @@ -220,8 +228,10 @@ void main()
// function call with array parameters
if(t.binding < 2)
dispatch_indirect_color(0, tex1, tex1, 5.0f, t);
else
else if(t.binding < 3)
add_color(tex2[t.idx]);
else
add_color(tex3[t.idx]);
}
}
}
Expand Down Expand Up @@ -269,6 +279,9 @@ void main()
else if(!descIndexing.shaderSampledImageArrayNonUniformIndexing)
Avail =
"Descriptor indexing feature 'shaderSampledImageArrayNonUniformIndexing' not available";
else if(!descIndexing.descriptorBindingVariableDescriptorCount)
Avail =
"Descriptor indexing feature 'descriptorBindingVariableDescriptorCount' not available";

static VkPhysicalDeviceDescriptorIndexingFeaturesEXT descIndexingEnable = {
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DESCRIPTOR_INDEXING_FEATURES_EXT,
Expand All @@ -277,6 +290,7 @@ void main()
descIndexingEnable.descriptorBindingPartiallyBound = VK_TRUE;
descIndexingEnable.runtimeDescriptorArray = VK_TRUE;
descIndexingEnable.shaderSampledImageArrayNonUniformIndexing = VK_TRUE;
descIndexingEnable.descriptorBindingVariableDescriptorCount = VK_TRUE;

devInfoNext = &descIndexingEnable;
}
Expand All @@ -291,13 +305,14 @@ void main()
VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_BINDING_FLAGS_CREATE_INFO_EXT,
};

VkDescriptorBindingFlagsEXT bindFlags[3] = {
VkDescriptorBindingFlagsEXT bindFlags[4] = {
VK_DESCRIPTOR_BINDING_PARTIALLY_BOUND_BIT_EXT,
0,
VK_DESCRIPTOR_BINDING_PARTIALLY_BOUND_BIT_EXT,
VK_DESCRIPTOR_BINDING_VARIABLE_DESCRIPTOR_COUNT_BIT_EXT,
};

descFlags.bindingCount = 3;
descFlags.bindingCount = ARRAYSIZE(bindFlags);
descFlags.pBindingFlags = bindFlags;

VkDescriptorSetLayout setlayout = createDescriptorSetLayout(
Expand All @@ -321,6 +336,12 @@ void main()
DESC_ARRAY2_SIZE,
VK_SHADER_STAGE_FRAGMENT_BIT | VK_SHADER_STAGE_COMPUTE_BIT,
},
{
3,
VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER,
UINT32_MAX,
VK_SHADER_STAGE_FRAGMENT_BIT | VK_SHADER_STAGE_COMPUTE_BIT,
},
})
.next(&descFlags));

Expand Down Expand Up @@ -479,11 +500,26 @@ void main()
}),
NULL, &descpool));

CHECK_VKR(vkAllocateDescriptorSets(
device,
vkh::DescriptorSetAllocateInfo(descpool,
{setlayout, setlayout, setlayout, setlayout, setlayout}),
descset));
const static uint32_t numDescriptorSets = ARRAYSIZE(descset);
std::vector<VkDescriptorSetLayout> setLayouts(numDescriptorSets, setlayout);
std::vector<uint32_t> counts(numDescriptorSets, DESC_ARRAY3_SIZE);

VkDescriptorSetVariableDescriptorCountAllocateInfoEXT countInfo = {
VK_STRUCTURE_TYPE_DESCRIPTOR_SET_VARIABLE_DESCRIPTOR_COUNT_ALLOCATE_INFO_EXT,
NULL,
numDescriptorSets,
counts.data(),
};

VkDescriptorSetAllocateInfo allocInfo = {
VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO,
&countInfo,
descpool,
numDescriptorSets,
setLayouts.data(),
};

CHECK_VKR(vkAllocateDescriptorSets(device, &allocInfo, descset));
}

VkSampler sampler = createSampler(vkh::SamplerCreateInfo(VK_FILTER_LINEAR));
Expand Down Expand Up @@ -516,6 +552,12 @@ void main()
up.descriptorCount = DESC_ARRAY2_SIZE - 20;

ups.push_back(up);

up.dstBinding = 3;
up.dstArrayElement = 0;
up.descriptorCount = DESC_ARRAY3_SIZE;

ups.push_back(up);
}

vkh::updateDescriptorSets(device, ups);
Expand Down Expand Up @@ -566,6 +608,11 @@ void main()
{
vkh::DescriptorImageInfo(imgview, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, sampler),
}),
vkh::WriteDescriptorSet(
descset[0], 3, TEX3_INDEX, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER,
{
vkh::DescriptorImageInfo(imgview, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, sampler),
}),
});

while(Running())
Expand All @@ -586,13 +633,14 @@ void main()
vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, layout, 0, 1, &descset[0], 0,
NULL);

Vec4i idx = {BUFIDX, INDEX1, INDEX2, 0};
Vec4i idx = {BUFIDX, INDEX1, INDEX2, TEX3_INDEX};
vkCmdPushConstants(cmd, layout, VK_SHADER_STAGE_FRAGMENT_BIT | VK_SHADER_STAGE_COMPUTE_BIT, 0,
sizeof(Vec4i), &idx);

static_assert(BUFIDX < DESC_ARRAY1_SIZE, "Buffer index is out of bounds");
static_assert(INDEX1 < DESC_ARRAY1_SIZE, "Index 1 is out of bounds");
static_assert(INDEX2 < DESC_ARRAY2_SIZE, "Index 2 is out of bounds");
static_assert(TEX3_INDEX < DESC_ARRAY3_SIZE, "Index 3 is out of bounds");

vkCmdFillBuffer(cmd, ssbo.buffer, 0, 1024 * 1024, 0);

Expand Down
6 changes: 4 additions & 2 deletions util/test/tests/Vulkan/VK_Descriptor_Indexing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ def check_capture(self):
# images 49 & 59 in bind 1 should be used for the first fixed index
# image 4 in bind 1 should be used for the global access from a function with no dynamic/patched parameters
# - images 381 & 386 in bind 2 should be used for the second fixed index
# - image 1 in bind 3 should be used
bind_info = {
0: { 'dynamicallyUsedCount': 1, 'used': [15] },
1: { 'dynamicallyUsedCount': 6, 'used': [4, 19, 20, 21, 49, 59] },
2: { 'dynamicallyUsedCount': 2, 'used': [381, 386] },
3: { 'dynamicallyUsedCount': 1, 'used': [1] },
}

if len(vkpipe.graphics.descriptorSets) != 1:
Expand Down Expand Up @@ -91,7 +93,7 @@ def check_capture(self):

pipe = self.controller.GetPipelineState()
ro = pipe.GetReadOnlyResources(rd.ShaderStage.Pixel, False)
self.check_eq(len(ro), 2)
self.check_eq(len(ro), 3)
self.check_eq(ro[0].dynamicallyUsedCount, 6)
self.check_eq(ro[0].firstIndex, 0)
self.check_eq(len(ro[0].resources), 128)
Expand All @@ -101,7 +103,7 @@ def check_capture(self):
self.check(not ro[0].resources[18].dynamicallyUsed)
self.check(ro[0].resources[19].dynamicallyUsed)
ro = pipe.GetReadOnlyResources(rd.ShaderStage.Pixel, True)
self.check_eq(len(ro), 2)
self.check_eq(len(ro), 3)
self.check_eq(ro[0].dynamicallyUsedCount, 6)
self.check_eq(ro[0].firstIndex, 4)
self.check_eq(len(ro[0].resources), 56)
Expand Down