[FIXED] zero descriptorCount passed to DescriptorPool::create

[siystar]

Pull Request Template

Description

Fixed zero descriptorCount entries passed to DescriptorPool::create(..).

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

auto context = vsg::Context::create(device);
vsg::ResourceRequirements requirements;
requirements.descriptorTypeMap[VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER] = 1;
requirements.externalNumDescriptorSets = 1;
context->reserve(requirements);
requirements.externalNumDescriptorSets = 2;
context->reserve(requirements);

vkCreateDescriptorPool(): pCreateInfo->pPoolSizes[0].descriptorCount is not greater than 0. The Vulkan spec states: descriptorCount must be greater than 0 (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorPoolSize-descriptorCount-00302)

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[psi29a]

Probably should go into RC3

[robertosfield]

I have added the suggest test code to the vsgvalidate example in the vsgvalidate branch of vsgExamples:

vsg-dev/vsgExamples@4cfae5d

I have merged this PR as the siystar-descriptorpool3 branch of the VSG:

https://github.com/vsg-dev/VulkanSceneGraph/tree/siystar-descriptorpool3

When I run the updated vsgvalidate with VSG master I see the error:

VUID-VkDescriptorPoolSize-descriptorCount-00302(ERROR / SPEC): msgNum: 1303077118 - Validation Error: [ VUID-VkDescriptorPoolSize-descriptorCount-00302 ] Object 0: handle = 0x561f
66928030, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0x4dab60fe | vkCreateDescriptorPool(): pCreateInfo->pPoolSizes[0].descriptorCount is not greater than 0. The Vulkan spec stat
es: descriptorCount must be greater than 0 (https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#VUID-VkDescriptorPoolSize-descriptorCount-00302)
    Objects: 1
        [0] 0x561f66928030, type: 3, name: NULL

With the siystar-descriptorpool3 branch I see a different error:

UID-VkDescriptorPoolCreateInfo-poolSizeCount-arraylength(ERROR / SPEC): msgNum: 149254764 - Validation Error: [ VUID-VkDescriptorPoolCreateInfo-poolSizeCount-arraylength ] Object
 0: handle = 0x557dbbb9aff0, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0x8e5726c | vkCreateDescriptorPool: parameter pCreateInfo->poolSizeCount must be greater than 0. The Vulka
n spec states: poolSizeCount must be greater than 0 (https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#VUID-VkDescriptorPoolCreateInfo-poolSizeCount-ar
raylength)
    Objects: 1
        [0] 0x557dbbb9aff0, type: 3, name: NULL

Thoughts?

[robertosfield]

Upping the number of required desriptors addresses the Vulkan debug error, as you hinted at with the comment:
// vkCreateDescriptorPool(): pCreateInfo->pPoolSizes[0].descriptorCount is not greater than 0. The Vulkan spec states: descriptorCount must be greater than 0 (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorPoolSize-descriptorCount-00302)

The following change to vsgvalidate.cpp fixes the Vulkan debug error:

$ git diff
diff --git a/examples/app/vsgvalidate/vsgvalidate.cpp b/examples/app/vsgvalidate/vsgvalidate.cpp
index e463095..a8b161c 100644
--- a/examples/app/vsgvalidate/vsgvalidate.cpp
+++ b/examples/app/vsgvalidate/vsgvalidate.cpp
@@ -148,6 +148,7 @@ int main(int argc, char** argv)

     vsg::info("Test 3, part 2");
     requirements.externalNumDescriptorSets = 2;

•    requirements.descriptorTypeMap[VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER] = 2;
     context->reserve(requirements);
  

However, this is a bit of odd "fix" and really hints a deeper question is should the Context::reserve(const ResourceRequirements& requirements) method be just specifying a new maximum total requirement or is it an additional requirement.

I will do another code review of the master and PR version of the Context::reserve(..) method and do some more pondering, For now it doesn't seem right to issue a vsg::DescriptorPool::create(..) call with invalid parameters so I think something more needs to be done.

[robertosfield]

Review the code the PR code looks better than the original but didn't catch the erroneous number of required_descriptorPoolSizes so I've added some additional checks and a warn(..) message to highlight the user that something has got into an invalid state : 9f5fa0a

I'm now happy with the code in the siystar-descriptorpool3 so will go ahead and merge with VSG master and close this PR.