UniqueHandle Constructor should be explicit

[spfuetzner]

... in order to prevent premature or multiple destruction. For example:

void f(vk::UniqueDevice const& dev) {}

void g(vk::Device dev)
{
    f(dev); // oops
}

[ioquatix]

I hit this bug too when migrating legacy code. In the end I decided not to use the Unique variants.

I would actually argue that the UniqueFoo resource management is beyond the scope of Vulkan-Hpp - what was the motivation for it's inclusion? It does add quite a bit of overhead actually and that goes against the design of this header IMHO.

[spfuetzner]

Imho, the RAII-less vk::Handles are not idiomatic for C++ and very alien to use. Most C++ programmers (really embracing the language and not using it as C with classes) would expect wrapper classes like Unique*. But they have to be implemented with care. Look at std::unique_ptr for example, it has an explicit constructor too for the same reason.

[ioquatix]

@spfuetzner I agree with you, but in terms of overhead it's quite large.

What would be wrong with using std::unique_ptr rather than the vk::Unique snowflake?

[ioquatix]

With the current system, unless you nest your dependencies, it's not clear the order that they would be destroyed. For example, creating several vk::Unique instances, e.g. vk::UniqueDevice and vk::UniqueQueryPool, how is the order of destruction guaranteed?

[spfuetzner]

@ioquatix std::unique_ptr would be hard to implement without extra allocation per object (very bad). It's template parameter has to be the dereferenced-pointer-type. In 32-bit this would not be possible because Vk* are integers, not pointers.

Your second question: UniqHandles are normal RAII objects and behave like others, e.g. std::vector, std::complex etc, except that you cannot make copies (like std::unique_ptr). Destruction is in the reverse order of construction, exception throwing in the middle is possible and all the other good stuff. Unique handles are not reference counted and cannot be shared, but that is not needed because you can move them (and so store them in std::vector for example). Nested dependencies are not needed if you have no sharing. You push these handles down the call stack via (const) references or pointers to them. And pushing unique handles up the call stack can be done with moving.

[ioquatix]

Destruction is in the reverse order of construction

I did not know this was guaranteed by C++?

[spfuetzner]

I did not know this was guaranteed by C++?

This is very basic stuff. Maybe you mean something different, then you should show some example code.

[ioquatix]

struct Renderer {
    vk::UniqueDevice _device;
    vk::UniqueOtherStuff _other_stuff;

    Renderer() {
        _device = ...;
    }
    
    void render_something() {
        _other_stuff = ...;
    }
 
    ~Renderer() {
        // What is order of destruction? How guarantee _device destroyed last?
    }
}

[ioquatix]

Ah, found this: http://stackoverflow.com/questions/7241385/why-the-destructor-in-c-de-allocated-memory-in-reverse-order-of-how-they-were

But yeah, I'm still not convinced that it would work in the example I gave. I'll have to try it out.

[spfuetzner]

_other_stuff is destroyed first because it is declared last. This is guaranteed by the language rules. This is also the order of initialization, no matter in what order the members are in the initialization list. Also resetting the UniqueHandle or calling operator= makes no difference.

For a minimal example using vkhpp and UniqueHandles, see my small project here:
https://github.com/spfuetzner/vkTest

[mtavenrath]

@ioquatix

I would actually argue that the UniqueFoo resource management is beyond the scope of Vulkan-Hpp
We had requests to add this feature and also found that multiple C++ Vulkan libraries implement a unqiue handle and the corresponding deleters. After a long discussion we decided that it can be useful feature for certain use cases and it's fully optional and thus doesn't affect runtime performance if you don't use it.

It does add quite a bit of overhead actually and that goes against the design of this header IMHO.
This was the main reason why we've been quite reluctant adding UniqueHandle..

@spfuetzner

In 32-bit this would not be possible because Vk* are integers, not pointers.
vulkan.h allows to override VK_DEFINE_NON_DISPATCHABLE_HANDLE. In theory the following definition should be ABI compatible:
#define VK_DEFINE_NON_DISPATCHABLE_HANDLE(object) typedef struct {uint64_t handle} object;
I haven't checked yet if this define would cause any trouble.

Before implementing UniqueHandle we tried to utilize std::unique_ptr. Unfortunately one compiler produced errors when using the pointer typedef in the Deleter in combination with operator-> in the handle and thus we had to 'invent' the wheel again. Long term we should get rid of our own class and use std::unique_ptr instead.