layers: Add DrawState checks for correct CmdBuffer reset
Can only reset individual CmdBuffers if they are allocated from a pool that
included the VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT. Also need to
intercept vkResetCommandPool() function and make sure that all individual
CmdBuffers allocated from that pool are reset when called. Finally, updated
naming of cmdBuffer states (RECORDING & RECORDED) to more closely match spec.
diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp
index f037d2f..5953de4 100644
--- a/layers/draw_state.cpp
+++ b/layers/draw_state.cpp
@@ -69,6 +69,12 @@
using std::unordered_map;
using std::unordered_set;
+// Track command pools and their command buffers
+struct CMD_POOL_INFO {
+ VkCommandPoolCreateFlags createFlags;
+ list<VkCommandBuffer> commandBuffers; // list container of cmd buffers allocated from this pool
+};
+
struct devExts {
VkBool32 debug_marker_enabled;
VkBool32 wsi_enabled;
@@ -92,7 +98,7 @@
unordered_map<VkBufferView, unique_ptr<VkBufferViewCreateInfo>> bufferViewMap;
unordered_map<VkBuffer, unique_ptr<VkBufferCreateInfo>> bufferMap;
unordered_map<VkPipeline, PIPELINE_NODE*> pipelineMap;
- unordered_map<VkCommandPool, list<VkCommandBuffer>> commandPoolMap;
+ unordered_map<VkCommandPool, CMD_POOL_INFO> commandPoolMap;
unordered_map<VkDescriptorPool, DESCRIPTOR_POOL_NODE*> descriptorPoolMap;
unordered_map<VkDescriptorSet, SET_NODE*> setMap;
unordered_map<VkDescriptorSetLayout, LAYOUT_NODE*> descriptorSetLayoutMap;
@@ -2808,7 +2814,7 @@
"CB %#" PRIxLEAST64 " was begun w/ VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT set, but has been submitted %#" PRIxLEAST64 " times.",
reinterpret_cast<uint64_t>(pCB->commandBuffer), pCB->submitCount);
}
- if (CB_UPDATE_COMPLETE != pCB->state) {
+ if (CB_RECORDED != pCB->state) {
// Flag error for using CB w/o vkEndCommandBuffer() called
// TODO : How to pass cb as srcObj?
skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, 0, 0, DRAWSTATE_NO_END_COMMAND_BUFFER, "DS",
@@ -2925,7 +2931,7 @@
}
// Remove commandBuffer reference from commandPoolMap
- dev_data->commandPoolMap[commandPool].remove(pCommandBuffers[i]);
+ dev_data->commandPoolMap[commandPool].commandBuffers.remove(pCommandBuffers[i]);
loader_platform_thread_unlock_mutex(&globalLock);
}
@@ -2940,7 +2946,7 @@
if (VK_SUCCESS == result) {
loader_platform_thread_lock_mutex(&globalLock);
- dev_data->commandPoolMap[*pCommandPool];
+ dev_data->commandPoolMap[*pCommandPool].createFlags = pCreateInfo->flags;
loader_platform_thread_unlock_mutex(&globalLock);
}
return result;
@@ -2953,11 +2959,11 @@
// Must remove cmdpool from cmdpoolmap, after removing all cmdbuffers in its list from the commandPoolMap
if (dev_data->commandPoolMap.find(commandPool) != dev_data->commandPoolMap.end()) {
- for (auto poolCb = dev_data->commandPoolMap[commandPool].begin(); poolCb != dev_data->commandPoolMap[commandPool].end();) {
+ for (auto poolCb = dev_data->commandPoolMap[commandPool].commandBuffers.begin(); poolCb != dev_data->commandPoolMap[commandPool].commandBuffers.end();) {
auto del_cb = dev_data->commandBufferMap.find(*poolCb);
delete (*del_cb).second; // delete CB info structure
dev_data->commandBufferMap.erase(del_cb); // Remove this command buffer from cbMap
- poolCb = dev_data->commandPoolMap[commandPool].erase(poolCb); // Remove CB reference from commandPoolMap's list
+ poolCb = dev_data->commandPoolMap[commandPool].commandBuffers.erase(poolCb); // Remove CB reference from commandPoolMap's list
}
}
dev_data->commandPoolMap.erase(commandPool);
@@ -2966,6 +2972,26 @@
dev_data->device_dispatch_table->DestroyCommandPool(device, commandPool, pAllocator);
}
+VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkResetCommandPool(
+ VkDevice device,
+ VkCommandPool commandPool,
+ VkCommandPoolResetFlags flags)
+{
+ layer_data *dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map);
+ VkResult result = VK_ERROR_VALIDATION_FAILED;
+
+ result = dev_data->device_dispatch_table->ResetCommandPool(device, commandPool, flags);
+ // Reset all of the CBs allocated from this pool
+ if (VK_SUCCESS == result) {
+ auto it = dev_data->commandPoolMap[commandPool].commandBuffers.begin();
+ while (it != dev_data->commandPoolMap[commandPool].commandBuffers.end()) {
+ resetCB(dev_data, (*it));
+ ++it;
+ }
+ }
+ return result;
+}
+
VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkDestroyFramebuffer(VkDevice device, VkFramebuffer framebuffer, const VkAllocationCallbacks* pAllocator)
{
get_my_data_ptr(get_dispatch_key(device), layer_data_map)->device_dispatch_table->DestroyFramebuffer(device, framebuffer, pAllocator);
@@ -3418,7 +3444,7 @@
if (dev_data->commandPoolMap.find(pCreateInfo->commandPool) != dev_data->commandPoolMap.end()) {
loader_platform_thread_lock_mutex(&globalLock);
// Add command buffer to its commandPool map
- dev_data->commandPoolMap[pCreateInfo->commandPool].push_back(pCommandBuffer[i]);
+ dev_data->commandPoolMap[pCreateInfo->commandPool].commandBuffers.push_back(pCommandBuffer[i]);
GLOBAL_CB_NODE* pCB = new GLOBAL_CB_NODE;
// Add command buffer to map
dev_data->commandBufferMap[pCommandBuffer[i]] = pCB;
@@ -3442,20 +3468,31 @@
if (pCB->createInfo.level == VK_COMMAND_BUFFER_LEVEL_PRIMARY) {
if (pBeginInfo->renderPass || pBeginInfo->framebuffer) {
// These should be NULL for a Primary CB
- skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, 0, 0, DRAWSTATE_BEGIN_CB_INVALID_STATE, "DS",
- "vkBeginCommandBuffer(): Primary Command Buffer (%p) may not specify framebuffer or renderpass parameters", (void*)commandBuffer);
+ skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, (uint64_t)commandBuffer, 0, DRAWSTATE_BEGIN_CB_INVALID_STATE, "DS",
+ "vkBeginCommandBuffer(): Primary Command Buffer (%p) may not specify framebuffer or renderpass parameters", (void*)commandBuffer);
}
} else {
if (!pBeginInfo->renderPass || !pBeginInfo->framebuffer) {
// These should NOT be null for an Secondary CB
- skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, 0, 0, DRAWSTATE_BEGIN_CB_INVALID_STATE, "DS",
- "vkBeginCommandBuffer(): Secondary Command Buffers (%p) must specify framebuffer and renderpass parameters", (void*)commandBuffer);
+ skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, (uint64_t)commandBuffer, 0, DRAWSTATE_BEGIN_CB_INVALID_STATE, "DS",
+ "vkBeginCommandBuffer(): Secondary Command Buffers (%p) must specify framebuffer and renderpass parameters", (void*)commandBuffer);
}
}
pCB->beginInfo = *pBeginInfo;
+ if (CB_RECORDING == pCB->state) {
+ skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, (uint64_t)commandBuffer, 0, DRAWSTATE_BEGIN_CB_INVALID_STATE, "DS",
+ "vkBeginCommandBuffer(): Cannot call Begin on CB (%#" PRIxLEAST64 ") in the RECORDING state. Must first call vkEndCommandBuffer().", (uint64_t)commandBuffer);
+ } else if (CB_RECORDED == pCB->state) {
+ VkCommandPool cmdPool = pCB->createInfo.commandPool;
+ if (!(VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT & dev_data->commandPoolMap[cmdPool].createFlags)) {
+ skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, (uint64_t)commandBuffer,
+ 0, DRAWSTATE_INVALID_COMMAND_BUFFER_RESET, "DS",
+ "Call to vkBeginCommandBuffer() on command buffer (%#" PRIxLEAST64 ") attempts to implicitly reset cmdBuffer created from command pool (%#" PRIxLEAST64 ") that does NOT have the VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT bit set.",
+ (uint64_t) commandBuffer, (uint64_t) cmdPool);
+ }
+ }
} else {
- // TODO : Need to pass commandBuffer as objType here
- skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, 0, 0, DRAWSTATE_INVALID_COMMAND_BUFFER, "DS",
+ skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, (uint64_t)commandBuffer, 0, DRAWSTATE_INVALID_COMMAND_BUFFER, "DS",
"In vkBeginCommandBuffer() and unable to find CommandBuffer Node for CB %p!", (void*)commandBuffer);
}
if (skipCall) {
@@ -3463,10 +3500,10 @@
}
VkResult result = dev_data->device_dispatch_table->BeginCommandBuffer(commandBuffer, pBeginInfo);
if ((VK_SUCCESS == result) && (pCB != NULL)) {
- if (CB_NEW != pCB->state) {
+ if (CB_RECORDED == pCB->state) {
resetCB(dev_data, commandBuffer);
}
- pCB->state = CB_UPDATE_ACTIVE;
+ pCB->state = CB_RECORDING;
}
return result;
}
@@ -3477,16 +3514,15 @@
VkResult result = VK_SUCCESS;
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
- /* TODO: preference is to always call API function after reporting any validation errors */
if (pCB) {
- if (pCB->state != CB_UPDATE_ACTIVE) {
+ if (pCB->state != CB_RECORDING) {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkEndCommandBuffer()");
}
}
if (VK_FALSE == skipCall) {
result = dev_data->device_dispatch_table->EndCommandBuffer(commandBuffer);
if (VK_SUCCESS == result) {
- pCB->state = CB_UPDATE_COMPLETE;
+ pCB->state = CB_RECORDED;
// Reset CB status flags
pCB->status = 0;
printCB(dev_data, commandBuffer);
@@ -3499,7 +3535,18 @@
VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkResetCommandBuffer(VkCommandBuffer commandBuffer, VkCommandBufferResetFlags flags)
{
+ VkBool32 skipCall = VK_FALSE;
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
+ GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
+ VkCommandPool cmdPool = pCB->createInfo.commandPool;
+ if (!(VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT & dev_data->commandPoolMap[cmdPool].createFlags)) {
+ skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_COMMAND_BUFFER, (uint64_t) commandBuffer,
+ 0, DRAWSTATE_INVALID_COMMAND_BUFFER_RESET, "DS",
+ "Attempt to reset command buffer (%#" PRIxLEAST64 ") created from command pool (%#" PRIxLEAST64 ") that does NOT have the VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT bit set.",
+ (uint64_t) commandBuffer, (uint64_t) cmdPool);
+ }
+ if (skipCall)
+ return VK_ERROR_VALIDATION_FAILED;
VkResult result = dev_data->device_dispatch_table->ResetCommandBuffer(commandBuffer, flags);
if (VK_SUCCESS == result) {
resetCB(dev_data, commandBuffer);
@@ -3513,7 +3560,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_BINDPIPELINE);
if ((VK_PIPELINE_BIND_POINT_COMPUTE == pipelineBindPoint) && (pCB->activeRenderPass)) {
skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, VK_OBJECT_TYPE_PIPELINE, (uint64_t) pipeline,
@@ -3556,7 +3603,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETVIEWPORTSTATE);
loader_platform_thread_lock_mutex(&globalLock);
pCB->status |= CBSTATUS_VIEWPORT_SET;
@@ -3580,7 +3627,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETSCISSORSTATE);
loader_platform_thread_lock_mutex(&globalLock);
pCB->status |= CBSTATUS_SCISSOR_SET;
@@ -3601,7 +3648,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETLINEWIDTHSTATE);
/* TODO: Do we still need this lock? */
loader_platform_thread_lock_mutex(&globalLock);
@@ -3626,7 +3673,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETDEPTHBIASSTATE);
pCB->status |= CBSTATUS_DEPTH_BIAS_SET;
pCB->depthBiasConstantFactor = depthBiasConstantFactor;
@@ -3646,7 +3693,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETBLENDSTATE);
pCB->status |= CBSTATUS_BLEND_SET;
memcpy(pCB->blendConstants, blendConstants, 4 * sizeof(float));
@@ -3667,7 +3714,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETDEPTHBOUNDSSTATE);
pCB->status |= CBSTATUS_DEPTH_BOUNDS_SET;
pCB->minDepthBounds = minDepthBounds;
@@ -3689,7 +3736,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETSTENCILREADMASKSTATE);
if (faceMask & VK_STENCIL_FACE_FRONT_BIT) {
pCB->front.compareMask = compareMask;
@@ -3717,7 +3764,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETSTENCILWRITEMASKSTATE);
if (faceMask & VK_STENCIL_FACE_FRONT_BIT) {
pCB->front.writeMask = writeMask;
@@ -3743,7 +3790,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETSTENCILREFERENCESTATE);
if (faceMask & VK_STENCIL_FACE_FRONT_BIT) {
pCB->front.reference = reference;
@@ -3766,7 +3813,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
if ((VK_PIPELINE_BIND_POINT_COMPUTE == pipelineBindPoint) && (pCB->activeRenderPass)) {
skipCall |= log_msg(dev_data->report_data, VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_RENDERPASS_CMD, "DS",
"Incorrectly binding compute DescriptorSets during active RenderPass (%#" PRIxLEAST64 ")", (uint64_t) pCB->activeRenderPass);
@@ -3845,7 +3892,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
VkDeviceSize offset_align = 0;
switch (indexType) {
case VK_INDEX_TYPE_UINT16:
@@ -3883,7 +3930,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
/* TODO: Need to track all the vertex buffers, not just last one */
pCB->lastVtxBinding = startBinding + bindingCount -1;
addCmd(dev_data, pCB, CMD_BINDVERTEXBUFFER);
@@ -3901,7 +3948,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
pCB->drawCount[DRAW]++;
skipCall |= validate_draw_state(dev_data, pCB, VK_FALSE);
// TODO : Need to pass commandBuffer as srcObj here
@@ -3926,7 +3973,7 @@
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
VkBool32 skipCall = VK_FALSE;
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
pCB->drawCount[DRAW_INDEXED]++;
skipCall |= validate_draw_state(dev_data, pCB, VK_TRUE);
// TODO : Need to pass commandBuffer as srcObj here
@@ -3951,7 +3998,7 @@
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
VkBool32 skipCall = VK_FALSE;
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
pCB->drawCount[DRAW_INDIRECT]++;
skipCall |= validate_draw_state(dev_data, pCB, VK_FALSE);
// TODO : Need to pass commandBuffer as srcObj here
@@ -3976,7 +4023,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
pCB->drawCount[DRAW_INDEXED_INDIRECT]++;
skipCall |= validate_draw_state(dev_data, pCB, VK_TRUE);
// TODO : Need to pass commandBuffer as srcObj here
@@ -4001,7 +4048,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_DISPATCH);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdDispatch()");
@@ -4018,7 +4065,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_DISPATCHINDIRECT);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdDispatchIndirect()");
@@ -4035,7 +4082,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_COPYBUFFER);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdCopyBuffer()");
@@ -4121,7 +4168,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_COPYIMAGE);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdCopyImage()");
@@ -4144,7 +4191,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_BLITIMAGE);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdBlitImage()");
@@ -4164,7 +4211,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_COPYBUFFERTOIMAGE);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdCopyBufferToImage()");
@@ -4185,7 +4232,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_COPYIMAGETOBUFFER);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdCopyImageToBuffer()");
@@ -4203,7 +4250,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_UPDATEBUFFER);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdUpdateBuffer()");
@@ -4220,7 +4267,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_FILLBUFFER);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdFillBuffer()");
@@ -4242,7 +4289,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
// Warn if this is issued prior to Draw Cmd and clearing the entire attachment
if (!hasDrawCmd(pCB) &&
(pCB->activeRenderPassBeginInfo.renderArea.extent.width == pRects[0].rect.extent.width) &&
@@ -4308,7 +4355,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_CLEARCOLORIMAGE);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdClearColorImage()");
@@ -4330,7 +4377,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_CLEARDEPTHSTENCILIMAGE);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdClearDepthStencilImage()");
@@ -4350,7 +4397,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_RESOLVEIMAGE);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdResolveImage()");
@@ -4367,7 +4414,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_SETEVENT);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdSetEvent()");
@@ -4384,7 +4431,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_RESETEVENT);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdResetEvent()");
@@ -4531,7 +4578,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_WAITEVENTS);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdWaitEvents()");
@@ -4549,7 +4596,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_PIPELINEBARRIER);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdPipelineBarrier()");
@@ -4567,7 +4614,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_BEGINQUERY);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdBeginQuery()");
@@ -4583,7 +4630,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_ENDQUERY);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdEndQuery()");
@@ -4599,7 +4646,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_RESETQUERYPOOL);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdResetQueryPool()");
@@ -4618,7 +4665,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_COPYQUERYPOOLRESULTS);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdCopyQueryPoolResults()");
@@ -4636,7 +4683,7 @@
layer_data* dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
GLOBAL_CB_NODE* pCB = getCBNode(dev_data, commandBuffer);
if (pCB) {
- if (pCB->state == CB_UPDATE_ACTIVE) {
+ if (pCB->state == CB_RECORDING) {
skipCall |= addCmd(dev_data, pCB, CMD_WRITETIMESTAMP);
} else {
skipCall |= report_error_no_cb_begin(dev_data, commandBuffer, "vkCmdWriteTimestamp()");
@@ -5522,6 +5569,8 @@
return (PFN_vkVoidFunction) vkCreateCommandPool;
if (!strcmp(funcName, "vkDestroyCommandPool"))
return (PFN_vkVoidFunction) vkDestroyCommandPool;
+ if (!strcmp(funcName, "vkResetCommandPool"))
+ return (PFN_vkVoidFunction) vkResetCommandPool;
if (!strcmp(funcName, "vkAllocateCommandBuffers"))
return (PFN_vkVoidFunction) vkAllocateCommandBuffers;
if (!strcmp(funcName, "vkFreeCommandBuffers"))
diff --git a/layers/draw_state.h b/layers/draw_state.h
index d2df8df..d935c15 100755
--- a/layers/draw_state.h
+++ b/layers/draw_state.h
@@ -79,7 +79,8 @@
DRAWSTATE_DESCRIPTOR_SET_NOT_BOUND, // DescriptorSet used by pipeline at draw time is not bound, or has been disturbed (which would have flagged previous warning)
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, // Primary/Secondary CB created with mismatched FB/RP information
+ 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_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
DRAWSTATE_MISSING_ATTACHMENT_REFERENCE, // Attachment reference must be present in active subpass
@@ -368,9 +369,9 @@
typedef enum _CB_STATE
{
- CB_NEW, // Newly created CB w/o any cmds
- CB_UPDATE_ACTIVE, // BeginCB has been called on this CB
- CB_UPDATE_COMPLETE // EndCB has been called on this CB
+ CB_NEW, // Newly created CB w/o any cmds
+ CB_RECORDING, // BeginCB has been called on this CB
+ CB_RECORDED // EndCB has been called on this CB
} CB_STATE;
// CB Status -- used to track status of various bindings on cmd buffer objects
typedef VkFlags CBStatusFlags;
diff --git a/layers/mem_tracker.cpp b/layers/mem_tracker.cpp
index e7462f2..5ce7974 100644
--- a/layers/mem_tracker.cpp
+++ b/layers/mem_tracker.cpp
@@ -2065,8 +2065,6 @@
VkBool32 skipCall = VK_FALSE;
VkResult result = VK_ERROR_VALIDATION_FAILED;
- // TODO: Check the commandPool's flags to see if reset is available for this pool.
-
auto it = my_data->commandPoolMap[commandPool].pCommandBuffers.begin();
// Verify that CB's in pool are complete (not in-flight)
while (it != my_data->commandPoolMap[commandPool].pCommandBuffers.end()) {
@@ -2081,6 +2079,7 @@
skipCall |= clear_cmd_buf_and_mem_references(my_data, (*it));
loader_platform_thread_unlock_mutex(&globalLock);
}
+ ++it;
}
if (VK_FALSE == skipCall) {
@@ -2137,8 +2136,6 @@
VkBool32 commandBufferComplete = VK_FALSE;
loader_platform_thread_lock_mutex(&globalLock);
- // TODO: Validate that this cmdBuffer's command pool allows reset
-
// Verify that CB is complete (not in-flight)
skipCall = checkCBCompleted(my_data, commandBuffer, &commandBufferComplete);
if (VK_FALSE == commandBufferComplete) {
diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md
index 6af4352..c4c4f34 100644
--- a/layers/vk_validation_layer_details.md
+++ b/layers/vk_validation_layer_details.md
@@ -14,7 +14,8 @@
| ----- | -------- | ---------------- | ------------ | -------- | ---------- |
| 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 level-related parameters | 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 |
+| 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 |
+| 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_DrawState 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_ParamChecker or VK_LAYER_LUNARG_ObjectTracker should really catch bad DSPool |
| Valid DescriptorSet | Validate that descriptor set was properly created and is currently valid | INVALID_SET | vkCmdBindDescriptorSets | None | Is this needed other places (like Update/Clear descriptors) |
@@ -86,7 +87,6 @@
23. For ClearAttachments function, verify that the index of referenced attachment actually exists
24. GetRenderAreaGranularity - The pname:renderPass parameter must be the same as the one given in the sname:VkRenderPassBeginInfo structure for which the render area is relevant.
28. Verify that all relevent dynamic state objects are bound (See https://cvs.khronos.org/bugzilla/show_bug.cgi?id=14323)
- 29. Flag an error if CommandBuffer has Begin called while it's being constructed - this is not a reset, this is a violation.
30. At PSO creation time, there is no case when NOT including a FS should flag an error since there exist dynamic state configurations that can be set to cause a FS to not be required. Instead, in the case when no FS is in the PSO, validation should detect at runtime if dynamic state will require a FS, and in those case issue a runtime warning about undefined behavior. (see bug https://cvs.khronos.org/bugzilla/show_bug.cgi?id=14429)
31. Error if a cmdbuffer is submitted on a queue whose family doesn't match the family of the pool from which it was created.
32. Update Gfx Pipe Create Info shadowing to remove new/delete and instead use unique_ptrs for auto clean-up
diff --git a/tests/layer_validation_tests.cpp b/tests/layer_validation_tests.cpp
index 65ed857..f79e58d 100644
--- a/tests/layer_validation_tests.cpp
+++ b/tests/layer_validation_tests.cpp
@@ -2277,6 +2277,53 @@
vkFreeCommandBuffers(m_device->device(), m_commandPool, 1, &draw_cmd);
}
+TEST_F(VkLayerTest, CommandBufferResetErrors)
+{
+ // Cause error due to Begin while recording CB
+ // Then cause 2 errors for attempting to reset CB w/o having
+ // VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT set for the pool from
+ // which CBs were allocated. Note that this bit is off by default.
+ m_errorMonitor->SetDesiredFailureMsg(VK_DBG_REPORT_ERROR_BIT,
+ "Cannot call Begin on CB");
+
+ ASSERT_NO_FATAL_FAILURE(InitState());
+
+ // Calls AllocateCommandBuffers
+ VkCommandBufferObj commandBuffer(m_device, m_commandPool);
+
+ // Force the failure by setting the Renderpass and Framebuffer fields with (fake) data
+ VkCommandBufferBeginInfo cmd_buf_info = {};
+ cmd_buf_info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
+ cmd_buf_info.pNext = NULL;
+ cmd_buf_info.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
+
+ // Begin CB to transition to recording state
+ vkBeginCommandBuffer(commandBuffer.GetBufferHandle(), &cmd_buf_info);
+ // Can't re-begin. This should trigger error
+ vkBeginCommandBuffer(commandBuffer.GetBufferHandle(), &cmd_buf_info);
+ if (!m_errorMonitor->DesiredMsgFound()) {
+ FAIL() << "Did not receive Error 'Cannot call Begin on CB (0x<ADDR>) in the RECORDING state...'";
+ m_errorMonitor->DumpFailureMsgs();
+ }
+ m_errorMonitor->SetDesiredFailureMsg(VK_DBG_REPORT_ERROR_BIT, "Attempt to reset command buffer ");
+ VkCommandBufferResetFlags flags = 0; // Don't care about flags for this test
+ // Reset attempt will trigger error due to incorrect CommandPool state
+ vkResetCommandBuffer(commandBuffer.GetBufferHandle(), flags);
+ if (!m_errorMonitor->DesiredMsgFound()) {
+ FAIL() << "Did not receive Error 'Attempt to reset command buffer (0x<ADDR>) created from command pool...'";
+ m_errorMonitor->DumpFailureMsgs();
+ }
+ m_errorMonitor->SetDesiredFailureMsg(VK_DBG_REPORT_ERROR_BIT, " attempts to implicitly reset cmdBuffer created from ");
+ // Transition CB to RECORDED state
+ vkEndCommandBuffer(commandBuffer.GetBufferHandle());
+ // Now attempting to Begin will implicitly reset, which triggers error
+ vkBeginCommandBuffer(commandBuffer.GetBufferHandle(), &cmd_buf_info);
+ if (!m_errorMonitor->DesiredMsgFound()) {
+ FAIL() << "Did not receive Error 'Call to vkBeginCommandBuffer() on command buffer (0x<ADDR>) attempts to implicitly reset...'";
+ m_errorMonitor->DumpFailureMsgs();
+ }
+}
+
TEST_F(VkLayerTest, InvalidPipelineCreateState)
{
// Attempt to Create Gfx Pipeline w/o a VS
diff --git a/tests/vktestbinding.h b/tests/vktestbinding.h
index 7cc7c5b..c631ec5 100644
--- a/tests/vktestbinding.h
+++ b/tests/vktestbinding.h
@@ -568,6 +568,7 @@
VkCommandPoolCreateInfo info = {};
info.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO;
info.queueFamilyIndex = queue_family_index;
+ info.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT;
return info;
}