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.

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

If using multiple sets in a shader, they may be ordered incorrectly when creating a descriptor [fixed] #86

Open
EvilTrev opened this issue Apr 29, 2023 · 1 comment

Comments

@EvilTrev
Copy link

EvilTrev commented Apr 29, 2023

This is because they are coming from an unordered_map and the sets might get shuffled around, the following code in Pipeline.cpp inserts them into a vector in essentially random order:

        // Create the pipeline layout handle.
        std::vector<VkDescriptorSetLayout> setLayouts;
        for (auto it : pipeline->m_descriptorSetLayouts)
            setLayouts.push_back(it.second->GetHandle());

Change it as follows (in two spots) to solve this issue:

        // Create the pipeline layout handle.
        std::vector<VkDescriptorSetLayout> setLayouts( pipeline->m_descriptorSetLayouts.size() );
        for (auto it : pipeline->m_descriptorSetLayouts)
            setLayouts[ it.first ] = it.second->GetHandle();
@EvilTrev
Copy link
Author

EvilTrev commented Apr 29, 2023

NOTE: This isn't a complete fix, if you have a situation where your sets have gaps (i.e. set 0 and set 2, but no set 1) then you would create an array too small and cause issues. A more complete fix is to include this version of CreateDescriptorSetLayout in Pipeline.cpp, which identifies shaders with gaps in the sets and fills them with empty descriptors. It also ensures the array size is large enough to avoid the issue mentioned:

    VkResult Pipeline::CreateDescriptorSetLayouts()
    {
        // Iterate over each set binding and create a DescriptorSetLayout instance.
        uint32_t maxSetUsed = 0;
        for (auto it : m_bindings)
        {
            DescriptorSetLayout* descriptorSetLayout = nullptr;
            auto result = m_device->GetDescriptorSetLayoutCache()->CreateLayout(it.first, it.second, &descriptorSetLayout);
            if (result != VK_SUCCESS)
                return result;

            m_descriptorSetLayouts.emplace(it.first, descriptorSetLayout);
            maxSetUsed = std::max( maxSetUsed, it.first );
        }
		
        // Did we create gaps?
        if ( maxSetUsed != m_descriptorSetLayouts.size() - 1 )
        {
            // Fill the holes with empty descriptor sets
            std::vector<VezPipelineResource> emptySet;
            for ( uint32_t i=0; i<maxSetUsed; ++i )
            {
                if ( m_descriptorSetLayouts.find(i) == m_descriptorSetLayouts.end() )
                {
                    DescriptorSetLayout* descriptorSetLayout = nullptr;
                    auto result = m_device->GetDescriptorSetLayoutCache()->CreateLayout(i, emptySet, &descriptorSetLayout);
                    if (result != VK_SUCCESS)
                        return result;
	
                    m_descriptorSetLayouts.emplace(i, descriptorSetLayout);
                }
            }
        }

        // Return success.
        return VK_SUCCESS;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant