From ad190dca307a079766a6f558ddda4e2774b2c136 Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Thu, 11 May 2017 08:52:51 -0600 Subject: [PATCH] layers: Validate shared presentable image cases Add validation support for shared presentable images as defined in VK_KHR_shared_presentable_image extension. For all uses of shared presentable images, make sure that the image is appropriately in VK_IMAGE_LAYOUT_PRESENT_SRC_KHR layout. For two cases where no layout validation was performed, added a TODO note (vkCmdBlitImage, vkCmdResolveImage) as basic layout validation should first be added upstream. Also locked the layout in the case where a front-buffered image is presented and then flag an error if an attempt is made to transition the image layout after that point. Change-Id: I06cda727e3a7f56ccff4bffd7503b5ff73e8a795 --- layers/buffer_validation.cpp | 48 ++++++++++++++++++++++++++++++++++++------ layers/core_validation.cpp | 28 ++++++++++++++++++++++++ layers/core_validation_types.h | 5 ++++- layers/descriptor_sets.cpp | 31 ++++++++++++++++++++------- 4 files changed, 96 insertions(+), 16 deletions(-) diff --git a/layers/buffer_validation.cpp b/layers/buffer_validation.cpp index 30b9168..3fd4e9a 100644 --- a/layers/buffer_validation.cpp +++ b/layers/buffer_validation.cpp @@ -472,6 +472,24 @@ bool ValidateBarriersToImages(layer_data *device_data, VkCommandBuffer cmdBuffer auto img_barrier = &pImageMemoryBarriers[i]; if (!img_barrier) continue; + auto image_state = GetImageState(device_data, img_barrier->image); + if (image_state) { + VkImageUsageFlags usage_flags = image_state->createInfo.usage; + skip |= ValidateBarrierLayoutToImageUsage(device_data, img_barrier, false, usage_flags, func_name); + skip |= ValidateBarrierLayoutToImageUsage(device_data, img_barrier, true, usage_flags, func_name); + + // Make sure layout is able to be transitioned, currently only presented shared presentable images are locked + if (image_state->layout_locked) { + // TODO: Add unique id for error when available + skip |= log_msg(core_validation::GetReportData(device_data), VK_DEBUG_REPORT_ERROR_BIT_EXT, + VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, 0, "DS", + "Attempting to transition shared presentable image 0x%" PRIxLEAST64 + " from layout %s to layout %s, but image has already been presented and cannot have its layout transitioned.", + reinterpret_cast(img_barrier->image), string_VkImageLayout(img_barrier->oldLayout), + string_VkImageLayout(img_barrier->newLayout)); + } + } + VkImageCreateInfo *image_create_info = &(GetImageState(device_data, img_barrier->image)->createInfo); // For a Depth/Stencil image both aspects MUST be set if (FormatIsDepthAndStencil(image_create_info->format)) { @@ -502,13 +520,6 @@ bool ValidateBarriersToImages(layer_data *device_data, VkCommandBuffer cmdBuffer skip |= ValidateImageAspectLayout(device_data, pCB, img_barrier, level, layer, VK_IMAGE_ASPECT_METADATA_BIT); } } - - IMAGE_STATE *image_state = GetImageState(device_data, img_barrier->image); - if (image_state) { - VkImageUsageFlags usage_flags = image_state->createInfo.usage; - skip |= ValidateBarrierLayoutToImageUsage(device_data, img_barrier, false, usage_flags, func_name); - skip |= ValidateBarrierLayoutToImageUsage(device_data, img_barrier, true, usage_flags, func_name); - } } return skip; } @@ -573,6 +584,14 @@ bool VerifyImageLayout(layer_data const *device_data, GLOBAL_CB_NODE const *cb_n "%s: For optimal performance image 0x%" PRIxLEAST64 " layout should be %s instead of GENERAL.", caller, reinterpret_cast(image), string_VkImageLayout(optimal_layout)); } + } else if (image_state->shared_presentable) { + if (VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR != explicit_layout) { + // TODO: Add unique error id when available. + skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, + __LINE__, msg_code, "DS", + "Layout for shared presentable image is %s but must be VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR.", + string_VkImageLayout(optimal_layout)); + } } else { *error = true; skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, @@ -878,6 +897,14 @@ bool VerifyClearImageLayout(layer_data *device_data, GLOBAL_CB_NODE *cb_node, IM reinterpret_cast(image_state->image), __LINE__, DRAWSTATE_INVALID_IMAGE_LAYOUT, "DS", "%s: Layout for cleared image should be TRANSFER_DST_OPTIMAL instead of GENERAL.", func_name); } + } else if (image_state->shared_presentable) { + if (VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR != dest_image_layout) { + // TODO: Add unique error id when available. + skip |= + log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__, 0, "DS", + "Layout for shared presentable cleared image is %s but can only be VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR.", + string_VkImageLayout(dest_image_layout)); + } } else { UNIQUE_VALIDATION_ERROR_CODE error_code = VALIDATION_ERROR_01086; if (strcmp(func_name, "vkCmdClearDepthStencilImage()") == 0) { @@ -1809,6 +1836,7 @@ bool PreCallValidateCmdResolveImage(layer_data *device_data, GLOBAL_CB_NODE *cb_ reinterpret_cast(cb_node->commandBuffer), __LINE__, VALIDATION_ERROR_01321, "IMAGE", "%s. %s", str, validation_error_map[VALIDATION_ERROR_01321]); } + // TODO: Need to validate image layouts, which will include layout validation for shared presentable images } else { assert(0); } @@ -1852,6 +1880,7 @@ bool PreCallValidateCmdBlitImage(layer_data *device_data, GLOBAL_CB_NODE *cb_nod skip |= ValidateCmdQueueFlags(device_data, cb_node, "vkCmdBlitImage()", VK_QUEUE_GRAPHICS_BIT, VALIDATION_ERROR_01299); skip |= ValidateCmd(device_data, cb_node, CMD_BLITIMAGE, "vkCmdBlitImage()"); skip |= insideRenderPass(device_data, cb_node, "vkCmdBlitImage()", VALIDATION_ERROR_01300); + // TODO: Need to validate image layouts, which will include layout validation for shared presentable images for (uint32_t i = 0; i < regionCount; i++) { // Warn for zero-sized regions @@ -2279,9 +2308,14 @@ bool ValidateLayouts(core_validation::layer_data *device_data, VkDevice device, auto attach_index = subpass.pColorAttachments[j].attachment; if (attach_index == VK_ATTACHMENT_UNUSED) continue; + // TODO: Need a way to validate shared presentable images here, currently just allowing + // VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR + // as an acceptable layout, but need to make sure shared presentable images ONLY use that layout switch (subpass.pColorAttachments[j].layout) { case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: // This is ideal. + case VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR: + // TODO: See note above, just assuming that attachment is shared presentable and allowing this for now. break; case VK_IMAGE_LAYOUT_GENERAL: diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 089ceaa..fb81f7a 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -10342,6 +10342,19 @@ static bool PreCallValidateCreateSwapchainKHR(layer_data *dev_data, const char * return true; } } + // Validate state for shared presentable case + if (VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR == pCreateInfo->presentMode || + VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR == pCreateInfo->presentMode) { + if (pCreateInfo->minImageCount != 1) { + // TODO: Add unique error id when available. + if (log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, + reinterpret_cast(dev_data->device), __LINE__, DRAWSTATE_SWAPCHAIN_CREATE_BEFORE_QUERY, "DS", + "%s called with presentMode %s, but minImageCount value is %d. For shared presentable image, minImageCount " + "must be 1.", + func_name, string_VkPresentModeKHR(pCreateInfo->presentMode), pCreateInfo->minImageCount)) + return true; + } + } return false; } @@ -10352,6 +10365,10 @@ static void PostCallRecordCreateSwapchainKHR(layer_data *dev_data, VkResult resu if (VK_SUCCESS == result) { std::lock_guard lock(global_lock); auto swapchain_state = unique_ptr(new SWAPCHAIN_NODE(pCreateInfo, *pSwapchain)); + if (VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR == pCreateInfo->presentMode || + VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR == pCreateInfo->presentMode) { + swapchain_state->shared_presentable = true; + } surface_state->swapchain = swapchain_state.get(); dev_data->swapchainMap[*pSwapchain] = std::move(swapchain_state); } else { @@ -10503,6 +10520,12 @@ VKAPI_ATTR VkResult VKAPI_CALL QueuePresentKHR(VkQueue queue, const VkPresentInf } else { auto image = swapchain_data->images[pPresentInfo->pImageIndices[i]]; auto image_state = GetImageState(dev_data, image); + + if (image_state->shared_presentable) { + image_state->layout_locked = true; + } else { + image_state->acquired = false; + } skip |= ValidateImageMemoryIsValid(dev_data, image_state, "vkQueuePresentKHR()"); if (!image_state->acquired) { @@ -10689,6 +10712,10 @@ static void PostCallRecordCreateSharedSwapchainsKHR(layer_data *dev_data, VkResu if (VK_SUCCESS == result) { for (uint32_t i = 0; i < swapchainCount; i++) { auto swapchain_state = unique_ptr(new SWAPCHAIN_NODE(&pCreateInfos[i], pSwapchains[i])); + if (VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR == pCreateInfos[i].presentMode || + VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR == pCreateInfos[i].presentMode) { + swapchain_state->shared_presentable = true; + } surface_state[i]->swapchain = swapchain_state.get(); dev_data->swapchainMap[pSwapchains[i]] = std::move(swapchain_state); } @@ -10807,6 +10834,7 @@ VKAPI_ATTR VkResult VKAPI_CALL AcquireNextImageKHR(VkDevice device, VkSwapchainK auto image = swapchain_data->images[*pImageIndex]; auto image_state = GetImageState(dev_data, image); image_state->acquired = true; + image_state->shared_presentable = swapchain_data->shared_presentable; } lock.unlock(); diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h index 67631e5..7e76350 100644 --- a/layers/core_validation_types.h +++ b/layers/core_validation_types.h @@ -254,8 +254,10 @@ class IMAGE_STATE : public BINDABLE { VkImageCreateInfo createInfo; bool valid; // If this is a swapchain image backing memory track valid here as it doesn't have DEVICE_MEM_INFO bool acquired; // If this is a swapchain image, has it been acquired by the app. + bool shared_presentable; // True for a front-buffered swapchain image + bool layout_locked; // A front-buffered image that has been presented can never have layout transitioned IMAGE_STATE(VkImage img, const VkImageCreateInfo *pCreateInfo) - : image(img), createInfo(*pCreateInfo), valid(false), acquired(false) { + : image(img), createInfo(*pCreateInfo), valid(false), acquired(false), shared_presentable(false), layout_locked(false) { if ((createInfo.sharingMode == VK_SHARING_MODE_CONCURRENT) && (createInfo.queueFamilyIndexCount > 0)) { uint32_t *pQueueFamilyIndices = new uint32_t[createInfo.queueFamilyIndexCount]; for (uint32_t i = 0; i < createInfo.queueFamilyIndexCount; i++) { @@ -342,6 +344,7 @@ class SWAPCHAIN_NODE { VkSwapchainKHR swapchain; std::vector images; bool replaced = false; + bool shared_presentable = false; SWAPCHAIN_NODE(const VkSwapchainCreateInfoKHR *pCreateInfo, VkSwapchainKHR swapchain) : createInfo(pCreateInfo), swapchain(swapchain) {} }; diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp index 3539a83..c36831a 100644 --- a/layers/descriptor_sets.cpp +++ b/layers/descriptor_sets.cpp @@ -895,14 +895,29 @@ bool cvdescriptorset::ValidateImageUpdate(VkImageView image_view, VkImageLayout error_usage_bit = "VK_IMAGE_USAGE_STORAGE_BIT"; } else if (VK_IMAGE_LAYOUT_GENERAL != image_layout) { std::stringstream error_str; - // TODO : Need to create custom enum error code for this case - error_str - << "ImageView (" << image_view << ") of VK_DESCRIPTOR_TYPE_STORAGE_IMAGE type is being updated with layout " - << string_VkImageLayout(image_layout) - << " but according to spec section 13.1 Descriptor Types, 'Load and store operations on storage images can " - "only be done on images in VK_IMAGE_LAYOUT_GENERAL layout.'"; - *error_msg = error_str.str(); - return false; + // TODO : Need to create custom enum error codes for these cases + if (image_node->shared_presentable) { + if (VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR != image_layout) { + error_str + << "ImageView (" << image_view + << ") of VK_DESCRIPTOR_TYPE_STORAGE_IMAGE type with a front-buffered image is being updated with " + "layout " + << string_VkImageLayout(image_layout) + << " but according to spec section 13.1 Descriptor Types, 'Front-buffered images that report support " + "for " + "VK_FORMAT_FEATURE_STORAGE_IMAGE_BIT must be in the VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR layout.'"; + *error_msg = error_str.str(); + return false; + } + } else if (VK_IMAGE_LAYOUT_GENERAL != image_layout) { + error_str + << "ImageView (" << image_view << ") of VK_DESCRIPTOR_TYPE_STORAGE_IMAGE type is being updated with layout " + << string_VkImageLayout(image_layout) + << " but according to spec section 13.1 Descriptor Types, 'Load and store operations on storage images can " + "only be done on images in VK_IMAGE_LAYOUT_GENERAL layout.'"; + *error_msg = error_str.str(); + return false; + } } break; } -- 2.7.4