diff options
| author | Tobin Ehlis <tobine@google.com> | 2017-04-07 12:20:30 -0600 |
|---|---|---|
| committer | Tobin Ehlis <tobine@google.com> | 2017-04-14 15:46:26 -0600 |
| commit | 7b59978c199c64bd8428cdae570f989002a7a2a4 (patch) | |
| tree | 73feaf1c10ead0280011020d5c81dc88357297e1 | |
| parent | 32cd102c36c6fca2679904aa9e59fdffc64ded59 (diff) | |
| download | usermoji-7b59978c199c64bd8428cdae570f989002a7a2a4.tar.xz | |
layers:Add image layout validation for descriptors
This change adds validation to make sure that an image layout at the
time the image is used in a descriptor matches the layout that was
given when the descriptor was updated.
Because image view covers a range of mip levels, loop over each level
and verify layouts one at a time.
Also Updated a number of validate functions to use cont ptr params for
data that they aren't changing.
| -rw-r--r-- | layers/buffer_validation.cpp | 29 | ||||
| -rw-r--r-- | layers/buffer_validation.h | 10 | ||||
| -rw-r--r-- | layers/core_validation.cpp | 17 | ||||
| -rw-r--r-- | layers/core_validation_types.h | 1 | ||||
| -rw-r--r-- | layers/descriptor_sets.cpp | 42 | ||||
| -rw-r--r-- | layers/descriptor_sets.h | 3 |
6 files changed, 73 insertions, 29 deletions
diff --git a/layers/buffer_validation.cpp b/layers/buffer_validation.cpp index 7f07a1d0..527b616b 100644 --- a/layers/buffer_validation.cpp +++ b/layers/buffer_validation.cpp @@ -68,7 +68,7 @@ void SetLayout(std::unordered_map<ImageSubresourcePair, IMAGE_LAYOUT_NODE> &imag imageLayoutMap[imgpair].layout = layout; } -bool FindLayoutVerifyNode(layer_data *device_data, GLOBAL_CB_NODE *pCB, ImageSubresourcePair imgpair, +bool FindLayoutVerifyNode(layer_data const *device_data, GLOBAL_CB_NODE const *pCB, ImageSubresourcePair imgpair, IMAGE_CMD_BUF_LAYOUT_NODE &node, const VkImageAspectFlags aspectMask) { const debug_report_data *report_data = core_validation::GetReportData(device_data); @@ -100,7 +100,7 @@ bool FindLayoutVerifyNode(layer_data *device_data, GLOBAL_CB_NODE *pCB, ImageSub return true; } -bool FindLayoutVerifyLayout(layer_data *device_data, ImageSubresourcePair imgpair, VkImageLayout &layout, +bool FindLayoutVerifyLayout(layer_data const *device_data, ImageSubresourcePair imgpair, VkImageLayout &layout, const VkImageAspectFlags aspectMask) { if (!(imgpair.subresource.aspectMask & aspectMask)) { return false; @@ -124,7 +124,7 @@ bool FindLayoutVerifyLayout(layer_data *device_data, ImageSubresourcePair imgpai } // Find layout(s) on the command buffer level -bool FindCmdBufLayout(layer_data *device_data, GLOBAL_CB_NODE *pCB, VkImage image, VkImageSubresource range, +bool FindCmdBufLayout(layer_data const *device_data, GLOBAL_CB_NODE const *pCB, VkImage image, VkImageSubresource range, IMAGE_CMD_BUF_LAYOUT_NODE &node) { ImageSubresourcePair imgpair = {image, true, range}; node = IMAGE_CMD_BUF_LAYOUT_NODE(VK_IMAGE_LAYOUT_MAX_ENUM, VK_IMAGE_LAYOUT_MAX_ENUM); @@ -528,9 +528,9 @@ void TransitionImageLayouts(layer_data *device_data, VkCommandBuffer cmdBuffer, } } -bool VerifyImageLayout(layer_data *device_data, GLOBAL_CB_NODE *cb_node, IMAGE_STATE *image_state, +bool VerifyImageLayout(layer_data const *device_data, GLOBAL_CB_NODE const *cb_node, IMAGE_STATE *image_state, VkImageSubresourceLayers subLayers, VkImageLayout explicit_layout, VkImageLayout optimal_layout, - const char *caller, UNIQUE_VALIDATION_ERROR_CODE msg_code) { + const char *caller, UNIQUE_VALIDATION_ERROR_CODE msg_code, bool *error) { const auto report_data = core_validation::GetReportData(device_data); const auto image = image_state->image; bool skip_call = false; @@ -541,6 +541,7 @@ bool VerifyImageLayout(layer_data *device_data, GLOBAL_CB_NODE *cb_node, IMAGE_S IMAGE_CMD_BUF_LAYOUT_NODE node; if (FindCmdBufLayout(device_data, cb_node, image, sub, node)) { if (node.layout != explicit_layout) { + *error = true; // TODO: Improve log message in the next pass skip_call |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, @@ -564,6 +565,7 @@ bool VerifyImageLayout(layer_data *device_data, GLOBAL_CB_NODE *cb_node, IMAGE_S reinterpret_cast<const uint64_t &>(image), string_VkImageLayout(optimal_layout)); } } else { + *error = true; skip_call |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, reinterpret_cast<uint64_t>(cb_node->commandBuffer), __LINE__, msg_code, "DS", "%s: Layout for image 0x%" PRIxLEAST64 " is %s but can only be %s or VK_IMAGE_LAYOUT_GENERAL. %s", @@ -1493,11 +1495,12 @@ bool PreCallValidateCmdCopyImage(layer_data *device_data, GLOBAL_CB_NODE *cb_nod VK_QUEUE_TRANSFER_BIT | VK_QUEUE_GRAPHICS_BIT | VK_QUEUE_COMPUTE_BIT, VALIDATION_ERROR_01193); skip |= ValidateCmd(device_data, cb_node, CMD_COPYIMAGE, "vkCmdCopyImage()"); skip |= insideRenderPass(device_data, cb_node, "vkCmdCopyImage()", VALIDATION_ERROR_01194); + bool hit_error = false; for (uint32_t i = 0; i < region_count; ++i) { skip |= VerifyImageLayout(device_data, cb_node, src_image_state, regions[i].srcSubresource, src_image_layout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, "vkCmdCopyImage()", VALIDATION_ERROR_01180); + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, "vkCmdCopyImage()", VALIDATION_ERROR_01180, &hit_error); skip |= VerifyImageLayout(device_data, cb_node, dst_image_state, regions[i].dstSubresource, dst_image_layout, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, "vkCmdCopyImage()", VALIDATION_ERROR_01183); + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, "vkCmdCopyImage()", VALIDATION_ERROR_01183, &hit_error); skip |= ValidateCopyImageTransferGranularityRequirements(device_data, cb_node, dst_image_state, ®ions[i], i, "vkCmdCopyImage()"); } @@ -3005,9 +3008,11 @@ bool PreCallValidateCmdCopyImageToBuffer(layer_data *device_data, VkImageLayout skip |= ValidateBufferUsageFlags(device_data, dst_buffer_state, VK_BUFFER_USAGE_TRANSFER_DST_BIT, true, VALIDATION_ERROR_01252, "vkCmdCopyImageToBuffer()", "VK_BUFFER_USAGE_TRANSFER_DST_BIT"); skip |= insideRenderPass(device_data, cb_node, "vkCmdCopyImageToBuffer()", VALIDATION_ERROR_01260); + bool hit_error = false; for (uint32_t i = 0; i < regionCount; ++i) { - skip |= VerifyImageLayout(device_data, cb_node, src_image_state, pRegions[i].imageSubresource, srcImageLayout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, "vkCmdCopyImageToBuffer()", VALIDATION_ERROR_01251); + skip |= + VerifyImageLayout(device_data, cb_node, src_image_state, pRegions[i].imageSubresource, srcImageLayout, + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, "vkCmdCopyImageToBuffer()", VALIDATION_ERROR_01251, &hit_error); skip |= ValidateCopyBufferImageTransferGranularityRequirements(device_data, cb_node, src_image_state, &pRegions[i], i, "vkCmdCopyImageToBuffer()"); } @@ -3077,9 +3082,11 @@ bool PreCallValidateCmdCopyBufferToImage(layer_data *device_data, VkImageLayout skip |= ValidateImageUsageFlags(device_data, dst_image_state, VK_IMAGE_USAGE_TRANSFER_DST_BIT, true, VALIDATION_ERROR_01231, "vkCmdCopyBufferToImage()", "VK_IMAGE_USAGE_TRANSFER_DST_BIT"); skip |= insideRenderPass(device_data, cb_node, "vkCmdCopyBufferToImage()", VALIDATION_ERROR_01242); + bool hit_error = false; for (uint32_t i = 0; i < regionCount; ++i) { - skip |= VerifyImageLayout(device_data, cb_node, dst_image_state, pRegions[i].imageSubresource, dstImageLayout, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, "vkCmdCopyBufferToImage()", VALIDATION_ERROR_01234); + skip |= + VerifyImageLayout(device_data, cb_node, dst_image_state, pRegions[i].imageSubresource, dstImageLayout, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, "vkCmdCopyBufferToImage()", VALIDATION_ERROR_01234, &hit_error); skip |= ValidateCopyBufferImageTransferGranularityRequirements(device_data, cb_node, dst_image_state, &pRegions[i], i, "vkCmdCopyBufferToImage()"); } diff --git a/layers/buffer_validation.h b/layers/buffer_validation.h index e4d1143f..1494afe0 100644 --- a/layers/buffer_validation.h +++ b/layers/buffer_validation.h @@ -51,9 +51,9 @@ uint32_t ResolveRemainingLayers(const VkImageSubresourceRange *range, uint32_t l bool VerifyClearImageLayout(layer_data *device_data, GLOBAL_CB_NODE *cb_node, IMAGE_STATE *image_state, VkImageSubresourceRange range, VkImageLayout dest_image_layout, const char *func_name); -bool VerifyImageLayout(layer_data *device_data, GLOBAL_CB_NODE *cb_node, IMAGE_STATE *image_state, +bool VerifyImageLayout(layer_data const *device_data, GLOBAL_CB_NODE const *cb_node, IMAGE_STATE *image_state, VkImageSubresourceLayers subLayers, VkImageLayout explicit_layout, VkImageLayout optimal_layout, - const char *caller, UNIQUE_VALIDATION_ERROR_CODE msg_code); + const char *caller, UNIQUE_VALIDATION_ERROR_CODE msg_code, bool *error); void RecordClearImageLayout(layer_data *dev_data, GLOBAL_CB_NODE *cb_node, VkImage image, VkImageSubresourceRange range, VkImageLayout dest_image_layout); @@ -68,13 +68,13 @@ bool PreCallValidateCmdClearDepthStencilImage(layer_data *dev_data, VkCommandBuf VkImageLayout imageLayout, uint32_t rangeCount, const VkImageSubresourceRange *pRanges); -bool FindLayoutVerifyNode(layer_data *device_data, GLOBAL_CB_NODE *pCB, ImageSubresourcePair imgpair, +bool FindLayoutVerifyNode(layer_data const *device_data, GLOBAL_CB_NODE const *pCB, ImageSubresourcePair imgpair, IMAGE_CMD_BUF_LAYOUT_NODE &node, const VkImageAspectFlags aspectMask); -bool FindLayoutVerifyLayout(layer_data *device_data, ImageSubresourcePair imgpair, VkImageLayout &layout, +bool FindLayoutVerifyLayout(layer_data const *device_data, ImageSubresourcePair imgpair, VkImageLayout &layout, const VkImageAspectFlags aspectMask); -bool FindCmdBufLayout(layer_data *device_data, GLOBAL_CB_NODE *pCB, VkImage image, VkImageSubresource range, +bool FindCmdBufLayout(layer_data const *device_data, GLOBAL_CB_NODE const *pCB, VkImage image, VkImageSubresource range, IMAGE_CMD_BUF_LAYOUT_NODE &node); bool FindGlobalLayout(layer_data *device_data, ImageSubresourcePair imgpair, VkImageLayout &layout); diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index ded7b803..b12e5d42 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -3022,13 +3022,14 @@ static bool ValidateDrawState(layer_data *dev_data, GLOBAL_CB_NODE *cb_node, con } // Validate the draw-time state for this descriptor set std::string err_str; - if (!descriptor_set->ValidateDrawState(set_binding_pair.second, state.dynamicOffsets[setIndex], &err_str)) { + if (!descriptor_set->ValidateDrawState(set_binding_pair.second, state.dynamicOffsets[setIndex], cb_node, function, + &err_str)) { auto set = descriptor_set->GetSet(); - result |= log_msg( - dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, - reinterpret_cast<const uint64_t &>(set), __LINE__, DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED, "DS", - "Descriptor set 0x%" PRIxLEAST64 " encountered the following validation error at %s() time: %s", - reinterpret_cast<const uint64_t &>(set), function, err_str.c_str()); + result |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, + VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, reinterpret_cast<const uint64_t &>(set), + __LINE__, DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED, "DS", + "Descriptor set 0x%" PRIxLEAST64 " encountered the following validation error at %s time: %s", + reinterpret_cast<const uint64_t &>(set), function, err_str.c_str()); } } } @@ -6207,6 +6208,10 @@ std::unordered_map<ImageSubresourcePair, IMAGE_LAYOUT_NODE> *GetImageLayoutMap(l return &device_data->imageLayoutMap; } +std::unordered_map<ImageSubresourcePair, IMAGE_LAYOUT_NODE> const *GetImageLayoutMap(layer_data const *device_data) { + return &device_data->imageLayoutMap; +} + std::unordered_map<VkBuffer, std::unique_ptr<BUFFER_STATE>> *GetBufferMap(layer_data *device_data) { return &device_data->bufferMap; } diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h index 246040fa..07f02334 100644 --- a/layers/core_validation_types.h +++ b/layers/core_validation_types.h @@ -862,6 +862,7 @@ const CHECK_DISABLED *GetDisables(layer_data *); std::unordered_map<VkImage, std::unique_ptr<IMAGE_STATE>> *GetImageMap(core_validation::layer_data *); std::unordered_map<VkImage, std::vector<ImageSubresourcePair>> *GetImageSubresourceMap(layer_data *); std::unordered_map<ImageSubresourcePair, IMAGE_LAYOUT_NODE> *GetImageLayoutMap(layer_data *); +std::unordered_map<ImageSubresourcePair, IMAGE_LAYOUT_NODE> const *GetImageLayoutMap(layer_data const *); std::unordered_map<VkBuffer, std::unique_ptr<BUFFER_STATE>> *GetBufferMap(layer_data *device_data); std::unordered_map<VkBufferView, std::unique_ptr<BUFFER_VIEW_STATE>> *GetBufferViewMap(layer_data *device_data); std::unordered_map<VkImageView, std::unique_ptr<IMAGE_VIEW_STATE>> *GetImageViewMap(layer_data *device_data); diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp index 84663c4b..de349457 100644 --- a/layers/descriptor_sets.cpp +++ b/layers/descriptor_sets.cpp @@ -24,6 +24,7 @@ #include "descriptor_sets.h" #include "vk_enum_string_helper.h" #include "vk_safe_struct.h" +#include "buffer_validation.h" #include <sstream> #include <algorithm> @@ -395,7 +396,8 @@ bool cvdescriptorset::DescriptorSet::IsCompatible(const DescriptorSetLayout *lay // that any update buffers are valid, and that any dynamic offsets are within the bounds of their buffers. // Return true if state is acceptable, or false and write an error message into error string bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t, descriptor_req> &bindings, - const std::vector<uint32_t> &dynamic_offsets, std::string *error) const { + const std::vector<uint32_t> &dynamic_offsets, const GLOBAL_CB_NODE *cb_node, + const char *caller, std::string *error) const { for (auto binding_pair : bindings) { auto binding = binding_pair.first; if (!p_layout_->HasBinding(binding)) { @@ -472,9 +474,15 @@ bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t, } } } else if (descriptor_class == ImageSampler || descriptor_class == Image) { - auto image_view = (descriptor_class == ImageSampler) - ? static_cast<ImageSamplerDescriptor *>(descriptors_[i].get())->GetImageView() - : static_cast<ImageDescriptor *>(descriptors_[i].get())->GetImageView(); + VkImageView image_view; + VkImageLayout image_layout; + if (descriptor_class == ImageSampler) { + image_view = static_cast<ImageSamplerDescriptor *>(descriptors_[i].get())->GetImageView(); + image_layout = static_cast<ImageSamplerDescriptor *>(descriptors_[i].get())->GetImageLayout(); + } else { + image_view = static_cast<ImageDescriptor *>(descriptors_[i].get())->GetImageView(); + image_layout = static_cast<ImageDescriptor *>(descriptors_[i].get())->GetImageLayout(); + } auto reqs = binding_pair.second; auto image_view_state = GetImageViewState(device_data_, image_view); @@ -493,7 +501,30 @@ bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t, auto image_node = GetImageState(device_data_, image_view_ci.image); assert(image_node); - + // Verify Image Layout + // TODO: VALIDATION_ERROR_02981 is the error physically closest to the spec language of interest, however + // there is no VUID for the actual spec language. Need to file a spec MR to add VU language for: + // imageLayout is the layout that the image subresources accessible from imageView will be in at the time + // this descriptor is accessed. + // Copy first mip level into sub_layers and loop over each mip level to verify layout + VkImageSubresourceLayers sub_layers; + sub_layers.aspectMask = image_view_ci.subresourceRange.aspectMask; + sub_layers.baseArrayLayer = image_view_ci.subresourceRange.baseArrayLayer; + sub_layers.layerCount = image_view_ci.subresourceRange.layerCount; + bool hit_error = false; + for (auto cur_level = image_view_ci.subresourceRange.baseMipLevel; + cur_level < image_view_ci.subresourceRange.levelCount; ++cur_level) { + sub_layers.mipLevel = cur_level; + VerifyImageLayout(device_data_, cb_node, image_node, sub_layers, image_layout, + VK_IMAGE_LAYOUT_UNDEFINED, caller, VALIDATION_ERROR_02981, &hit_error); + if (hit_error) { + *error = + "Image layout specified at vkUpdateDescriptorSets() time doesn't match actual image layout at " + "time descriptor is used. See previous error callback for specific details."; + return false; + } + } + // Verify Sample counts if ((reqs & DESCRIPTOR_REQ_SINGLE_SAMPLE) && image_node->createInfo.samples != VK_SAMPLE_COUNT_1_BIT) { std::stringstream error_str; error_str << "Descriptor in binding #" << binding << " at global descriptor index " << i @@ -502,7 +533,6 @@ bool cvdescriptorset::DescriptorSet::ValidateDrawState(const std::map<uint32_t, *error = error_str.str(); return false; } - if ((reqs & DESCRIPTOR_REQ_MULTI_SAMPLE) && image_node->createInfo.samples == VK_SAMPLE_COUNT_1_BIT) { std::stringstream error_str; error_str << "Descriptor in binding #" << binding << " at global descriptor index " << i diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h index 293b481b..0e8cdb59 100644 --- a/layers/descriptor_sets.h +++ b/layers/descriptor_sets.h @@ -352,7 +352,8 @@ class DescriptorSet : public BASE_NODE { // Is this set compatible with the given layout? bool IsCompatible(const DescriptorSetLayout *, std::string *) const; // For given bindings validate state at time of draw is correct, returning false on error and writing error details into string* - bool ValidateDrawState(const std::map<uint32_t, descriptor_req> &, const std::vector<uint32_t> &, std::string *) const; + bool ValidateDrawState(const std::map<uint32_t, descriptor_req> &, const std::vector<uint32_t> &, const GLOBAL_CB_NODE *, + const char *caller, std::string *) const; // For given set of bindings, add any buffers and images that will be updated to their respective unordered_sets & return number // of objects inserted uint32_t GetStorageUpdates(const std::map<uint32_t, descriptor_req> &, std::unordered_set<VkBuffer> *, |
