Merge pull request #110 from crud89/optional-default-descriptor-bindings

Improve descriptor set handling.

docs/release-logs/0.4.1.md

@@ -10,9 +10,10 @@
 - Improvements to C++ core guideline conformance ([See PR #103](https://github.com/crud89/LiteFX/pull/103)).
 - New event infrastructure. ([See PR #81](https://github.com/crud89/LiteFX/pull/81))
 - Add support for user-defined debug markers. ([See PR #82](https://github.com/crud89/LiteFX/pull/82))
-- Improved resource allocation and binding: ([See PR #83](https://github.com/crud89/LiteFX/pull/83))
+- Improved resource allocation and binding: ([See PR #83](https://github.com/crud89/LiteFX/pull/83) and [PR #110](https://github.com/crud89/LiteFX/pull/110))
   - Resources can now be created without querying the descriptor set layout or descriptor layout in advance.
   - When allocating descriptor sets, default bindings can be provided to make bind-once scenarios more straightforward.
+  - Descriptor sets can also be allocated without providing any binding index (in which case continous counting is assumed) or resources (which enables late binding or resource updating).
 - Improved handling of temporary command buffers. ([See PR #89](https://github.com/crud89/LiteFX/pull/89))
   - Command buffers can now be submitted with shared ownership to a command queue, which then stores them and releases the references, if the submit fence is passed (during `waitFor`).
   - Command buffer transfers can now receive resources with shared ownership. Resource references are released in a similar fashion.
@@ -29,6 +30,7 @@
 - Raise minimum Vulkan SDK version to 1.3.204.1. ([See PR #86](https://github.com/crud89/LiteFX/pull/86) and [See PR #88](https://github.com/crud89/LiteFX/pull/88))
 - `VK_EXT_debug_utils` is now enabled by default for the Vulkan backend in debug builds. ([See PR #82](https://github.com/crud89/LiteFX/pull/82))
 - Images are now implicitly transitioned during transfer operations. ([See PR #93](https://github.com/crud89/LiteFX/pull/93))
+- Empty descriptor sets are now allowed and may be automatically created to fill gaps in descriptor set space indices. ([See PR#110](https://github.com/crud89/LiteFX/pull/110))
 
 **❎ DirectX 12:**
 

src/Backends/DirectX12/src/descriptor_set_layout.cpp

@@ -240,13 +240,18 @@ UniquePtr<DirectX12DescriptorSet> DirectX12DescriptorSetLayout::allocate(UInt32
     auto descriptorSet = makeUnique<DirectX12DescriptorSet>(*this, std::move(bufferHeap), std::move(samplerHeap));
 
     // Apply the default bindings.
-    for (auto& binding : bindings)
+    for (UInt32 i{ 0 }; auto & binding : bindings)
+    {
         std::visit(type_switch{
-            [&descriptorSet, &binding](const ISampler& sampler) { descriptorSet->update(binding.binding, sampler, binding.firstDescriptor); },
-            [&descriptorSet, &binding](const IBuffer& buffer) { descriptorSet->update(binding.binding, buffer, binding.firstElement, binding.elements, binding.firstDescriptor); },
-            [&descriptorSet, &binding](const IImage& image) { descriptorSet->update(binding.binding, image, binding.firstDescriptor, binding.firstLevel, binding.levels, binding.firstElement, binding.elements); }
+            [](const std::monostate&) { }, // Default: don't bind anything.
+            [&descriptorSet, &binding, i](const ISampler& sampler) { descriptorSet->update(binding.binding.value_or(i), sampler, binding.firstDescriptor); },
+            [&descriptorSet, &binding, i](const IBuffer& buffer) { descriptorSet->update(binding.binding.value_or(i), buffer, binding.firstElement, binding.elements, binding.firstDescriptor); },
+            [&descriptorSet, &binding, i](const IImage& image) { descriptorSet->update(binding.binding.value_or(i), image, binding.firstDescriptor, binding.firstLevel, binding.levels, binding.firstElement, binding.elements); }
         }, binding.resource);
 
+        ++i;
+    }
+
     // Return the descriptor set.
     return descriptorSet;
 }

src/Backends/DirectX12/src/pipeline_layout.cpp

@@ -60,6 +60,17 @@ class DirectX12PipelineLayout::DirectX12PipelineLayoutImpl : public Implement<Di
 public:
     ComPtr<ID3D12RootSignature> initialize()
     {
+        // Sort and check if there are duplicate space indices.
+        std::ranges::sort(m_descriptorSetLayouts, [](const UniquePtr<DirectX12DescriptorSetLayout>& a, const UniquePtr<DirectX12DescriptorSetLayout>& b) { return a->space() < b->space(); });
+
+        for (Tuple spaces : m_descriptorSetLayouts | std::views::transform([](const UniquePtr<DirectX12DescriptorSetLayout>& layout) { return layout->space(); }) | std::views::adjacent_transform<2>([](UInt32 a, UInt32 b) { return std::make_tuple(a, b); }))
+        {
+            auto [a, b] = spaces;
+
+            if (a == b) [[unlikely]]
+                throw InvalidArgumentException("Two layouts defined for the same descriptor set {}. Each descriptor set must use it's own space.", a);
+        }
+
         // Define the descriptor range from descriptor set layouts.
         Array<D3D12_ROOT_PARAMETER1> descriptorParameters;
         Array<D3D12_STATIC_SAMPLER_DESC> staticSamplers;

src/Backends/Vulkan/src/descriptor_set_layout.cpp

@@ -60,6 +60,14 @@ class VulkanDescriptorSetLayout::VulkanDescriptorSetLayoutImpl : public Implemen
     VkDescriptorSetLayout initialize(UInt32 poolSize, UInt32 maxUnboundedArraySize)
     {
         LITEFX_TRACE(VULKAN_LOG, "Defining layout for descriptor set {0} {{ Stages: {1}, Pool Size: {2} }}...", m_space, m_stages, poolSize);
+
+        // Figure out the proper pool size.
+        if (m_descriptorLayouts.empty() && poolSize > 0)
+        {
+            LITEFX_WARNING(VULKAN_LOG, "The descriptor set layout does not contain any descriptors but pool size is set to `{}` and will be reset to `0`. Allocating empty descriptor sets is not valid will raise an error.", poolSize);
+            poolSize = 0;
+        }
+
         m_poolSize = poolSize;
 
         // Parse the shader stage descriptor.
@@ -167,6 +175,10 @@ class VulkanDescriptorSetLayout::VulkanDescriptorSetLayoutImpl : public Implemen
 
     void addDescriptorPool()
     {
+        // Don't add a pool for empty descriptor sets.
+        if (m_poolSize == 0) [[unlikely]]
+            return;
+
         LITEFX_TRACE(VULKAN_LOG, "Allocating descriptor pool with {5} sets {{ Uniforms: {0}, Storages: {1}, Images: {2}, Samplers: {3}, Input attachments: {4} }}...", m_poolSizes[m_poolSizeMapping[VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER]].descriptorCount, m_poolSizes[m_poolSizeMapping[VK_DESCRIPTOR_TYPE_STORAGE_BUFFER]].descriptorCount, m_poolSizes[m_poolSizeMapping[VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE]].descriptorCount, m_poolSizes[m_poolSizeMapping[VK_DESCRIPTOR_TYPE_SAMPLER]].descriptorCount, m_poolSizes[m_poolSizeMapping[VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT]].descriptorCount, m_poolSize);
 
         // Filter pool sizes, since descriptorCount must be greater than 0, according to the specs.
@@ -190,6 +202,10 @@ class VulkanDescriptorSetLayout::VulkanDescriptorSetLayoutImpl : public Implemen
 
     VkDescriptorSet tryAllocate(UInt32 descriptors)
     {
+        // If the descriptor set layout is empty, no descriptor set can be allocated.
+        if (m_poolSize == 0) [[unlikely]]
+            throw RuntimeException("Cannot allocate descriptor set from empty layout.");
+
         VkDescriptorSetVariableDescriptorCountAllocateInfo variableCountInfo;
         VkDescriptorSetAllocateInfo descriptorSetInfo = {
             .sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO,
@@ -336,13 +352,18 @@ UniquePtr<VulkanDescriptorSet> VulkanDescriptorSetLayout::allocate(UInt32 descri
     }
 
     // Apply the default bindings.
-    for (auto& binding : bindings)
+    for (UInt32 i{ 0 }; auto & binding : bindings)
+    {
         std::visit(type_switch{
-            [&descriptorSet, &binding](const ISampler& sampler) { descriptorSet->update(binding.binding, sampler, binding.firstDescriptor); },
-            [&descriptorSet, &binding](const IBuffer& buffer) { descriptorSet->update(binding.binding, buffer, binding.firstElement, binding.elements, binding.firstDescriptor); },
-            [&descriptorSet, &binding](const IImage& image) { descriptorSet->update(binding.binding, image, binding.firstDescriptor, binding.firstLevel, binding.levels, binding.firstElement, binding.elements); }
+            [](const std::monostate&) { }, // Default: don't bind anything.
+            [&descriptorSet, &binding, i](const ISampler& sampler) { descriptorSet->update(binding.binding.value_or(i), sampler, binding.firstDescriptor); },
+            [&descriptorSet, &binding, i](const IBuffer& buffer) { descriptorSet->update(binding.binding.value_or(i), buffer, binding.firstElement, binding.elements, binding.firstDescriptor); },
+            [&descriptorSet, &binding, i](const IImage& image) { descriptorSet->update(binding.binding.value_or(i), image, binding.firstDescriptor, binding.firstLevel, binding.levels, binding.firstElement, binding.elements); }
         }, binding.resource);
 
+        ++i;
+    }
+
     // Return the descriptor set.
     return descriptorSet;
 }

src/Backends/Vulkan/src/pipeline_layout.cpp

@@ -35,6 +35,30 @@ class VulkanPipelineLayout::VulkanPipelineLayoutImpl : public Implement<VulkanPi
         // Since Vulkan does not know spaces, descriptor sets are mapped to their indices based on the order they are defined. Hence we need to sort the descriptor set layouts accordingly.
         std::ranges::sort(m_descriptorSetLayouts, [](const UniquePtr<VulkanDescriptorSetLayout>& a, const UniquePtr<VulkanDescriptorSetLayout>& b) { return a->space() < b->space(); });
 
+        // Find unused and duplicate descriptor sets.
+        Array<UInt32> emptySets;
+
+        for (UInt32 i{ 0 }; Tuple spaces : m_descriptorSetLayouts | std::views::transform([](const UniquePtr<VulkanDescriptorSetLayout>& layout) { return layout->space(); }) | std::views::adjacent_transform<2>([](UInt32 a, UInt32 b) { return std::make_tuple(a, b); }))
+        {
+            auto [a, b] = spaces;
+
+            if (a == b) [[unlikely]]
+                throw InvalidArgumentException("Two layouts defined for the same descriptor set {}. Each descriptor set must use it's own space.", a);
+
+            while (i != a)
+                emptySets.push_back(i++);
+        }
+
+        // Add empty sets.
+        if (!emptySets.empty())
+        {
+            for (auto s : emptySets)
+                m_descriptorSetLayouts.push_back(UniquePtr<VulkanDescriptorSetLayout>{ new VulkanDescriptorSetLayout(m_device, { }, s, ShaderStage::Vertex | ShaderStage::Fragment, 0, 0) }); // No descriptor can ever be allocated from an empty descriptor set.
+
+            // Re-order them.
+            std::ranges::sort(m_descriptorSetLayouts, [](const UniquePtr<VulkanDescriptorSetLayout>& a, const UniquePtr<VulkanDescriptorSetLayout>& b) { return a->space() < b->space(); });
+        }
+
         // Store the pipeline layout on the push constants.
         if (m_pushConstantsLayout != nullptr)
             m_pushConstantsLayout->pipelineLayout(*this->m_parent);

src/Rendering/include/litefx/rendering_api.hpp

@@ -3327,21 +3327,26 @@ namespace LiteFX::Rendering {
     /// <seealso cref="IDescriptorSetLayout" />
     struct LITEFX_RENDERING_API DescriptorBinding {
     public:
-        using resource_container = Variant<Ref<IBuffer>, Ref<IImage>, Ref<ISampler>>;
+        using resource_container = Variant<std::monostate, Ref<IBuffer>, Ref<IImage>, Ref<ISampler>>;
 
     public:
         /// <summary>
-        /// The binding point to bind the resource at.
+        /// The binding point to bind the resource at. If not provided (i.e., `std::nullopt`), the index within the collection of `DescriptorBindings` is used.
         /// </summary>
-        UInt32 binding;
+        Optional<UInt32> binding = std::nullopt;
 
         /// <summary>
-        /// The resource to bind.
+        /// The resource to bind or `std::monostate` if no resource should be bound.
         /// </summary>
+        /// <remarks>
+        /// Note that not providing any resource does not perform any binding, in which case a resource needs to be manually bound to the descriptor set later 
+        /// (<see cref="IDescriptorSet::update" />). This is useful in situations where you frequently update the resource bound to a descriptor set or where you do no have
+        /// access to the resource at the time the descriptor set is allocated.
+        /// </remarks>
         /// <seealso cref="IBuffer" />
         /// <seealso cref="IImage" />
         /// <seealso cref="ISampler" />
-        resource_container resource;
+        resource_container resource = {};
 
         /// <summary>
         /// The index of the descriptor in a descriptor array at which binding the resource arrays starts.

src/Samples/BasicRendering/src/sample.cpp

@@ -117,7 +117,7 @@ void SampleApp::initBuffers(IRenderBackend* backend)
     auto& geometryPipeline = m_device->state().pipeline("Geometry");
     auto& cameraBindingLayout = geometryPipeline.layout()->descriptorSet(DescriptorSets::Constant);
     auto cameraBuffer = m_device->factory().createBuffer("Camera", cameraBindingLayout, 0, BufferUsage::Resource);
-    auto cameraBindings = cameraBindingLayout.allocate({ { 0, *cameraBuffer } });
+    auto cameraBindings = cameraBindingLayout.allocate({ { .resource = *cameraBuffer } });
 
     // Update the camera. Since the descriptor set already points to the proper buffer, all changes are implicitly visible.
     this->updateCamera(*commandBuffer, *cameraBuffer);
@@ -127,9 +127,9 @@ void SampleApp::initBuffers(IRenderBackend* backend)
     auto& transformBindingLayout = geometryPipeline.layout()->descriptorSet(DescriptorSets::PerFrame);
     auto transformBuffer = m_device->factory().createBuffer("Transform", transformBindingLayout, 0, BufferUsage::Dynamic, 3);
     auto transformBindings = transformBindingLayout.allocateMultiple(3, {
-        { { .binding = 0, .resource = *transformBuffer, .firstElement = 0, .elements = 1 } },
-        { { .binding = 0, .resource = *transformBuffer, .firstElement = 1, .elements = 1 } },
-        { { .binding = 0, .resource = *transformBuffer, .firstElement = 2, .elements = 1 } }
+        { { .resource = *transformBuffer, .firstElement = 0, .elements = 1 } },
+        { { .resource = *transformBuffer, .firstElement = 1, .elements = 1 } },
+        { { .resource = *transformBuffer, .firstElement = 2, .elements = 1 } }
     });
 
     // End and submit the command buffer.