Vk: make the Instance immovable as well.

doc/snippets/MagnumVk.cpp

@@ -66,6 +66,26 @@ using namespace Magnum;
 
 #define DOXYGEN_IGNORE(...) __VA_ARGS__
 
+/* [Instance-delayed-creation] */
+class MyApplication {
+    public:
+        explicit MyApplication();
+
+    private:
+        Vk::Instance _instance{NoCreate};
+};
+
+MyApplication::MyApplication() {
+    // decide on layers, extensions, ...
+
+    _instance.create(Vk::InstanceCreateInfo{DOXYGEN_IGNORE()}
+        DOXYGEN_IGNORE()
+    );
+}
+/* [Instance-delayed-creation] */
+
+namespace B {
+
 /* [Device-delayed-creation] */
 class MyApplication {
     public:
@@ -84,6 +104,8 @@ MyApplication::MyApplication(DOXYGEN_IGNORE(Vk::Instance& instance)) {
 }
 /* [Device-delayed-creation] */
 
+}
+
 int main() {
 
 {

src/Magnum/Vk/CMakeLists.txt

@@ -33,7 +33,6 @@ set(MagnumVk_SRCS
     Fence.cpp
     Framebuffer.cpp
     Handle.cpp
-    Instance.cpp
     Queue.cpp
     Result.cpp
     Shader.cpp
@@ -52,6 +51,7 @@ set(MagnumVk_GracefulAssert_SRCS
     ExtensionProperties.cpp
     Image.cpp
     ImageView.cpp
+    Instance.cpp
     LayerProperties.cpp
     Memory.cpp
     RenderPass.cpp)

src/Magnum/Vk/Device.h

@@ -175,7 +175,8 @@ moving a new instance over the empty one. Here you have to use the
 @snippet MagnumVk.cpp Device-delayed-creation
 
 Similar case is with @ref wrap() --- instead of being @cpp static @ce, you have
-to call it on a @ref Device(NoCreateT) "NoCreate"'d instance.
+to call it on a @ref Device(NoCreateT) "NoCreate"'d instance. The @ref Instance
+class behaves equivalently.
 */
 class MAGNUM_VK_EXPORT Device {
     public:

src/Magnum/Vk/Instance.cpp

@@ -37,6 +37,7 @@
 #include "Magnum/Vk/Extensions.h"
 #include "Magnum/Vk/ExtensionProperties.h"
 #include "Magnum/Vk/Handle.h"
+#include "Magnum/Vk/Result.h"
 #include "Magnum/Vk/Version.h"
 #include "Magnum/Vk/Implementation/Arguments.h"
 #include "Magnum/Vk/Implementation/InstanceState.h"
@@ -258,22 +259,51 @@ InstanceCreateInfo& InstanceCreateInfo::addEnabledExtensions(const std::initiali
     return addEnabledExtensions(Containers::arrayView(extensions));
 }
 
-Instance Instance::wrap(const VkInstance handle, const Version version, const Containers::ArrayView<const Containers::StringView> enabledExtensions, const HandleFlags flags) {
+void Instance::wrap(const VkInstance handle, const Version version, const Containers::ArrayView<const Containers::StringView> enabledExtensions, const HandleFlags flags) {
+    CORRADE_ASSERT(!_handle,
+        "Vk::Instance::wrap(): instance already created", );
+
     /* Compared to the constructor nothing is printed here as it would be just
        repeating what was passed to the constructor */
-    Instance out{NoCreate};
-    out._handle = handle;
-    out._flags = flags;
-    out.initializeExtensions(enabledExtensions);
-    out.initialize(version, 0, nullptr);
-    return out;
+    _handle = handle;
+    _flags = flags;
+    initializeExtensions(enabledExtensions);
+    initialize(version, 0, nullptr);
+}
+
+void Instance::wrap(const VkInstance handle, const Version version, const std::initializer_list<Containers::StringView> enabledExtensions, const HandleFlags flags) {
+    wrap(handle, version, Containers::arrayView(enabledExtensions), flags);
+}
+
+Instance::Instance(const InstanceCreateInfo& info): Instance{NoCreate} {
+    create(info);
+}
+
+Instance::Instance(): Instance{NoCreate} {
+    create();
+}
+
+Instance::Instance(NoCreateT): _handle{}, _functionPointers{} {}
+
+Instance::~Instance() {
+    if(_handle && (_flags & HandleFlag::DestroyOnDestruction))
+        _functionPointers.DestroyInstance(_handle, nullptr);
+}
+
+void Instance::create(const InstanceCreateInfo& info) {
+    if(tryCreate(info) != Result::Success) std::exit(1);
 }
 
-Instance Instance::wrap(const VkInstance handle, const Version version, const std::initializer_list<Containers::StringView> enabledExtensions, const HandleFlags flags) {
-    return wrap(handle, version, Containers::arrayView(enabledExtensions), flags);
+void Instance::create() {
+    if(tryCreate() != Result::Success) std::exit(1);
 }
 
-Instance::Instance(const InstanceCreateInfo& info): _flags{HandleFlag::DestroyOnDestruction} {
+Result Instance::tryCreate(const InstanceCreateInfo& info) {
+    CORRADE_ASSERT(!_handle,
+        "Vk::Instance::tryCreate(): instance already created", {});
+
+    _flags = HandleFlag::DestroyOnDestruction;
+
     const Version version = info._state && info._state->version != Version::None ? info._state->version : enumerateInstanceVersion();
 
     /* Print all enabled layers and extensions if we're not told to be quiet */
@@ -293,44 +323,22 @@ Instance::Instance(const InstanceCreateInfo& info): _flags{HandleFlag::DestroyOn
         }
     }
 
-    MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(vkCreateInstance(info, nullptr, &_handle));
+    if(const VkResult result = vkCreateInstance(info, nullptr, &_handle)) {
+        Error{} << "Vk::Instance::tryCreate(): instance creation failed:" << Result(result);
+        return Result(result);
+    }
 
     initializeExtensions<const char*>({info->ppEnabledExtensionNames, info->enabledExtensionCount});
     if(info._state)
         initialize(version, info._state->argc, info._state->argv);
     else
         initialize(version, 0, nullptr);
-}
-
-Instance::Instance(): Instance{InstanceCreateInfo{}} {}
-
-Instance::Instance(NoCreateT): _handle{}, _functionPointers{} {}
 
-Instance::Instance(Instance&& other) noexcept: _handle{other._handle},
-    _flags{other._flags}, _version{other._version},
-    _extensionStatus{other._extensionStatus}, _state{std::move(other._state)},
-    /* Can't use {} with GCC 4.8 here because it tries to initialize the first
-       member instead of doing a copy */
-    _functionPointers(other._functionPointers)
-{
-    other._handle = nullptr;
-    other._functionPointers = {};
-}
-
-Instance::~Instance() {
-    if(_handle && (_flags & HandleFlag::DestroyOnDestruction))
-        _functionPointers.DestroyInstance(_handle, nullptr);
+    return Result::Success;
 }
 
-Instance& Instance::operator=(Instance&& other) noexcept {
-    using std::swap;
-    swap(other._handle, _handle);
-    swap(other._flags, _flags);
-    swap(other._version, _version);
-    swap(other._extensionStatus, _extensionStatus);
-    swap(other._state, _state);
-    swap(other._functionPointers, _functionPointers);
-    return *this;
+Result Instance::tryCreate() {
+    return tryCreate(InstanceCreateInfo{});
 }
 
 template<class T> void Instance::initializeExtensions(const Containers::ArrayView<const T> enabledExtensions) {

src/Magnum/Vk/Instance.h

@@ -171,6 +171,21 @@ Compared to the above, the same custom code would then look like this:
 
 Similarly you can use @ref Device::populateGlobalFunctionPointers() to populate
 device-level global function pointers.
+
+@section Vk-Instance-disabled-move Disabled move and delayed instances creation
+
+Similarly to @ref Device, for safety reasons as all instance-dependent objects
+internally have to keep a pointer to the originating @ref Instance to access
+Vulkan function pointers, the @ref Instance class is not movable. This leads to
+a difference compared to other Vulkan object wrappers, where you can use the
+@ref NoCreate tag to construct an empty instance (for example as a class
+member) and do a delayed creation by moving a new instance over the empty one.
+Here you have to use the @ref create() function instead:
+
+@snippet MagnumVk.cpp Instance-delayed-creation
+
+Similar case is with @ref wrap() --- instead of being @cpp static @ce, you have
+to call it on a @ref Instance(NoCreateT) "NoCreate"'d instance.
 */
 class MAGNUM_VK_EXPORT Instance {
     public:
@@ -183,6 +198,17 @@ class MAGNUM_VK_EXPORT Instance {
          *      on the instance
          * @param flags         Handle flags
          *
+         * <b></b>
+         *
+         * @m_class{m-note m-warning}
+         *
+         * @par
+         *      Unlike with other Vulkan object wrappers, this isn't a
+         *      @cpp static @ce function returning a new @ref Instance, instead
+         *      it's expected to be called on a @ref NoCreate "NoCreate"'d
+         *      instance. See @ref Vk-Instance-disabled-move for more
+         *      information.
+         *
          * The @p handle is expected to be of an existing Vulkan instance. The
          * @p version and @p enabledExtensions parameters populate internal
          * info about supported version and extensions and will be reflected in
@@ -200,15 +226,16 @@ class MAGNUM_VK_EXPORT Instance {
          * behavior.
          * @see @ref release()
          */
-        static Instance wrap(VkInstance handle, Version version, Containers::ArrayView<const Containers::StringView> enabledExtensions, HandleFlags flags = {});
+        void wrap(VkInstance handle, Version version, Containers::ArrayView<const Containers::StringView> enabledExtensions, HandleFlags flags = {});
 
         /** @overload */
-        static Instance wrap(VkInstance handle, Version version, std::initializer_list<Containers::StringView> enabledExtensions, HandleFlags flags = {});
+        void wrap(VkInstance handle, Version version, std::initializer_list<Containers::StringView> enabledExtensions, HandleFlags flags = {});
 
         /**
          * @brief Constructor
          *
-         * @see @fn_vk_keyword{CreateInstance}
+         * Equivalent to calling @ref Instance(NoCreateT) followed by
+         * @ref create(const InstanceCreateInfo&).
          */
         #ifdef DOXYGEN_GENERATING_OUTPUT
         explicit Instance(const InstanceCreateInfo& info = InstanceCreateInfo{});
@@ -230,8 +257,12 @@ class MAGNUM_VK_EXPORT Instance {
         /** @brief Copying is not allowed */
         Instance(const Instance&) = delete;
 
-        /** @brief Move constructor */
-        Instance(Instance&& other) noexcept;
+        /**
+         * @brief Moving is not allowed
+         *
+         * See @ref Vk-Instance-disabled-move for more information.
+         */
+        Instance(Instance&&) = delete;
 
         /**
          * @brief Destructor
@@ -246,8 +277,12 @@ class MAGNUM_VK_EXPORT Instance {
         /** @brief Copying is not allowed */
         Instance& operator=(const Instance&) = delete;
 
-        /** @brief Move assignment */
-        Instance& operator=(Instance&& other) noexcept;
+        /**
+         * @brief Moving is not allowed
+         *
+         * See @ref Vk-Instance-disabled-move for more information.
+         */
+        Instance& operator=(Instance&&) = delete;
 
         /** @brief Underlying @type_vk{Instance} handle */
         VkInstance handle() { return _handle; }
@@ -257,6 +292,44 @@ class MAGNUM_VK_EXPORT Instance {
         /** @brief Handle flags */
         HandleFlags handleFlags() const { return _flags; }
 
+        /**
+         * @brief Create an instance
+         * @param info      Instance creation info
+         *
+         * Meant to be called on a @ref Instance(NoCreateT) "NoCreate"'d
+         * instance. After creating the instance, populates instance-level
+         * function pointers and runtime information about enabled extensions
+         * based on @p info.
+         *
+         * If instance creation fails, a message is printed to error output and
+         * the application exits --- if you need a different behavior, use
+         * @ref tryCreate() instead.
+         * @see @ref Instance(const InstanceCreateInfo&),
+         *      @see @fn_vk_keyword{CreateInstance}
+         */
+        #ifdef DOXYGEN_GENERATING_OUTPUT
+        void create(const InstanceCreateInfo& info = InstanceCreateInfo{});
+        #else
+        /* To avoid dependency on InstanceCreateInfo.h */
+        void create(const InstanceCreateInfo& info);
+        void create();
+        #endif
+
+        /**
+         * @brief Try to create an instance
+         *
+         * Unlike @ref create(), instead of exiting on error, prints a message
+         * to error output and returns a corresponding result value. On success
+         * returns @ref Result::Success.
+         */
+        #ifdef DOXYGEN_GENERATING_OUTPUT
+        Result tryCreate(const InstanceCreateInfo& info = InstanceCreateInfo{});
+        #else
+        /* To avoid dependency on InstanceCreateInfo.h */
+        Result tryCreate(const InstanceCreateInfo& info);
+        Result tryCreate();
+        #endif
+
         /**
          * @brief Version supported by the instance
          *

src/Magnum/Vk/Test/CMakeLists.txt

@@ -163,7 +163,7 @@ if(BUILD_VK_TESTS)
     corrade_add_test(VkLayerPropertiesVkTest LayerPropertiesVkTest.cpp LIBRARIES MagnumVkTestLib)
     corrade_add_test(VkImageVkTest ImageVkTest.cpp LIBRARIES MagnumVk MagnumVulkanTester)
     corrade_add_test(VkImageViewVkTest ImageViewVkTest.cpp LIBRARIES MagnumVk MagnumVulkanTester)
-    corrade_add_test(VkInstanceVkTest InstanceVkTest.cpp LIBRARIES MagnumVk)
+    corrade_add_test(VkInstanceVkTest InstanceVkTest.cpp LIBRARIES MagnumVkTestLib)
     corrade_add_test(VkMemoryVkTest MemoryVkTest.cpp LIBRARIES MagnumVk MagnumVulkanTester)
     corrade_add_test(VkRenderPassVkTest RenderPassVkTest.cpp LIBRARIES MagnumVkTestLib MagnumVulkanTester)
     corrade_add_test(VkShaderVkTest ShaderVkTest.cpp LIBRARIES MagnumVk MagnumVulkanTester)

src/Magnum/Vk/Test/InstanceTest.cpp

@@ -38,6 +38,7 @@ struct InstanceTest: TestSuite::Tester {
 
     void constructNoCreate();
     void constructCopy();
+    void constructMove();
 };
 
 InstanceTest::InstanceTest() {
@@ -45,7 +46,8 @@ InstanceTest::InstanceTest() {
               &InstanceTest::createInfoConstructFromVk,
 
               &InstanceTest::constructNoCreate,
-              &InstanceTest::constructCopy});
+              &InstanceTest::constructCopy,
+              &InstanceTest::constructMove});
 }
 
 void InstanceTest::createInfoConstructNoInit() {
@@ -89,6 +91,11 @@ void InstanceTest::constructCopy() {
     CORRADE_VERIFY(!std::is_copy_assignable<Instance>{});
 }
 
+void InstanceTest::constructMove() {
+    CORRADE_VERIFY(!std::is_move_constructible<Instance>{});
+    CORRADE_VERIFY(!std::is_move_assignable<Instance>{});
+}
+
 }}}}
 
 CORRADE_TEST_MAIN(Magnum::Vk::Test::InstanceTest)

src/Magnum/Vk/Test/InstanceVkTest.cpp

@@ -56,10 +56,14 @@ struct InstanceVkTest: TestSuite::Tester {
     void constructLayerExtension();
     void constructCommandLineDisable();
     void constructCommandLineEnable();
-    void constructMove();
-    void constructUnknownLayer();
-    void constructUnknownExtension();
+
+    void tryCreateAlreadyCreated();
+    void tryCreateUnknownLayer();
+    void tryCreateUnknownExtension();
+
     void wrap();
+    void wrapAlreadyCreated();
+
     void populateGlobalFunctionPointers();
 };
 
@@ -161,10 +165,13 @@ InstanceVkTest::InstanceVkTest() {
                        &InstanceVkTest::constructCommandLineEnable},
         Containers::arraySize(ConstructCommandLineData));
 
-    addTests({&InstanceVkTest::constructMove,
-              &InstanceVkTest::constructUnknownLayer,
-              &InstanceVkTest::constructUnknownExtension,
+    addTests({&InstanceVkTest::tryCreateAlreadyCreated,
+              &InstanceVkTest::tryCreateUnknownLayer,
+              &InstanceVkTest::tryCreateUnknownExtension,
+
               &InstanceVkTest::wrap,
+              &InstanceVkTest::wrapAlreadyCreated,
+
               &InstanceVkTest::populateGlobalFunctionPointers});
 }
 
@@ -430,66 +437,38 @@ void InstanceVkTest::constructCommandLineEnable() {
     CORRADE_COMPARE(!!instance->CreateDebugReportCallbackEXT, data.debugReportEnabled);
 }
 
-void InstanceVkTest::constructMove() {
-    if(std::getenv("MAGNUM_DISABLE_EXTENSIONS"))
-        CORRADE_SKIP("Can't test with the MAGNUM_DISABLE_EXTENSIONS environment variable set");
+void InstanceVkTest::tryCreateAlreadyCreated() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
 
-    InstanceExtensionProperties extensions = enumerateInstanceExtensionProperties();
-    if(!extensions.isSupported<Extensions::KHR::get_physical_device_properties2>())
-        CORRADE_SKIP("VK_KHR_get_physical_device_properties2 not supported, can't test");
+    Instance instance;
+    CORRADE_VERIFY(instance.handle());
 
-    Instance a{InstanceCreateInfo{}
-        .setApplicationInfo("InstanceVkTest", version(0, 0, 1))
-        .addEnabledExtensions<Extensions::KHR::get_physical_device_properties2>()};
-    VkInstance handle = a.handle();
-    Version version = a.version();
-    CORRADE_VERIFY(handle);
-    CORRADE_VERIFY(version != Version{});
-
-    Instance b = std::move(a);
-    CORRADE_VERIFY(!a.handle());
-    CORRADE_COMPARE(b.handleFlags(), HandleFlag::DestroyOnDestruction);
-    CORRADE_COMPARE(b.handle(), handle);
-    CORRADE_COMPARE(b.version(), version);
-    CORRADE_VERIFY(b.isExtensionEnabled<Extensions::KHR::get_physical_device_properties2>());
-    /* Function pointers in a are left in whatever state they were before, as
-       that doesn't matter */
-    CORRADE_VERIFY(b->CreateDevice);
-
-    Instance c{NoCreate};
-    c = std::move(b);
-    CORRADE_VERIFY(!b.handle());
-    CORRADE_COMPARE(b.handleFlags(), HandleFlags{});
-    CORRADE_COMPARE(c.handleFlags(), HandleFlag::DestroyOnDestruction);
-    CORRADE_COMPARE(c.handle(), handle);
-    CORRADE_COMPARE(c.version(), version);
-    CORRADE_VERIFY(c.isExtensionEnabled<Extensions::KHR::get_physical_device_properties2>());
-    /* Everything is swapped, including function pointers */
-    CORRADE_VERIFY(!b->CreateDevice);
-    CORRADE_VERIFY(c->CreateDevice);
-
-    CORRADE_VERIFY(std::is_nothrow_move_constructible<Instance>::value);
-    CORRADE_VERIFY(std::is_nothrow_move_assignable<Instance>::value);
+    std::ostringstream out;
+    Error redirectError{&out};
+    instance.tryCreate();
+    CORRADE_COMPARE(out.str(), "Vk::Instance::tryCreate(): instance already created\n");
 }
 
-void InstanceVkTest::constructUnknownLayer() {
-    CORRADE_SKIP("Currently this hits an internal assert, which can't be tested.");
+void InstanceVkTest::tryCreateUnknownLayer() {
+    Instance instance{NoCreate};
 
     std::ostringstream out;
     Error redirectError{&out};
-    Instance instance{InstanceCreateInfo{}
-        .addEnabledLayers({"VK_LAYER_this_doesnt_exist"_s})};
-    CORRADE_COMPARE(out.str(), "TODO");
+    CORRADE_COMPARE(instance.tryCreate(InstanceCreateInfo{}
+        .addEnabledLayers({"VK_LAYER_this_doesnt_exist"_s})), Result::ErrorLayerNotPresent);
+    CORRADE_COMPARE(out.str(), "Vk::Instance::tryCreate(): instance creation failed: Vk::Result::ErrorLayerNotPresent\n");
 }
 
-void InstanceVkTest::constructUnknownExtension() {
-    CORRADE_SKIP("Currently this hits an internal assert, which can't be tested.");
+void InstanceVkTest::tryCreateUnknownExtension() {
+    Instance instance{NoCreate};
 
     std::ostringstream out;
     Error redirectError{&out};
-    Instance instance{InstanceCreateInfo{}
-        .addEnabledExtensions({"VK_this_doesnt_exist"_s})};
-    CORRADE_COMPARE(out.str(), "TODO");
+    CORRADE_COMPARE(instance.tryCreate(InstanceCreateInfo{}
+        .addEnabledExtensions({"VK_this_doesnt_exist"_s})), Result::ErrorExtensionNotPresent);
+    CORRADE_COMPARE(out.str(), "Vk::Instance::tryCreate(): instance creation failed: Vk::Result::ErrorExtensionNotPresent\n");
 }
 
 void InstanceVkTest::wrap() {
@@ -515,7 +494,8 @@ void InstanceVkTest::wrap() {
 
     {
         /* Wrapping should load the basic function pointers */
-        auto wrapped = Instance::wrap(instance, Version::Vk11, {
+        Instance wrapped{NoCreate};
+        wrapped.wrap(instance, Version::Vk11, {
             Extensions::EXT::debug_report::string()
         }, HandleFlag::DestroyOnDestruction);
         CORRADE_VERIFY(wrapped->DestroyInstance);
@@ -540,11 +520,26 @@ void InstanceVkTest::wrap() {
     }
 
     /* ...so we can wrap it again, non-owned, and then destroy it manually */
-    auto wrapped = Instance::wrap(instance, Version::Vk10, {});
+    Instance wrapped{NoCreate};
+    wrapped.wrap(instance, Version::Vk10, {});
     CORRADE_VERIFY(wrapped->DestroyInstance);
     wrapped->DestroyInstance(instance, nullptr);
 }
 
+void InstanceVkTest::wrapAlreadyCreated() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    Instance instance;
+    CORRADE_VERIFY(instance.handle());
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    instance.wrap({}, {}, {});
+    CORRADE_COMPARE(out.str(), "Vk::Instance::wrap(): instance already created\n");
+}
+
 void InstanceVkTest::populateGlobalFunctionPointers() {
     vkDestroyInstance = nullptr;
 

src/Magnum/Vk/VulkanTester.cpp

@@ -43,9 +43,9 @@ VulkanTester::VulkanTester(): VulkanTester{NoCreate} {
 }
 
 VulkanTester::VulkanTester(NoCreateT): VulkanTester{NoCreate, NoCreate} {
-    _instance = Vk::Instance{Vk::InstanceCreateInfo{arguments().first, arguments().second}
+    _instance.create(Vk::InstanceCreateInfo{arguments().first, arguments().second}
         .setApplicationInfo(testName(), {})
-    };
+    );
 }
 
 VulkanTester::VulkanTester(NoCreateT, NoCreateT): TestSuite::Tester{TestSuite::Tester::TesterConfiguration{}.setSkippedArgumentPrefixes({"magnum"})}, _instance{NoCreate}, _device{NoCreate}, _queue{NoCreate} {}