From 50fa6b2ecc9b54a6408fd89465221cec13c5ad8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Jan 2021 13:45:58 +0100 Subject: [PATCH] Vk: make the Device immovable. There was a nasty issue when moving the Device -- because the Queue instances are populated with the Device pointer in the constructor, moving the Device makes the Queue device pointer invalid and those instances are useless. Discovered this when using VulkanTester to test an upcoming Queue::submit() -- until then, the Device pointer in the Queue wasn't really used for anything and so this wasn't known. Making the Device immovable is thus the only sane way to keep the current Queue retrieval workflow, which I'd say is much cleaner than having to manually query the device for queues later using some error-prone indices and whatnot. On the other hand, this finally makes it possible to have a failable device creation, instead of the device creation failing on an assert (or failing silently when CORRADE_NO_ASSERT is defined). This is consistent with how the GL wrapper works. Also, because all device-dependent objects need to keep a pointer to the originating Device in order to access Vulkan function pointers, having it immovable makes it considerably faster. I'll make Instance immovable with tryCreate() next because it seems like a good thing to do there as well. --- doc/snippets/MagnumVk.cpp | 18 ++++ src/Magnum/Vk/Device.cpp | 113 +++++++++++----------- src/Magnum/Vk/Device.h | 143 +++++++++++++++++++++------- src/Magnum/Vk/Test/DeviceTest.cpp | 9 +- src/Magnum/Vk/Test/DeviceVkTest.cpp | 132 ++++++++++++------------- src/Magnum/Vk/VulkanTester.cpp | 4 +- 6 files changed, 258 insertions(+), 161 deletions(-) diff --git a/doc/snippets/MagnumVk.cpp b/doc/snippets/MagnumVk.cpp index b8fe74927..c8ea77013 100644 --- a/doc/snippets/MagnumVk.cpp +++ b/doc/snippets/MagnumVk.cpp @@ -66,6 +66,24 @@ using namespace Magnum; #define DOXYGEN_IGNORE(...) __VA_ARGS__ +/* [Device-delayed-creation] */ +class MyApplication { + public: + explicit MyApplication(DOXYGEN_IGNORE(Vk::Instance& instance)); + + private: + Vk::Device _device{NoCreate}; +}; + +MyApplication::MyApplication(DOXYGEN_IGNORE(Vk::Instance& instance)) { + // decide on extensions, features, ... + + _device.create(instance, Vk::DeviceCreateInfo{DOXYGEN_IGNORE(Vk::pickDevice(instance))} + DOXYGEN_IGNORE() + ); +} +/* [Device-delayed-creation] */ + int main() { { diff --git a/src/Magnum/Vk/Device.cpp b/src/Magnum/Vk/Device.cpp index 021a6aef9..0c6a448fc 100644 --- a/src/Magnum/Vk/Device.cpp +++ b/src/Magnum/Vk/Device.cpp @@ -44,6 +44,7 @@ #include "Magnum/Vk/Extensions.h" #include "Magnum/Vk/ExtensionProperties.h" #include "Magnum/Vk/Queue.h" +#include "Magnum/Vk/Result.h" #include "Magnum/Vk/Version.h" #include "Magnum/Vk/Implementation/Arguments.h" #include "Magnum/Vk/Implementation/InstanceState.h" @@ -569,41 +570,68 @@ constexpr Version KnownVersionsForExtensions[]{ } -Device Device::wrap(Instance& instance, const VkDevice handle, const Version version, const Containers::ArrayView enabledExtensions, const DeviceFeatures& enabledFeatures, const HandleFlags flags) { +void Device::wrap(Instance& instance, const VkDevice handle, const Version version, const Containers::ArrayView enabledExtensions, const DeviceFeatures& enabledFeatures, const HandleFlags flags) { + CORRADE_ASSERT(!_handle, + "Vk::Device::wrap(): device already created", ); + /* Compared to the constructor nothing is printed here as it would be just - repeating what was passed to the constructor */ - Device out{NoCreate}; - out._handle = handle; - out._flags = flags; - out.initializeExtensions(enabledExtensions); - out.initialize(instance, version, enabledFeatures); - return out; + repeating what was passed via the arguments */ + _handle = handle; + _flags = flags; + initializeExtensions(enabledExtensions); + initialize(instance, version, enabledFeatures); } -Device Device::wrap(Instance& instance, const VkDevice handle, const Version version, const std::initializer_list enabledExtensions, const DeviceFeatures& enabledFeatures, const HandleFlags flags) { - return wrap(instance, handle, version, Containers::arrayView(enabledExtensions), enabledFeatures, flags); +void Device::wrap(Instance& instance, const VkDevice handle, const Version version, const std::initializer_list enabledExtensions, const DeviceFeatures& enabledFeatures, const HandleFlags flags) { + wrap(instance, handle, version, Containers::arrayView(enabledExtensions), enabledFeatures, flags); } -Device::Device(Instance& instance, const DeviceCreateInfo& info): Device{instance, info, DeviceProperties::wrap(instance, info._physicalDevice)} {} +Device::Device(Instance& instance, const DeviceCreateInfo& info): Device{NoCreate} { + create(instance, info); +} -Device::Device(Instance& instance, DeviceCreateInfo&& info): Device{instance, info, std::move(info._state->properties)} {} +Device::Device(Instance& instance, DeviceCreateInfo&& info): Device{NoCreate} { + create(instance, std::move(info)); +} + +Device::Device(NoCreateT): _handle{}, _functionPointers{} {} + +Device::~Device() { + if(_handle && (_flags & HandleFlag::DestroyOnDestruction)) + _functionPointers.DestroyDevice(_handle, nullptr); +} + +void Device::create(Instance& instance, const DeviceCreateInfo& info) { + if(tryCreate(instance, info) != Result::Success) std::exit(1); +} + +void Device::create(Instance& instance, DeviceCreateInfo&& info) { + if(tryCreate(instance, std::move(info)) != Result::Success) std::exit(1); +} + +Result Device::tryCreate(Instance& instance, const DeviceCreateInfo& info) { + return tryCreateInternal(instance, info, DeviceProperties::wrap(instance, info._physicalDevice)); +} + +Result Device::tryCreate(Instance& instance, DeviceCreateInfo&& info) { + return tryCreateInternal(instance, info, std::move(info._state->properties)); +} + +Result Device::tryCreateInternal(Instance& instance, const DeviceCreateInfo& info, DeviceProperties&& properties) { + CORRADE_ASSERT(!_handle, + "Vk::Device::tryCreate(): device already created", {}); + CORRADE_ASSERT(info._info.queueCreateInfoCount, + "Vk::Device::tryCreate(): needs at least one queue", {}); + + _flags = HandleFlag::DestroyOnDestruction; + _properties.emplace(std::move(properties)); -Device::Device(Instance& instance, const DeviceCreateInfo& info, DeviceProperties&& properties): - #ifdef CORRADE_GRACEFUL_ASSERT - _handle{}, /* Otherwise the destructor dies if we hit the queue assert */ - #endif - _flags{HandleFlag::DestroyOnDestruction}, - _properties{Containers::InPlaceInit, std::move(properties)} -{ /* The properties should always be a valid instance, either moved from outside or created again from VkPhysicalDevice, in case it couldn't be moved. If it's not, something in DeviceCreateInfo or here got messed up. */ CORRADE_INTERNAL_ASSERT(_properties->handle()); - CORRADE_ASSERT(info._info.queueCreateInfoCount, - "Vk::Device: needs to be created with at least one queue", ); - /* Check that all enabled features were actually reported as supported. I happily assumed the drivers would do that, but as far as my testing goes it happens only for the VkPhysicalDeviceFeatures2, and not for @@ -615,8 +643,8 @@ Device::Device(Instance& instance, const DeviceCreateInfo& info, DevicePropertie not) fail later in an unpredictable way. Fortunately it's rather easy to check thanks to how these are designed :D */ CORRADE_ASSERT(info._state->enabledFeatures <= _properties->features(), - "Vk::Device: some enabled features are not supported:" << - (info._state->enabledFeatures & ~_properties->features()), ); + "Vk::Device::tryCreate(): some enabled features are not supported:" << + (info._state->enabledFeatures & ~_properties->features()), {}); const Version version = info._state->version != Version::None ? info._state->version : _properties->version(); @@ -641,7 +669,10 @@ Device::Device(Instance& instance, const DeviceCreateInfo& info, DevicePropertie } } - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(instance->CreateDevice(info._physicalDevice, info, nullptr, &_handle)); + if(const VkResult result = instance->CreateDevice(info._physicalDevice, info, nullptr, &_handle)) { + Error{} << "Vk::Device::tryCreate(): device creation failed:" << Result(result); + return Result(result); + } initializeExtensions({info->ppEnabledExtensionNames, info->enabledExtensionCount}); initialize(instance, version, info._state->enabledFeatures); @@ -655,7 +686,7 @@ Device::Device(Instance& instance, const DeviceCreateInfo& info, DevicePropertie for(const Version version: KnownVersionsForExtensions) { for(const Extension extension: Extension::extensions(version)) { if(extension.index() != i) continue; - CORRADE_ASSERT_UNREACHABLE("Vk::Device: some enabled features need" << extension.string() << "enabled", ); + CORRADE_ASSERT_UNREACHABLE("Vk::Device::tryCreate(): some enabled features need" << extension.string() << "enabled", {}); } } } @@ -693,36 +724,8 @@ Device::Device(Instance& instance, const DeviceCreateInfo& info, DevicePropertie *info._state->queueOutput[queueOutputIndex++] = Queue::wrap(*this, queue); } } -} - -Device::Device(NoCreateT): _handle{}, _functionPointers{} {} - -Device::Device(Device&& other) noexcept: _handle{other._handle}, - _flags{other._flags}, _version{other._version}, - _enabledExtensions{other._enabledExtensions}, _properties{std::move(other._properties)}, _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 = {}; -} - -Device::~Device() { - if(_handle && (_flags & HandleFlag::DestroyOnDestruction)) - _functionPointers.DestroyDevice(_handle, nullptr); -} -Device& Device::operator=(Device&& other) noexcept { - using std::swap; - swap(other._handle, _handle); - swap(other._flags, _flags); - swap(other._version, _version); - swap(other._enabledExtensions, _enabledExtensions); - swap(other._properties, _properties); - swap(other._state, _state); - swap(other._functionPointers, _functionPointers); - return *this; + return Result::Success; } template void Device::initializeExtensions(const Containers::ArrayView enabledExtensions) { diff --git a/src/Magnum/Vk/Device.h b/src/Magnum/Vk/Device.h index 4706183a3..f39bb1af4 100644 --- a/src/Magnum/Vk/Device.h +++ b/src/Magnum/Vk/Device.h @@ -160,6 +160,22 @@ Compared to the above, the same custom code would then look like this: Similarly you can use @ref Instance::populateGlobalFunctionPointers() to populate instance-level global function pointers. + +@section Vk-Device-disabled-move Disabled move and delayed device creation + +Due to the way @ref Queue instances are populated on device creation, and +for safety reasons as all device-dependent objects internally have to keep a +pointer to the originating @ref Device to access Vulkan function pointers, the +@ref Device 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 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. */ class MAGNUM_VK_EXPORT Device { public: @@ -175,6 +191,17 @@ class MAGNUM_VK_EXPORT Device { * the device * @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 Device, instead + * it's expected to be called on a @ref NoCreate "NoCreate"'d + * instance. See @ref Vk-Device-disabled-move for more + * information. + * * The @p handle is expected to be originating from @p instance. The * @p version, @p enabledExtensions and @p enabledFeatures parameters * populate internal info about supported version, enabled extensions @@ -189,59 +216,48 @@ class MAGNUM_VK_EXPORT Device { * not recommended to call this function repeatedly for creating * short-lived device instances, even though it's technically correct. * - * Unlike a device created using a constructor, the Vulkan device is by - * default not deleted on destruction, use @p flags for different - * behavior. + * Unlike a device created using the constructor or @ref create(), the + * Vulkan device is by default not deleted on destruction. Use @p flags + * for different behavior. * @see @ref release() */ - static Device wrap(Instance& instance, VkDevice handle, Version version, Containers::ArrayView enabledExtensions, const DeviceFeatures& enabledFeatures, HandleFlags flags = {}); + void wrap(Instance& instance, VkDevice handle, Version version, Containers::ArrayView enabledExtensions, const DeviceFeatures& enabledFeatures, HandleFlags flags = {}); /** @overload */ - static Device wrap(Instance& instance, VkDevice handle, Version version, std::initializer_list enabledExtensions, const DeviceFeatures& enabledFeatures, HandleFlags flags = {}); + void wrap(Instance& instance, VkDevice handle, Version version, std::initializer_list enabledExtensions, const DeviceFeatures& enabledFeatures, HandleFlags flags = {}); /** * @brief Constructor - * @param instance Vulkan instance to create the device on - * @param info Device creation info * - * After creating the device requests device queues added via - * @ref DeviceCreateInfo::addQueues(), populating the @ref Queue - * references. - * @see @fn_vk_keyword{CreateDevice}, @fn_vk_keyword{GetDeviceQueue2}, - * @fn_vk_keyword{GetDeviceQueue} - * @todoc link to a concrete addQueues() overload above once doxygen - * finally GROWS UP and can link to &-qualified functions FFS + * Equivalent to calling @ref Device(NoCreateT) followed by + * @ref create(Instance&, const DeviceCreateInfo&). */ explicit Device(Instance& instance, const DeviceCreateInfo& info); /** - * @brief Construct with reusing already populated device properties + * @brief Construct, reusing already populated device properties * - * Compared to @ref Device(Instance&, const DeviceCreateInfo&), it can - * take ownership of the @ref DeviceProperties added to @p info earlier - * via @ref DeviceCreateInfo::DeviceCreateInfo(DeviceProperties&&, const ExtensionProperties*, Flags) - * or any of the other r-value-taking constructors. - * - * With that, the @ref properties() getter and any APIs relying on it - * can reuse what was possibly already queried without having to repeat - * the potentially complex queries second time. + * Equivalent to calling @ref Device(NoCreateT) followed by + * @ref create(Instance&, DeviceCreateInfo&&). */ explicit Device(Instance& instance, DeviceCreateInfo&& info); /** * @brief Construct without creating the device * - * The constructed instance is equivalent to moved-from state. Useful - * in cases where you will overwrite the instance later anyway. Move - * another object over it to make it useful. + * Use @ref create() or @ref tryCreate() to create the device. */ explicit Device(NoCreateT); /** @brief Copying is not allowed */ Device(const Device&) = delete; - /** @brief Move constructor */ - Device(Device&& other) noexcept; + /** + * @brief Moving is not allowed + * + * See @ref Vk-Device-disabled-move for more information. + */ + Device(Device&& other) = delete; /** * @brief Destructor @@ -256,8 +272,12 @@ class MAGNUM_VK_EXPORT Device { /** @brief Copying is not allowed */ Device& operator=(const Device&) = delete; - /** @brief Move assignment */ - Device& operator=(Device&& other) noexcept; + /** + * @brief Moving is not allowed + * + * See @ref Vk-Device-disabled-move for more information. + */ + Device& operator=(Device&&) = delete; /** @brief Underlying @type_vk{Device} handle */ VkDevice handle() { return _handle; } @@ -267,6 +287,63 @@ class MAGNUM_VK_EXPORT Device { /** @brief Handle flags */ HandleFlags handleFlags() const { return _flags; } + /** + * @brief Create a device + * @param instance Vulkan instance to create the device on + * @param info Device creation info + * + * Meant to be called on a @ref Device(NoCreateT) "NoCreate"'d + * instance. After creating the device populates device-level + * function pointers and runtime information about enabled extensions + * and features based on @p info, and finally requests device queues + * added via @ref DeviceCreateInfo::addQueues(), populating the + * @ref Queue references. + * + * If device 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 Device(Instance&, const DeviceCreateInfo&), + * @fn_vk_keyword{CreateDevice}, @fn_vk_keyword{GetDeviceQueue2}, + * @fn_vk_keyword{GetDeviceQueue} + * @todoc link to a concrete addQueues() overload above once doxygen + * finally GROWS UP and can link to &-qualified functions FFS + */ + void create(Instance& instance, const DeviceCreateInfo& info); + + /** + * @brief Create a device, reusing already populated device properties + * + * Compared to @ref create(Instance&, const DeviceCreateInfo&), it can + * take ownership of the @ref DeviceProperties added to @p info earlier + * via @ref DeviceCreateInfo::DeviceCreateInfo(DeviceProperties&&, const ExtensionProperties*, Flags) + * or any of the other r-value-taking constructors. + * + * With that, the @ref properties() getter and any APIs relying on it + * can reuse what was possibly already queried without having to repeat + * the potentially complex queries second time. + * @see @ref Device(Instance&, DeviceCreateInfo&&), + * @ref tryCreate(Instance&, DeviceCreateInfo&&) + */ + void create(Instance& instance, DeviceCreateInfo&& info); + + /** + * @brief Try to create a device + * + * Unlike @ref create(Instance&, const DeviceCreateInfo&), instead of + * exiting on error, prints a message to error output and returns a + * corresponding result value. On success returns @ref Result::Success. + */ + Result tryCreate(Instance& instance, const DeviceCreateInfo& info); + + /** + * @brief Try to create a device, reusing already populated device properties + * + * Unlike @ref create(Instance&, DeviceCreateInfo&&), instead of + * exiting on error, prints a message to error output and returns a + * corresponding result value. On success returns @ref Result::Success. + */ + Result tryCreate(Instance& instance, DeviceCreateInfo&& info); + /** * @brief Device properties * @@ -368,9 +445,9 @@ class MAGNUM_VK_EXPORT Device { private: friend Implementation::DeviceState; - /* Common guts for Device(Instance&, DeviceCreateInfo&) and - Device(Instance&, DeviceCreateInfo&&) */ - explicit Device(Instance& isntance, const DeviceCreateInfo&, DeviceProperties&&); + /* Common guts for tryCreate(Instance&, DeviceCreateInfo&) and + tryCreate(Instance&, DeviceCreateInfo&&) */ + Result tryCreateInternal(Instance& isntance, const DeviceCreateInfo&, DeviceProperties&&); template MAGNUM_VK_LOCAL void initializeExtensions(Containers::ArrayView enabledExtensions); MAGNUM_VK_LOCAL void initialize(Instance& instance, Version version, const DeviceFeatures& enabledFeatures); diff --git a/src/Magnum/Vk/Test/DeviceTest.cpp b/src/Magnum/Vk/Test/DeviceTest.cpp index 197f39ddc..6b04bbb30 100644 --- a/src/Magnum/Vk/Test/DeviceTest.cpp +++ b/src/Magnum/Vk/Test/DeviceTest.cpp @@ -38,6 +38,7 @@ struct DeviceTest: TestSuite::Tester { void constructNoCreate(); void constructCopy(); + void constructMove(); }; DeviceTest::DeviceTest() { @@ -45,7 +46,8 @@ DeviceTest::DeviceTest() { &DeviceTest::createInfoConstructFromVk, &DeviceTest::constructNoCreate, - &DeviceTest::constructCopy}); + &DeviceTest::constructCopy, + &DeviceTest::constructMove}); } void DeviceTest::createInfoConstructNoInit() { @@ -85,6 +87,11 @@ void DeviceTest::constructCopy() { CORRADE_VERIFY(!std::is_copy_assignable{}); } +void DeviceTest::constructMove() { + CORRADE_VERIFY(!std::is_move_constructible{}); + CORRADE_VERIFY(!std::is_move_assignable{}); +} + }}}} CORRADE_TEST_MAIN(Magnum::Vk::Test::DeviceTest) diff --git a/src/Magnum/Vk/Test/DeviceVkTest.cpp b/src/Magnum/Vk/Test/DeviceVkTest.cpp index 746becad4..60166b54e 100644 --- a/src/Magnum/Vk/Test/DeviceVkTest.cpp +++ b/src/Magnum/Vk/Test/DeviceVkTest.cpp @@ -76,13 +76,15 @@ struct DeviceVkTest: VulkanTester { void constructExtensionsCommandLineEnable(); void constructMultipleQueues(); void constructRawQueue(); - void constructMove(); - void constructUnknownExtension(); void constructFeatureNotSupported(); void constructFeatureWithoutExtension(); void constructNoQueue(); + void tryCreateAlreadyCreated(); + void tryCreateUnknownExtension(); + void wrap(); + void wrapAlreadyCreated(); void populateGlobalFunctionPointers(); }; @@ -172,14 +174,16 @@ DeviceVkTest::DeviceVkTest(): VulkanTester{NoCreate} { addTests({&DeviceVkTest::constructMultipleQueues, &DeviceVkTest::constructRawQueue, - - &DeviceVkTest::constructMove, - &DeviceVkTest::constructUnknownExtension, &DeviceVkTest::constructFeatureNotSupported, &DeviceVkTest::constructFeatureWithoutExtension, &DeviceVkTest::constructNoQueue, + &DeviceVkTest::tryCreateAlreadyCreated, + &DeviceVkTest::tryCreateUnknownExtension, + &DeviceVkTest::wrap, + &DeviceVkTest::wrapAlreadyCreated, + &DeviceVkTest::populateGlobalFunctionPointers}); } @@ -874,66 +878,6 @@ void DeviceVkTest::constructRawQueue() { CORRADE_VERIFY(rawQueue); } -void DeviceVkTest::constructMove() { - if(std::getenv("MAGNUM_VULKAN_VERSION")) - CORRADE_SKIP("Can't test with the MAGNUM_VULKAN_VERSION environment variable set"); - - DeviceProperties deviceProperties = pickDevice(instance()); - ExtensionProperties extensions = deviceProperties.enumerateExtensionProperties(); - if(!extensions.isSupported()) - CORRADE_SKIP("VK_KHR_maintenance1 not supported, can't test"); - - Queue queue{NoCreate}; - Device a{instance(), DeviceCreateInfo{deviceProperties} - .addQueues(0, {0.0f}, {queue}) - .addEnabledExtensions() - }; - VkDevice handle = a.handle(); - Version version = a.version(); - CORRADE_VERIFY(handle); - CORRADE_VERIFY(version != Version{}); - CORRADE_VERIFY(version != Version::None); - - Device 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_COMPARE(b.properties().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->CreateBuffer); - - Device 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_COMPARE(c.properties().version(), version); - CORRADE_VERIFY(c.isExtensionEnabled()); - /* Everything is swapped, including function pointers */ - CORRADE_VERIFY(!b->CreateBuffer); - CORRADE_VERIFY(c->CreateBuffer); - - CORRADE_VERIFY(std::is_nothrow_move_constructible::value); - CORRADE_VERIFY(std::is_nothrow_move_assignable::value); -} - -void DeviceVkTest::constructUnknownExtension() { - CORRADE_SKIP("Currently this hits an internal assert, which can't be tested."); - - std::ostringstream out; - Error redirectError{&out}; - Queue queue{NoCreate}; - Device device{instance(), DeviceCreateInfo{pickDevice(instance())} - .addQueues(0, {0.0f}, {queue}) - .addEnabledExtensions({"VK_this_doesnt_exist"_s})}; - CORRADE_COMPARE(out.str(), "TODO"); -} - void DeviceVkTest::constructFeatureNotSupported() { DeviceProperties properties = pickDevice(instance()); if(properties.features() & DeviceFeature::SparseBinding) @@ -947,7 +891,7 @@ void DeviceVkTest::constructFeatureNotSupported() { Device device{instance(), DeviceCreateInfo{properties} .addQueues(0, {0.0f}, {queue}) .setEnabledFeatures(DeviceFeature::SparseBinding|DeviceFeature::SparseResidency16Samples)}; - CORRADE_COMPARE(out.str(), "Vk::Device: some enabled features are not supported: Vk::DeviceFeature::SparseBinding|Vk::DeviceFeature::SparseResidency16Samples\n"); + CORRADE_COMPARE(out.str(), "Vk::Device::tryCreate(): some enabled features are not supported: Vk::DeviceFeature::SparseBinding|Vk::DeviceFeature::SparseResidency16Samples\n"); } void DeviceVkTest::constructFeatureWithoutExtension() { @@ -965,7 +909,7 @@ void DeviceVkTest::constructFeatureWithoutExtension() { std::ostringstream out; Error redirectError{&out}; Device device{instance(), info}; - CORRADE_COMPARE(out.str(), "Vk::Device: some enabled features need VK_KHR_sampler_ycbcr_conversion enabled\n"); + CORRADE_COMPARE(out.str(), "Vk::Device::tryCreate(): some enabled features need VK_KHR_sampler_ycbcr_conversion enabled\n"); } void DeviceVkTest::constructNoQueue() { @@ -976,7 +920,36 @@ void DeviceVkTest::constructNoQueue() { std::ostringstream out; Error redirectError{&out}; Device device{instance(), DeviceCreateInfo{pickDevice(instance())}}; - CORRADE_COMPARE(out.str(), "Vk::Device: needs to be created with at least one queue\n"); + CORRADE_COMPARE(out.str(), "Vk::Device::tryCreate(): needs at least one queue\n"); +} + +void DeviceVkTest::tryCreateAlreadyCreated() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Queue queue{NoCreate}; + Device device{instance(), DeviceCreateInfo{pickDevice(instance())} + .addQueues(0, {0.0f}, {queue}) + }; + CORRADE_VERIFY(device.handle()); + + std::ostringstream out; + Error redirectError{&out}; + device.tryCreate(instance(), DeviceCreateInfo{pickDevice(instance())}); + CORRADE_COMPARE(out.str(), "Vk::Device::tryCreate(): device already created\n"); +} + +void DeviceVkTest::tryCreateUnknownExtension() { + Queue queue{NoCreate}; + Device device{NoCreate}; + + std::ostringstream out; + Error redirectError{&out}; + CORRADE_COMPARE(device.tryCreate(instance(), DeviceCreateInfo{pickDevice(instance())} + .addQueues(0, {0.0f}, {queue}) + .addEnabledExtensions({"VK_this_doesnt_exist"_s})), Result::ErrorExtensionNotPresent); + CORRADE_COMPARE(out.str(), "Vk::Device::tryCreate(): device creation failed: Vk::Result::ErrorExtensionNotPresent\n"); } void DeviceVkTest::wrap() { @@ -1024,7 +997,8 @@ void DeviceVkTest::wrap() { { /* Wrapping should load the basic function pointers */ - auto wrapped = Device::wrap(instance2, device, Version::Vk11, { + Device wrapped{NoCreate}; + wrapped.wrap(instance2, device, Version::Vk11, { Extensions::EXT::debug_marker::string() }, DeviceFeature::RobustBufferAccess, HandleFlag::DestroyOnDestruction); CORRADE_VERIFY(wrapped->DestroyDevice); @@ -1052,11 +1026,29 @@ void DeviceVkTest::wrap() { } /* ...so we can wrap it again, non-owned, and then destroy it manually */ - auto wrapped = Device::wrap(instance2, device, Version::Vk10, {}, {}); + Device wrapped{NoCreate}; + wrapped.wrap(instance2, device, Version::Vk10, {}, {}); CORRADE_VERIFY(wrapped->DestroyDevice); wrapped->DestroyDevice(device, nullptr); } +void DeviceVkTest::wrapAlreadyCreated() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Queue queue{NoCreate}; + Device device{instance(), DeviceCreateInfo{pickDevice(instance())} + .addQueues(0, {0.0f}, {queue}) + }; + CORRADE_VERIFY(device.handle()); + + std::ostringstream out; + Error redirectError{&out}; + device.wrap(instance(), {}, {}, {}, {}); + CORRADE_COMPARE(out.str(), "Vk::Device::wrap(): device already created\n"); +} + void DeviceVkTest::populateGlobalFunctionPointers() { vkDestroyDevice = nullptr; diff --git a/src/Magnum/Vk/VulkanTester.cpp b/src/Magnum/Vk/VulkanTester.cpp index 1e9c9ef6f..3ed997e17 100644 --- a/src/Magnum/Vk/VulkanTester.cpp +++ b/src/Magnum/Vk/VulkanTester.cpp @@ -37,9 +37,9 @@ namespace Magnum { namespace Vk { VulkanTester::VulkanTester(): VulkanTester{NoCreate} { DeviceProperties deviceProperties = pickDevice(_instance); UnsignedInt graphicsQueue = deviceProperties.pickQueueFamily(Vk::QueueFlag::Graphics); - _device = Vk::Device{_instance, Vk::DeviceCreateInfo{std::move(deviceProperties)} + _device.create(_instance, Vk::DeviceCreateInfo{std::move(deviceProperties)} .addQueues(graphicsQueue, {0.0f}, {_queue}) - }; + ); } VulkanTester::VulkanTester(NoCreateT): VulkanTester{NoCreate, NoCreate} {