From 0825db39ab3e0820cc1b22423ceed313fcf619d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 12 Jan 2021 19:07:57 +0100 Subject: [PATCH] Vk: rethink render pass creation to make it clearer (but more verbose). Originally I wanted to save people typing and provide "reasonable defaults" for the image layout, however shortly after I realized I can't express even the simplest case in the Vulkan Triangle example with those. Furthermore the case of having default load/store operations are so rare that it isn't worth an extra API which tricks the user into thinking the clear/discard operations are meant to be done somewhere else. I realized this because it was actually harder to explain these in a second step than introducing them right from the beginning. Finally, because there's so many different structures to fill, the implicit constructors weren't a good idea either, as it again tricked users to think just PixelFormat or just an index is the parameter, with nothing else, and then left them wondering where all the other important params go. Same with {} used instead AttachmentLoadOperation::Load / AttachmentStoreOperation::Store, unnecessary confusion there so don't even suggest that. --- doc/snippets/MagnumVk.cpp | 61 +++----- src/Magnum/Vk/Image.h | 12 +- src/Magnum/Vk/RenderPass.cpp | 12 -- src/Magnum/Vk/RenderPass.h | 66 ++++----- src/Magnum/Vk/RenderPassCreateInfo.h | 92 +++--------- src/Magnum/Vk/Test/FramebufferVkTest.cpp | 44 ++++-- src/Magnum/Vk/Test/RenderPassTest.cpp | 169 +++++++++++------------ src/Magnum/Vk/Test/RenderPassVkTest.cpp | 54 ++++++-- 8 files changed, 242 insertions(+), 268 deletions(-) diff --git a/doc/snippets/MagnumVk.cpp b/doc/snippets/MagnumVk.cpp index 6c1f4e387..c2504b555 100644 --- a/doc/snippets/MagnumVk.cpp +++ b/doc/snippets/MagnumVk.cpp @@ -403,8 +403,8 @@ Vk::ImageView depthView{device, Vk::ImageViewCreateInfo2D{depth}}; Vk::RenderPass renderPass{device, Vk::RenderPassCreateInfo{} /* created before */ .setAttachments({ - color.format(), - depth.format() + Vk::AttachmentDescription{color.format(), DOXYGEN_IGNORE({}, {}, {}, {})}, + Vk::AttachmentDescription{depth.format(), DOXYGEN_IGNORE({}, {}, {}, {})}, }) DOXYGEN_IGNORE() }; @@ -621,7 +621,7 @@ indices.bindMemory(memory, indicesOffset); } { -Vk::Device device{DOXYGEN_IGNORE(NoCreate)}; +Vk::Device device{NoCreate}; /* The include should be a no-op here since it was already included above */ /* [RenderPass-creation] */ #include @@ -630,52 +630,29 @@ DOXYGEN_IGNORE() Vk::RenderPass renderPass{device, Vk::RenderPassCreateInfo{} .setAttachments({ - Vk::PixelFormat::RGBA8Srgb, - Vk::PixelFormat::Depth24UnormStencil8UI + Vk::AttachmentDescription{Vk::PixelFormat::RGBA8Srgb, + Vk::AttachmentLoadOperation::Clear, + Vk::AttachmentStoreOperation::Store, + Vk::ImageLayout::Undefined, + Vk::ImageLayout::ColorAttachment}, + Vk::AttachmentDescription{Vk::PixelFormat::Depth24UnormStencil8UI, + Vk::AttachmentLoadOperation::Clear, + Vk::AttachmentStoreOperation::DontCare, + Vk::ImageLayout::Undefined, + Vk::ImageLayout::DepthStencilAttachment} }) .addSubpass(Vk::SubpassDescription{} - .setColorAttachments({0}) - .setDepthStencilAttachment(1) + .setColorAttachments({ + Vk::AttachmentReference{0, Vk::ImageLayout::ColorAttachment} + }) + .setDepthStencilAttachment( + Vk::AttachmentReference{1, Vk::ImageLayout::DepthStencilAttachment} + ) ) }; /* [RenderPass-creation] */ } -{ -Vk::Device device{DOXYGEN_IGNORE(NoCreate)}; -/* [RenderPass-creation-load-store] */ -Vk::RenderPass renderPass{device, Vk::RenderPassCreateInfo{} - .setAttachments({ - {Vk::PixelFormat::RGBA8Srgb, Vk::AttachmentLoadOperation::Clear, {}}, - {Vk::PixelFormat::Depth24UnormStencil8UI, Vk::AttachmentLoadOperation::Clear, {}}, - }) - DOXYGEN_IGNORE() -}; -/* [RenderPass-creation-load-store] */ -} - -{ -Vk::Device device{DOXYGEN_IGNORE(NoCreate)}; -/* [RenderPass-creation-layout] */ -Vk::RenderPass renderPass{device, Vk::RenderPassCreateInfo{} - .setAttachments({ - {Vk::PixelFormat::RGBA8Srgb, - Vk::AttachmentLoadOperation::Clear, {}, - Vk::ImageLayout::ColorAttachment, - Vk::ImageLayout::ColorAttachment}, - {Vk::PixelFormat::Depth24UnormStencil8UI, - Vk::AttachmentLoadOperation::Clear, {}, - Vk::ImageLayout::DepthStencilAttachment, - Vk::ImageLayout::DepthStencilAttachment}, - }) - .addSubpass(Vk::SubpassDescription{} - .setColorAttachments({{0, Vk::ImageLayout::ColorAttachment}}) - .setDepthStencilAttachment({1, Vk::ImageLayout::ColorAttachment}) - ) -}; -/* [RenderPass-creation-layout] */ -} - { Vk::Device device{DOXYGEN_IGNORE(NoCreate)}; /* The include should be a no-op here since it was already included above */ diff --git a/src/Magnum/Vk/Image.h b/src/Magnum/Vk/Image.h index 35534ec7a..7e8d01502 100644 --- a/src/Magnum/Vk/Image.h +++ b/src/Magnum/Vk/Image.h @@ -49,9 +49,11 @@ namespace Implementation { struct DeviceState; } */ enum class ImageLayout: Int { /** - * Undefined. Can only be used as the initial layout in - * @ref ImageCreateInfo structures (and there it's the default). Images in - * this layout are not accessible by the device, the image has to be + * Undefined. Can be used as the initial layout in @ref ImageCreateInfo + * structures (and there it's the default) and as the initial layout in + * render pass @ref AttachmentDescription (in which case it tells the + * driver that we don't care about the previous contents). Images in this + * layout are not accessible by the device, the image has to be * transitioned to a defined layout such as @ref ImageLayout::General * first; contents of the memory are not guaranteed to be preserved during * the transition. @@ -84,9 +86,7 @@ enum class ImageLayout: Int { Preinitialized = VK_IMAGE_LAYOUT_PREINITIALIZED, /** - * General layout, supports all types of device access. This is the - * conservative default used everywhere except the @ref ImageCreateInfo - * structures, which uses @ref ImageLayout::Undefined. + * General layout, supports all types of device access. * * @m_class{m-note m-success} * diff --git a/src/Magnum/Vk/RenderPass.cpp b/src/Magnum/Vk/RenderPass.cpp index 8bcfda4d2..c907eeacd 100644 --- a/src/Magnum/Vk/RenderPass.cpp +++ b/src/Magnum/Vk/RenderPass.cpp @@ -57,12 +57,6 @@ AttachmentDescription::AttachmentDescription(const Magnum::PixelFormat format, c AttachmentDescription::AttachmentDescription(const Magnum::CompressedPixelFormat format, const AttachmentLoadOperation loadOperation, const AttachmentStoreOperation storeOperation, const ImageLayout initialLayout, const ImageLayout finalLayout, const Int samples, const Flags flags): AttachmentDescription{pixelFormat(format), loadOperation, storeOperation, initialLayout, finalLayout, samples, flags} {} -AttachmentDescription::AttachmentDescription(const PixelFormat format, const AttachmentLoadOperation loadOperation, const AttachmentStoreOperation storeOperation, const Int samples, const Flags flags): AttachmentDescription{format, loadOperation, storeOperation, ImageLayout::General, ImageLayout::General, samples, flags} {} - -AttachmentDescription::AttachmentDescription(const Magnum::PixelFormat format, const AttachmentLoadOperation loadOperation, const AttachmentStoreOperation storeOperation, const Int samples, const Flags flags): AttachmentDescription{pixelFormat(format), loadOperation, storeOperation, samples, flags} {} - -AttachmentDescription::AttachmentDescription(const Magnum::CompressedPixelFormat format, const AttachmentLoadOperation loadOperation, const AttachmentStoreOperation storeOperation, const Int samples, const Flags flags): AttachmentDescription{pixelFormat(format), loadOperation, storeOperation, samples, flags} {} - AttachmentDescription::AttachmentDescription(const PixelFormat format, const std::pair depthStencilLoadOperation, const std::pair depthStencilStoreOperation, const ImageLayout initialLayout, const ImageLayout finalLayout, const Int samples, const Flags flags): _description{} { _description.sType = VK_STRUCTURE_TYPE_ATTACHMENT_DESCRIPTION_2; _description.flags = VkAttachmentDescriptionFlags(flags); @@ -80,12 +74,6 @@ AttachmentDescription::AttachmentDescription(const Magnum::PixelFormat format, c AttachmentDescription::AttachmentDescription(const Magnum::CompressedPixelFormat format, const std::pair depthStencilLoadOperation, const std::pair depthStencilStoreOperation, const ImageLayout initialLayout, const ImageLayout finalLayout, const Int samples, const Flags flags): AttachmentDescription{pixelFormat(format), depthStencilLoadOperation, depthStencilStoreOperation, initialLayout, finalLayout, samples, flags} {} -AttachmentDescription::AttachmentDescription(const PixelFormat format, const std::pair depthStencilLoadOperation, const std::pair depthStencilStoreOperation, const Int samples, const Flags flags): AttachmentDescription{format, depthStencilLoadOperation, depthStencilStoreOperation, ImageLayout::General, ImageLayout::General, samples, flags} {} - -AttachmentDescription::AttachmentDescription(const Magnum::PixelFormat format, const std::pair depthStencilLoadOperation, const std::pair depthStencilStoreOperation, const Int samples, const Flags flags): AttachmentDescription{pixelFormat(format), depthStencilLoadOperation, depthStencilStoreOperation, samples, flags} {} - -AttachmentDescription::AttachmentDescription(const Magnum::CompressedPixelFormat format, const std::pair depthStencilLoadOperation, const std::pair depthStencilStoreOperation, const Int samples, const Flags flags): AttachmentDescription{pixelFormat(format), depthStencilLoadOperation, depthStencilStoreOperation, samples, flags} {} - AttachmentDescription::AttachmentDescription(NoInitT) noexcept {} AttachmentDescription::AttachmentDescription(const VkAttachmentDescription2& description): diff --git a/src/Magnum/Vk/RenderPass.h b/src/Magnum/Vk/RenderPass.h index 54b84f9ea..c0763eb56 100644 --- a/src/Magnum/Vk/RenderPass.h +++ b/src/Magnum/Vk/RenderPass.h @@ -53,45 +53,45 @@ connected together in a @ref Framebuffer. @section Vk-RenderPass-creation Render pass creation -@ref RenderPassCreateInfo is a set of attachments, described by -@ref AttachmentDescription instances, subpasses operating on those attachments, -described by a @ref SubpassDescription using @ref AttachmentReference -instances, and subpass dependencies, described by @ref SubpassDependency. +A @ref RenderPassCreateInfo consists of: + +- a set of attachments, described by @ref AttachmentDescription instances, +- subpasses operating on those attachments, described by a + @ref SubpassDescription using @ref AttachmentReference instances, +- and subpass dependencies, described by @ref SubpassDependency. A render pass has to have at least one subpass. It's common to have just one -subpass but while the subpass isn't required to operate on any attachments, +subpass, but while the subpass isn't required to operate on any attachments, such case is rather rare. Following is a simple setup for one subpass operating -on a color and a combined depth/stencil attachment. The main parameter an -@ref AttachmentDescription needs is attachment format; the numbers passed to -@ref SubpassDescription::setColorAttachments() and -@ref SubpassDescription::setDepthStencilAttachment() are indices into the -@ref RenderPassCreateInfo::setAttachments() array, and it's actually -@ref AttachmentReference instances: +on a color and a combined depth/stencil attachment. Each +@ref AttachmentDescription describes a concrete attachment and particular +@ref SubpassDescription::setColorAttachments() / +@ref SubpassDescription::setDepthStencilAttachment() "setColorAttachments()" +then reference it by an index. + +In addition to a format, each @ref AttachmentDescription has to specify an +@ref AttachmentLoadOperation that happens at the render pass begin --- whether +we want to preserve the contents, clear them or discard --- and similarly an +@ref AttachmentStoreOperation at the end. Then, the @ref ImageLayout parameters +specify what implicit layout transitions (if any) need to happen between start +of the render pass and the first subpass, between subpasses and between last +subpass and end of the render pass. @snippet MagnumVk.cpp RenderPass-creation -The above again does a conservative estimate that you'd want to preserve the -attachment contents between render passes. Usually you'd want to clear the -framebuffer first instead of reusing its previous contents, which is done by -passing appropriate @ref AttachmentLoadOperation / -@ref AttachmentStoreOperation to the @ref AttachmentDescription constructor. -@ref AttachmentLoadOperation::Load and @ref AttachmentStoreOperation::Store are -conveniently the zero values, which means you can use @cpp {} @ce instead of -typing them out in full: - -@snippet MagnumVk.cpp RenderPass-creation-load-store - -Vulkan makes heavy use of image layouts for optimal memory access -and in all the cases above, @ref ImageLayout::General is used as an implicit -conservative layout. It's guaranteed to work for all device access, but it -might not always be optimal. A complete description of image layouts and their -use is out of scope of this reference, but for example, if the attached images -would be always only used as a render target, the above setup could be made -more optimal by explicitly specifying both a concrete initial and final layout -in the @ref AttachmentDescription constructors and in -each @link AttachmentReference @endlink: - -@snippet MagnumVk.cpp RenderPass-creation-layout + + +@m_class{m-note m-info} + +@par + @ref ImageLayout::Undefined also doubles as a way of saying "transition + from whatever layout is there" --- when we render to an attachment for the + first time, it will most probably be in @ref ImageLayout::Undefined, but + subsequently it won't anymore and instead be + @ref ImageLayout::ColorAttachment, + @ref ImageLayout::DepthStencilAttachment "DepthStencilAttachment" or just + any other. With @ref ImageLayout::Undefined we don't need to explicitly + handle that case. */ class MAGNUM_VK_EXPORT RenderPass { public: diff --git a/src/Magnum/Vk/RenderPassCreateInfo.h b/src/Magnum/Vk/RenderPassCreateInfo.h index 723555568..c80f51b7a 100644 --- a/src/Magnum/Vk/RenderPassCreateInfo.h +++ b/src/Magnum/Vk/RenderPassCreateInfo.h @@ -52,14 +52,7 @@ subpass. @m_enum_values_as_keywords */ enum class AttachmentLoadOperation: Int { - /** - * Previous contents are preserved. This is the conservative default when - * using the @ref AttachmentDescription::AttachmentDescription(PixelFormat, Int, Flags) - * constructor. - * - * This value is also guaranteed to be @cpp 0 @ce, which means you're - * encouraged to simply use @cpp {} @ce in function calls and elsewhere. - */ + /** Previous contents are preserved. */ Load = VK_ATTACHMENT_LOAD_OP_LOAD, /** @@ -103,14 +96,7 @@ treated at the end of a subpass. @m_enum_values_as_keywords */ enum class AttachmentStoreOperation: Int { - /** - * Generated contents are written to memory. This is the conservative - * default when using the @ref AttachmentDescription::AttachmentDescription(PixelFormat, Int, Flags) - * constructor. - * - * This value is also guaranteed to be @cpp 0 @ce, which means you're - * encouraged to simply use @cpp {} @ce in function calls and elsewhere. - */ + /** Generated contents are written to memory. */ Store = VK_ATTACHMENT_STORE_OP_STORE, /** @@ -215,24 +201,13 @@ class MAGNUM_VK_EXPORT AttachmentDescription { * See also @ref AttachmentDescription(PixelFormat, std::pair, std::pair, ImageLayout, ImageLayout, Int, Flags) * for a constructing a combined depth/stencil attachment description. */ - /*implicit*/ AttachmentDescription(PixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); + /* These were implicit at first, but I realized code gets way too + confusing with all the {{}} so it's not anymore */ + explicit AttachmentDescription(PixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::PixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); + explicit AttachmentDescription(Magnum::PixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::CompressedPixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); - - /** - * @brief Construct with implicit conservative layout - * - * Equivalent to calling @ref AttachmentDescription(PixelFormat, AttachmentLoadOperation, AttachmentStoreOperation, ImageLayout, ImageLayout, Int, Flags) - * with both @p initialLayout and @p finalLayout set to - * @ref ImageLayout::General. - */ - /*implicit*/ AttachmentDescription(PixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, Int samples = 1, Flags flags = {}); - /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::PixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, Int samples = 1, Flags flags = {}); - /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::CompressedPixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, Int samples = 1, Flags flags = {}); + explicit AttachmentDescription(Magnum::CompressedPixelFormat format, AttachmentLoadOperation loadOperation, AttachmentStoreOperation storeOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); /** * @brief Construct for a combined depth/stencil attachment @@ -254,8 +229,10 @@ class MAGNUM_VK_EXPORT AttachmentDescription { * @param samples Sample count * @param flags Attachment description flags * - * The following @type_vk{AttachmentDescription} fields are pre-filled - * in addition to `sType`, everything else is zero-filled: + * Compared to @ref AttachmentDescription(PixelFormat, AttachmentLoadOperation, AttachmentStoreOperation, ImageLayout, ImageLayout, Int, Flags) + * allows you to specify different load/store operation for depth and + * stencil. The following @type_vk{AttachmentDescription2} fields are + * pre-filled in addition to `sType`, everything else is zero-filled: * * - `flags` * - `format` @@ -268,38 +245,13 @@ class MAGNUM_VK_EXPORT AttachmentDescription { * @todo Implement @vk_extension{KHR,separate_depth_stencil_layouts} * and provide a pair of layouts as well */ - /*implicit*/ AttachmentDescription(PixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); - /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::PixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); - /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::CompressedPixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); - - /** - * @brief Construct for a combined depth/stencil attachment with implicit conservative layout - * - * Equivalent to calling @ref AttachmentDescription(PixelFormat, std::pair, std::pair, ImageLayout, ImageLayout, Int, Flags) - * with both @p initialLayout and @p finalLayout set to - * @ref ImageLayout::General. - */ - /*implicit*/ AttachmentDescription(PixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, Int samples = 1, Flags flags = {}); - /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::PixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, Int samples = 1, Flags flags = {}); - /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::CompressedPixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, Int samples = 1, Flags flags = {}); - - /** - * @brief Construct with implicit conservative load/store operation and layout - * - * Equivalent to calling @ref AttachmentDescription(PixelFormat, std::pair, std::pair, ImageLayout, ImageLayout, Int, Flags) - * with @ref AttachmentLoadOperation::Load and - * @ref AttachmentStoreOperation::Store and both @p initialLayout and - * @p finalLayout set to @ref ImageLayout::General. - */ - /*implicit*/ AttachmentDescription(PixelFormat format, Int samples = 1, Flags flags = {}): AttachmentDescription{format, AttachmentLoadOperation{}, AttachmentStoreOperation{}, samples, flags} {} + /* These were implicit at first, but I realized code gets way too + confusing with all the {{}} so it's not anymore */ + explicit AttachmentDescription(PixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::PixelFormat format, Int samples = 1, Flags flags = {}): AttachmentDescription{format, AttachmentLoadOperation{}, AttachmentStoreOperation{}, samples, flags} {} + explicit AttachmentDescription(Magnum::PixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); /** @overload */ - /*implicit*/ AttachmentDescription(Magnum::CompressedPixelFormat format, Int samples = 1, Flags flags = {}): AttachmentDescription{format, AttachmentLoadOperation{}, AttachmentStoreOperation{}, samples, flags} {} + explicit AttachmentDescription(Magnum::CompressedPixelFormat format, std::pair depthStencilLoadOperation, std::pair depthStencilStoreOperation, ImageLayout initialLayout, ImageLayout finalLayout, Int samples = 1, Flags flags = {}); /** * @brief Construct without initializing the contents @@ -412,14 +364,10 @@ class MAGNUM_VK_EXPORT AttachmentReference { * - `attachment` * - `layout` */ - /*implicit*/ AttachmentReference(UnsignedInt attachment, ImageLayout layout = - #ifdef DOXYGEN_GENERATING_OUTPUT - /* To avoid Image.h dependency */ - ImageLayout::General - #else - ImageLayout(VK_IMAGE_LAYOUT_GENERAL) - #endif - ); + /* This was implicit at first, but I realized code gets way too + confusing with all the {{}} especially with setColorAttachments() + that takes two lists so it's not anymore */ + explicit AttachmentReference(UnsignedInt attachment, ImageLayout layout); /** * @brief Construct with no attachment diff --git a/src/Magnum/Vk/Test/FramebufferVkTest.cpp b/src/Magnum/Vk/Test/FramebufferVkTest.cpp index c2ff4c37b..53ae441c7 100644 --- a/src/Magnum/Vk/Test/FramebufferVkTest.cpp +++ b/src/Magnum/Vk/Test/FramebufferVkTest.cpp @@ -63,12 +63,24 @@ void FramebufferVkTest::construct() { RenderPass renderPass{device(), RenderPassCreateInfo{} .setAttachments({ - color.format(), - depth.format() + AttachmentDescription{color.format(), + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::ColorAttachment}, + AttachmentDescription{depth.format(), + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::DepthStencilAttachment} }) .addSubpass(SubpassDescription{} - .setColorAttachments({0}) - .setDepthStencilAttachment(1) + .setColorAttachments({ + AttachmentReference{0, ImageLayout::ColorAttachment} + }) + .setDepthStencilAttachment( + AttachmentReference{1, ImageLayout::DepthStencilAttachment} + ) ) }; @@ -91,8 +103,16 @@ void FramebufferVkTest::constructMove() { PixelFormat::RGBA8Unorm, {256, 256}, 1}, MemoryFlag::DeviceLocal}; ImageView colorView{device(), ImageViewCreateInfo2D{color}}; RenderPass renderPass{device(), RenderPassCreateInfo{} - .setAttachments({color.format()}) - .addSubpass(SubpassDescription{}.setColorAttachments({0})) + .setAttachments({ + AttachmentDescription{color.format(), + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::ColorAttachment} + }) + .addSubpass(SubpassDescription{}.setColorAttachments({ + AttachmentReference{0, ImageLayout::ColorAttachment} + })) }; Framebuffer a{device(), FramebufferCreateInfo{renderPass, { @@ -123,8 +143,16 @@ void FramebufferVkTest::wrap() { PixelFormat::RGBA8Unorm, {256, 256}, 1}, MemoryFlag::DeviceLocal}; ImageView colorView{device(), ImageViewCreateInfo2D{color}}; RenderPass renderPass{device(), RenderPassCreateInfo{} - .setAttachments({color.format()}) - .addSubpass(SubpassDescription{}.setColorAttachments({0})) + .setAttachments({ + AttachmentDescription{color.format(), + AttachmentLoadOperation::Load, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::ColorAttachment} + }) + .addSubpass(SubpassDescription{}.setColorAttachments({ + AttachmentReference{0, ImageLayout::ColorAttachment} + })) }; VkFramebuffer framebuffer{}; diff --git a/src/Magnum/Vk/Test/RenderPassTest.cpp b/src/Magnum/Vk/Test/RenderPassTest.cpp index 220a1ad25..887de1480 100644 --- a/src/Magnum/Vk/Test/RenderPassTest.cpp +++ b/src/Magnum/Vk/Test/RenderPassTest.cpp @@ -52,10 +52,7 @@ struct RenderPassTest: TestSuite::Tester { structures. */ template void attachmentDescriptionConstruct(); - template void attachmentDescriptionConstructImplicitLayout(); template void attachmentDescriptionConstructDepthStencil(); - template void attachmentDescriptionConstructDepthStencilImplicitLayout(); - template void attachmentDescriptionConstructImplicitLoadStoreLayout(); void attachmentDescriptionConstructNoInit(); template void attachmentDescriptionConstructFromVk(); template void attachmentDescriptionConvertToVk(); @@ -122,18 +119,9 @@ RenderPassTest::RenderPassTest() { addTests({&RenderPassTest::attachmentDescriptionConstruct, &RenderPassTest::attachmentDescriptionConstruct, &RenderPassTest::attachmentDescriptionConstruct, - &RenderPassTest::attachmentDescriptionConstructImplicitLayout, - &RenderPassTest::attachmentDescriptionConstructImplicitLayout, - &RenderPassTest::attachmentDescriptionConstructImplicitLayout, &RenderPassTest::attachmentDescriptionConstructDepthStencil, &RenderPassTest::attachmentDescriptionConstructDepthStencil, &RenderPassTest::attachmentDescriptionConstructDepthStencil, - &RenderPassTest::attachmentDescriptionConstructDepthStencilImplicitLayout, - &RenderPassTest::attachmentDescriptionConstructDepthStencilImplicitLayout, - &RenderPassTest::attachmentDescriptionConstructDepthStencilImplicitLayout, - &RenderPassTest::attachmentDescriptionConstructImplicitLoadStoreLayout, - &RenderPassTest::attachmentDescriptionConstructImplicitLoadStoreLayout, - &RenderPassTest::attachmentDescriptionConstructImplicitLoadStoreLayout, &RenderPassTest::attachmentDescriptionConstructNoInit, &RenderPassTest::attachmentDescriptionConstructFromVk, &RenderPassTest::attachmentDescriptionConstructFromVk, @@ -272,23 +260,6 @@ template void RenderPassTest::attachmentDescriptionConstruct() { CORRADE_COMPARE(description->finalLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); } -template void RenderPassTest::attachmentDescriptionConstructImplicitLayout() { - setTestCaseTemplateName(PixelFormatTraits::name()); - - AttachmentDescription description{PixelFormatTraits::format(), - AttachmentLoadOperation::Clear, AttachmentStoreOperation::DontCare, - 4, AttachmentDescription::Flag::MayAlias}; - CORRADE_COMPARE(description->flags, VK_ATTACHMENT_DESCRIPTION_MAY_ALIAS_BIT); - CORRADE_COMPARE(description->format, PixelFormatTraits::expected()); - CORRADE_COMPARE(description->samples, VK_SAMPLE_COUNT_4_BIT); - CORRADE_COMPARE(description->loadOp, VK_ATTACHMENT_LOAD_OP_CLEAR); - CORRADE_COMPARE(description->stencilLoadOp, VK_ATTACHMENT_LOAD_OP_LOAD); - CORRADE_COMPARE(description->storeOp, VK_ATTACHMENT_STORE_OP_DONT_CARE); - CORRADE_COMPARE(description->stencilStoreOp, VK_ATTACHMENT_STORE_OP_STORE); - CORRADE_COMPARE(description->initialLayout, VK_IMAGE_LAYOUT_GENERAL); - CORRADE_COMPARE(description->finalLayout, VK_IMAGE_LAYOUT_GENERAL); -} - template void RenderPassTest::attachmentDescriptionConstructDepthStencil() { setTestCaseTemplateName(PixelFormatTraits::name()); @@ -308,40 +279,6 @@ template void RenderPassTest::attachmentDescriptionConstructDepthStenci CORRADE_COMPARE(description->finalLayout, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); } -template void RenderPassTest::attachmentDescriptionConstructDepthStencilImplicitLayout() { - setTestCaseTemplateName(PixelFormatTraits::name()); - - AttachmentDescription description{PixelFormatTraits::format(), - {AttachmentLoadOperation::Clear, AttachmentLoadOperation::DontCare}, - {AttachmentStoreOperation::Store, AttachmentStoreOperation::DontCare}, - 4, AttachmentDescription::Flag::MayAlias}; - CORRADE_COMPARE(description->flags, VK_ATTACHMENT_DESCRIPTION_MAY_ALIAS_BIT); - CORRADE_COMPARE(description->format, PixelFormatTraits::expected()); - CORRADE_COMPARE(description->samples, VK_SAMPLE_COUNT_4_BIT); - CORRADE_COMPARE(description->loadOp, VK_ATTACHMENT_LOAD_OP_CLEAR); - CORRADE_COMPARE(description->stencilLoadOp, VK_ATTACHMENT_LOAD_OP_DONT_CARE); - CORRADE_COMPARE(description->storeOp, VK_ATTACHMENT_STORE_OP_STORE); - CORRADE_COMPARE(description->stencilStoreOp, VK_ATTACHMENT_STORE_OP_DONT_CARE); - CORRADE_COMPARE(description->initialLayout, VK_IMAGE_LAYOUT_GENERAL); - CORRADE_COMPARE(description->finalLayout, VK_IMAGE_LAYOUT_GENERAL); -} - -template void RenderPassTest::attachmentDescriptionConstructImplicitLoadStoreLayout() { - setTestCaseTemplateName(PixelFormatTraits::name()); - - AttachmentDescription description{PixelFormatTraits::format(), - 4, AttachmentDescription::Flag::MayAlias}; - CORRADE_COMPARE(description->flags, VK_ATTACHMENT_DESCRIPTION_MAY_ALIAS_BIT); - CORRADE_COMPARE(description->format, PixelFormatTraits::expected()); - CORRADE_COMPARE(description->samples, VK_SAMPLE_COUNT_4_BIT); - CORRADE_COMPARE(description->loadOp, VK_ATTACHMENT_LOAD_OP_LOAD); - CORRADE_COMPARE(description->stencilLoadOp, VK_ATTACHMENT_LOAD_OP_LOAD); - CORRADE_COMPARE(description->storeOp, VK_ATTACHMENT_STORE_OP_STORE); - CORRADE_COMPARE(description->stencilStoreOp, VK_ATTACHMENT_STORE_OP_STORE); - CORRADE_COMPARE(description->initialLayout, VK_IMAGE_LAYOUT_GENERAL); - CORRADE_COMPARE(description->finalLayout, VK_IMAGE_LAYOUT_GENERAL); -} - void RenderPassTest::attachmentDescriptionConstructNoInit() { AttachmentDescription description{NoInit}; description->sType = VK_STRUCTURE_TYPE_FORMAT_PROPERTIES_2; @@ -475,17 +412,25 @@ void RenderPassTest::subpassDescriptionConstructNoInit() { void RenderPassTest::subpassDescriptionConstructInputAttachments() { SubpassDescription description; - description.setInputAttachments({15, {}, 2}); + description.setInputAttachments({ + AttachmentReference{15, ImageLayout::ShaderReadOnly}, + {}, + AttachmentReference{2, ImageLayout::ShaderReadOnly} + }); CORRADE_COMPARE(description->inputAttachmentCount, 3); CORRADE_VERIFY(description->pInputAttachments); CORRADE_COMPARE(description->pInputAttachments[0].attachment, 15); + CORRADE_COMPARE(description->pInputAttachments[0].layout, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); CORRADE_COMPARE(description->pInputAttachments[1].attachment, VK_ATTACHMENT_UNUSED); CORRADE_COMPARE(description->pInputAttachments[2].attachment, 2); } void RenderPassTest::subpassDescriptionConstructColorAttachments() { SubpassDescription description; - description.setColorAttachments({{}, 23}); + description.setColorAttachments({ + {}, + AttachmentReference{23, ImageLayout::ColorAttachment} + }); CORRADE_COMPARE(description->colorAttachmentCount, 2); CORRADE_VERIFY(description->pColorAttachments); CORRADE_VERIFY(!description->pResolveAttachments); @@ -495,7 +440,13 @@ void RenderPassTest::subpassDescriptionConstructColorAttachments() { void RenderPassTest::subpassDescriptionConstructColorResolveAttachments() { SubpassDescription description; - description.setColorAttachments({{}, 23}, {1, 0}); + description.setColorAttachments({ + {}, + AttachmentReference{23, ImageLayout::ColorAttachment} + }, { + AttachmentReference{1, ImageLayout::ColorAttachment}, + AttachmentReference{0, ImageLayout::ColorAttachment} + }); CORRADE_COMPARE(description->colorAttachmentCount, 2); CORRADE_VERIFY(description->pColorAttachments); @@ -515,14 +466,16 @@ void RenderPassTest::subpassDescriptionConstructColorResolveAttachmentsWrongCoun std::ostringstream out; Error redirectError{&out}; - description.setColorAttachments({0, 1}, {2, 3, 5}); + description.setColorAttachments({{}, {}}, {{}, {}, {}}); CORRADE_COMPARE(out.str(), "Vk::SubpassDescription::setColorAttachments(): resolve attachments expected to be either empty or have a size of 2 but got 3\n"); } void RenderPassTest::subpassDescriptionConstructDepthStencilAttachment() { SubpassDescription description; - description.setDepthStencilAttachment(11); + description.setDepthStencilAttachment( + AttachmentReference{11, ImageLayout::DepthStencilAttachment} + ); CORRADE_VERIFY(description->pDepthStencilAttachment); CORRADE_COMPARE(description->pDepthStencilAttachment->attachment, 11); } @@ -633,7 +586,10 @@ void RenderPassTest::subpassDescriptionConstructCopy() { void RenderPassTest::subpassDescriptionConstructMove() { SubpassDescription a; - a.setInputAttachments({24, 35}); + a.setInputAttachments({ + AttachmentReference{24, ImageLayout::ShaderReadOnly}, + AttachmentReference{35, ImageLayout::ShaderReadOnly} + }); CORRADE_COMPARE(a->inputAttachmentCount, 2); CORRADE_COMPARE(a->pInputAttachments[1].attachment, 35); @@ -659,10 +615,20 @@ template void RenderPassTest::subpassDescriptionConvertToVk() { SubpassDescription description{}; description.setInputAttachments({ - 24, {35, ImageLayout::ShaderReadOnly}, 17 + AttachmentReference{24, ImageLayout::ShaderReadOnly}, + AttachmentReference{35, ImageLayout::ShaderReadOnly}, + AttachmentReference{17, ImageLayout::ShaderReadOnly}, + }) + .setColorAttachments({ + AttachmentReference{1, ImageLayout::ColorAttachment}, + AttachmentReference{3, ImageLayout::ColorAttachment}, + }, { + AttachmentReference{25, ImageLayout::ColorAttachment}, + AttachmentReference{12, ImageLayout::ColorAttachment} }) - .setColorAttachments({1, 3}, {{25, ImageLayout::ColorAttachment}, 12}) - .setDepthStencilAttachment({5, ImageLayout::DepthStencilAttachment}) + .setDepthStencilAttachment( + AttachmentReference{5, ImageLayout::DepthStencilAttachment} + ) .setPreserveAttachments({0, 15, 23, 17}); Containers::Array array = Traits::convert(description); @@ -720,7 +686,10 @@ template void RenderPassTest::subpassDescriptionConvertToVkNoResolveAtt setTestCaseTemplateName(Traits::name()); SubpassDescription description{}; - description.setColorAttachments({1, 3}); + description.setColorAttachments({ + AttachmentReference{1, ImageLayout::ColorAttachment}, + AttachmentReference{3, ImageLayout::ColorAttachment} + }); Containers::Array array = Traits::convert(description); const T& to = array[0]; @@ -818,14 +787,23 @@ void RenderPassTest::createInfoConstructNoInit() { void RenderPassTest::createInfoConstructAttachments() { RenderPassCreateInfo info; info.setAttachments({ - {PixelFormat::RGBA16F, AttachmentLoadOperation::Clear, AttachmentStoreOperation::DontCare}, - {PixelFormat::RGB8Snorm, 4} + AttachmentDescription{PixelFormat::RGBA16F, + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::ColorAttachment}, + AttachmentDescription{PixelFormat::RGB8Snorm, + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::General, + 4} }); CORRADE_COMPARE(info->attachmentCount, 2); CORRADE_VERIFY(info->pAttachments); CORRADE_COMPARE(info->pAttachments[0].format, VK_FORMAT_R16G16B16A16_SFLOAT); - CORRADE_COMPARE(info->pAttachments[0].loadOp, VK_ATTACHMENT_LOAD_OP_CLEAR); - CORRADE_COMPARE(info->pAttachments[0].storeOp, VK_ATTACHMENT_STORE_OP_DONT_CARE); + CORRADE_COMPARE(info->pAttachments[0].initialLayout, VK_IMAGE_LAYOUT_UNDEFINED); + CORRADE_COMPARE(info->pAttachments[0].finalLayout, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL); CORRADE_COMPARE(info->pAttachments[1].format, VK_FORMAT_R8G8B8_SNORM); CORRADE_COMPARE(info->pAttachments[1].samples, VK_SAMPLE_COUNT_4_BIT); } @@ -833,12 +811,19 @@ void RenderPassTest::createInfoConstructAttachments() { void RenderPassTest::createInfoConstructSubpasses() { RenderPassCreateInfo info; info.addSubpass(SubpassDescription{} - .setColorAttachments({15, 34, 1}) + .setColorAttachments({ + AttachmentReference{15, ImageLayout::ColorAttachment}, + AttachmentReference{34, ImageLayout::ColorAttachment}, + AttachmentReference{1, ImageLayout::ColorAttachment} + }) .setPreserveAttachments({22}) ); info.addSubpass(SubpassDescription{} - .setInputAttachments({17, {}}) - .setDepthStencilAttachment(1) + .setInputAttachments({ + AttachmentReference{17, ImageLayout::ShaderReadOnly}, + {} + }) + .setDepthStencilAttachment(AttachmentReference{1, ImageLayout::DepthStencilAttachment}) ); CORRADE_COMPARE(info->subpassCount, 2); CORRADE_VERIFY(info->pSubpasses); @@ -953,7 +938,14 @@ void RenderPassTest::createInfoConstructCopy() { void RenderPassTest::createInfoConstructMove() { RenderPassCreateInfo a; - a.setAttachments({PixelFormat::Depth32F, PixelFormat::RGB8Snorm}); + a.setAttachments({ + AttachmentDescription{PixelFormat::Depth32F, + {}, {}, + {}, {}}, + AttachmentDescription{PixelFormat::RGB8Snorm, + {}, {}, + {}, {}} + }); CORRADE_COMPARE(a->attachmentCount, 2); CORRADE_COMPARE(a->pAttachments[1].format, VK_FORMAT_R8G8B8_SNORM); @@ -980,11 +972,18 @@ template void RenderPassTest::createInfoConvertToVk() { RenderPassCreateInfo info; info.setAttachments({ - AttachmentDescription{PixelFormat::RGB16UI}, - AttachmentDescription{PixelFormat{}, {}, {AttachmentStoreOperation::Store, AttachmentStoreOperation::DontCare}} + AttachmentDescription{PixelFormat::RGB16UI, + AttachmentLoadOperation{}, AttachmentStoreOperation{}, + ImageLayout{}, ImageLayout{}}, + AttachmentDescription{PixelFormat{}, {}, {AttachmentStoreOperation::Store, AttachmentStoreOperation::DontCare}, ImageLayout{}, ImageLayout{}} }) - .addSubpass(SubpassDescription{}.setColorAttachments({1, {15, ImageLayout::ShaderReadOnly}})) - .addSubpass(SubpassDescription{}.setDepthStencilAttachment({15, ImageLayout::ShaderReadOnly})) + .addSubpass(SubpassDescription{}.setColorAttachments({ + AttachmentReference{1, ImageLayout{}}, + AttachmentReference{15, ImageLayout::ShaderReadOnly} + })) + .addSubpass(SubpassDescription{}.setDepthStencilAttachment( + AttachmentReference{15, ImageLayout::ShaderReadOnly} + )) .addSubpass(SubpassDescription{}.setPreserveAttachments({57})) .setDependencies({SubpassDependency{dependency}}); Containers::Array array = Traits::convert(info); diff --git a/src/Magnum/Vk/Test/RenderPassVkTest.cpp b/src/Magnum/Vk/Test/RenderPassVkTest.cpp index 699b7e0e2..6010b8125 100644 --- a/src/Magnum/Vk/Test/RenderPassVkTest.cpp +++ b/src/Magnum/Vk/Test/RenderPassVkTest.cpp @@ -70,8 +70,16 @@ RenderPassVkTest::RenderPassVkTest() { void RenderPassVkTest::construct() { { RenderPass renderPass{device(), RenderPassCreateInfo{} - .setAttachments({PixelFormat::RGBA8Unorm}) - .addSubpass(SubpassDescription{}.setColorAttachments({0})) + .setAttachments({ + AttachmentDescription{PixelFormat::RGBA8Unorm, + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::General} + }) + .addSubpass(SubpassDescription{}.setColorAttachments({ + AttachmentReference{0, ImageLayout::General} + })) }; CORRADE_VERIFY(renderPass.handle()); CORRADE_COMPARE(renderPass.handleFlags(), HandleFlag::DestroyOnDestruction); @@ -103,8 +111,16 @@ void RenderPassVkTest::constructSubpassNoAttachments() { void RenderPassVkTest::constructMove() { RenderPass a{device(), RenderPassCreateInfo{} - .setAttachments({PixelFormat::RGBA8Unorm}) - .addSubpass(SubpassDescription{}.setColorAttachments({0})) + .setAttachments({ + AttachmentDescription{PixelFormat::RGBA8Unorm, + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::ColorAttachment} + }) + .addSubpass(SubpassDescription{}.setColorAttachments({ + AttachmentReference{0, ImageLayout::ColorAttachment} + })) }; VkRenderPass handle = a.handle(); @@ -128,8 +144,16 @@ void RenderPassVkTest::wrap() { VkRenderPass renderPass{}; CORRADE_COMPARE(Result(device()->CreateRenderPass(device(), RenderPassCreateInfo{} - .setAttachments({PixelFormat::RGBA8Unorm}) - .addSubpass(SubpassDescription{}.setColorAttachments({0})) + .setAttachments({ + AttachmentDescription{PixelFormat::RGBA8Unorm, + AttachmentLoadOperation::Clear, + AttachmentStoreOperation::Store, + ImageLayout::Undefined, + ImageLayout::ColorAttachment} + }) + .addSubpass(SubpassDescription{}.setColorAttachments({ + AttachmentReference{0, ImageLayout::ColorAttachment} + })) .vkRenderPassCreateInfo(), nullptr, &renderPass)), Result::Success); @@ -160,12 +184,22 @@ void RenderPassVkTest::cmdBeginEnd() { RenderPass renderPass{device(), RenderPassCreateInfo{} .setAttachments({ - {color.format(), AttachmentLoadOperation::Clear, {}}, - {depth.format(), AttachmentLoadOperation::Clear, {}}, + AttachmentDescription{color.format(), + AttachmentLoadOperation::Clear, {}, + ImageLayout::Undefined, + ImageLayout::ColorAttachment}, + AttachmentDescription{depth.format(), + AttachmentLoadOperation::Clear, {}, + ImageLayout::Undefined, + ImageLayout::ColorAttachment}, }) .addSubpass(SubpassDescription{} - .setColorAttachments({0}) - .setDepthStencilAttachment(1) + .setColorAttachments({ + AttachmentReference{0, ImageLayout::ColorAttachment} + }) + .setDepthStencilAttachment( + AttachmentReference{1, ImageLayout::DepthStencilAttachment} + ) ) /* Further subpasses with no attachments so we can test nextSubpass() but don't need to specify subpass dependencies (which I have no idea