From: Tobin Ehlis Date: Mon, 28 Mar 2016 17:18:19 +0000 (-0600) Subject: layers: GH195 Fix core_validation dynamic state checks X-Git-Tag: submit/tizen/20181227.054638~3478 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=098ec63bdb640195fe01e22778954a34b4ffe4bb;p=platform%2Fupstream%2FVulkan-Tools.git layers: GH195 Fix core_validation dynamic state checks Overhauled validation of dynamic state based on latest details in the VkPipelineDynamicStateCreateInfo section of the spec. Because some of the state checks need to be predicated on different pipeline state, removed the previous flag-based system for check predication and made it more clear and explicit exactly which pipeline states were gating each dynamic state check with "if" clauses in validate_draw_state_flags(). The biggest change here is for blend constants, which was the focus of the referenced GH195. For this case, added blendConstantsEnabled bool to PIPELINE_NODE and enable it by loop through all attachments to identify if any have blendEnable set and use the blend constant in any of their blendFactors. Also updated all the tests that were affected by this change. Change-Id: Iaba5d28986c83547575be1ff70b9ae7602435417 --- diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 6948facf..6a7216e7 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -2167,16 +2167,12 @@ static VkBool32 hasDrawCmd(GLOBAL_CB_NODE *pCB) { } // Check object status for selected flag state -static VkBool32 validate_status(layer_data *my_data, GLOBAL_CB_NODE *pNode, CBStatusFlags enable_mask, CBStatusFlags status_mask, - CBStatusFlags status_flag, VkFlags msg_flags, DRAW_STATE_ERROR error_code, const char *fail_msg) { - // If non-zero enable mask is present, check it against status but if enable_mask - // is 0 then no enable required so we should always just check status - if ((!enable_mask) || (enable_mask & pNode->status)) { - if ((pNode->status & status_mask) != status_flag) { - // TODO : How to pass dispatchable objects as srcObject? Here src obj should be cmd buffer - return log_msg(my_data->report_data, msg_flags, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, 0, __LINE__, error_code, - "DS", "CB object %#" PRIxLEAST64 ": %s", (uint64_t)(pNode->commandBuffer), fail_msg); - } +static VkBool32 validate_status(layer_data *my_data, GLOBAL_CB_NODE *pNode, CBStatusFlags status_mask, VkFlags msg_flags, + DRAW_STATE_ERROR error_code, const char *fail_msg) { + if (!(pNode->status & status_mask)) { + return log_msg(my_data->report_data, msg_flags, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, + reinterpret_cast(pNode->commandBuffer), __LINE__, error_code, "DS", + "CB object %#" PRIxLEAST64 ": %s", reinterpret_cast(pNode->commandBuffer), fail_msg); } return VK_FALSE; } @@ -2201,39 +2197,43 @@ static VkBool32 isDynamic(const PIPELINE_NODE *pPipeline, const VkDynamicState s } // Validate state stored as flags at time of draw call -static VkBool32 validate_draw_state_flags(layer_data *my_data, GLOBAL_CB_NODE *pCB, VkBool32 indexedDraw) { +static VkBool32 validate_draw_state_flags(layer_data *dev_data, GLOBAL_CB_NODE *pCB, const PIPELINE_NODE *pPipe, + VkBool32 indexedDraw) { VkBool32 result; - result = - validate_status(my_data, pCB, CBSTATUS_NONE, CBSTATUS_VIEWPORT_SET, CBSTATUS_VIEWPORT_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, - DRAWSTATE_VIEWPORT_NOT_BOUND, "Dynamic viewport state not set for this command buffer"); - result |= - validate_status(my_data, pCB, CBSTATUS_NONE, CBSTATUS_SCISSOR_SET, CBSTATUS_SCISSOR_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, - DRAWSTATE_SCISSOR_NOT_BOUND, "Dynamic scissor state not set for this command buffer"); - result |= validate_status(my_data, pCB, CBSTATUS_NONE, CBSTATUS_LINE_WIDTH_SET, CBSTATUS_LINE_WIDTH_SET, - VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_LINE_WIDTH_NOT_BOUND, - "Dynamic line width state not set for this command buffer"); - result |= validate_status(my_data, pCB, CBSTATUS_NONE, CBSTATUS_DEPTH_BIAS_SET, CBSTATUS_DEPTH_BIAS_SET, - VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_DEPTH_BIAS_NOT_BOUND, - "Dynamic depth bias state not set for this command buffer"); - result |= validate_status(my_data, pCB, CBSTATUS_COLOR_BLEND_WRITE_ENABLE, CBSTATUS_BLEND_SET, CBSTATUS_BLEND_SET, - VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_BLEND_NOT_BOUND, - "Dynamic blend object state not set for this command buffer"); - result |= validate_status(my_data, pCB, CBSTATUS_DEPTH_WRITE_ENABLE, CBSTATUS_DEPTH_BOUNDS_SET, CBSTATUS_DEPTH_BOUNDS_SET, - VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_DEPTH_BOUNDS_NOT_BOUND, - "Dynamic depth bounds state not set for this command buffer"); - result |= validate_status(my_data, pCB, CBSTATUS_STENCIL_TEST_ENABLE, CBSTATUS_STENCIL_READ_MASK_SET, - CBSTATUS_STENCIL_READ_MASK_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_STENCIL_NOT_BOUND, - "Dynamic stencil read mask state not set for this command buffer"); - result |= validate_status(my_data, pCB, CBSTATUS_STENCIL_TEST_ENABLE, CBSTATUS_STENCIL_WRITE_MASK_SET, - CBSTATUS_STENCIL_WRITE_MASK_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_STENCIL_NOT_BOUND, - "Dynamic stencil write mask state not set for this command buffer"); - result |= validate_status(my_data, pCB, CBSTATUS_STENCIL_TEST_ENABLE, CBSTATUS_STENCIL_REFERENCE_SET, - CBSTATUS_STENCIL_REFERENCE_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_STENCIL_NOT_BOUND, - "Dynamic stencil reference state not set for this command buffer"); - if (indexedDraw) - result |= validate_status(my_data, pCB, CBSTATUS_NONE, CBSTATUS_INDEX_BUFFER_BOUND, CBSTATUS_INDEX_BUFFER_BOUND, - VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_INDEX_BUFFER_NOT_BOUND, + result = validate_status(dev_data, pCB, CBSTATUS_VIEWPORT_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_VIEWPORT_NOT_BOUND, + "Dynamic viewport state not set for this command buffer"); + result |= validate_status(dev_data, pCB, CBSTATUS_SCISSOR_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, DRAWSTATE_SCISSOR_NOT_BOUND, + "Dynamic scissor state not set for this command buffer"); + if ((pPipe->iaStateCI.topology == VK_PRIMITIVE_TOPOLOGY_LINE_LIST) || + (pPipe->iaStateCI.topology == VK_PRIMITIVE_TOPOLOGY_LINE_STRIP)) { + result |= validate_status(dev_data, pCB, CBSTATUS_LINE_WIDTH_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_LINE_WIDTH_NOT_BOUND, "Dynamic line width state not set for this command buffer"); + } + if (pPipe->rsStateCI.depthBiasEnable) { + result |= validate_status(dev_data, pCB, CBSTATUS_DEPTH_BIAS_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_DEPTH_BIAS_NOT_BOUND, "Dynamic depth bias state not set for this command buffer"); + } + if (pPipe->blendConstantsEnabled) { + result |= validate_status(dev_data, pCB, CBSTATUS_BLEND_CONSTANTS_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_BLEND_NOT_BOUND, "Dynamic blend constants state not set for this command buffer"); + } + if (pPipe->dsStateCI.depthBoundsTestEnable) { + result |= validate_status(dev_data, pCB, CBSTATUS_DEPTH_BOUNDS_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_DEPTH_BOUNDS_NOT_BOUND, "Dynamic depth bounds state not set for this command buffer"); + } + if (pPipe->dsStateCI.stencilTestEnable) { + result |= validate_status(dev_data, pCB, CBSTATUS_STENCIL_READ_MASK_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_STENCIL_NOT_BOUND, "Dynamic stencil read mask state not set for this command buffer"); + result |= validate_status(dev_data, pCB, CBSTATUS_STENCIL_WRITE_MASK_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_STENCIL_NOT_BOUND, "Dynamic stencil write mask state not set for this command buffer"); + result |= validate_status(dev_data, pCB, CBSTATUS_STENCIL_REFERENCE_SET, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_STENCIL_NOT_BOUND, "Dynamic stencil reference state not set for this command buffer"); + } + if (indexedDraw) { + result |= validate_status(dev_data, pCB, CBSTATUS_INDEX_BUFFER_BOUND, VK_DEBUG_REPORT_ERROR_BIT_EXT, + DRAWSTATE_INDEX_BUFFER_NOT_BOUND, "Index buffer object not bound to this command buffer when Indexed Draw attempted"); + } return result; } @@ -2920,9 +2920,9 @@ static VkBool32 validate_dynamic_offsets(layer_data *my_data, const GLOBAL_CB_NO // Validate overall state at the time of a draw call static VkBool32 validate_draw_state(layer_data *my_data, GLOBAL_CB_NODE *pCB, VkBool32 indexedDraw) { - // First check flag states - VkBool32 result = validate_draw_state_flags(my_data, pCB, indexedDraw); PIPELINE_NODE *pPipe = getPipeline(my_data, pCB->lastBound[VK_PIPELINE_BIND_POINT_GRAPHICS].pipeline); + // First check flag states + VkBool32 result = validate_draw_state_flags(my_data, pCB, pPipe, indexedDraw); // Now complete other state checks // TODO : Currently only performing next check if *something* was bound (non-zero last bound) // There is probably a better way to gate when this check happens, and to know if something *should* have been bound @@ -3062,7 +3062,7 @@ static VkBool32 verifyPipelineCreateState(layer_data *my_data, const VkDevice de if (pPipeline->graphicsPipelineCI.pColorBlendState != NULL) { if (!my_data->physDevProperties.features.independentBlend) { - if (pPipeline->attachments.size() > 0) { + if (pPipeline->attachments.size() > 1) { VkPipelineColorBlendAttachmentState *pAttachments = &pPipeline->attachments[0]; for (size_t i = 1; i < pPipeline->attachments.size(); i++) { if ((pAttachments[0].blendEnable != pAttachments[i].blendEnable) || @@ -4481,17 +4481,6 @@ static void resetCB(layer_data *my_data, const VkCommandBuffer cb) { // Set PSO-related status bits for CB, including dynamic state set via PSO static void set_cb_pso_status(GLOBAL_CB_NODE *pCB, const PIPELINE_NODE *pPipe) { - for (auto const & att : pPipe->attachments) { - if (0 != att.colorWriteMask) { - pCB->status |= CBSTATUS_COLOR_BLEND_WRITE_ENABLE; - } - } - if (pPipe->dsStateCI.depthWriteEnable) { - pCB->status |= CBSTATUS_DEPTH_WRITE_ENABLE; - } - if (pPipe->dsStateCI.stencilTestEnable) { - pCB->status |= CBSTATUS_STENCIL_TEST_ENABLE; - } // Account for any dynamic state not set via this PSO if (!pPipe->dynStateCI.dynamicStateCount) { // All state is static pCB->status = CBSTATUS_ALL; @@ -4515,7 +4504,7 @@ static void set_cb_pso_status(GLOBAL_CB_NODE *pCB, const PIPELINE_NODE *pPipe) { psoDynStateMask &= ~CBSTATUS_DEPTH_BIAS_SET; break; case VK_DYNAMIC_STATE_BLEND_CONSTANTS: - psoDynStateMask &= ~CBSTATUS_BLEND_SET; + psoDynStateMask &= ~CBSTATUS_BLEND_CONSTANTS_SET; break; case VK_DYNAMIC_STATE_DEPTH_BOUNDS: psoDynStateMask &= ~CBSTATUS_DEPTH_BOUNDS_SET; @@ -6466,6 +6455,27 @@ vkMergePipelineCaches(VkDevice device, VkPipelineCache dstCache, uint32_t srcCac return result; } +// utility function to set collective state for pipeline +void set_pipeline_state(PIPELINE_NODE *pPipe) { + // If any attachment used by this pipeline has blendEnable, set top-level blendEnable + if (pPipe->graphicsPipelineCI.pColorBlendState) { + for (size_t i = 0; i < pPipe->attachments.size(); ++i) { + if (VK_TRUE == pPipe->attachments[i].blendEnable) { + if (((pPipe->attachments[i].dstAlphaBlendFactor >= VK_BLEND_FACTOR_CONSTANT_COLOR) && + (pPipe->attachments[i].dstAlphaBlendFactor <= VK_BLEND_FACTOR_ONE_MINUS_CONSTANT_ALPHA)) || + ((pPipe->attachments[i].dstColorBlendFactor >= VK_BLEND_FACTOR_CONSTANT_COLOR) && + (pPipe->attachments[i].dstColorBlendFactor <= VK_BLEND_FACTOR_ONE_MINUS_CONSTANT_ALPHA)) || + ((pPipe->attachments[i].srcAlphaBlendFactor >= VK_BLEND_FACTOR_CONSTANT_COLOR) && + (pPipe->attachments[i].srcAlphaBlendFactor <= VK_BLEND_FACTOR_ONE_MINUS_CONSTANT_ALPHA)) || + ((pPipe->attachments[i].srcColorBlendFactor >= VK_BLEND_FACTOR_CONSTANT_COLOR) && + (pPipe->attachments[i].srcColorBlendFactor <= VK_BLEND_FACTOR_ONE_MINUS_CONSTANT_ALPHA))) { + pPipe->blendConstantsEnabled = true; + } + } + } + } +} + VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkCreateGraphicsPipelines(VkDevice device, VkPipelineCache pipelineCache, uint32_t count, const VkGraphicsPipelineCreateInfo *pCreateInfos, const VkAllocationCallbacks *pAllocator, @@ -7158,6 +7168,7 @@ vkCmdBindPipeline(VkCommandBuffer commandBuffer, VkPipelineBindPoint pipelineBin if (pPN) { pCB->lastBound[pipelineBindPoint].pipeline = pipeline; set_cb_pso_status(pCB, pPN); + set_pipeline_state(pPN); skipCall |= validatePipelineState(dev_data, pCB, pipelineBindPoint, pipeline); } else { skipCall |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT, @@ -7241,7 +7252,7 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdSetBlendConstants(VkCommandBuffe GLOBAL_CB_NODE *pCB = getCBNode(dev_data, commandBuffer); if (pCB) { skipCall |= addCmd(dev_data, pCB, CMD_SETBLENDSTATE, "vkCmdSetBlendConstants()"); - pCB->status |= CBSTATUS_BLEND_SET; + pCB->status |= CBSTATUS_BLEND_CONSTANTS_SET; } loader_platform_thread_unlock_mutex(&globalLock); if (VK_FALSE == skipCall) diff --git a/layers/core_validation.h b/layers/core_validation.h index a157b43b..31132a93 100644 --- a/layers/core_validation.h +++ b/layers/core_validation.h @@ -424,11 +424,12 @@ typedef struct _PIPELINE_NODE { std::vector vertexBindingDescriptions; std::vector vertexAttributeDescriptions; std::vector attachments; + bool blendConstantsEnabled; // Blend constants enabled for any attachments // Default constructor _PIPELINE_NODE() : pipeline{}, graphicsPipelineCI{}, vertexInputCI{}, iaStateCI{}, tessStateCI{}, vpStateCI{}, rsStateCI{}, msStateCI{}, cbStateCI{}, dsStateCI{}, dynStateCI{}, vsCI{}, tcsCI{}, tesCI{}, gsCI{}, fsCI{}, computePipelineCI{}, active_shaders(0), - active_slots(), vertexBindingDescriptions(), vertexAttributeDescriptions(), attachments() {} + active_slots(), vertexBindingDescriptions(), vertexAttributeDescriptions(), attachments(), blendConstantsEnabled(false) {} } PIPELINE_NODE; class BASE_NODE { @@ -723,21 +724,20 @@ typedef enum _CB_STATE { // CB Status -- used to track status of various bindings on cmd buffer objects typedef VkFlags CBStatusFlags; typedef enum _CBStatusFlagBits { - CBSTATUS_NONE = 0x00000000, // No status is set - CBSTATUS_VIEWPORT_SET = 0x00000001, // Viewport has been set - CBSTATUS_LINE_WIDTH_SET = 0x00000002, // Line width has been set - CBSTATUS_DEPTH_BIAS_SET = 0x00000004, // Depth bias has been set - CBSTATUS_COLOR_BLEND_WRITE_ENABLE = 0x00000008, // PSO w/ CB Enable set has been set - CBSTATUS_BLEND_SET = 0x00000010, // Blend state object has been set - CBSTATUS_DEPTH_WRITE_ENABLE = 0x00000020, // PSO w/ Depth Enable set has been set - CBSTATUS_STENCIL_TEST_ENABLE = 0x00000040, // PSO w/ Stencil Enable set has been set - CBSTATUS_DEPTH_BOUNDS_SET = 0x00000080, // Depth bounds state object has been set - CBSTATUS_STENCIL_READ_MASK_SET = 0x00000100, // Stencil read mask has been set - CBSTATUS_STENCIL_WRITE_MASK_SET = 0x00000200, // Stencil write mask has been set - CBSTATUS_STENCIL_REFERENCE_SET = 0x00000400, // Stencil reference has been set - CBSTATUS_INDEX_BUFFER_BOUND = 0x00000800, // Index buffer has been set - CBSTATUS_SCISSOR_SET = 0x00001000, // Scissor has been set - CBSTATUS_ALL = 0x00001FFF, // All dynamic state set + // clang-format off + CBSTATUS_NONE = 0x00000000, // No status is set + CBSTATUS_VIEWPORT_SET = 0x00000001, // Viewport has been set + CBSTATUS_LINE_WIDTH_SET = 0x00000002, // Line width has been set + CBSTATUS_DEPTH_BIAS_SET = 0x00000004, // Depth bias has been set + CBSTATUS_BLEND_CONSTANTS_SET = 0x00000008, // Blend constants state has been set + CBSTATUS_DEPTH_BOUNDS_SET = 0x00000010, // Depth bounds state object has been set + CBSTATUS_STENCIL_READ_MASK_SET = 0x00000020, // Stencil read mask has been set + CBSTATUS_STENCIL_WRITE_MASK_SET = 0x00000040, // Stencil write mask has been set + CBSTATUS_STENCIL_REFERENCE_SET = 0x00000080, // Stencil reference has been set + CBSTATUS_INDEX_BUFFER_BOUND = 0x00000100, // Index buffer has been set + CBSTATUS_SCISSOR_SET = 0x00000200, // Scissor has been set + CBSTATUS_ALL = 0x000003FF, // All dynamic state set + // clang-format on } CBStatusFlagBits; typedef struct stencil_data {