diff --git a/src/Magnum/Vk/CommandBuffer.h b/src/Magnum/Vk/CommandBuffer.h index 0ced89cc1..a8588f31d 100644 --- a/src/Magnum/Vk/CommandBuffer.h +++ b/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} */ diff --git a/src/Magnum/Vk/RenderPass.cpp b/src/Magnum/Vk/RenderPass.cpp index 11267ec49..156adff8e 100644 --- a/src/Magnum/Vk/RenderPass.cpp +++ b/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 vkSubpassDescriptionExtrasInto(cons } Containers::Array 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 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); } diff --git a/src/Magnum/Vk/RenderPassCreateInfo.h b/src/Magnum/Vk/RenderPassCreateInfo.h index b91d412bb..c04fc03ae 100644 --- a/src/Magnum/Vk/RenderPassCreateInfo.h +++ b/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 diff --git a/src/Magnum/Vk/Test/RenderPassTest.cpp b/src/Magnum/Vk/Test/RenderPassTest.cpp index 6c2326af0..45a8cf57c 100644 --- a/src/Magnum/Vk/Test/RenderPassTest.cpp +++ b/src/Magnum/Vk/Test/RenderPassTest.cpp @@ -57,12 +57,14 @@ struct RenderPassTest: TestSuite::Tester { void attachmentDescriptionConstructNoInit(); template void attachmentDescriptionConstructFromVk(); template void attachmentDescriptionConvertToVk(); + void attachmentDescriptionConvertDisallowed(); void attachmentReferenceConstruct(); void attachmentReferenceConstructUnused(); void attachmentReferenceConstructNoInit(); template void attachmentReferenceConstructFromVk(); template void attachmentReferenceConvertToVk(); + void attachmentReferenceConvertDisallowed(); void subpassDescriptionConstruct(); void subpassDescriptionConstructNoInit(); @@ -79,12 +81,14 @@ struct RenderPassTest: TestSuite::Tester { template void subpassDescriptionConvertToVk(); template void subpassDescriptionConvertToVkNoAttachments(); template void subpassDescriptionConvertToVkNoResolveAttachments(); + void subpassDescriptionConvertDisallowed(); void subpassDescriptionRvalue(); void subpassDependencyConstruct(); void subpassDependencyConstructNoInit(); template void subpassDependencyConstructFromVk(); template void subpassDependencyConvertToVk(); + void subpassDependencyConvertDisallowed(); void createInfoConstruct(); void createInfoConstructNoInit(); @@ -131,6 +135,7 @@ RenderPassTest::RenderPassTest() { &RenderPassTest::attachmentDescriptionConstructFromVk, &RenderPassTest::attachmentDescriptionConvertToVk, &RenderPassTest::attachmentDescriptionConvertToVk, + &RenderPassTest::attachmentDescriptionConvertDisallowed, &RenderPassTest::attachmentReferenceConstruct, &RenderPassTest::attachmentReferenceConstructUnused, @@ -141,6 +146,7 @@ RenderPassTest::RenderPassTest() { &RenderPassTest::attachmentReferenceConstructFromVk, &RenderPassTest::attachmentReferenceConvertToVk, &RenderPassTest::attachmentReferenceConvertToVk, + &RenderPassTest::attachmentReferenceConvertDisallowed, &RenderPassTest::subpassDescriptionConstruct, &RenderPassTest::subpassDescriptionConstructNoInit, @@ -163,6 +169,7 @@ RenderPassTest::RenderPassTest() { &RenderPassTest::subpassDescriptionConvertToVkNoAttachments, &RenderPassTest::subpassDescriptionConvertToVkNoResolveAttachments, &RenderPassTest::subpassDescriptionConvertToVkNoResolveAttachments, + &RenderPassTest::subpassDescriptionConvertDisallowed, &RenderPassTest::subpassDescriptionRvalue, &RenderPassTest::subpassDependencyConstruct, @@ -173,6 +180,7 @@ RenderPassTest::RenderPassTest() { &RenderPassTest::subpassDependencyConstructFromVk, &RenderPassTest::subpassDependencyConvertToVk, &RenderPassTest::subpassDependencyConvertToVk, + &RenderPassTest::subpassDependencyConvertDisallowed, &RenderPassTest::createInfoConstruct, &RenderPassTest::createInfoConstructNoInit, @@ -341,6 +349,20 @@ template 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 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 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{}) @@ -800,6 +850,20 @@ template 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)}; diff --git a/src/Magnum/Vk/Test/RenderPassVkTest.cpp b/src/Magnum/Vk/Test/RenderPassVkTest.cpp index 6010b8125..2d24d27b8 100644 --- a/src/Magnum/Vk/Test/RenderPassVkTest.cpp +++ b/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()) + 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)