From 249062984890d77591e91019b476e01a60abcbf4 Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Mon, 18 Jul 2016 19:01:43 -0600 Subject: layers: GH773 Check improper renderpass layout for only first use Only verify that a LOAD_OP attachment does not have first layout of *READ_ONLY* type on its first use. The spec language this is checking is from section "7.1 Render Pass Creation" : The first use of an attachment must not specify a layout equal to VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL or VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL if the attachment specifies that the loadOp is VK_ATTACHMENT_LOAD_OP_CLEAR. Previously we were checking this condition on all uses of an attachment which would incorrectly flag errors on proper uses beyond the first use by later subpasses. Note that validation for CreateRenderPass has a fair amount of duplication and still needs to be cleaned up to consolidate work that's currently being done before and after the call down the chain. This is simply a singular bug fix. --- layers/core_validation.cpp | 63 +++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 26 deletions(-) (limited to 'layers/core_validation.cpp') diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 4ca938fe..b7946378 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -8956,30 +8956,10 @@ static bool ValidateLayoutVsAttachmentDescription(debug_report_data *report_data static bool ValidateLayouts(const layer_data *my_data, VkDevice device, const VkRenderPassCreateInfo *pCreateInfo) { bool skip = false; + // Track when we're observing the first use of an attachment + std::vector attach_first_use(pCreateInfo->attachmentCount, true); for (uint32_t i = 0; i < pCreateInfo->subpassCount; ++i) { const VkSubpassDescription &subpass = pCreateInfo->pSubpasses[i]; - for (uint32_t j = 0; j < subpass.inputAttachmentCount; ++j) { - auto attach_index = subpass.pInputAttachments[j].attachment; - if (attach_index == VK_ATTACHMENT_UNUSED) - continue; - - if (subpass.pInputAttachments[j].layout != VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL && - subpass.pInputAttachments[j].layout != VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL) { - if (subpass.pInputAttachments[j].layout == VK_IMAGE_LAYOUT_GENERAL) { - // TODO: Verify Valid Use in spec. I believe this is allowed (valid) but may not be optimal performance - skip |= log_msg(my_data->report_data, VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT, - (VkDebugReportObjectTypeEXT)0, 0, __LINE__, DRAWSTATE_INVALID_IMAGE_LAYOUT, "DS", - "Layout for input attachment is GENERAL but should be READ_ONLY_OPTIMAL."); - } else { - skip |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, - DRAWSTATE_INVALID_IMAGE_LAYOUT, "DS", - "Layout for input attachment is %s but can only be READ_ONLY_OPTIMAL or GENERAL.", - string_VkImageLayout(subpass.pInputAttachments[j].layout)); - } - } - skip |= ValidateLayoutVsAttachmentDescription(my_data->report_data, subpass.pInputAttachments[j].layout, attach_index, - pCreateInfo->pAttachments[attach_index]); - } for (uint32_t j = 0; j < subpass.colorAttachmentCount; ++j) { auto attach_index = subpass.pColorAttachments[j].attachment; if (attach_index == VK_ATTACHMENT_UNUSED) @@ -8998,8 +8978,11 @@ static bool ValidateLayouts(const layer_data *my_data, VkDevice device, const Vk string_VkImageLayout(subpass.pColorAttachments[j].layout)); } } - skip |= ValidateLayoutVsAttachmentDescription(my_data->report_data, subpass.pColorAttachments[j].layout, attach_index, - pCreateInfo->pAttachments[attach_index]); + if (attach_first_use[attach_index]) { + skip |= ValidateLayoutVsAttachmentDescription(my_data->report_data, subpass.pColorAttachments[j].layout, + attach_index, pCreateInfo->pAttachments[attach_index]); + } + attach_first_use[attach_index] = false; } if ((subpass.pDepthStencilAttachment != NULL) && (subpass.pDepthStencilAttachment->attachment != VK_ATTACHMENT_UNUSED)) { if (subpass.pDepthStencilAttachment->layout != VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL) { @@ -9017,8 +9000,36 @@ static bool ValidateLayouts(const layer_data *my_data, VkDevice device, const Vk } } auto attach_index = subpass.pDepthStencilAttachment->attachment; - skip |= ValidateLayoutVsAttachmentDescription(my_data->report_data, subpass.pDepthStencilAttachment->layout, - attach_index, pCreateInfo->pAttachments[attach_index]); + if (attach_first_use[attach_index]) { + skip |= ValidateLayoutVsAttachmentDescription(my_data->report_data, subpass.pDepthStencilAttachment->layout, + attach_index, pCreateInfo->pAttachments[attach_index]); + } + attach_first_use[attach_index] = false; + } + for (uint32_t j = 0; j < subpass.inputAttachmentCount; ++j) { + auto attach_index = subpass.pInputAttachments[j].attachment; + if (attach_index == VK_ATTACHMENT_UNUSED) + continue; + + if (subpass.pInputAttachments[j].layout != VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL && + subpass.pInputAttachments[j].layout != VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL) { + if (subpass.pInputAttachments[j].layout == VK_IMAGE_LAYOUT_GENERAL) { + // TODO: Verify Valid Use in spec. I believe this is allowed (valid) but may not be optimal performance + skip |= log_msg(my_data->report_data, VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT, + (VkDebugReportObjectTypeEXT)0, 0, __LINE__, DRAWSTATE_INVALID_IMAGE_LAYOUT, "DS", + "Layout for input attachment is GENERAL but should be READ_ONLY_OPTIMAL."); + } else { + skip |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, + DRAWSTATE_INVALID_IMAGE_LAYOUT, "DS", + "Layout for input attachment is %s but can only be READ_ONLY_OPTIMAL or GENERAL.", + string_VkImageLayout(subpass.pInputAttachments[j].layout)); + } + } + if (attach_first_use[attach_index]) { + skip |= ValidateLayoutVsAttachmentDescription(my_data->report_data, subpass.pInputAttachments[j].layout, + attach_index, pCreateInfo->pAttachments[attach_index]); + } + attach_first_use[attach_index] = false; } } return skip; -- cgit v1.2.3