From e22b00057b9d0de018e364c12707ce77abed6b86 Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Fri, 15 Jan 2016 13:34:44 -0700 Subject: [PATCH] layers: MR139, draw_state VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT checks Added a set to track which secondary cmdBuffers are contained within a primary cmdBuffer. Added new DRAWSTATE_INVALID_CB_SIMULTANEOUS_USE enum value for new error/warning cases, along with line in documentation. New validation issues and state updates are listed below. While tracking cmdBuffers at QueueSubmit time: 1. Flag an error if a cmdBuffer is already in flight and does not have VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set 2. Account for tracking of secondary cmdBuffers (and their in_use resources) that are submitted within a primary command buffer When submitting secondary command buffers into primary command buffer at CmdExecuteCommands time: 1. Flag an error if secondary cmdBuffer is in flight and doesn't have VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set. 2. Warn user if secondary cmdBuffer doesn't have VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set but primary cmdBuffer does. This causes primary cmdBuffer to be treated as if VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT isn't set. 3. Add secondary cmdBuffers to set off of primary cmdBuffer for tracking purposes 4. Add secondary cmdBuffers to inFlight set --- layers/draw_state.cpp | 42 +++++++++++++++++++++++++-- layers/draw_state.h | 3 ++ layers/vk_validation_layer_details.md | 1 + 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp index 416995d1..f8c72088 100644 --- a/layers/draw_state.cpp +++ b/layers/draw_state.cpp @@ -2605,6 +2605,7 @@ static void resetCB(layer_data* my_data, const VkCommandBuffer cb) pCB->waitedEvents.clear(); pCB->waitedEventsBeforeQueryReset.clear(); pCB->queryToStateMap.clear(); + pCB->secondaryCommandBuffers.clear(); } } @@ -3184,17 +3185,26 @@ void trackCommandBuffers(layer_data* my_data, VkQueue queue, uint32_t cmdBufferC my_data->fenceMap[fence].priorFence = priorFence; my_data->fenceMap[fence].needsSignaled = true; for (uint32_t i = 0; i < cmdBufferCount; ++i) { + for (auto secondaryCmdBuffer : my_data->commandBufferMap[pCmdBuffers[i]]->secondaryCommandBuffers) { + my_data->fenceMap[fence].cmdBuffers.push_back(secondaryCmdBuffer); + } my_data->fenceMap[fence].cmdBuffers.push_back(pCmdBuffers[i]); } } else { if (queue_data != my_data->queueMap.end()) { for (uint32_t i = 0; i < cmdBufferCount; ++i) { + for (auto secondaryCmdBuffer : my_data->commandBufferMap[pCmdBuffers[i]]->secondaryCommandBuffers) { + queue_data->second.untrackedCmdBuffers.push_back(secondaryCmdBuffer); + } queue_data->second.untrackedCmdBuffers.push_back(pCmdBuffers[i]); } } } if (queue_data != my_data->queueMap.end()) { for (uint32_t i = 0; i < cmdBufferCount; ++i) { + for (auto secondaryCmdBuffer : my_data->commandBufferMap[pCmdBuffers[i]]->secondaryCommandBuffers) { + my_data->inFlightCmdBuffers.insert(secondaryCmdBuffer); + } my_data->inFlightCmdBuffers.insert(pCmdBuffers[i]); } } @@ -3205,8 +3215,6 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueSubmit(VkQueue queue, uint VkBool32 skipCall = VK_FALSE; GLOBAL_CB_NODE* pCB = NULL; layer_data* dev_data = get_my_data_ptr(get_dispatch_key(queue), layer_data_map); - // TODO : Any pCommandBuffers must have USAGE_SIMULTANEOUS_USE_BIT set or cannot already be executing on device - // Same goes for any secondary CBs under the primary CB for (uint32_t submit_idx = 0; submit_idx < submitCount; submit_idx++) { const VkSubmitInfo *submit = &pSubmits[submit_idx]; for (uint32_t i=0; i < submit->waitSemaphoreCount; ++i) { @@ -3231,7 +3239,13 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueSubmit(VkQueue queue, uint pCB = getCBNode(dev_data, submit->pCommandBuffers[i]); loader_platform_thread_lock_mutex(&globalLock); pCB->submitCount++; // increment submit count + // Track in-use for resources off of primary and any secondary CBs skipCall |= validateAndIncrementResources(dev_data, pCB); + if (!pCB->secondaryCommandBuffers.empty()) { + for (auto secondaryCmdBuffer : pCB->secondaryCommandBuffers) { + skipCall |= validateAndIncrementResources(dev_data, dev_data->commandBufferMap[secondaryCmdBuffer]); + } + } if ((pCB->beginInfo.flags & VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT) && (pCB->submitCount > 1)) { skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, 0, __LINE__, DRAWSTATE_COMMAND_BUFFER_SINGLE_SUBMIT_VIOLATION, "DS", "CB %#" PRIxLEAST64 " was begun w/ VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT set, but has been submitted %#" PRIxLEAST64 " times.", @@ -3244,6 +3258,13 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueSubmit(VkQueue queue, uint loader_platform_thread_unlock_mutex(&globalLock); return VK_ERROR_VALIDATION_FAILED_EXT; } + // If USAGE_SIMULTANEOUS_USE_BIT not set then CB cannot already be executing on device + if (!(pCB->beginInfo.flags & VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT)) { + if (dev_data->inFlightCmdBuffers.find(pCB->commandBuffer) != dev_data->inFlightCmdBuffers.end()) { + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, reinterpret_cast(pCB->commandBuffer), __LINE__, DRAWSTATE_INVALID_CB_SIMULTANEOUS_USE, "DS", + "Attempt to simultaneously execute CB %#" PRIxLEAST64 " w/o VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set!", reinterpret_cast(pCB->commandBuffer)); + } + } loader_platform_thread_unlock_mutex(&globalLock); } trackCommandBuffers(dev_data, queue, submit->commandBufferCount, submit->pCommandBuffers, fence); @@ -3314,6 +3335,7 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueWaitIdle(VkQueue queue) { layer_data* dev_data = get_my_data_ptr(get_dispatch_key(queue), layer_data_map); decrementResources(dev_data, queue); + // TODO : Is this a bug? Can't clear all device CmdBuffers on QueueWaitIdle for (auto cmdBuffer : dev_data->inFlightCmdBuffers) { cleanInFlightCmdBuffer(dev_data, cmdBuffer); } @@ -5869,6 +5891,22 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdExecuteCommands(VkCommandBuffer } } } + // Secondary cmdBuffers are considered pending execution starting w/ being recorded + if (!(pSubCB->beginInfo.flags & VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT)) { + if (dev_data->inFlightCmdBuffers.find(pSubCB->commandBuffer) != dev_data->inFlightCmdBuffers.end()) { + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, reinterpret_cast(pCB->commandBuffer), __LINE__, DRAWSTATE_INVALID_CB_SIMULTANEOUS_USE, "DS", + "Attempt to simultaneously execute CB %#" PRIxLEAST64 " w/o VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set!", reinterpret_cast(pCB->commandBuffer)); + } + if (pCB->beginInfo.flags & VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT) { + // Warn that non-simultaneous secondary cmd buffer renders primary non-simultaneous + skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_WARN_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, reinterpret_cast(pCommandBuffers[i]), __LINE__, DRAWSTATE_INVALID_CB_SIMULTANEOUS_USE, "DS", + "vkCmdExecuteCommands(): Secondary Command Buffer (%#" PRIxLEAST64 ") does not have VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set and will cause primary command buffer (%#" PRIxLEAST64 ") to be treated as if it does not have VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set, even though it does.", + reinterpret_cast(pCommandBuffers[i]), reinterpret_cast(pCB->commandBuffer)); + pCB->beginInfo.flags &= ~VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT; + } + } + pCB->secondaryCommandBuffers.insert(pSubCB->commandBuffer); + dev_data->inFlightCmdBuffers.insert(pSubCB->commandBuffer); } skipCall |= validatePrimaryCommandBuffer(dev_data, pCB, "vkCmdExecuteComands"); skipCall |= addCmd(dev_data, pCB, CMD_EXECUTECOMMANDS, "vkCmdExecuteComands()"); diff --git a/layers/draw_state.h b/layers/draw_state.h index 1ab42286..9f17bb9a 100755 --- a/layers/draw_state.h +++ b/layers/draw_state.h @@ -86,6 +86,7 @@ typedef enum _DRAW_STATE_ERROR DRAWSTATE_INVALID_DYNAMIC_OFFSET_COUNT, // DescriptorSets bound with different number of dynamic descriptors that were included in dynamicOffsetCount DRAWSTATE_CLEAR_CMD_BEFORE_DRAW, // Clear cmd issued before any Draw in CommandBuffer, should use RenderPass Ops instead DRAWSTATE_BEGIN_CB_INVALID_STATE, // CB state at Begin call is bad. Can be Primary/Secondary CB created with mismatched FB/RP information or CB in RECORDING state + DRAWSTATE_INVALID_CB_SIMULTANEOUS_USE, // CmdBuffer is being used in violation of VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT rules (i.e. simultaneous use w/o that bit set) DRAWSTATE_INVALID_COMMAND_BUFFER_RESET, // Attempting to call Reset (or Begin on recorded cmdBuffer) that was allocated from Pool w/o VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT bit set DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, // Count for viewports and scissors mismatch and/or state doesn't match count DRAWSTATE_INVALID_IMAGE_ASPECT, // Image aspect is invalid for the current operation @@ -524,6 +525,8 @@ typedef struct _GLOBAL_CB_NODE { unordered_map imageLayoutMap; vector drawData; DRAW_DATA currentDrawData; + // If cmd buffer is primary, track secondary command buffers pending execution + std::unordered_set secondaryCommandBuffers; } GLOBAL_CB_NODE; typedef struct _SWAPCHAIN_NODE { diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md index d2dcbd24..ce002ad3 100644 --- a/layers/vk_validation_layer_details.md +++ b/layers/vk_validation_layer_details.md @@ -15,6 +15,7 @@ The VK_LAYER_LUNARG_draw_state layer tracks state leading into Draw cmds. This i | Valid Pipeline Layouts | Verify that sets being bound are compatible with their PipelineLayout and that the last-bound PSO PipelineLayout at Draw time is compatible with all bound sets used by that PSO | PIPELINE_LAYOUTS_INCOMPATIBLE | vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | TBD | None | | Validate DbgMarker exensions | Validates that DbgMarker extensions have been enabled before use | INVALID_EXTENSION | vkCmdDbgMarkerBegin vkCmdDbgMarkerEnd | TBD | None | | Valid BeginCommandBuffer state | Must not call Begin on command buffers that are being recorded, and primary command buffers must specify VK_NULL_HANDLE for RenderPass or Framebuffer parameters, while secondary command buffers must provide non-null parameters, | BEGIN_CB_INVALID_STATE | vkBeginCommandBuffer | PrimaryCommandBufferFramebufferAndRenderpass SecondaryCommandBufferFramebufferAndRenderpass | None | +| Command Buffer Simultaneous Use | Violation of VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT rules. Most likely attempting to simultaneously use a CmdBuffer w/o having that bit set. This also warns if you add secondary command buffer w/o that bit set to a primary command buffer that does have that bit set. | INVALID_CB_SIMULTANEOUS_USE | vkQueueSubmit vkCmdExecuteCommands | TODO | Write test | | Valid Command Buffer Reset | Can only reset individual command buffer that was allocated from a pool with VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT set | INVALID_COMMAND_BUFFER_RESET | vkBeginCommandBuffer vkResetCommandBuffer | CommandBufferResetErrors | None | | PSO Bound | Verify that a properly created and valid pipeline object is bound to the CommandBuffer specified in these calls | NO_PIPELINE_BOUND | vkCmdBindDescriptorSets vkCmdBindVertexBuffers | PipelineNotBound | This check is currently more related to VK_LAYER_LUNARG_draw_state data structures and less about verifying that PSO is bound at all appropriate points in API. For API purposes, need to make sure this is checked at Draw time and any other relevant calls. | | Valid DescriptorPool | Verifies that the descriptor set pool object was properly created and is valid | INVALID_POOL | vkResetDescriptorPool vkAllocateDescriptorSets | None | This is just an internal layer data structure check. VK_LAYER_LUNARG_param_checker or VK_LAYER_LUNARG_object_tracker should really catch bad DSPool | -- 2.34.1