0

I have been changing some vulkan code to use the vulkan.hpp structures and methods.

Since I like RAIIs, I am using the Unique wrappers, to not have to explicitely handle resource management.

So far I have been able to create 2 versions of each wrapping method I am making, one version creates a non unique object, and another method calls the first and then initializes a unique handle from the first. Example:

vector<vk::UniqueFramebuffer> CreateFramebuffersUnique(vk::SurfaceKHR surface,
    vk::PhysicalDevice phys_device, vk::Device device, vk::RenderPass render_pass,
    vector<vk::ImageView> image_views)
{
    auto framebuffers =
        CreateFramebuffers(surface, phys_device, device, render_pass, image_views);
    vector<vk::UniqueFramebuffer> u_framebuffers;
    for(auto &fb : framebuffers)
        u_framebuffers.push_back(vk::UniqueFramebuffer(fb, device));

    return u_framebuffers;
}

The above method creates an array of framebuffers and then re-initializes each framebuffer as a unique framebuffer prior of returning it.

I tried doing the same with command buffers:

vector<vk::UniqueCommandBuffer> CreateCommandBuffersUnique(vk::SurfaceKHR &surface,
    vk::PhysicalDevice &phys_device, vk::Device &device, vk::RenderPass &render_pass,
    vk::Pipeline &graphics_pipeline, vk::CommandPool &command_pool,
    vector<vk::Framebuffer> &framebuffers)
{
    auto command_buffers = CreateCommandBuffers(surface, phys_device, device, render_pass,
        graphics_pipeline, command_pool, framebuffers);

    vector<vk::UniqueCommandBuffer> u_command_buffers;
    for(auto &cb : command_buffers)
        u_command_buffers.push_back(vk::UniqueCommandBuffer(cb, device));

    return u_command_buffers;
}

The above technically works, but upon program termination, the validation layers complain about a mistake upon resorce de allocation:

