Proposal: vk::{CommandBuffer,DescriptorSet,Queue}WithDispatcher (non-owning handles that store the inherited device dispatcher)

[stripe2933]

Overview

Related: #929

Using RAII handle is very handy, as it manages the resource lifetime automatically and owns the inherited dispatcher from instance/device.

However, some Vulkan objects are not following the RAII nature: command buffer, descriptor set and queue for instance. Their lifetimes are tied to command pool, descriptor pool and device, respectively. Using the RAII version of them makes the code less performant (calling vkFreeCommandBuffer() and vkFreeDescriptorSet() for individual command buffer/descriptor set), needs specific creation flag (VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT for vk::raii::DescriptorSet), or completely non-sense (queue).

However, using non-RAII version of them will make losing the benefits of automatic inherited dispatcher. User need to pass the proper dispatcher to each call manually, which is cumbersome and likely to be missed.

vk::raii::Device device { ... };
vk::raii::CommandPool commandPool { device, ... };

vk::CommandBuffer cb = (*device).allocateCommandBuffers(..., *device.getDispatcher());
cb.begin(..., *device.getDispatcher());
cb.bindPipeline(..., *device.getDispatcher());
cb.draw(..., *device.getDispatcher());
cb.end(*device.getDispatcher());

vk::Queue queue = (*device).getQueue(..., *device.getDispatcher());
queue.submit(..., *device.getDispatcher());
queue.waitIdle(*device.getDispatcher());

Using RAII handle can avoid passing the manual dispatcher parameter, but need to call release() before destroying the handle, which violates the RAII idiom.

vk::raii::Device device { ... };
vk::raii::CommandPool commandPool { device, ... };

vk::raii::CommandBuffer cb = device.allocateCommandBuffers(...)[0];
cb.begin(...);
cb.bindPipeline(...);
cb.draw(...);
cb.end();

vk::raii::Queue queue = device.getQueue(...);
queue.submit(...);
queue.waitIdle();

// queue.release(); // optional, as vk::Queue::~Queue() does nothing
cb.release(); // must be called before cb being destroyed, to avoid calling vkFreeCommandBuffer()

Proposal

1. Add vk::{CommandBuffer,DescriptorSet,Queue}WithDispatcher classes, non-owning handles and store the inherited dispatcher from the device.

namespace VULKAN_HPP_NAMESPACE {
    class CommandBufferWithDispatcher {
    public:
        using CType   = VkCommandBuffer;
        using CppType = VULKAN_HPP_NAMESPACE::CommandBuffer;
    
        static VULKAN_HPP_CONST_OR_CONSTEXPR ObjectType               objectType            = ObjectType::eCommandBuffer;
        static VULKAN_HPP_CONST_OR_CONSTEXPR DebugReportObjectTypeEXT debugReportObjectType = DebugReportObjectTypeEXT::eCommandBuffer;

        CommandBufferWithDispatcher(CommandBuffer commandBuffer, detail::DeviceDispatcher const & d)
            : m_commandBuffer { commandBuffer }
            , m_dispatcher { d } { }
            
        // copy/move constructors/assignment operators, swap, ...
        
        //=== VK_VERSION_1_0 ===

        // wrapper function for command vkBeginCommandBuffer, see https://registry.khronos.org/vulkan/specs/latest/man/html/vkBeginCommandBuffer.html
        typename ResultValueType<void>::type begin( CommandBufferBeginInfo const & beginInfo ) const;

        // wrapper function for command vkEndCommandBuffer, see https://registry.khronos.org/vulkan/specs/latest/man/html/vkEndCommandBuffer.html
        typename ResultValueType<void>::type end() const;

        // wrapper function for command vkResetCommandBuffer, see https://registry.khronos.org/vulkan/specs/latest/man/html/vkResetCommandBuffer.html
        typename ResultValueType<void>::type reset( CommandBufferResetFlags flags VULKAN_HPP_DEFAULT_ASSIGNMENT( {} ) ) const;

        // ...

    private:
        CommandBuffer m_commandBuffer;
        detail::DeviceDispatcher m_dispatcher;
    };
    
    class DescriptorSetWithDispatcher { ... };
    class QueueWithDispatcher { ... };
    
namespace VULKAN_HPP_RAII_NAMESPACE {
    class Device { 
    public:
        // ...
        
