From 7e5c0c26004626cf6826dfe2779a738a1f9f1fff Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Wed, 14 Sep 2016 11:21:55 -0600 Subject: [PATCH] layers: Add support code to handle in_use for missing objects Add case block to in-use Increment/Decrement functions to handle the missing object types that may be bound to a command buffer. The actual bindings for these missing types will be added in a future commit. Here are the types that were added: VkBufferView VkImageView VkDescriptorPool VkCommandPool VkFramebuffer VkRenderPass VkDeviceMemory Also added new error enums for all of these type except RENDERPASS which already had an appropriate error enum. Unify object BASE_NODE look-up for removeCommandBufferBinding() and DecrementBoundResources() to use common GetStateStructPtrFromObject() function. Finally, had to update the state-wrapping class for DescriptorPool, DeviceMemory and RenderPass to derive from BASE_NODE so that in_use can be tracked. --- layers/core_validation.cpp | 180 +++++++++++++++++++++------------- layers/core_validation.h | 4 +- layers/core_validation_error_enums.h | 6 ++ layers/core_validation_types.h | 6 +- layers/vk_validation_layer_details.md | 6 ++ 5 files changed, 127 insertions(+), 75 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 4ab2b2d..42d36b9 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -448,6 +448,10 @@ static const char *object_type_to_string(VkDebugReportObjectTypeEXT type) { return "image"; case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_EXT: return "buffer"; + case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_VIEW_EXT: + return "image view"; + case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_VIEW_EXT: + return "buffer view"; case VK_DEBUG_REPORT_OBJECT_TYPE_SWAPCHAIN_KHR_EXT: return "swapchain"; case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: @@ -458,10 +462,18 @@ static const char *object_type_to_string(VkDebugReportObjectTypeEXT type) { return "event"; case VK_DEBUG_REPORT_OBJECT_TYPE_QUERY_POOL_EXT: return "query pool"; + case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_POOL_EXT: + return "descriptor pool"; + case VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_POOL_EXT: + return "command pool"; case VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT: return "pipeline"; case VK_DEBUG_REPORT_OBJECT_TYPE_SAMPLER_EXT: return "sampler"; + case VK_DEBUG_REPORT_OBJECT_TYPE_RENDER_PASS_EXT: + return "renderpass"; + case VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_MEMORY_EXT: + return "device memory"; case VK_DEBUG_REPORT_OBJECT_TYPE_SEMAPHORE_EXT: return "semaphore"; default: @@ -3932,61 +3944,86 @@ static bool addCmd(layer_data *my_data, GLOBAL_CB_NODE *pCB, const CMD_TYPE cmd, } return skip_call; } -// Tie the VK_OBJECT to the cmd buffer which includes: -// Add object_binding to cmd buffer -// Add cb_binding to object -static void addCommandBufferBinding(std::unordered_set *cb_bindings, VK_OBJECT obj, GLOBAL_CB_NODE *cb_node) { - cb_bindings->insert(cb_node); - cb_node->object_bindings.insert(obj); -} -// For a given object, if cb_node is in that objects cb_bindings, remove cb_node -static void removeCommandBufferBinding(layer_data *dev_data, VK_OBJECT const *object, GLOBAL_CB_NODE *cb_node) { - switch (object->type) { - case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT: { - auto img_node = getImageNode(dev_data, reinterpret_cast(object->handle)); - if (img_node) - img_node->cb_bindings.erase(cb_node); +// For given object struct return a ptr of BASE_NODE type for its wrapping struct +BASE_NODE *GetStateStructPtrFromObject(layer_data *dev_data, VK_OBJECT object_struct) { + BASE_NODE *base_ptr = nullptr; + switch (object_struct.type) { + case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: { + base_ptr = getSetNode(dev_data, reinterpret_cast(object_struct.handle)); + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_SAMPLER_EXT: { + base_ptr = getSamplerNode(dev_data, reinterpret_cast(object_struct.handle)); + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_QUERY_POOL_EXT: { + base_ptr = getQueryPoolNode(dev_data, reinterpret_cast(object_struct.handle)); + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT: { + base_ptr = getPipeline(dev_data, reinterpret_cast(object_struct.handle)); break; } case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_EXT: { - auto buf_node = getBufferNode(dev_data, reinterpret_cast(object->handle)); - if (buf_node) - buf_node->cb_bindings.erase(cb_node); + base_ptr = getBufferNode(dev_data, reinterpret_cast(object_struct.handle)); + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_VIEW_EXT: { + base_ptr = getBufferViewState(dev_data, reinterpret_cast(object_struct.handle)); + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT: { + base_ptr = getImageNode(dev_data, reinterpret_cast(object_struct.handle)); + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_VIEW_EXT: { + base_ptr = getImageViewState(dev_data, reinterpret_cast(object_struct.handle)); break; } case VK_DEBUG_REPORT_OBJECT_TYPE_EVENT_EXT: { - auto evt_node = getEventNode(dev_data, reinterpret_cast(object->handle)); - if (evt_node) - evt_node->cb_bindings.erase(cb_node); + base_ptr = getEventNode(dev_data, reinterpret_cast(object_struct.handle)); break; } - case VK_DEBUG_REPORT_OBJECT_TYPE_QUERY_POOL_EXT: { - auto qp_node = getQueryPoolNode(dev_data, reinterpret_cast(object->handle)); - if (qp_node) - qp_node->cb_bindings.erase(cb_node); + case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_POOL_EXT: { + base_ptr = getPoolNode(dev_data, reinterpret_cast(object_struct.handle)); break; } - case VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT: { - auto pipe_node = getPipeline(dev_data, reinterpret_cast(object->handle)); - if (pipe_node) - pipe_node->cb_bindings.erase(cb_node); + case VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_POOL_EXT: { + base_ptr = getCommandPoolNode(dev_data, reinterpret_cast(object_struct.handle)); break; } - case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: { - auto set_node = getSetNode(dev_data, reinterpret_cast(object->handle)); - if (set_node) - set_node->RemoveBoundCommandBuffer(cb_node); + case VK_DEBUG_REPORT_OBJECT_TYPE_FRAMEBUFFER_EXT: { + base_ptr = getFramebuffer(dev_data, reinterpret_cast(object_struct.handle)); break; } - case VK_DEBUG_REPORT_OBJECT_TYPE_SAMPLER_EXT: { - auto sampler_node = getSamplerNode(dev_data, reinterpret_cast(object->handle)); - if (sampler_node) - sampler_node->cb_bindings.erase(cb_node); + case VK_DEBUG_REPORT_OBJECT_TYPE_RENDER_PASS_EXT: { + base_ptr = getRenderPass(dev_data, reinterpret_cast(object_struct.handle)); + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_MEMORY_EXT: { + base_ptr = getMemObjInfo(dev_data, reinterpret_cast(object_struct.handle)); break; } default: - assert(0); // unhandled object type + // TODO : Any other objects to be handled here? + assert(0); + break; } + return base_ptr; +} + +// Tie the VK_OBJECT to the cmd buffer which includes: +// Add object_binding to cmd buffer +// Add cb_binding to object +static void addCommandBufferBinding(std::unordered_set *cb_bindings, VK_OBJECT obj, GLOBAL_CB_NODE *cb_node) { + cb_bindings->insert(cb_node); + cb_node->object_bindings.insert(obj); +} +// For a given object, if cb_node is in that objects cb_bindings, remove cb_node +static void removeCommandBufferBinding(layer_data *dev_data, VK_OBJECT const *object, GLOBAL_CB_NODE *cb_node) { + BASE_NODE *base_obj = GetStateStructPtrFromObject(dev_data, *object); + if (base_obj) + base_obj->cb_bindings.erase(cb_node); } // Reset the command buffer state // Maintain the createInfo and set state to CB_NEW, but clear all other state @@ -4531,16 +4568,51 @@ static bool ValidateAndIncrementBoundObjects(layer_data *dev_data, GLOBAL_CB_NOD error_code = DRAWSTATE_INVALID_BUFFER; break; } + case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_VIEW_EXT: { + base_obj = getBufferViewState(dev_data, reinterpret_cast(obj.handle)); + error_code = DRAWSTATE_INVALID_BUFFER_VIEW; + break; + } case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT: { base_obj = getImageNode(dev_data, reinterpret_cast(obj.handle)); error_code = DRAWSTATE_INVALID_IMAGE; break; } + case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_VIEW_EXT: { + base_obj = getImageViewState(dev_data, reinterpret_cast(obj.handle)); + error_code = DRAWSTATE_INVALID_IMAGE_VIEW; + break; + } case VK_DEBUG_REPORT_OBJECT_TYPE_EVENT_EXT: { base_obj = getEventNode(dev_data, reinterpret_cast(obj.handle)); error_code = DRAWSTATE_INVALID_EVENT; break; } + case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_POOL_EXT: { + base_obj = getPoolNode(dev_data, reinterpret_cast(obj.handle)); + error_code = DRAWSTATE_INVALID_DESCRIPTOR_POOL; + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_POOL_EXT: { + base_obj = getCommandPoolNode(dev_data, reinterpret_cast(obj.handle)); + error_code = DRAWSTATE_INVALID_COMMAND_POOL; + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_FRAMEBUFFER_EXT: { + base_obj = getFramebuffer(dev_data, reinterpret_cast(obj.handle)); + error_code = DRAWSTATE_INVALID_FRAMEBUFFER; + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_RENDER_PASS_EXT: { + base_obj = getRenderPass(dev_data, reinterpret_cast(obj.handle)); + error_code = DRAWSTATE_INVALID_RENDERPASS; + break; + } + case VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_MEMORY_EXT: { + base_obj = getMemObjInfo(dev_data, reinterpret_cast(obj.handle)); + error_code = DRAWSTATE_INVALID_DEVICE_MEMORY; + break; + } default: // TODO : Merge handling of other objects types into this code break; @@ -4625,39 +4697,7 @@ static inline void removeInFlightCmdBuffer(layer_data *dev_data, VkCommandBuffer static void DecrementBoundResources(layer_data *dev_data, GLOBAL_CB_NODE const *cb_node) { BASE_NODE *base_obj = nullptr; for (auto obj : cb_node->object_bindings) { - switch (obj.type) { - case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: { - base_obj = getSetNode(dev_data, reinterpret_cast(obj.handle)); - break; - } - case VK_DEBUG_REPORT_OBJECT_TYPE_SAMPLER_EXT: { - base_obj = getSamplerNode(dev_data, reinterpret_cast(obj.handle)); - break; - } - case VK_DEBUG_REPORT_OBJECT_TYPE_QUERY_POOL_EXT: { - base_obj = getQueryPoolNode(dev_data, reinterpret_cast(obj.handle)); - break; - } - case VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT: { - base_obj = getPipeline(dev_data, reinterpret_cast(obj.handle)); - break; - } - case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_EXT: { - base_obj = getBufferNode(dev_data, reinterpret_cast(obj.handle)); - break; - } - case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT: { - base_obj = getImageNode(dev_data, reinterpret_cast(obj.handle)); - break; - } - case VK_DEBUG_REPORT_OBJECT_TYPE_EVENT_EXT: { - base_obj = getEventNode(dev_data, reinterpret_cast(obj.handle)); - break; - } - default: - // TODO : Merge handling of other objects types into this code - break; - } + base_obj = GetStateStructPtrFromObject(dev_data, obj); if (base_obj) { base_obj->in_use.fetch_sub(1); } diff --git a/layers/core_validation.h b/layers/core_validation.h index d7374f6..aa3058e 100644 --- a/layers/core_validation.h +++ b/layers/core_validation.h @@ -176,7 +176,7 @@ class QUERY_POOL_NODE : public BASE_NODE { VkQueryPoolCreateInfo createInfo; }; -class FRAMEBUFFER_NODE : BASE_NODE { +class FRAMEBUFFER_NODE : public BASE_NODE { public: using BASE_NODE::in_use; using BASE_NODE::cb_bindings; @@ -196,7 +196,7 @@ typedef struct stencil_data { } CBStencilData; // Track command pools and their command buffers -struct COMMAND_POOL_NODE { +struct COMMAND_POOL_NODE : public BASE_NODE { VkCommandPoolCreateFlags createFlags; uint32_t queueFamilyIndex; // TODO: why is this std::list? diff --git a/layers/core_validation_error_enums.h b/layers/core_validation_error_enums.h index e55ef3a..0bb18d8 100644 --- a/layers/core_validation_error_enums.h +++ b/layers/core_validation_error_enums.h @@ -62,11 +62,17 @@ enum DRAW_STATE_ERROR { DRAWSTATE_INVALID_BARRIER, // Invalid Barrier DRAWSTATE_INVALID_BUFFER, // Invalid Buffer DRAWSTATE_INVALID_IMAGE, // Invalid Image + DRAWSTATE_INVALID_BUFFER_VIEW, // Invalid BufferView + DRAWSTATE_INVALID_IMAGE_VIEW, // Invalid ImageView DRAWSTATE_INVALID_QUERY, // Invalid Query DRAWSTATE_INVALID_QUERY_POOL, // Invalid QueryPool + DRAWSTATE_INVALID_DESCRIPTOR_POOL, // Invalid DescriptorPool + DRAWSTATE_INVALID_COMMAND_POOL, // Invalid CommandPool DRAWSTATE_INVALID_FENCE, // Invalid Fence DRAWSTATE_INVALID_EVENT, // Invalid Event DRAWSTATE_INVALID_SAMPLER, // Invalid Sampler + DRAWSTATE_INVALID_FRAMEBUFFER, // Invalid Framebuffer + DRAWSTATE_INVALID_DEVICE_MEMORY, // Invalid DeviceMemory DRAWSTATE_VTX_INDEX_OUT_OF_BOUNDS, // binding in vkCmdBindVertexData() too // large for PSO's // pVertexBindingDescriptions array diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h index 48642a7..21f5510 100644 --- a/layers/core_validation_types.h +++ b/layers/core_validation_types.h @@ -108,7 +108,7 @@ enum descriptor_req { DESCRIPTOR_REQ_MULTI_SAMPLE = DESCRIPTOR_REQ_SINGLE_SAMPLE << 1, }; -struct DESCRIPTOR_POOL_NODE { +struct DESCRIPTOR_POOL_NODE : BASE_NODE { VkDescriptorPool pool; uint32_t maxSets; // Max descriptor sets allowed in this pool uint32_t availableSets; // Available descriptor sets in this pool @@ -244,7 +244,7 @@ struct MEMORY_RANGE { }; // Data struct for tracking memory object -struct DEVICE_MEM_INFO { +struct DEVICE_MEM_INFO : public BASE_NODE { void *object; // Dispatchable object used to create this memory (device of swapchain) bool global_valid; // If allocation is mapped, set to "true" to be picked up by subsequently bound ranges VkDeviceMemory mem; @@ -318,7 +318,7 @@ struct DAGNode { std::vector next; }; -struct RENDER_PASS_NODE { +struct RENDER_PASS_NODE : public BASE_NODE { VkRenderPass renderPass; VkRenderPassCreateInfo const *pCreateInfo; std::vector hasSelfDependency; diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md index fdc132b..3c97fda 100644 --- a/layers/vk_validation_layer_details.md +++ b/layers/vk_validation_layer_details.md @@ -91,6 +91,12 @@ The Draw State portion of the core validation layer tracks state leading into Dr | Verify Memory Buffer Not Deleted | Validate Command Buffer not submitted with deleted memory buffer | INVALID_BUFFER | vkQueueSubmit | VertexBufferInvalid | None | | Verify Image Not Deleted | Validate Command Buffer not submitted with deleted image | INVALID_IMAGE | vkQueueSubmit | TODO | Write test (or record here if we already have one) | | Verify Query Pool Not Deleted | Validate Command Buffer not submitted with deleted query pool | INVALID_QUERY_POOL | vkQueueSubmit | TODO | Write test (or record here if we already have one) | +| Verify BufferView Not Deleted | Validate Command Buffer not submitted with deleted buffer view | INVALID_BUFFER_VIEW | vkQueueSubmit | TODO | Write test (or record here if we already have one) | +| Verify ImageView Not Deleted | Validate Command Buffer not submitted with deleted image view | INVALID_IMAGE_VIEW | vkQueueSubmit | TODO | Write test (or record here if we already have one) | +| Verify DescriptorPool Not Deleted | Validate Command Buffer not submitted with deleted descriptor pool | INVALID_DESCRIPTOR_POOL | vkQueueSubmit | TODO | Write test (or record here if we already have one) | +| Verify CommandPool Not Deleted | Validate Command Buffer not submitted with deleted command pool | INVALID_COMMAND_POOL | vkQueueSubmit | TODO | Write test (or record here if we already have one) | +| Verify Framebuffer Not Deleted | Validate Command Buffer not submitted with deleted framebuffer | INVALID_FRAMEBUFFER | vkQueueSubmit | TODO | Write test (or record here if we already have one) | +| Verify DeviceMemory Not Deleted | Validate Command Buffer not submitted with deleted device memory | INVALID_DEVICE_MEMORY | vkQueueSubmit | TODO | Write test (or record here if we already have one) | | Verify Memory Buffer Destroy | Validate memory buffers are not destroyed more than once | DOUBLE_DESTROY | vkDestroyBuffer | VertexBufferInvalid | None | | Verify Object Not In Use | Validate that object being freed or modified is not in use | OBJECT_INUSE | vkDestroyBuffer vkFreeDescriptorSets vkUpdateDescriptorSets vkDestroySemaphore | InUseDestroyedSignaled InvalidCmdBufferDescriptorSetImageSamplerDestroyed | NA | | Verify Get Queries| Validate that queries are properly setup, initialized and synchronized | INVALID_QUERY | vkGetFenceStatus vkQueueWaitIdle vkWaitForFences vkDeviceWaitIdle vkCmdBeginQuery vkCmdEndQuery | InvalidQueueIndexInvalidQuery | May need to check existing case against object_tracker and remove any redundant checks. Then write tests for remaining case. Currently there are 8 cases for this check with 1 each in cleanInFlightCmdBuffer(), EndCommandBuffer(), CmdEndQuery(), validateQuery(), and 4 cases in GetQueryPoolResults() | -- 2.7.4