Vk: DeviceCreateInfo::addQueues() overload taking QueueFlags directly.

More convenient to use since one doesn't have to explicitly store a
DeviceProperties instance to call pickQueueFamily() on it and then move
it to DeviceCreateInfo to keep it efficient.

This required a slight redesign of how DeviceProperties are stored in
DeviceCreateInfo, now we need the instance to be always valid (but it
can get never used). The wrap() API isn't doing any extra work so this
won't add any inefficiency.

doc/snippets/MagnumVk.cpp

@@ -79,24 +79,20 @@ Vk::CommandBuffer commandBuffer = graphicsCommandPool.allocate();
 
 {
 Vk::Instance instance;
-/* [Device-usage-pick] */
-Vk::DeviceProperties props = Vk::pickDevice(instance);
-/* [Device-usage-pick] */
-
 /* [Device-usage-construct-queue] */
 Vk::Queue queue{NoCreate};
-Vk::Device device{instance, Vk::DeviceCreateInfo{props}
-    .addQueues(props.pickQueueFamily(Vk::QueueFlag::Graphics), {0.0f}, {queue})
+Vk::Device device{instance, Vk::DeviceCreateInfo{Vk::pickDevice(instance)}
+    .addQueues(Vk::QueueFlag::Graphics, {0.0f}, {queue})
 };
 /* [Device-usage-construct-queue] */
 }
 
 {
 Vk::Instance instance;
-Vk::DeviceProperties props{NoCreate};
+Vk::DeviceProperties properties{NoCreate};
 using namespace Containers::Literals;
 /* [Device-usage-extensions] */
-Vk::Device device{instance, Vk::DeviceCreateInfo{props}
+Vk::Device device{instance, Vk::DeviceCreateInfo{DOXYGEN_IGNORE(properties)}
     DOXYGEN_IGNORE()
     .addEnabledExtensions<                         // predefined extensions
         Vk::Extensions::EXT::index_type_uint8,
@@ -108,12 +104,12 @@ Vk::Device device{instance, Vk::DeviceCreateInfo{props}
 
 {
 Vk::Instance instance;
-Vk::DeviceProperties props{NoCreate};
 using namespace Containers::Literals;
 /* [Device-usage-check-supported] */
-Vk::ExtensionProperties extensions = props.enumerateExtensionProperties();
+Vk::DeviceProperties properties = Vk::pickDevice(instance);
+Vk::ExtensionProperties extensions = properties.enumerateExtensionProperties();
 
-Vk::DeviceCreateInfo info{props};
+Vk::DeviceCreateInfo info{properties};
 if(extensions.isSupported<Vk::Extensions::EXT::index_type_uint8>())
     info.addEnabledExtensions<Vk::Extensions::EXT::index_type_uint8>();
 if(extensions.isSupported("VK_NV_mesh_shader"_s))

src/Magnum/Vk/Device.cpp

@@ -63,10 +63,9 @@ 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(). */
+    /* Gets populated at the very end of DeviceCreateInfo(DeviceProperties&)
+       and then possibly overwritten in DeviceCreateInfo(DeviceProperties&&).
+       Either way, it's meant to be valid after the constructor exits. */
     DeviceProperties properties{NoCreate};
 };
 
@@ -122,6 +121,16 @@ DeviceCreateInfo::DeviceCreateInfo(DeviceProperties& deviceProperties, const Ext
         if(_state->version < Version::Vk11 && extensionProperties->isSupported<Extensions::KHR::bind_memory2>())
             addEnabledExtensions<Extensions::KHR::bind_memory2>();
     }
+
+    /* Conservatively populate the device properties.
+       - In case the DeviceCreateInfo(DeviceProperties&&) constructor is used,
+         it'll get overwritten straight away with a populated instance.
+       - In case the addQueues(QueueFlags) API is not used and DeviceCreateInfo
+         isn't subsequently moved to the Device, it'll never get touched again
+         and Device will wrap() its own.
+       - In case addQueues(QueueFlags) is used it'll get populated and then
+         possibly discarded if it isn't subsequently moved to the Device. */
+    _state->properties = DeviceProperties::wrap(*deviceProperties._instance, deviceProperties._handle);
 }
 
 DeviceCreateInfo::DeviceCreateInfo(DeviceProperties&& deviceProperties, const ExtensionProperties* extensionProperties, const Flags flags): DeviceCreateInfo{deviceProperties, extensionProperties, flags} {
@@ -260,6 +269,24 @@ DeviceCreateInfo&& DeviceCreateInfo::addQueues(const UnsignedInt family, const s
     return std::move(*this);
 }
 
+DeviceCreateInfo& DeviceCreateInfo::addQueues(const QueueFlags flags, const Containers::ArrayView<const Float> priorities, const Containers::ArrayView<const Containers::Reference<Queue>> output) & {
+    return addQueues(_state->properties.pickQueueFamily(flags), priorities, output);
+}
+
+DeviceCreateInfo&& DeviceCreateInfo::addQueues(const QueueFlags flags, const Containers::ArrayView<const Float> priorities, const Containers::ArrayView<const Containers::Reference<Queue>> output) && {
+    addQueues(flags, priorities, output);
+    return std::move(*this);
+}
+
+DeviceCreateInfo& DeviceCreateInfo::addQueues(const QueueFlags flags, const std::initializer_list<Float> priorities, const std::initializer_list<Containers::Reference<Queue>> output) & {
+    return addQueues(flags, Containers::arrayView(priorities), Containers::arrayView(output));
+}
+
+DeviceCreateInfo&& DeviceCreateInfo::addQueues(const QueueFlags flags, const std::initializer_list<Float> priorities, const std::initializer_list<Containers::Reference<Queue>> output) && {
+    addQueues(flags, priorities, output);
+    return std::move(*this);
+}
+
 DeviceCreateInfo& DeviceCreateInfo::addQueues(const VkDeviceQueueCreateInfo& info) & {
     /* This can happen in case we used the NoInit or VkDeviceCreateInfo
        constructor */
@@ -295,7 +322,7 @@ 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{instance, info, DeviceProperties{NoCreate}} {}
+Device::Device(Instance& instance, const DeviceCreateInfo& info): Device{instance, info, DeviceProperties::wrap(instance, info._physicalDevice)} {}
 
 Device::Device(Instance& instance, DeviceCreateInfo&& info): Device{instance, info, std::move(info._state->properties)} {}
 
@@ -303,19 +330,18 @@ Device::Device(Instance& instance, const DeviceCreateInfo& info, DevicePropertie
     #ifdef CORRADE_GRACEFUL_ASSERT
     _handle{}, /* Otherwise the destructor dies if we hit the queue assert */
     #endif
-    _flags{HandleFlag::DestroyOnDestruction}
+    _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", );
 
-    /* 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->version();
 

src/Magnum/Vk/Device.h

@@ -207,7 +207,8 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo {
          * @return Reference to self (for method chaining)
          *
          * At least one queue has to be added.
-         * @see @ref DeviceProperties::pickQueueFamily()
+         * @see @ref DeviceProperties::pickQueueFamily(),
+         *      @ref addQueues(QueueFlags, Containers::ArrayView<const Float>, Containers::ArrayView<const Containers::Reference<Queue>>)
          */
         DeviceCreateInfo& addQueues(UnsignedInt family, Containers::ArrayView<const Float> priorities, Containers::ArrayView<const Containers::Reference<Queue>> output) &;
         /** @overload */
@@ -217,6 +218,27 @@ class MAGNUM_VK_EXPORT DeviceCreateInfo {
         /** @overload */
         DeviceCreateInfo&& addQueues(UnsignedInt family, std::initializer_list<Float> priorities, std::initializer_list<Containers::Reference<Queue>> output) &&;
 
+        /**
+         * @brief Add queues of family matching given flags
+         *
+         * Equivalent to picking a queue family first using
+         * @ref DeviceProperties::pickQueueFamily() based on @p flags and then
+         * calling @ref addQueues(UnsignedInt, Containers::ArrayView<const Float>, Containers::ArrayView<const Containers::Reference<Queue>>)
+         * with the family index.
+         *
+         * Note that @ref DeviceProperties::pickQueueFamily() exits in case it
+         * doesn't find any family satisfying given @p flags --- for a
+         * failproof scenario you may want to go with the above overload and
+         * @ref DeviceProperties::tryPickQueueFamily() instead.
+         */
+        DeviceCreateInfo& addQueues(QueueFlags flags, Containers::ArrayView<const Float> priorities, Containers::ArrayView<const Containers::Reference<Queue>> output) &;
+        /** @overload */
+        DeviceCreateInfo&& addQueues(QueueFlags flags, Containers::ArrayView<const Float> priorities, Containers::ArrayView<const Containers::Reference<Queue>> output) &&;
+        /** @overload */
+        DeviceCreateInfo& addQueues(QueueFlags flags, std::initializer_list<Float> priorities, std::initializer_list<Containers::Reference<Queue>> output) &;
+        /** @overload */
+        DeviceCreateInfo&& addQueues(QueueFlags flags, std::initializer_list<Float> priorities, std::initializer_list<Containers::Reference<Queue>> output) &&;
+
         /**
          * @brief Add queues using raw info
          * @return Reference to self (for method chaining)
@@ -269,23 +291,20 @@ even a software implementation. If the application needs something specific,
 you can use @ref enumerateDevices() instead, pick a device from the list
 manually, provide the users with a list to choose from etc.
 
-@snippet MagnumVk.cpp Device-usage-pick
-
-After a device is picked, a @ref Device can be created using
-@ref DeviceCreateInfo. At the very least you'll need to set up queues, as every
-Vulkan device needs at least one. That's done by creating an empty @ref Queue
-instance and then referencing it from @ref DeviceCreateInfo::addQueues(). After
-the device is constructed, the queue gets populated and is ready to be used.
+The picked device is then passed to @ref DeviceCreateInfo. At the very least
+you'll also need to set up queues, as every Vulkan device needs at least one.
+That's done by creating an empty @ref Queue instance and then referencing it
+from @ref DeviceCreateInfo::addQueues(). After the device is constructed, the
+queue gets populated and is ready to be used.
 
 @snippet MagnumVk.cpp Device-usage-construct-queue
 
-In the above snippet, we requested a graphics queue --- the
-@ref DeviceProperties instance we made earlier acts as knowledge base for a
-particular device, providing info about available queues, extensions, features,
-memory heaps and implementation limits --- and we used a convenience API to
-pick the first available graphics queue. As with device picking, you can
-also iterate through all @ref DeviceProperties::queueFamilyCount() and choose
-one manually.
+In the above snippet, we requested a graphics queue via a convenience API. The
+information about available queues and other device properties is stored in a
+@ref DeviceProperties that got returned from @ref pickDevice() and
+@ref DeviceCreateInfo called @ref DeviceProperties::pickQueueFamily() for us.
+As with device picking, you can also iterate through all
+@ref DeviceProperties::queueFamilyCount() and choose one manually.
 
 Same as with @ref Instance, the above won't enable any additional extensions
 except for what the engine itself needs or what's supplied on the command line. Use @ref DeviceCreateInfo::addEnabledExtensions() to enable them, you can use

src/Magnum/Vk/Test/DeviceVkTest.cpp

@@ -24,6 +24,7 @@
 */
 
 #include <sstream>
+#include <Corrade/Containers/Optional.h>
 #include <Corrade/Containers/StringStl.h>
 #include <Corrade/TestSuite/Compare/Numeric.h>
 #include <Corrade/Utility/DebugStl.h>
@@ -57,6 +58,7 @@ struct DeviceVkTest: VulkanTester {
     void createInfoRvalue();
 
     void construct();
+    void constructQueueFromFlags();
     void constructExtensions();
     void constructTransferDeviceProperties();
     void constructExtensionsCommandLineDisable();
@@ -137,6 +139,7 @@ DeviceVkTest::DeviceVkTest(): VulkanTester{NoCreate} {
               &DeviceVkTest::createInfoRvalue,
 
               &DeviceVkTest::construct,
+              &DeviceVkTest::constructQueueFromFlags,
               &DeviceVkTest::constructExtensions,
               &DeviceVkTest::constructTransferDeviceProperties});
 
@@ -244,9 +247,14 @@ void DeviceVkTest::createInfoWrongQueueOutputCount() {
 }
 
 void DeviceVkTest::createInfoRvalue() {
+    /* Verify that there actually are graphics queues so we don't exit inside
+       addQueues() */
+    CORRADE_VERIFY(pickDevice(instance()).tryPickQueueFamily(QueueFlag::Graphics));
+
     Float zero[1]{};
-    Queue a{NoCreate}, b{NoCreate};
-    Containers::Reference<Queue> reference[1]{a};
+    Queue a{NoCreate}, b{NoCreate}, c{NoCreate}, d{NoCreate};
+    Containers::Reference<Queue> referenceA[1]{a};
+    Containers::Reference<Queue> referenceC[1]{c};
 
     VkDeviceQueueCreateInfo rawQueueInfo{};
     rawQueueInfo.sType = VK_STRUCTURE_TYPE_DEVICE_QUEUE_CREATE_INFO;
@@ -260,8 +268,10 @@ void DeviceVkTest::createInfoRvalue() {
         .addEnabledExtensions(Containers::ArrayView<const Extension>{})
         .addEnabledExtensions(std::initializer_list<Extension>{})
         .addEnabledExtensions<>()
-        .addQueues(0, zero, reference)
+        .addQueues(0, zero, referenceA)
         .addQueues(0, {0.0f}, {b})
+        .addQueues(QueueFlag::Graphics, zero, referenceC)
+        .addQueues(QueueFlag::Graphics, {0.0f}, {d})
         .addQueues(rawQueueInfo);
 
     /* Just to test something, main point is that the above compiles, links and
@@ -306,6 +316,21 @@ void DeviceVkTest::construct() {
     CORRADE_VERIFY(true);
 }
 
+void DeviceVkTest::constructQueueFromFlags() {
+    DeviceProperties deviceProperties = pickDevice(instance());
+
+    /* Verify that there actually are graphics queues so we don't exit after */
+    CORRADE_VERIFY(deviceProperties.tryPickQueueFamily(QueueFlag::Graphics));
+
+    Queue queue{NoCreate};
+    Device device{instance(), DeviceCreateInfo{deviceProperties}
+        .addQueues(QueueFlag::Graphics, {0.0f}, {queue})};
+    CORRADE_VERIFY(device.handle());
+
+    /* The queue should be filled in like usual */
+    CORRADE_VERIFY(queue.handle());
+}
+
 void DeviceVkTest::constructExtensions() {
     if(std::getenv("MAGNUM_DISABLE_EXTENSIONS"))
         CORRADE_SKIP("Can't test with the MAGNUM_DISABLE_EXTENSIONS environment variable set");