        VULKAN_HPP_NODISCARD typename ResultValueType<std::vector<CommandBufferWithDispatcher>>::type allocateCommandBuffersWithDispatcher( CommandBufferAllocateInfo const & allocateInfo ) const {
            VULKAN_HPP_NAMESPACE::detail::resultCheck( result, VULKAN_HPP_RAII_NAMESPACE_STRING "::Device::allocateCommandBuffers" );
            std::vector<CommandBufferWithDispatcher> commandBuffersWithDispatcher;
            if ( result == Result::eSuccess )
            {
              commandBuffersWithDispatcher.reserve( commandBuffers.size() );
              for ( auto & commandBuffer : commandBuffers )
              {
                commandBuffersWithDispatcher.emplace_back(commandBuffer, getDispatcher());
              }
            }
            return VULKAN_HPP_NAMESPACE::detail::createResultValueType( result, std::move( commandBuffersWithDispatcher ) );
        }
        
        VULKAN_HPP_NODISCARD typename ResultValueType<std::vector<DescriptorSetWithDispatcher>>::type allocateDescriptorSetsWithDispatcher( DescriptorSetAllocateInfo const & allocateInfo ) const { ... }
        
        VULKAN_HPP_NODISCARD QueueWithDispatcher getQueueWithDispatcher( uint32_t queueFamilyIndex, uint32_t queueIndex ) const VULKAN_HPP_NOEXCEPT_WHEN_NO_EXCEPTIONS { ... };
        VULKAN_HPP_NODISCARD QueueWithDispatcher getQueueWithDispatcher2( DeviceQueueInfo2 const & queueInfo ) const VULKAN_HPP_NOEXCEPT_WHEN_NO_EXCEPTIONS { ... };
    
    private:
        // ...
    };
}
}

vk::raii::Device device { ... };
vk::raii::CommandPool commandPool { device, ... };

vk::CommandBufferWithDispatcher cb = device.allocateCommandBuffersWithDispatcher(...)[0];
cb.begin();
cb.bindPipeline(...);
cb.draw(...);
cb.end();

vk::QueueWithDispatcher queue = device.getQueueWithDispatcher(...);
queue.submit(...);
queue.waitIdle();

These classes are 16-byte (non-RAII handle and inherited device dispatcher), which are leaner than the current RAII handles (32-byte for vk::raii::CommandBuffer and vk::raii::DescriptorSet). They can be constructed from the original handle and dispatcher instance, or obtained by vk::raii::Device methods. In the case, vk::detail::DeviceDispatcher will be used for template parameter.

The new class definitions should be in vulkan_raii.hpp as they are supposed to be used with RAII handles.

2. Mark vk::raii::Queue as deprecated in favor of vk::QueueWithDispatcher

Basically, the new vk::QueueWithDispatcher is equivalent to the current vk::raii::Queue. As the current one neither create resource in the constructor nor destroying resource in the destructor, it should be renamed to non-RAII version.

[asuessenbach]

Thanks a lot for this interesting idea on how to resolve this CommandBuffer/DescriptorSet/Queue issue.

But instead of introducing another set of classes (even though that's just two) I could imagine to add some special handling in the creation process of CommandPool/CommandBuffers:

• add an optional parameter bool freeIndividually = true, or so, to the vk::raii::CommandBuffers constructor and vk::raii::Device::allocateCommandBuffers
• only if that parameter is true, the commandPool is passed into the vk::raii::CommandBuffer constructor
• vk::raii::CommandBuffer::clear (and thus, the vk::raii::CommandBuffer destructor) checks if m_commandPool is not null before freeing that command buffer.

For the DescriptorPool/DescriptorSet, it would be similar, but slightly more complex:

• store if the eFreeDescriptorSet bit is set in the DescriptorPoolCreateFlags in the vk::raii::DescriptorPool
• add an optional parameter bool freeIndividually = true, or so, to the vk::raii::DescriptorSets constructor and vk::raii::Device::allocateDescriptorSets
• only if both, eFreeDescriptorSet and freeIndividually are true, the descriptorPool is passed into the vk::raii::DescriptorPool constructor
• vk::raii::DescriptorPool::clear (and thus, the vk::raii::DescriptorSet destructor) checks if m_descriptorPool is not null before freeing that descriptor set

For the Queue, there's nothing to do, as it already doesn't do anything (vk-related) in its destructor.

What do you think about this approach?

[stripe2933]

Personally, I would prefer these three Vulkan handles not to be placed inside the raii:: namespace. In Vulkan, resource allocation and deallocation are explicit, and placing them under raii:: implies that their destructors automatically release the resources.

However, since the Vulkan-Hpp headers are already quite large, if keeping the header size minimal is a priority, then following your approach and reusing the existing classes is also reasonable.

If we go with your approach of using the existing classes, then for vk::raii::DescriptorSet, it should be sufficient to check only freeIndividually == true. Setting the eFreeDescriptorSet flag is the responsibility of the class user, and any violations will be caught by the validation layers.