Vk: add assertions to prevent information loss in Vulkan 1.1 fallbacks.

Usually the validation layers should be expected to catch this, but in
this case we omit the pNext structure chain before it has a chance to be
caught by the layer, so the assert is needed.

src/Magnum/Vk/CommandBuffer.h

@@ -302,8 +302,8 @@ class MAGNUM_VK_EXPORT CommandBuffer {
          *
          * If Vulkan 1.2 is not supported and the
          * @vk_extension{KHR,create_renderpass2} extension is not enabled on
-         * the device, only the `contents` field from @p beginInfo is used to
-         * begin the render pass.
+         * the device, @p beginInfo has to have the `pNext` chain empty and
+         * only the `contents` field from it is used to begin the render pass.
          * @see @fn_vk_keyword{CmdBeginRenderPass2},
          *      @fn_vk_keyword{CmdBeginRenderPass}
          */
@@ -321,8 +321,9 @@ class MAGNUM_VK_EXPORT CommandBuffer {
          *
          * If Vulkan 1.2 is not supported and the
          * @vk_extension{KHR,create_renderpass2} extension is not enabled on
-         * the device, @p endInfo is ignored and only the `contents` field from
-         * @p beginInfo is used to begin the next subpass
+         * the device, both @p endInfo and @p beginInfo have to have the
+         * `pNext` chain empty and only the `contents` field from @p beginInfo
+         * is used to begin the next subpass.
          * @see @fn_vk_keyword{CmdNextSubpass2},
          *      @fn_vk_keyword{CmdNextSubpass}
          */
@@ -344,7 +345,7 @@ class MAGNUM_VK_EXPORT CommandBuffer {
          *
          * If Vulkan 1.2 is not supported and the
          * @vk_extension{KHR,create_renderpass2} extension is not enabled on
-         * the device, @p endInfo is ignored.
+         * the device, @p endInfo has to have the `pNext` chain empty.
          * @see @fn_vk_keyword{CmdEndRenderPass2},
          *      @fn_vk_keyword{CmdEndRenderPass}
          */

src/Magnum/Vk/RenderPass.cpp

@@ -100,6 +100,8 @@ namespace {
 
 /* Used by RenderPassCreateInfo::vkRenderPassCreateInfo() as well */
 VkAttachmentDescription vkAttachmentDescription(const VkAttachmentDescription2& description) {
+    CORRADE_ASSERT(!description.pNext,
+        "Vk::AttachmentDescription: disallowing conversion to VkAttachmentDescription with non-empty pNext to prevent information loss", {});
     return {
         description.flags,
         description.format,
@@ -150,6 +152,8 @@ namespace {
 
 /* Used in SubpassDescription::vkSubpassDescription() as well */
 VkAttachmentReference vkAttachmentReference(const VkAttachmentReference2& reference) {
+    CORRADE_ASSERT(!reference.pNext,
+        "Vk::AttachmentReference: disallowing conversion to VkAttachmentReference with non-empty pNext to prevent information loss", {});
     return {
         reference.attachment,
         reference.layout
@@ -437,6 +441,10 @@ std::pair<VkSubpassDescription, std::size_t> vkSubpassDescriptionExtrasInto(cons
 }
 
 Containers::Array<VkSubpassDescription> SubpassDescription::vkSubpassDescription() const {
+    /* pNext in nested structures checked in other helpers */
+    CORRADE_ASSERT(!_description.pNext,
+        "Vk::SubpassDescription: disallowing conversion to VkSubpassDescription with non-empty pNext to prevent information loss", {});
+
     /* Allocate an array to fit VkSubpassDescription together with all
        converted VkAttachmentReference instances it needs. Expect the default
        deleter is used so we don't need to wrap some other below. */
@@ -492,6 +500,8 @@ namespace {
 
 /* Used by RenderPassCreateInfo::vkRenderPassCreateInfo() as well */
 VkSubpassDependency vkSubpassDependency(const VkSubpassDependency2& dependency) {
+    CORRADE_ASSERT(!dependency.pNext,
+        "Vk::SubpassDependency: disallowing conversion to VkSubpassDependency with non-empty pNext to prevent information loss", {});
     return {
         dependency.srcSubpass,
         dependency.dstSubpass,
@@ -666,6 +676,9 @@ RenderPassCreateInfo& RenderPassCreateInfo::setDependencies(std::initializer_lis
 }
 
 Containers::Array<VkRenderPassCreateInfo> RenderPassCreateInfo::vkRenderPassCreateInfo() const {
+    /* pNext exists in the "V1" structure as well, thus no "information loss"
+       assert here. For nested structures it's checked in other helpers. */
+
     /* Check that all the structs we copy into the contiguous array have the
        expected alignment requirements. Subpass descriptions have the largest
        (on 64bit) due to the internal pointers, so they'll go first. */
@@ -911,6 +924,8 @@ CommandBuffer& CommandBuffer::beginRenderPass(const RenderPassBeginInfo& info, c
 }
 
 void CommandBuffer::beginRenderPassImplementationDefault(CommandBuffer& self, const VkRenderPassBeginInfo& info, const VkSubpassBeginInfo& beginInfo) {
+    CORRADE_ASSERT(!beginInfo.pNext,
+        "Vk::CommandBuffer::beginRenderPass(): disallowing conversion of SubpassBeginInfo to VkSubpassContents with non-empty pNext to prevent information loss", );
     return (**self._device).CmdBeginRenderPass(self, &info, beginInfo.contents);
 }
 
@@ -931,7 +946,11 @@ CommandBuffer& CommandBuffer::nextSubpass(const SubpassEndInfo& endInfo, const S
     return *this;
 }
 
-void CommandBuffer::nextSubpassImplementationDefault(CommandBuffer& self, const VkSubpassEndInfo&, const VkSubpassBeginInfo& beginInfo) {
+void CommandBuffer::nextSubpassImplementationDefault(CommandBuffer& self, const VkSubpassEndInfo& endInfo, const VkSubpassBeginInfo& beginInfo) {
+    CORRADE_ASSERT(!endInfo.pNext,
+        "Vk::CommandBuffer::nextRenderPass(): disallowing omission of SubpassEndInfo with non-empty pNext to prevent information loss", );
+    CORRADE_ASSERT(!beginInfo.pNext,
+        "Vk::CommandBuffer::nextRenderPass(): disallowing conversion of SubpassBeginInfo to VkSubpassContents with non-empty pNext to prevent information loss", );
     return (**self._device).CmdNextSubpass(self, beginInfo.contents);
 }
 
@@ -960,7 +979,9 @@ CommandBuffer& CommandBuffer::endRenderPass(const SubpassEndInfo& endInfo) {
     return *this;
 }
 
-void CommandBuffer::endRenderPassImplementationDefault(CommandBuffer& self, const VkSubpassEndInfo&) {
+void CommandBuffer::endRenderPassImplementationDefault(CommandBuffer& self, const VkSubpassEndInfo& endInfo) {
+    CORRADE_ASSERT(!endInfo.pNext,
+        "Vk::CommandBuffer::endRenderPass(): disallowing omission of SubpassEndInfo with non-empty pNext to prevent information loss", );
     return (**self._device).CmdEndRenderPass(self);
 }
 

src/Magnum/Vk/RenderPassCreateInfo.h

@@ -133,7 +133,9 @@ then get a @type_vk{AttachmentDescription} back again using
 
 For direct editing of the Vulkan structure, it's recommended to edit the
 @type_vk{AttachmentDescription2} fields and then perform the conversion
-instead of editing the resulting @type_vk{AttachmentDescription}.
+instead of editing the resulting @type_vk{AttachmentDescription}, as additional
+safety checks may be done during the conversion to ensure no information is
+lost.
 
 Please note that the conversion to @type_vk{AttachmentDescription} will ignore
 all fields that are present only in @type_vk{AttachmentDescription2} and its
@@ -340,7 +342,8 @@ back again using @ref vkAttachmentReference().
 
 For direct editing of the Vulkan structure, it's recommended to edit the
 @type_vk{AttachmentReference2} fields and then perform the conversion instead
-of editing the resulting @type_vk{AttachmentReference}.
+of editing the resulting @type_vk{AttachmentReference}, as additional safety
+checks may be done during the conversion to ensure no information is lost.
 
 Please note that the conversion to @type_vk{AttachmentReference} will ignore
 all fields that are present only in @type_vk{AttachmentReference2} --- in
@@ -467,7 +470,8 @@ so be sure to keep it in scope for as long as needed.
 
 For direct editing of the Vulkan structure, it's recommended to edit the
 @type_vk{SubpassDescription2} fields and then perform the conversion instead of
-editing the resulting @type_vk{SubpassDescription}.
+editing the resulting @type_vk{SubpassDescription}, as additional safety checks
+may be done during the conversion to ensure no information is lost.
 
 Please note that the conversion to @type_vk{SubpassDescription} will ignore all
 fields that are present only in @type_vk{SubpassDescription2} and its
@@ -706,7 +710,8 @@ using @ref vkSubpassDependency().
 
 For direct editing of the Vulkan structure, it's recommended to edit the
 @type_vk{SubpassDependency2} fields and then perform the conversion instead
-of editing the resulting @type_vk{SubpassDependency}.
+of editing the resulting @type_vk{SubpassDependency}, as additional safety
+checks may be done during the conversion to ensure no information is lost.
 
 Please note that the conversion to @type_vk{SubpassDependency} will ignore
 all fields that are present only in @type_vk{SubpassDependency2} --- in
@@ -845,7 +850,8 @@ so be sure to keep it in scope for as long as needed.
 
 For direct editing of the Vulkan structure, it's recommended to edit the
 @type_vk{RenderPassCreateInfo2} fields and then perform the conversion instead
-of editing the resulting @type_vk{RenderPassCreateInfo}.
+of editing the resulting @type_vk{RenderPassCreateInfo}, as additional safety
+checks may be done during the conversion to ensure no information is lost.
 
 Please note that the conversion to @type_vk{RenderPassCreateInfo} will ignore
 all fields that are present only in @type_vk{RenderPassCreateInfo2} and its

src/Magnum/Vk/Test/RenderPassTest.cpp

@@ -57,12 +57,14 @@ struct RenderPassTest: TestSuite::Tester {
     void attachmentDescriptionConstructNoInit();
     template<class From, class To> void attachmentDescriptionConstructFromVk();
     template<class T> void attachmentDescriptionConvertToVk();
+    void attachmentDescriptionConvertDisallowed();
 
     void attachmentReferenceConstruct();
     void attachmentReferenceConstructUnused();
     void attachmentReferenceConstructNoInit();
     template<class From, class To> void attachmentReferenceConstructFromVk();
     template<class T> void attachmentReferenceConvertToVk();
+    void attachmentReferenceConvertDisallowed();
 
     void subpassDescriptionConstruct();
     void subpassDescriptionConstructNoInit();
@@ -79,12 +81,14 @@ struct RenderPassTest: TestSuite::Tester {
     template<class T> void subpassDescriptionConvertToVk();
     template<class T> void subpassDescriptionConvertToVkNoAttachments();
     template<class T> void subpassDescriptionConvertToVkNoResolveAttachments();
+    void subpassDescriptionConvertDisallowed();
     void subpassDescriptionRvalue();
 
     void subpassDependencyConstruct();
     void subpassDependencyConstructNoInit();
     template<class From, class To> void subpassDependencyConstructFromVk();
     template<class T> void subpassDependencyConvertToVk();
+    void subpassDependencyConvertDisallowed();
 
     void createInfoConstruct();
     void createInfoConstructNoInit();
@@ -131,6 +135,7 @@ RenderPassTest::RenderPassTest() {
               &RenderPassTest::attachmentDescriptionConstructFromVk<VkAttachmentDescription, VkAttachmentDescription>,
               &RenderPassTest::attachmentDescriptionConvertToVk<VkAttachmentDescription2>,
               &RenderPassTest::attachmentDescriptionConvertToVk<VkAttachmentDescription>,
+              &RenderPassTest::attachmentDescriptionConvertDisallowed,
 
               &RenderPassTest::attachmentReferenceConstruct,
               &RenderPassTest::attachmentReferenceConstructUnused,
@@ -141,6 +146,7 @@ RenderPassTest::RenderPassTest() {
               &RenderPassTest::attachmentReferenceConstructFromVk<VkAttachmentReference, VkAttachmentReference>,
               &RenderPassTest::attachmentReferenceConvertToVk<VkAttachmentReference2>,
               &RenderPassTest::attachmentReferenceConvertToVk<VkAttachmentReference>,
+              &RenderPassTest::attachmentReferenceConvertDisallowed,
 
               &RenderPassTest::subpassDescriptionConstruct,
               &RenderPassTest::subpassDescriptionConstructNoInit,
@@ -163,6 +169,7 @@ RenderPassTest::RenderPassTest() {
               &RenderPassTest::subpassDescriptionConvertToVkNoAttachments<VkSubpassDescription>,
               &RenderPassTest::subpassDescriptionConvertToVkNoResolveAttachments<VkSubpassDescription2>,
               &RenderPassTest::subpassDescriptionConvertToVkNoResolveAttachments<VkSubpassDescription>,
+              &RenderPassTest::subpassDescriptionConvertDisallowed,
               &RenderPassTest::subpassDescriptionRvalue,
 
               &RenderPassTest::subpassDependencyConstruct,
@@ -173,6 +180,7 @@ RenderPassTest::RenderPassTest() {
               &RenderPassTest::subpassDependencyConstructFromVk<VkSubpassDependency, VkSubpassDependency>,
               &RenderPassTest::subpassDependencyConvertToVk<VkSubpassDependency2>,
               &RenderPassTest::subpassDependencyConvertToVk<VkSubpassDependency>,
+              &RenderPassTest::subpassDependencyConvertDisallowed,
 
               &RenderPassTest::createInfoConstruct,
               &RenderPassTest::createInfoConstructNoInit,
@@ -341,6 +349,20 @@ template<class T> void RenderPassTest::attachmentDescriptionConvertToVk() {
     CORRADE_COMPARE(out.finalLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
 }
 
+void RenderPassTest::attachmentDescriptionConvertDisallowed() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    AttachmentDescription description{PixelFormat{}, AttachmentLoadOperation{}, AttachmentStoreOperation{}, ImageLayout{}, ImageLayout{}};
+    description->pNext = &description;
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    description.vkAttachmentDescription();
+    CORRADE_COMPARE(out.str(), "Vk::AttachmentDescription: disallowing conversion to VkAttachmentDescription with non-empty pNext to prevent information loss\n");
+}
+
 void RenderPassTest::attachmentReferenceConstruct() {
     AttachmentReference reference{3, ImageLayout::ColorAttachment};
     CORRADE_COMPARE(reference->attachment, 3);
@@ -387,6 +409,20 @@ template<class T> void RenderPassTest::attachmentReferenceConvertToVk() {
     CORRADE_COMPARE(out.layout, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
 }
 
+void RenderPassTest::attachmentReferenceConvertDisallowed() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    AttachmentReference reference{0, ImageLayout{}};
+    reference->pNext = &reference;
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    reference.vkAttachmentReference();
+    CORRADE_COMPARE(out.str(), "Vk::AttachmentReference: disallowing conversion to VkAttachmentReference with non-empty pNext to prevent information loss\n");
+}
+
 void RenderPassTest::subpassDescriptionConstruct() {
     /** @todo use a real flag once it exists */
     SubpassDescription description{SubpassDescription::Flag(VK_INCOMPLETE)};
@@ -703,6 +739,20 @@ template<class T> void RenderPassTest::subpassDescriptionConvertToVkNoResolveAtt
     CORRADE_VERIFY(!to.pResolveAttachments);
 }
 
+void RenderPassTest::subpassDescriptionConvertDisallowed() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    SubpassDescription description;
+    description->pNext = &description;
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    description.vkSubpassDescription();
+    CORRADE_COMPARE(out.str(), "Vk::SubpassDescription: disallowing conversion to VkSubpassDescription with non-empty pNext to prevent information loss\n");
+}
+
 void RenderPassTest::subpassDescriptionRvalue() {
     SubpassDescription&& description = SubpassDescription{}
         .setInputAttachments(Containers::ArrayView<const AttachmentReference>{})
@@ -800,6 +850,20 @@ template<class T> void RenderPassTest::subpassDependencyConvertToVk() {
     CORRADE_COMPARE(out.dependencyFlags, VK_DEPENDENCY_BY_REGION_BIT);
 }
 
+void RenderPassTest::subpassDependencyConvertDisallowed() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    SubpassDependency dependency{0, PipelineStages{}, Accesses{}, 1, PipelineStages{}, Accesses{}};
+    dependency->pNext = &dependency;
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    dependency.vkSubpassDependency();
+    CORRADE_COMPARE(out.str(), "Vk::SubpassDependency: disallowing conversion to VkSubpassDependency with non-empty pNext to prevent information loss\n");
+}
+
 void RenderPassTest::createInfoConstruct() {
     /** @todo use a real flag once it exists */
     RenderPassCreateInfo info{RenderPassCreateInfo::Flag(VK_INCOMPLETE)};

src/Magnum/Vk/Test/RenderPassVkTest.cpp

@@ -32,6 +32,7 @@
 #include "Magnum/Vk/CommandBuffer.h"
 #include "Magnum/Vk/CommandPoolCreateInfo.h"
 #include "Magnum/Vk/DeviceProperties.h"
+#include "Magnum/Vk/Extensions.h"
 #include "Magnum/Vk/FramebufferCreateInfo.h"
 #include "Magnum/Vk/Handle.h"
 #include "Magnum/Vk/ImageCreateInfo.h"
@@ -54,6 +55,7 @@ struct RenderPassVkTest: VulkanTester {
     void wrap();
 
     void cmdBeginEnd();
+    void cmdBeginEndDisallowedConversion();
 };
 
 RenderPassVkTest::RenderPassVkTest() {
@@ -64,7 +66,8 @@ RenderPassVkTest::RenderPassVkTest() {
 
               &RenderPassVkTest::wrap,
 
-              &RenderPassVkTest::cmdBeginEnd});
+              &RenderPassVkTest::cmdBeginEnd,
+              &RenderPassVkTest::cmdBeginEndDisallowedConversion});
 }
 
 void RenderPassVkTest::construct() {
@@ -227,6 +230,37 @@ void RenderPassVkTest::cmdBeginEnd() {
     CORRADE_VERIFY(cmd.handle());
 }
 
+void RenderPassVkTest::cmdBeginEndDisallowedConversion() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    if(device().isVersionSupported(Version::Vk12) ||device().isExtensionEnabled<Extensions::KHR::create_renderpass2>())
+        CORRADE_SKIP("KHR_create_renderpass2 enabled on the device, can't test");
+
+    CommandPool pool{device(), CommandPoolCreateInfo{
+        device().properties().pickQueueFamily(QueueFlag::Graphics)}};
+    CommandBuffer cmd = pool.allocate();
+    SubpassEndInfo endInfo;
+    endInfo->pNext = &endInfo;
+    SubpassBeginInfo beginInfo;
+    beginInfo->pNext = &beginInfo;
+
+    /* The commands shouldn't do anything, so it should be fine to just call
+       them without any render pass set up */
+    std::ostringstream out;
+    Error redirectError{&out};
+    cmd.beginRenderPass(RenderPassBeginInfo{NoInit}, beginInfo)
+       .nextSubpass(beginInfo)
+       .nextSubpass(endInfo)
+       .endRenderPass(endInfo);
+    CORRADE_COMPARE(out.str(),
+        "Vk::CommandBuffer::beginRenderPass(): disallowing conversion of SubpassBeginInfo to VkSubpassContents with non-empty pNext to prevent information loss\n"
+        "Vk::CommandBuffer::nextRenderPass(): disallowing conversion of SubpassBeginInfo to VkSubpassContents with non-empty pNext to prevent information loss\n"
+        "Vk::CommandBuffer::nextRenderPass(): disallowing omission of SubpassEndInfo with non-empty pNext to prevent information loss\n"
+        "Vk::CommandBuffer::endRenderPass(): disallowing omission of SubpassEndInfo with non-empty pNext to prevent information loss\n");
+}
+
 }}}}
 
 CORRADE_TEST_MAIN(Magnum::Vk::Test::RenderPassVkTest)