validation layer: vkFreeCommandBuffers: required parameter commandPool specified as VK_NULL_HANDLE
UNASSIGNED-GeneralParameterError-RequiredParameter
4096
validation layer: Invalid CommandPool Object 0x0. The Vulkan spec states: commandPool must be a valid VkCommandPool handle (https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#VUID-vkFreeCommandBuffers-commandPool-parameter)
VUID-vkFreeCommandBuffers-commandPool-parameter

This happens because the command pool field of the unique handle is not set properly:

(gdb) p t_command_buffers[0]
$6 = {<vk::PoolFree<vk::Device, vk::CommandPool, vk::DispatchLoaderStatic>> = {m_owner = {m_device = 0x555555ec14e0}, m_pool = {m_commandPool = 0x0},
    m_dispatch = 0x7fffffffe2f7}, m_value = {m_commandBuffer = 0x555555fe6390}}

I checked, and although there is a commandBuffer.getPool(), there is no setPool().

Any suggestions on properly setting the field?

Makogan
  • 8,208
  • 7
  • 44
  • 112

2 Answers2

1

You're not including the source or the signature of the CreateCommandBuffers function you're calling internally from CreateCommandBuffersUnique. However, lets assume it returns a type of std::vector<vk::CommandBuffer>

It looks like you're then looping over those and wrapping them manually in vk::UniqueCommandBuffer instances. However, you're passing the cb (assumed here to be a vk::CommandBuffer and a vk::Device to the UniqueCommandBuffer constructor here

u_command_buffers.push_back(vk::UniqueCommandBuffer(cb, device));

I'm actually not clear on how that even compiles, unless vk::Device has an operator ()(). Regardless, the second parameter to a vk::UniqueCommandBuffer should be a deleter object.

So it turns out that all of the UniqueXXX types derive from UniqueHandle<>, which in turn derives from UniqueHandleTraits<...> which is specialized for each type, in order to give it a default deleter type. Most of the UniqueHandleTraits<...> specializations use an ObjectDestroy<...> template because most objects only need their creating Device to destroy them. However, the UniqueHandleTraits<CommandBuffer> and UniqueHandleTraits<DescriptorSet> specializations use PoolFree<...> for their deleters.

Therefore, your use of

vk::UniqueCommandBuffer(cb, device));

implicitly turns into

vk::UniqueCommandBuffer(cb, 
    vk::PoolFree<Device, CommandPool,Dispatch> { device, {}, {} }
)

That {} after device is where the pool is supposed to be specified.

See the corresponding code from vk::Device::allocateCommandBuffersUnique

PoolFree<Device,CommandPool,Dispatch> deleter( *this, allocateInfo.commandPool, d );
for ( size_t i=0 ; i<allocateInfo.commandBufferCount ; i++ )
{
  commandBuffers.push_back( UniqueCommandBuffer( buffer[i], deleter ) );
}

To fix your code you either need to use vk::Device::allocateCommandBuffersUnique or replicate it's behavior, specifically create a deleter object or lambda and pass it as the second parameter to the UniqueCommandBuffer ctor.

Based on my examination of the UniqueHandleTraits, it looks like you'd be able to fix your code just by changing this line:

    u_framebuffers.push_back(vk::UniqueFramebuffer(fb, device));

to

    u_framebuffers.push_back(vk::UniqueFramebuffer(fb, { device, command_pool }));
Jherico
  • 28,584
  • 8
  • 61
  • 87
  • The above syntax not only compiles but works for almost all objects (I used it for swap chains, devices, framebuffers, commandpool... the only object it didn;t work on was CommandBuffers), I will append a temptative solution that fixes the problem, at least for now. But you are correct about the types of the return values of each function – Makogan Apr 28 '19 at 01:32
  • The other types you mention aren't allocated from a pool. In fact if you look at vulkan.hpp and search for `UniqueHandleTraits` you'll see that most of vulkan types have specialized deleters based on `ObjectDestroy` but pool allocated resources like `CommandBuffer` and `DescriptorSet` use a deleter specializations based on `PoolFree`. It looks like the reason your code even compiles is that `PoolFree` has default arguments for everyhing in it's ctor, so your code essentially calls `PoolFree instance(device, {}, {})` – Jherico Apr 28 '19 at 02:46
0

Upon playing around I have found a solution that solves the issue, although I would prefer doing things more closely to the example I provided with framebuffers:

vector<vk::UniqueCommandBuffer> CreateCommandBuffersUnique(vk::SurfaceKHR &surface,
    vk::PhysicalDevice &phys_device, vk::Device &device, vk::RenderPass &render_pass,
    vk::Pipeline &graphics_pipeline, vk::CommandPool &command_pool,
    vector<vk::Framebuffer> &framebuffers)
{
    vk::CommandBufferAllocateInfo alloc_info(command_pool,
        vk::CommandBufferLevel::ePrimary, framebuffers.size());
    auto[result, u_command_buffers] = device.allocateCommandBuffersUnique(alloc_info);
    if (result != vk::Result::eSuccess)
        Log::RecordLogError("Failed to allocate command buffers!");

    auto command_buffers = CreateCommandBuffers(surface, phys_device, device, render_pass,
        graphics_pipeline, command_pool, framebuffers);

    for(auto &cb : command_buffers)
    {
        u_command_buffers[&cb - &command_buffers[0]].reset(cb);
    }

    return std::move(u_command_buffers);
}
Makogan
  • 8,208
  • 7
  • 44
  • 112
  • 1
    Don't use `std::move` here. see https://stackoverflow.com/questions/4986673/c11-rvalues-and-move-semantics-confusion-return-statement/4986802#4986802 – Jherico Apr 28 '19 at 03:00
  • 1
    How can I prevent the constructor from being called in that case? i.e how do I extend the life of the unique handle? – Makogan Apr 28 '19 at 05:15