DescriptorSet::compile()

[KAlO2]

Hello @robertosfield,

DescriptorSet::compile()

void DescriptorSet::compile(Context& context)
{
    if (!_implementation[context.deviceID])
    {
        // make sure all the contributing objects are compiled
        if (setLayout) setLayout->compile(context);  // checked
        for (auto& descriptor : descriptors) descriptor->compile(context);

        _implementation[context.deviceID] = context.allocateDescriptorSet(setLayout);  // not checked
        _implementation[context.deviceID]->assign(context, descriptors);
    }
}

The function here seems a bit suspicious.
You checked earlier that setLayout is not empty, but later you didn't check, which leds to a crash on my side.
Please ensure consistency of behavior.

ref_ptr<DescriptorSet::Implementation> DescriptorPools::allocateDescriptorSet(DescriptorSetLayout* descriptorSetLayout)
{
    for (auto itr = descriptorPools.rbegin(); itr != descriptorPools.rend(); ++itr)
    {
        auto dsi = (*itr)->allocateDescriptorSet(descriptorSetLayout);
        if (dsi) return dsi;
    }

    DescriptorPoolSizes descriptorPoolSizes;
    descriptorSetLayout->getDescriptorPoolSizes(descriptorPoolSizes);  // descriptorSetLayout is nullptr
    ...
}

[robertosfield]

Could you provide an example/model that illustrates the crash. If the scene graph that you are compiling is valid then I'll need to review all the code to make sure a null descriptorSetLayout is permissible, if not then we'll need to add a higher level check for this case a emit a warning/throw an error.

[KAlO2]

By tweaking the official svgExamples/commands/vsgdraw example and removing its texture-related code,
you can triggered the crash mentioned above.

test code vsgdraw.cpp

Or, just simplify it to the two lines of code below. Can descriptorSetLayout be null if no texture is used?

vsg::ref_ptr<vsg::DescriptorSetLayout> descriptorSetLayout;  // = vsg::DescriptorSetLayout::create();
auto descriptorSet = vsg::DescriptorSet::create(descriptorSetLayout, vsg::Descriptors{});

---

Perhaps another check should be performed in the DescriptorSet's constructor. It took me minutes to figure out that the DescriptorSetLayout's size did not match the Descriptors' size.

vsg::DescriptorSetLayoutBindings descriptorBindings{
// intentionally commented out to demonstrate the problem
// {0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1, VK_SHADER_STAGE_FRAGMENT_BIT, nullptr}
};

vsg::ref_ptr<vsg::DescriptorSetLayout> descriptorSetLayout = vsg::DescriptorSetLayout::create(descriptorBindings);
auto texture = vsg::DescriptorImage::create(vsg::Sampler::create(), textureData, 0, 0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER);
auto descriptorSet = vsg::DescriptorSet::create(descriptorSetLayout, vsg::Descriptors{texture});

[robertosfield]

I have tried out your modified vsgdraw.cpp and was able to reproduce the error conditions and as the problem combinations of DescriptorSet/DescriptorSetLayout are related to invalid setup I've added a checked for a null setLayouts and the DescriptorSetLayout.bindigs and the DescriptorSets.descriptor sizes not being equal, when an erroneous condition is found exceptions are throw.

Changes merged with VSG master with PR #1643

These changes will be part of the next 1.1.13 dev releases.