diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index f9c36be43d..2b33b727bb 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -867,8 +867,9 @@ static bool validateSubpassCompatibility(layer_data const *dev_data, const char if (i < secondary_desc.inputAttachmentCount) { secondary_input_attach = secondary_desc.pInputAttachments[i].attachment; } - skip |= validateAttachmentCompatibility(dev_data, type1_string, rp1_state, type2_string, rp2_state, primary_input_attach, - secondary_input_attach, caller, error_code); + if (validateAttachmentCompatibility(dev_data, type1_string, rp1_state, type2_string, rp2_state, primary_input_attach, + secondary_input_attach, caller, error_code)) + return true; } uint32_t maxColorAttachmentCount = std::max(primary_desc.colorAttachmentCount, secondary_desc.colorAttachmentCount); for (uint32_t i = 0; i < maxColorAttachmentCount; ++i) { @@ -879,8 +880,10 @@ static bool validateSubpassCompatibility(layer_data const *dev_data, const char if (i < secondary_desc.colorAttachmentCount) { secondary_color_attach = secondary_desc.pColorAttachments[i].attachment; } - skip |= validateAttachmentCompatibility(dev_data, type1_string, rp1_state, type2_string, rp2_state, primary_color_attach, - secondary_color_attach, caller, error_code); + if (validateAttachmentCompatibility(dev_data, type1_string, rp1_state, type2_string, rp2_state, primary_color_attach, + secondary_color_attach, caller, error_code)) + return true; + uint32_t primary_resolve_attach = VK_ATTACHMENT_UNUSED, secondary_resolve_attach = VK_ATTACHMENT_UNUSED; if (i < primary_desc.colorAttachmentCount && primary_desc.pResolveAttachments) { primary_resolve_attach = primary_desc.pResolveAttachments[i].attachment; @@ -888,8 +891,9 @@ static bool validateSubpassCompatibility(layer_data const *dev_data, const char if (i < secondary_desc.colorAttachmentCount && secondary_desc.pResolveAttachments) { secondary_resolve_attach = secondary_desc.pResolveAttachments[i].attachment; } - skip |= validateAttachmentCompatibility(dev_data, type1_string, rp1_state, type2_string, rp2_state, primary_resolve_attach, - secondary_resolve_attach, caller, error_code); + if (validateAttachmentCompatibility(dev_data, type1_string, rp1_state, type2_string, rp2_state, primary_resolve_attach, + secondary_resolve_attach, caller, error_code)) + return true; } uint32_t primary_depthstencil_attach = VK_ATTACHMENT_UNUSED, secondary_depthstencil_attach = VK_ATTACHMENT_UNUSED; if (primary_desc.pDepthStencilAttachment) { @@ -8036,90 +8040,93 @@ VKAPI_ATTR void VKAPI_CALL CmdBeginRenderPass(VkCommandBuffer commandBuffer, con layer_data *dev_data = GetLayerDataPtr(get_dispatch_key(commandBuffer), layer_data_map); unique_lock_t lock(global_lock); GLOBAL_CB_NODE *cb_node = GetCBNode(dev_data, commandBuffer); - auto render_pass_state = pRenderPassBegin ? GetRenderPassState(dev_data, pRenderPassBegin->renderPass) : nullptr; - auto framebuffer = pRenderPassBegin ? GetFramebufferState(dev_data, pRenderPassBegin->framebuffer) : nullptr; - if (cb_node) { - if (render_pass_state) { - uint32_t clear_op_size = 0; // Make sure pClearValues is at least as large as last LOAD_OP_CLEAR - cb_node->activeFramebuffer = pRenderPassBegin->framebuffer; - for (uint32_t i = 0; i < render_pass_state->createInfo.attachmentCount; ++i) { - MT_FB_ATTACHMENT_INFO &fb_info = framebuffer->attachments[i]; - auto pAttachment = &render_pass_state->createInfo.pAttachments[i]; - if (FormatSpecificLoadAndStoreOpSettings(pAttachment->format, pAttachment->loadOp, pAttachment->stencilLoadOp, - VK_ATTACHMENT_LOAD_OP_CLEAR)) { - clear_op_size = static_cast(i) + 1; - std::function function = [=]() { - SetImageMemoryValid(dev_data, GetImageState(dev_data, fb_info.image), true); - return false; - }; - cb_node->queue_submit_functions.push_back(function); - } else if (FormatSpecificLoadAndStoreOpSettings(pAttachment->format, pAttachment->loadOp, - pAttachment->stencilLoadOp, VK_ATTACHMENT_LOAD_OP_DONT_CARE)) { - std::function function = [=]() { - SetImageMemoryValid(dev_data, GetImageState(dev_data, fb_info.image), false); - return false; - }; - cb_node->queue_submit_functions.push_back(function); - } else if (FormatSpecificLoadAndStoreOpSettings(pAttachment->format, pAttachment->loadOp, - pAttachment->stencilLoadOp, VK_ATTACHMENT_LOAD_OP_LOAD)) { - std::function function = [=]() { - return ValidateImageMemoryIsValid(dev_data, GetImageState(dev_data, fb_info.image), - "vkCmdBeginRenderPass()"); - }; - cb_node->queue_submit_functions.push_back(function); - } - if (render_pass_state->attachment_first_read[i]) { - std::function function = [=]() { - return ValidateImageMemoryIsValid(dev_data, GetImageState(dev_data, fb_info.image), - "vkCmdBeginRenderPass()"); - }; - cb_node->queue_submit_functions.push_back(function); - } - } - if (clear_op_size > pRenderPassBegin->clearValueCount) { - skip |= log_msg( - dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_RENDER_PASS_EXT, - HandleToUint64(render_pass_state->renderPass), __LINE__, VALIDATION_ERROR_1200070c, "DS", - "In vkCmdBeginRenderPass() the VkRenderPassBeginInfo struct has a clearValueCount of %u but there must " - "be at least %u entries in pClearValues array to account for the highest index attachment in renderPass " - "0x%" PRIx64 - " that uses VK_ATTACHMENT_LOAD_OP_CLEAR is %u. Note that the pClearValues array " - "is indexed by attachment number so even if some pClearValues entries between 0 and %u correspond to " - "attachments that aren't cleared they will be ignored. %s", - pRenderPassBegin->clearValueCount, clear_op_size, HandleToUint64(render_pass_state->renderPass), clear_op_size, - clear_op_size - 1, validation_error_map[VALIDATION_ERROR_1200070c]); - } - skip |= VerifyRenderAreaBounds(dev_data, pRenderPassBegin); - skip |= VerifyFramebufferAndRenderPassLayouts(dev_data, cb_node, pRenderPassBegin, - GetFramebufferState(dev_data, pRenderPassBegin->framebuffer)); - if (framebuffer->rp_state->renderPass != render_pass_state->renderPass) { - skip |= validateRenderPassCompatibility(dev_data, "render pass", render_pass_state, "framebuffer", - framebuffer->rp_state.get(), "vkCmdBeginRenderPass()", - VALIDATION_ERROR_12000710); - } - skip |= insideRenderPass(dev_data, cb_node, "vkCmdBeginRenderPass()", VALIDATION_ERROR_17a00017); - skip |= ValidateDependencies(dev_data, framebuffer, render_pass_state); - skip |= validatePrimaryCommandBuffer(dev_data, cb_node, "vkCmdBeginRenderPass()", VALIDATION_ERROR_17a00019); - skip |= ValidateCmdQueueFlags(dev_data, cb_node, "vkCmdBeginRenderPass()", VK_QUEUE_GRAPHICS_BIT, - VALIDATION_ERROR_17a02415); - skip |= ValidateCmd(dev_data, cb_node, CMD_BEGINRENDERPASS, "vkCmdBeginRenderPass()"); - cb_node->activeRenderPass = render_pass_state; - // This is a shallow copy as that is all that is needed for now - cb_node->activeRenderPassBeginInfo = *pRenderPassBegin; - cb_node->activeSubpass = 0; - cb_node->activeSubpassContents = contents; - cb_node->framebuffers.insert(pRenderPassBegin->framebuffer); - // Connect this framebuffer and its children to this cmdBuffer - AddFramebufferBinding(dev_data, cb_node, framebuffer); - // Connect this RP to cmdBuffer - addCommandBufferBinding(&render_pass_state->cb_bindings, - {HandleToUint64(render_pass_state->renderPass), kVulkanObjectTypeRenderPass}, cb_node); - // transition attachments to the correct layouts for beginning of renderPass and first subpass - TransitionBeginRenderPassLayouts(dev_data, cb_node, render_pass_state, framebuffer); - } - } - lock.unlock(); + assert(cb_node); + assert(pRenderPassBegin); + auto render_pass_state = GetRenderPassState(dev_data, pRenderPassBegin->renderPass); + auto framebuffer = GetFramebufferState(dev_data, pRenderPassBegin->framebuffer); + assert(render_pass_state); + assert(framebuffer); + + uint32_t clear_op_size = 0; // Make sure pClearValues is at least as large as last LOAD_OP_CLEAR + for (uint32_t i = 0; i < render_pass_state->createInfo.attachmentCount; ++i) { + auto pAttachment = &render_pass_state->createInfo.pAttachments[i]; + if (FormatSpecificLoadAndStoreOpSettings(pAttachment->format, pAttachment->loadOp, pAttachment->stencilLoadOp, + VK_ATTACHMENT_LOAD_OP_CLEAR)) { + clear_op_size = static_cast(i) + 1; + } + } + if (clear_op_size > pRenderPassBegin->clearValueCount) { + skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_RENDER_PASS_EXT, + HandleToUint64(render_pass_state->renderPass), __LINE__, VALIDATION_ERROR_1200070c, "DS", + "In vkCmdBeginRenderPass() the VkRenderPassBeginInfo struct has a clearValueCount of %u but there must " + "be at least %u entries in pClearValues array to account for the highest index attachment in renderPass " + "0x%" PRIx64 + " that uses VK_ATTACHMENT_LOAD_OP_CLEAR is %u. Note that the pClearValues array " + "is indexed by attachment number so even if some pClearValues entries between 0 and %u correspond to " + "attachments that aren't cleared they will be ignored. %s", + pRenderPassBegin->clearValueCount, clear_op_size, HandleToUint64(render_pass_state->renderPass), + clear_op_size, clear_op_size - 1, validation_error_map[VALIDATION_ERROR_1200070c]); + } + skip |= VerifyRenderAreaBounds(dev_data, pRenderPassBegin); + skip |= VerifyFramebufferAndRenderPassLayouts(dev_data, cb_node, pRenderPassBegin, + GetFramebufferState(dev_data, pRenderPassBegin->framebuffer)); + if (framebuffer->rp_state->renderPass != render_pass_state->renderPass) { + skip |= validateRenderPassCompatibility(dev_data, "render pass", render_pass_state, "framebuffer", + framebuffer->rp_state.get(), "vkCmdBeginRenderPass()", VALIDATION_ERROR_12000710); + } + skip |= insideRenderPass(dev_data, cb_node, "vkCmdBeginRenderPass()", VALIDATION_ERROR_17a00017); + skip |= ValidateDependencies(dev_data, framebuffer, render_pass_state); + skip |= validatePrimaryCommandBuffer(dev_data, cb_node, "vkCmdBeginRenderPass()", VALIDATION_ERROR_17a00019); + skip |= ValidateCmdQueueFlags(dev_data, cb_node, "vkCmdBeginRenderPass()", VK_QUEUE_GRAPHICS_BIT, VALIDATION_ERROR_17a02415); + skip |= ValidateCmd(dev_data, cb_node, CMD_BEGINRENDERPASS, "vkCmdBeginRenderPass()"); if (!skip) { + // Perform state updates prior to call down chain + cb_node->activeFramebuffer = pRenderPassBegin->framebuffer; + for (uint32_t i = 0; i < render_pass_state->createInfo.attachmentCount; ++i) { + MT_FB_ATTACHMENT_INFO &fb_info = framebuffer->attachments[i]; + auto pAttachment = &render_pass_state->createInfo.pAttachments[i]; + if (FormatSpecificLoadAndStoreOpSettings(pAttachment->format, pAttachment->loadOp, pAttachment->stencilLoadOp, + VK_ATTACHMENT_LOAD_OP_CLEAR)) { + std::function function = [=]() { + SetImageMemoryValid(dev_data, GetImageState(dev_data, fb_info.image), true); + return false; + }; + cb_node->queue_submit_functions.push_back(function); + } else if (FormatSpecificLoadAndStoreOpSettings(pAttachment->format, pAttachment->loadOp, pAttachment->stencilLoadOp, + VK_ATTACHMENT_LOAD_OP_DONT_CARE)) { + std::function function = [=]() { + SetImageMemoryValid(dev_data, GetImageState(dev_data, fb_info.image), false); + return false; + }; + cb_node->queue_submit_functions.push_back(function); + } else if (FormatSpecificLoadAndStoreOpSettings(pAttachment->format, pAttachment->loadOp, pAttachment->stencilLoadOp, + VK_ATTACHMENT_LOAD_OP_LOAD)) { + std::function function = [=]() { + return ValidateImageMemoryIsValid(dev_data, GetImageState(dev_data, fb_info.image), "vkCmdBeginRenderPass()"); + }; + cb_node->queue_submit_functions.push_back(function); + } + if (render_pass_state->attachment_first_read[i]) { + std::function function = [=]() { + return ValidateImageMemoryIsValid(dev_data, GetImageState(dev_data, fb_info.image), "vkCmdBeginRenderPass()"); + }; + cb_node->queue_submit_functions.push_back(function); + } + } + cb_node->activeRenderPass = render_pass_state; + // This is a shallow copy as that is all that is needed for now + cb_node->activeRenderPassBeginInfo = *pRenderPassBegin; + cb_node->activeSubpass = 0; + cb_node->activeSubpassContents = contents; + cb_node->framebuffers.insert(pRenderPassBegin->framebuffer); + // Connect this framebuffer and its children to this cmdBuffer + AddFramebufferBinding(dev_data, cb_node, framebuffer); + // Connect this RP to cmdBuffer + addCommandBufferBinding(&render_pass_state->cb_bindings, + {HandleToUint64(render_pass_state->renderPass), kVulkanObjectTypeRenderPass}, cb_node); + // transition attachments to the correct layouts for beginning of renderPass and first subpass + TransitionBeginRenderPassLayouts(dev_data, cb_node, render_pass_state, framebuffer); + lock.unlock(); dev_data->dispatch_table.CmdBeginRenderPass(commandBuffer, pRenderPassBegin, contents); } } diff --git a/tests/layer_validation_tests.cpp b/tests/layer_validation_tests.cpp index 78094e8547..7891205cfc 100644 --- a/tests/layer_validation_tests.cpp +++ b/tests/layer_validation_tests.cpp @@ -6013,6 +6013,8 @@ TEST_F(VkLayerTest, RenderPassInUseDestroyedSignaled) { // Wait for queue to complete so we can safely destroy rp vkQueueWaitIdle(m_device->m_queue); + m_errorMonitor->SetUnexpectedError("If renderPass is not VK_NULL_HANDLE, renderPass must be a valid VkRenderPass handle"); + m_errorMonitor->SetUnexpectedError("Unable to remove RenderPass obj"); vkDestroyRenderPass(m_device->device(), rp, nullptr); } @@ -12355,7 +12357,8 @@ TEST_F(VkLayerTest, NumSamplesMismatch) { TEST_F(VkLayerTest, RenderPassIncompatible) { TEST_DESCRIPTION( "Hit RenderPass incompatible cases. " - "Initial case is drawing with an active renderpass that's " + "First attempt BeginRenderPass() with incompatible FrameBuffer," + "then attempt to draw with an active renderpass that's " "not compatible with the bound pipeline state object's creation renderpass"); VkResult err; @@ -12426,17 +12429,20 @@ TEST_F(VkLayerTest, RenderPassIncompatible) { rpbi.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO; rpbi.framebuffer = m_framebuffer; rpbi.renderPass = rp; + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_12000710); vkCmdBeginRenderPass(m_commandBuffer->handle(), &rpbi, VK_SUBPASS_CONTENTS_INLINE); + m_errorMonitor->VerifyFound(); + // Now we want to bind the RenderPass to trigger Draw-time error so allow the error for this call + // The better way to do this to avoid the error would be to create separate FB with compatible RP + // and use that FB for this begin instead of m_framebuffer + m_errorMonitor->SetUnexpectedError("vkCmdBeginRenderPass(): RenderPasses incompatible between "); + vkCmdBeginRenderPass(m_commandBuffer->handle(), &rpbi, VK_SUBPASS_CONTENTS_INLINE); + // m_errorMonitor->VerifyFound(); vkCmdBindPipeline(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_GRAPHICS, pipe.handle()); m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, VALIDATION_ERROR_1a200366); // Render triangle (the error should trigger on the attempt to draw). m_commandBuffer->Draw(3, 1, 0, 0); - - // Finalize recording of the command buffer - m_commandBuffer->EndRenderPass(); - m_commandBuffer->end(); - m_errorMonitor->VerifyFound(); vkDestroyPipelineLayout(m_device->device(), pipeline_layout, NULL);