From e45b4f9816f56b7af194cbcb3285b896da4ab75e Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Tue, 10 Jan 2017 09:51:22 +0000 Subject: layers: Transition each aspect individually in TransitionImageLayouts This fixes the possibility of spurious validation errors like the following, when attempting to transition multiple aspects of an image after only a subset of those aspects have previously been used in the command buffer: "Cannot query for VkImage 0x599 layout when combined aspect mask 6 has multiple initial layout types: VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL and VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL" The specific case where this bug was encountered was where both the depth and stencil aspects of an image were initially in the TRANSFER_DST_OPTIMAL layout. The first command in the command buffer to reference the image was a vkCmdClearDepthStencilImage on only the depth aspect of the image, followed later by a vkCmdPipelineBarrier to transition both aspects to the DEPTH_STENCIL_ATTACHMENT_OPTIMAL layout. Since TransitionImageLayouts tries to look up the initial state for all aspects at the same time, it was picking up the state based on the depth aspect because of the previous use of that (which had both initialLayout and layout set to TRANSFER_DST_OPTIMAL), and then calling SetLayout for all aspects with only the new layout. Since the stencil aspect didn't have any currently recorded state, it was being added with both initialLayout and layout set to the new layout, causing a mismatch between the initialLayout for the two aspects and therefore the spurious error above. Fix by updating the state for each aspect individually. --- layers/core_validation.cpp | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index e50c2981..d77aa7ca 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -9097,6 +9097,32 @@ CmdResetEvent(VkCommandBuffer commandBuffer, VkEvent event, VkPipelineStageFlags dev_data->dispatch_table.CmdResetEvent(commandBuffer, event, stageMask); } +static bool TransitionImageAspectLayout(layer_data *dev_data, GLOBAL_CB_NODE *pCB, const VkImageMemoryBarrier *mem_barrier, + uint32_t level, uint32_t layer, VkImageAspectFlags aspect) +{ + if (!(mem_barrier->subresourceRange.aspectMask & aspect)) { + return false; + } + VkImageSubresource sub = {aspect, level, layer}; + IMAGE_CMD_BUF_LAYOUT_NODE node; + if (!FindLayout(pCB, mem_barrier->image, sub, node)) { + SetLayout(pCB, mem_barrier->image, sub, + IMAGE_CMD_BUF_LAYOUT_NODE(mem_barrier->oldLayout, mem_barrier->newLayout)); + return false; + } + bool skip = false; + if (mem_barrier->oldLayout == VK_IMAGE_LAYOUT_UNDEFINED) { + // TODO: Set memory invalid which is in mem_tracker currently + } else if (node.layout != mem_barrier->oldLayout) { + skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, + __LINE__, DRAWSTATE_INVALID_IMAGE_LAYOUT, "DS", + "You cannot transition the layout of aspect %d from %s when current layout is %s.", + aspect, string_VkImageLayout(mem_barrier->oldLayout), string_VkImageLayout(node.layout)); + } + SetLayout(pCB, mem_barrier->image, sub, mem_barrier->newLayout); + return skip; +} + // TODO: Separate validation and layout state updates static bool TransitionImageLayouts(VkCommandBuffer cmdBuffer, uint32_t memBarrierCount, const VkImageMemoryBarrier *pImgMemBarriers) { @@ -9118,22 +9144,10 @@ static bool TransitionImageLayouts(VkCommandBuffer cmdBuffer, uint32_t memBarrie uint32_t level = mem_barrier->subresourceRange.baseMipLevel + j; for (uint32_t k = 0; k < layerCount; k++) { uint32_t layer = mem_barrier->subresourceRange.baseArrayLayer + k; - VkImageSubresource sub = {mem_barrier->subresourceRange.aspectMask, level, layer}; - IMAGE_CMD_BUF_LAYOUT_NODE node; - if (!FindLayout(pCB, mem_barrier->image, sub, node)) { - SetLayout(pCB, mem_barrier->image, sub, - IMAGE_CMD_BUF_LAYOUT_NODE(mem_barrier->oldLayout, mem_barrier->newLayout)); - continue; - } - if (mem_barrier->oldLayout == VK_IMAGE_LAYOUT_UNDEFINED) { - // TODO: Set memory invalid which is in mem_tracker currently - } else if (node.layout != mem_barrier->oldLayout) { - skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, - __LINE__, DRAWSTATE_INVALID_IMAGE_LAYOUT, "DS", "You cannot transition the layout from %s " - "when current layout is %s.", - string_VkImageLayout(mem_barrier->oldLayout), string_VkImageLayout(node.layout)); - } - SetLayout(pCB, mem_barrier->image, sub, mem_barrier->newLayout); + skip |= TransitionImageAspectLayout(dev_data, pCB, mem_barrier, level, layer, VK_IMAGE_ASPECT_COLOR_BIT); + skip |= TransitionImageAspectLayout(dev_data, pCB, mem_barrier, level, layer, VK_IMAGE_ASPECT_DEPTH_BIT); + skip |= TransitionImageAspectLayout(dev_data, pCB, mem_barrier, level, layer, VK_IMAGE_ASPECT_STENCIL_BIT); + skip |= TransitionImageAspectLayout(dev_data, pCB, mem_barrier, level, layer, VK_IMAGE_ASPECT_METADATA_BIT); } } } -- cgit v1.2.3