diff --git a/doc/snippets/MagnumVk.cpp b/doc/snippets/MagnumVk.cpp index 2cce629f7..b3dd5514f 100644 --- a/doc/snippets/MagnumVk.cpp +++ b/doc/snippets/MagnumVk.cpp @@ -63,11 +63,10 @@ info->pNext = &validationFeatures; { Vk::Instance instance; /* [CommandPool-usage] */ -Vk::DeviceProperties props = DOXYGEN_IGNORE(pickDevice(instance)); Vk::Device device{DOXYGEN_IGNORE(NoCreate)}; Vk::CommandPool graphicsCommandPool{device, Vk::CommandPoolCreateInfo{ - props.pickQueueFamily(Vk::QueueFlag::Graphics) + device.properties().pickQueueFamily(Vk::QueueFlag::Graphics) }}; /* [CommandPool-usage] */ diff --git a/src/Magnum/Vk/Device.cpp b/src/Magnum/Vk/Device.cpp index 5f747470d..b809db08e 100644 --- a/src/Magnum/Vk/Device.cpp +++ b/src/Magnum/Vk/Device.cpp @@ -63,6 +63,11 @@ struct DeviceCreateInfo::State { std::size_t nextQueuePriority = 0; bool quietLog = false; Version version = Version::None; + /* Gets populated in the DeviceCreateInfo constructor that takes a + DeviceProperties&&, in which case it's then moved to the newly created + Device instance. If not populated, the Device instance gets nothing and + it'll gets populated on first access to Device::properties(). */ + DeviceProperties properties{NoCreate}; }; DeviceCreateInfo::DeviceCreateInfo(DeviceProperties& deviceProperties, const ExtensionProperties* extensionProperties, const Flags flags): _physicalDevice{deviceProperties}, _info{}, _state{Containers::InPlaceInit} { @@ -119,6 +124,10 @@ DeviceCreateInfo::DeviceCreateInfo(DeviceProperties& deviceProperties, const Ext } } +DeviceCreateInfo::DeviceCreateInfo(DeviceProperties&& deviceProperties, const ExtensionProperties* extensionProperties, const Flags flags): DeviceCreateInfo{deviceProperties, extensionProperties, flags} { + _state->properties = std::move(deviceProperties); +} + DeviceCreateInfo::DeviceCreateInfo(NoInitT) noexcept {} DeviceCreateInfo::DeviceCreateInfo(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo& info): _physicalDevice{physicalDevice}, @@ -128,7 +137,7 @@ DeviceCreateInfo::DeviceCreateInfo(VkPhysicalDevice physicalDevice, const VkDevi DeviceCreateInfo::~DeviceCreateInfo() = default; -DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const Containers::ArrayView extensions) { +DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const Containers::ArrayView extensions) & { if(extensions.empty()) return *this; /* This can happen in case we used the NoInit or VkDeviceCreateInfo constructor */ @@ -160,11 +169,21 @@ DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const Containers::Array return *this; } -DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const std::initializer_list extensions) { +DeviceCreateInfo&& DeviceCreateInfo::addEnabledExtensions(const Containers::ArrayView extensions) && { + addEnabledExtensions(extensions); + return std::move(*this); +} + +DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const std::initializer_list extensions) & { return addEnabledExtensions(Containers::arrayView(extensions)); } -DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const Containers::ArrayView extensions) { +DeviceCreateInfo&& DeviceCreateInfo::addEnabledExtensions(const std::initializer_list extensions) && { + addEnabledExtensions(extensions); + return std::move(*this); +} + +DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const Containers::ArrayView extensions) & { if(extensions.empty()) return *this; /* This can happen in case we used the NoInit or VkDeviceCreateInfo constructor */ @@ -185,11 +204,21 @@ DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const Containers::Array return *this; } -DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const std::initializer_list extensions) { +DeviceCreateInfo&& DeviceCreateInfo::addEnabledExtensions(const Containers::ArrayView extensions) && { + addEnabledExtensions(extensions); + return std::move(*this); +} + +DeviceCreateInfo& DeviceCreateInfo::addEnabledExtensions(const std::initializer_list extensions) & { return addEnabledExtensions(Containers::arrayView(extensions)); } -DeviceCreateInfo& DeviceCreateInfo::addQueues(const UnsignedInt family, const Containers::ArrayView priorities, const Containers::ArrayView> output) { +DeviceCreateInfo&& DeviceCreateInfo::addEnabledExtensions(const std::initializer_list extensions) && { + addEnabledExtensions(extensions); + return std::move(*this); +} + +DeviceCreateInfo& DeviceCreateInfo::addQueues(const UnsignedInt family, const Containers::ArrayView priorities, const Containers::ArrayView> output) & { CORRADE_ASSERT(!priorities.empty(), "Vk::DeviceCreateInfo::addQueues(): at least one queue priority has to be specified", *this); CORRADE_ASSERT(output.size() == priorities.size(), "Vk::DeviceCreateInfo::addQueues(): expected" << priorities.size() << "outuput queue references but got" << output.size(), *this); @@ -217,11 +246,21 @@ DeviceCreateInfo& DeviceCreateInfo::addQueues(const UnsignedInt family, const Co return addQueues(info); } -DeviceCreateInfo& DeviceCreateInfo::addQueues(const UnsignedInt family, const std::initializer_list priorities, const std::initializer_list> output) { +DeviceCreateInfo&& DeviceCreateInfo::addQueues(const UnsignedInt family, const Containers::ArrayView priorities, const Containers::ArrayView> output) && { + addQueues(family, priorities, output); + return std::move(*this); +} + +DeviceCreateInfo& DeviceCreateInfo::addQueues(const UnsignedInt family, const std::initializer_list priorities, const std::initializer_list> output) & { return addQueues(family, Containers::arrayView(priorities), Containers::arrayView(output)); } -DeviceCreateInfo& DeviceCreateInfo::addQueues(const VkDeviceQueueCreateInfo& info) { +DeviceCreateInfo&& DeviceCreateInfo::addQueues(const UnsignedInt family, const std::initializer_list priorities, const std::initializer_list> output) && { + addQueues(family, priorities, output); + return std::move(*this); +} + +DeviceCreateInfo& DeviceCreateInfo::addQueues(const VkDeviceQueueCreateInfo& info) & { /* This can happen in case we used the NoInit or VkDeviceCreateInfo constructor */ if(!_state) _state.emplace(); @@ -236,6 +275,11 @@ DeviceCreateInfo& DeviceCreateInfo::addQueues(const VkDeviceQueueCreateInfo& inf return *this; } +DeviceCreateInfo&& DeviceCreateInfo::addQueues(const VkDeviceQueueCreateInfo& info) && { + addQueues(info); + return std::move(*this); +} + Device Device::wrap(Instance& instance, const VkDevice handle, const Version version, const Containers::ArrayView enabledExtensions, const HandleFlags flags) { /* Compared to the constructor nothing is printed here as it would be just repeating what was passed to the constructor */ @@ -251,7 +295,11 @@ Device Device::wrap(Instance& instance, const VkDevice handle, const Version ver return wrap(instance, handle, version, Containers::arrayView(enabledExtensions), flags); } -Device::Device(Instance& instance, const DeviceCreateInfo& info): +Device::Device(Instance& instance, const DeviceCreateInfo& info): Device{instance, info, DeviceProperties{NoCreate}} {} + +Device::Device(Instance& instance, DeviceCreateInfo&& info): Device{instance, info, std::move(info._state->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 @@ -260,7 +308,16 @@ Device::Device(Instance& instance, const DeviceCreateInfo& info): CORRADE_ASSERT(info._info.queueCreateInfoCount, "Vk::Device: needs to be created with at least one queue", ); - const Version version = info._state->version != Version::None ? info._state->version : DeviceProperties::wrap(instance, info._physicalDevice).apiVersion(); + /* If the passed properties are populated, use them. Otherwise create a new + instance as we'd have no other way to remember the VkPhysicalDevice + handle otherwise */ + if(properties.handle()) + _properties.emplace(std::move(properties)); + else + _properties.emplace(DeviceProperties::wrap(instance, info._physicalDevice)); + + const Version version = info._state->version != Version::None ? + info._state->version : _properties->apiVersion(); /* Print all enabled extensions if we're not told to be quiet */ if(!info._state || !info._state->quietLog) { @@ -315,7 +372,7 @@ Device::Device(NoCreateT): _handle{}, _functionPointers{} {} Device::Device(Device&& other) noexcept: _handle{other._handle}, _flags{other._flags}, _version{other._version}, - _extensionStatus{other._extensionStatus}, _state{std::move(other._state)}, + _extensionStatus{other._extensionStatus}, _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) @@ -335,6 +392,7 @@ Device& Device::operator=(Device&& other) noexcept { swap(other._flags, _flags); swap(other._version, _version); swap(other._extensionStatus, _extensionStatus); + swap(other._properties, _properties); swap(other._state, _state); swap(other._functionPointers, _functionPointers); return *this; diff --git a/src/Magnum/Vk/Device.h b/src/Magnum/Vk/Device.h index 94ecaa2a2..1613b56ab 100644 --- a/src/Magnum/Vk/Device.h +++ b/src/Magnum/Vk/Device.h @@ -95,7 +95,8 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo { * @param extensionProperties Existing @ref ExtensionProperties * instance for querying available Vulkan extensions. If * @cpp nullptr @ce, a new instance may be created internally if - * needed. + * needed. If a r-value is passed, the instance is later available + * through @ref Device::properties(). * @param flags Device creation flags * * The following @type_vk{DeviceCreateInfo} fields are pre-filled in @@ -107,12 +108,22 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo { */ explicit DeviceCreateInfo(DeviceProperties& deviceProperties, const ExtensionProperties* extensionProperties, Flags flags = {}); - /** @overload */ - explicit DeviceCreateInfo(DeviceProperties&& deviceProperties, const ExtensionProperties* extensionProperties, Flags flags = {}): DeviceCreateInfo{deviceProperties, extensionProperties, flags} {} - /** @overload */ explicit DeviceCreateInfo(DeviceProperties& deviceProperties, Flags flags = {}): DeviceCreateInfo{deviceProperties, nullptr, flags} {} + /** + * @brief Construct with allowing to reuse already populated device properties + * + * Compared to @ref DeviceCreateInfo(DeviceProperties&, const ExtensionProperties*, Flags), + * if the @ref Device is subsequently constructed via + * @ref Device::Device(Instance&, DeviceCreateInfo&&), the + * @p deviceProperties instance gets directly transferred to the + * device, meaning @ref Device::properties() and any APIs relying on it + * can reuse what was possibly already queried without having to repeat + * the potentially complex queries second time. + */ + explicit DeviceCreateInfo(DeviceProperties&& deviceProperties, const ExtensionProperties* extensionProperties, Flags flags = {}); + /** @overload */ explicit DeviceCreateInfo(DeviceProperties&& deviceProperties, Flags flags = {}): DeviceCreateInfo{std::move(deviceProperties), nullptr, flags} {} @@ -135,6 +146,18 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo { ~DeviceCreateInfo(); + /* All the && overloads below are there in order to allow code like + + Device device{instance, DeviceCreateInfo{pickDevice(instance)} + .addQueues(...) + .addEnabledExtensions(...) + ... + }; + + to work and correctly move the DeviceProperties to the Device. + When adding new APIs, expand DeviceVkTest::createInfoRvalue() to + verify everything still works. */ + /** * @brief Add enabled device extensions * @return Reference to self (for method chaining) @@ -147,17 +170,30 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo { * null-terminated, use the @link Containers::Literals::operator""_s() @endlink * literal to prevent that where possible. */ - DeviceCreateInfo& addEnabledExtensions(Containers::ArrayView extensions); + DeviceCreateInfo& addEnabledExtensions(Containers::ArrayView extensions) &; + /** @overload */ + DeviceCreateInfo&& addEnabledExtensions(Containers::ArrayView extensions) &&; + /** @overload */ + DeviceCreateInfo& addEnabledExtensions(std::initializer_list extension) &; /** @overload */ - DeviceCreateInfo& addEnabledExtensions(std::initializer_list extension); + DeviceCreateInfo&& addEnabledExtensions(std::initializer_list extension) &&; /** @overload */ - DeviceCreateInfo& addEnabledExtensions(Containers::ArrayView extensions); + DeviceCreateInfo& addEnabledExtensions(Containers::ArrayView extensions) &; /** @overload */ - DeviceCreateInfo& addEnabledExtensions(std::initializer_list extension); + DeviceCreateInfo&& addEnabledExtensions(Containers::ArrayView extensions) &&; /** @overload */ - template DeviceCreateInfo& addEnabledExtensions() { + DeviceCreateInfo& addEnabledExtensions(std::initializer_list extension) &; + /** @overload */ + DeviceCreateInfo&& addEnabledExtensions(std::initializer_list extension) &&; + /** @overload */ + template DeviceCreateInfo& addEnabledExtensions() & { static_assert(Implementation::IsExtension::value, "expected only Vulkan device extensions"); - return addEnabledExtensions({E{}...}); + return addEnabledExtensions(std::initializer_list{E{}...}); + } + /** @overload */ + template DeviceCreateInfo&& addEnabledExtensions() && { + addEnabledExtensions(); + return std::move(*this); } /** @@ -173,9 +209,13 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo { * At least one queue has to be added. * @see @ref DeviceProperties::pickQueueFamily() */ - DeviceCreateInfo& addQueues(UnsignedInt family, Containers::ArrayView priorities, Containers::ArrayView> output); + DeviceCreateInfo& addQueues(UnsignedInt family, Containers::ArrayView priorities, Containers::ArrayView> output) &; + /** @overload */ + DeviceCreateInfo&& addQueues(UnsignedInt family, Containers::ArrayView priorities, Containers::ArrayView> output) &&; /** @overload */ - DeviceCreateInfo& addQueues(UnsignedInt family, std::initializer_list priorities, std::initializer_list> output); + DeviceCreateInfo& addQueues(UnsignedInt family, std::initializer_list priorities, std::initializer_list> output) &; + /** @overload */ + DeviceCreateInfo&& addQueues(UnsignedInt family, std::initializer_list priorities, std::initializer_list> output) &&; /** * @brief Add queues using raw info @@ -185,7 +225,9 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo { * queue properties using the `pNext` chain. The info is uses as-is, * with all pointers expected to stay in scope until device creation. */ - DeviceCreateInfo& addQueues(const VkDeviceQueueCreateInfo& info); + DeviceCreateInfo& addQueues(const VkDeviceQueueCreateInfo& info) &; + /** @overload */ + DeviceCreateInfo&& addQueues(const VkDeviceQueueCreateInfo& info) &&; /** @brief Underlying @type_vk{DeviceCreateInfo} structure */ VkDeviceCreateInfo& operator*() { return _info; } @@ -353,6 +395,20 @@ class MAGNUM_VK_EXPORT Device { */ explicit Device(Instance& instance, const DeviceCreateInfo& info); + /** + * @brief Construct with 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. + */ + explicit Device(Instance& instance, DeviceCreateInfo&& info); + /** * @brief Construct without creating the device * @@ -392,6 +448,15 @@ class MAGNUM_VK_EXPORT Device { /** @brief Handle flags */ HandleFlags handleFlags() const { return _flags; } + /** + * @brief Device properties + * + * If a r-value @ref DeviceProperties instance was propagated to + * @ref DeviceCreateInfo and then to @ref Device, it's reused here. + * Otherwise the contents are populated on first use. + */ + DeviceProperties& properties() { return *_properties; } + /** * @brief Version supported by the device * @@ -477,6 +542,10 @@ 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&&); + template MAGNUM_VK_LOCAL void initializeExtensions(Containers::ArrayView enabledExtensions); MAGNUM_VK_LOCAL void initialize(Instance& instance, Version version); @@ -487,6 +556,7 @@ class MAGNUM_VK_EXPORT Device { HandleFlags _flags; Version _version; Math::BoolVector _extensionStatus; + Containers::Pointer _properties; Containers::Pointer _state; /* This member is bigger than you might think */ diff --git a/src/Magnum/Vk/Test/DeviceVkTest.cpp b/src/Magnum/Vk/Test/DeviceVkTest.cpp index 7eaa6804e..5e8481cf8 100644 --- a/src/Magnum/Vk/Test/DeviceVkTest.cpp +++ b/src/Magnum/Vk/Test/DeviceVkTest.cpp @@ -54,9 +54,11 @@ struct DeviceVkTest: VulkanTester { void createInfoCopiedStrings(); void createInfoNoQueuePriorities(); void createInfoWrongQueueOutputCount(); + void createInfoRvalue(); void construct(); void constructExtensions(); + void constructTransferDeviceProperties(); void constructExtensionsCommandLineDisable(); void constructExtensionsCommandLineEnable(); void constructMultipleQueues(); @@ -64,7 +66,9 @@ struct DeviceVkTest: VulkanTester { void constructMove(); void constructUnknownExtension(); void constructNoQueue(); + void wrap(); + void populateGlobalFunctionPointers(); }; @@ -126,9 +130,11 @@ DeviceVkTest::DeviceVkTest(): VulkanTester{NoCreate} { &DeviceVkTest::createInfoCopiedStrings, &DeviceVkTest::createInfoNoQueuePriorities, &DeviceVkTest::createInfoWrongQueueOutputCount, + &DeviceVkTest::createInfoRvalue, &DeviceVkTest::construct, - &DeviceVkTest::constructExtensions}); + &DeviceVkTest::constructExtensions, + &DeviceVkTest::constructTransferDeviceProperties}); addInstancedTests({&DeviceVkTest::constructExtensionsCommandLineDisable, &DeviceVkTest::constructExtensionsCommandLineEnable}, @@ -233,6 +239,33 @@ void DeviceVkTest::createInfoWrongQueueOutputCount() { CORRADE_COMPARE(out.str(), "Vk::DeviceCreateInfo::addQueues(): expected 3 outuput queue references but got 2\n"); } +void DeviceVkTest::createInfoRvalue() { + Float zero[1]{}; + Queue a{NoCreate}, b{NoCreate}; + Containers::Reference reference[1]{a}; + + VkDeviceQueueCreateInfo rawQueueInfo{}; + rawQueueInfo.sType = VK_STRUCTURE_TYPE_DEVICE_QUEUE_CREATE_INFO; + rawQueueInfo.pQueuePriorities = zero; + rawQueueInfo.queueFamilyIndex = 0; + rawQueueInfo.queueCount = 1; + + DeviceCreateInfo&& info = DeviceCreateInfo{pickDevice(instance())} + .addEnabledExtensions(Containers::ArrayView{}) + .addEnabledExtensions(std::initializer_list{}) + .addEnabledExtensions(Containers::ArrayView{}) + .addEnabledExtensions(std::initializer_list{}) + .addEnabledExtensions<>() + .addQueues(0, zero, reference) + .addQueues(0, {0.0f}, {b}) + .addQueues(rawQueueInfo); + + /* Just to test something, main point is that the above compiles, links and + returns a &&. Can't test anything related to the contents because the + destructor gets called at the end of the expression. */ + CORRADE_VERIFY(&info); +} + void DeviceVkTest::construct() { if(std::getenv("MAGNUM_VULKAN_VERSION")) CORRADE_SKIP("Can't test with the MAGNUM_VULKAN_VERSION environment variable set"); @@ -258,6 +291,11 @@ void DeviceVkTest::construct() { /* The queue should be also filled in */ CORRADE_VERIFY(queue.handle()); + + /* Device properties should be lazy-populated and different from the + above instances because we didn't transfer the ownership */ + CORRADE_COMPARE(device.properties().name(), deviceProperties.name()); + CORRADE_VERIFY(&device.properties().properties() != &deviceProperties.properties()); } /* Shouldn't crash or anything */ @@ -303,6 +341,19 @@ void DeviceVkTest::constructExtensions() { CORRADE_VERIFY(device->TrimCommandPoolKHR); } +void DeviceVkTest::constructTransferDeviceProperties() { + DeviceProperties deviceProperties = pickDevice(instance()); + const void* vkProperties = &deviceProperties.properties(); + Queue queue{NoCreate}; + Device device{instance(), DeviceCreateInfo{std::move(deviceProperties)} + .addQueues(0, {0.0f}, {queue}) + }; + + /* Device properties should be the same address as in the original instance + because the ownership got transferred through */ + CORRADE_COMPARE(&device.properties().properties(), vkProperties); +} + void DeviceVkTest::constructExtensionsCommandLineDisable() { auto&& data = ConstructCommandLineData[testCaseInstanceId()]; setTestCaseDescription(data.nameDisable); @@ -496,6 +547,9 @@ void DeviceVkTest::constructRawQueue() { } 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()) @@ -517,6 +571,7 @@ void DeviceVkTest::constructMove() { CORRADE_COMPARE(b.handleFlags(), HandleFlag::DestroyOnDestruction); CORRADE_COMPARE(b.handle(), handle); CORRADE_COMPARE(b.version(), version); + CORRADE_COMPARE(b.properties().apiVersion(), version); CORRADE_VERIFY(b.isExtensionEnabled()); /* Function pointers in a are left in whatever state they were before, as that doesn't matter */ @@ -529,6 +584,7 @@ void DeviceVkTest::constructMove() { CORRADE_COMPARE(c.handleFlags(), HandleFlag::DestroyOnDestruction); CORRADE_COMPARE(c.handle(), handle); CORRADE_COMPARE(c.version(), version); + CORRADE_COMPARE(c.properties().apiVersion(), version); CORRADE_VERIFY(c.isExtensionEnabled()); /* Everything is swapped, including function pointers */ CORRADE_VERIFY(!b->CreateBuffer);