GPUOpen-LibrariesAndSDKs / V-EZ

MIT License
864 stars 70 forks source link

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

Open EvilTrev opened 1 year ago

EvilTrev commented 1 year ago

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 commented 1 year ago

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;
    }