From d4f3f2d2ed0f71b00f14939259d8b37fd224a257 Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Mon, 11 Jan 2016 13:18:40 -0700 Subject: layers: MR127, Update DrawState handling of non-updated descriptors At CmdBindDescriptorSets() time, only warn on non-updated descriptor if that descriptor is not empty. At draw time, if a descriptor is being used and it has not been updated, trigger an error. --- layers/draw_state.cpp | 22 ++++++++++++++-------- layers/vk_validation_layer_details.md | 2 +- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp index 6c341f1a..ceac2cea 100644 --- a/layers/draw_state.cpp +++ b/layers/draw_state.cpp @@ -1405,9 +1405,15 @@ static VkBool32 validate_draw_state(layer_data* my_data, GLOBAL_CB_NODE* pCB, Vk result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, (uint64_t)setHandle, __LINE__, DRAWSTATE_PIPELINE_LAYOUTS_INCOMPATIBLE, "DS", "VkDescriptorSet (%#" PRIxLEAST64 ") bound as set #%u is not compatible with overlapping VkPipelineLayout %#" PRIxLEAST64 " due to: %s", (uint64_t)setHandle, setIndex, (uint64_t)pPipe->graphicsPipelineCI.layout, errorString.c_str()); - } else { // Valid set is bound and layout compatible, validate any dynamic offsets - // Pull the set node, and for each dynamic descriptor, make sure dynamic offset doesn't overstep buffer + } else { // Valid set is bound and layout compatible, validate that it's updated and verify any dynamic offsets + // Pull the set node SET_NODE* pSet = getSetNode(my_data, pCB->boundDescriptorSets[setIndex]); + // Make sure set has been updated + if (!pSet->pUpdateStructs) { + result |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, (uint64_t) pSet->set, __LINE__, DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED, "DS", + "DS %#" PRIxLEAST64 " bound but it was never updated. It is now being used to draw so this will result in undefined behavior.", (uint64_t) pSet->set); + } + // For each dynamic descriptor, make sure dynamic offset doesn't overstep buffer result |= validate_dynamic_offsets(my_data, pSet); } } @@ -3696,6 +3702,8 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkCreateDescriptorSetLayout(VkDev memcpy(*ppIS, pCreateInfo->pBindings[i].pImmutableSamplers, pCreateInfo->pBindings[i].descriptorCount*sizeof(VkSampler)); } } + pNewNode->layout = *pSetLayout; + pNewNode->startIndex = 0; if (totalCount > 0) { pNewNode->descriptorTypes.resize(totalCount); pNewNode->stageFlags.resize(totalCount); @@ -3714,11 +3722,10 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkCreateDescriptorSetLayout(VkDev } offset += j; } + pNewNode->endIndex = pNewNode->startIndex + totalCount - 1; + } else { // no descriptors + pNewNode->endIndex = 0; } - pNewNode->layout = *pSetLayout; - pNewNode->startIndex = 0; - pNewNode->endIndex = pNewNode->startIndex + totalCount - 1; - assert(pNewNode->endIndex >= pNewNode->startIndex); // Put new node at Head of global Layer list loader_platform_thread_lock_mutex(&globalLock); dev_data->descriptorSetLayoutMap[*pSetLayout] = pNewNode; @@ -4255,8 +4262,7 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdBindDescriptorSets(VkCommandBuff loader_platform_thread_unlock_mutex(&globalLock); skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_INFO_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, (uint64_t) pDescriptorSets[i], __LINE__, DRAWSTATE_NONE, "DS", "DS %#" PRIxLEAST64 " bound on pipeline %s", (uint64_t) pDescriptorSets[i], string_VkPipelineBindPoint(pipelineBindPoint)); - if (!pSet->pUpdateStructs) { - // TODO: Verify against Valid Usage + if (!pSet->pUpdateStructs && (pSet->descriptorCount != 0)) { skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_WARN_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT, (uint64_t) pDescriptorSets[i], __LINE__, DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED, "DS", "DS %#" PRIxLEAST64 " bound but it was never updated. You may want to either update it or not bind it.", (uint64_t) pDescriptorSets[i]); } diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md index 692a8c4f..1011c3e5 100644 --- a/layers/vk_validation_layer_details.md +++ b/layers/vk_validation_layer_details.md @@ -50,7 +50,7 @@ The VK_LAYER_LUNARG_draw_state layer tracks state leading into Draw cmds. This i | Valid RenderPass | Flag error if attempt made to Begin/End/Continue a NULL or otherwise invalid RenderPass object | INVALID_RENDERPASS | vkCmdBeginRenderPass vkCmdEndRenderPass vkBeginCommandBuffer | NullRenderPass | NA | | RenderPass Compatibility | Verify that active renderpass is compatible with renderpass specified in secondary command buffer, and that renderpass specified for a framebuffer is compatible with renderpass specified in secondary command buffer | RENDERPASS_INCOMPATIBLE | vkCmdExecuteCommands vkBeginCommandBuffer | TBD | None | | Framebuffer Compatibility | If a framebuffer is passed to secondary command buffer in vkBeginCommandBuffer, then it must match active renderpass (if any) at time of vkCmdExecuteCommands | FRAMEBUFFER_INCOMPATIBLE | vkCmdExecuteCommands | TBD | None | -| DescriptorSet Updated | Warn user if DescriptorSet bound that was never updated | DESCRIPTOR_SET_NOT_UPDATED | vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | DescriptorSetCompatibility | NA | +| DescriptorSet Updated | Warn user if DescriptorSet bound that was never updated and is not empty. Trigger error at draw time if a set being used was never updated. | DESCRIPTOR_SET_NOT_UPDATED | vkCmdBindDescriptorSets vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | DescriptorSetCompatibility | NA | | DescriptorSet Bound | Error if DescriptorSet not bound that is used by currently bound VkPipeline at draw time | DESCRIPTOR_SET_NOT_BOUND | vkCmdBindDescriptorSets | DescriptorSetNotUpdated | NA | | Dynamic Offset Count | Error if dynamicOffsetCount at CmdBindDescriptorSets time is not equal to the actual number of dynamic descriptors in all sets being bound. | INVALID_DYNAMIC_OFFSET_COUNT | vkCmdBindDescriptorSets | InvalidDynamicOffsetCases | None | | Dynamic Offsets | At draw time, for a *_DYNAMIC type descriptor, the combination of dynamicOffset along with offset and range from its descriptor update must be less than the size of its buffer. | DYNAMIC_OFFSET_OVERFLOW | vkCmdDraw vkCmdDrawIndexed vkCmdDrawIndirect vkCmdDrawIndexedIndirect | InvalidDynamicOffsetCases | None | -- cgit v1.2.3