diff --git a/doc/snippets/MagnumVk.cpp b/doc/snippets/MagnumVk.cpp index c8ea77013..d3a8574ea 100644 --- a/doc/snippets/MagnumVk.cpp +++ b/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() { { diff --git a/src/Magnum/Vk/CMakeLists.txt b/src/Magnum/Vk/CMakeLists.txt index ed1896432..c6ef27cb4 100644 --- a/src/Magnum/Vk/CMakeLists.txt +++ b/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) diff --git a/src/Magnum/Vk/Device.h b/src/Magnum/Vk/Device.h index f39bb1af4..7468e782e 100644 --- a/src/Magnum/Vk/Device.h +++ b/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: diff --git a/src/Magnum/Vk/Instance.cpp b/src/Magnum/Vk/Instance.cpp index b93401e3b..a079f9b90 100644 --- a/src/Magnum/Vk/Instance.cpp +++ b/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 enabledExtensions, const HandleFlags flags) { +void Instance::wrap(const VkInstance handle, const Version version, const Containers::ArrayView 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 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 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({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 = {}; + return Result::Success; } -Instance::~Instance() { - if(_handle && (_flags & HandleFlag::DestroyOnDestruction)) - _functionPointers.DestroyInstance(_handle, nullptr); -} - -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 void Instance::initializeExtensions(const Containers::ArrayView enabledExtensions) { diff --git a/src/Magnum/Vk/Instance.h b/src/Magnum/Vk/Instance.h index 8c0e7868c..827da3a7f 100644 --- a/src/Magnum/Vk/Instance.h +++ b/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 * + * + * + * @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 enabledExtensions, HandleFlags flags = {}); + void wrap(VkInstance handle, Version version, Containers::ArrayView enabledExtensions, HandleFlags flags = {}); /** @overload */ - static Instance wrap(VkInstance handle, Version version, std::initializer_list enabledExtensions, HandleFlags flags = {}); + void wrap(VkInstance handle, Version version, std::initializer_list 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 * diff --git a/src/Magnum/Vk/Test/CMakeLists.txt b/src/Magnum/Vk/Test/CMakeLists.txt index adac619c1..77c525dc3 100644 --- a/src/Magnum/Vk/Test/CMakeLists.txt +++ b/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) diff --git a/src/Magnum/Vk/Test/InstanceTest.cpp b/src/Magnum/Vk/Test/InstanceTest.cpp index 874ff2501..b62c25e0a 100644 --- a/src/Magnum/Vk/Test/InstanceTest.cpp +++ b/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{}); } +void InstanceTest::constructMove() { + CORRADE_VERIFY(!std::is_move_constructible{}); + CORRADE_VERIFY(!std::is_move_assignable{}); +} + }}}} CORRADE_TEST_MAIN(Magnum::Vk::Test::InstanceTest) diff --git a/src/Magnum/Vk/Test/InstanceVkTest.cpp b/src/Magnum/Vk/Test/InstanceVkTest.cpp index 76622c7b6..7ff71b846 100644 --- a/src/Magnum/Vk/Test/InstanceVkTest.cpp +++ b/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()) - 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()}; - 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()); - /* 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()); - /* Everything is swapped, including function pointers */ - CORRADE_VERIFY(!b->CreateDevice); - CORRADE_VERIFY(c->CreateDevice); - - CORRADE_VERIFY(std::is_nothrow_move_constructible::value); - CORRADE_VERIFY(std::is_nothrow_move_assignable::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; diff --git a/src/Magnum/Vk/VulkanTester.cpp b/src/Magnum/Vk/VulkanTester.cpp index 3ed997e17..68eee9da3 100644 --- a/src/Magnum/Vk/VulkanTester.cpp +++ b/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} {}