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 6948facf062e87d8fa246c3f67aa865e3f471a12..6a7216e75df2bf5e0db50691a90f4a3914e78577 100644 (file)
--- 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<const uint64_t &>(pNode->commandBuffer), __LINE__, error_code, "DS",
+                       "CB object %#" PRIxLEAST64 ": %s", reinterpret_cast<const uint64_t &>(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 a157b43b0ab17b23a9bb6f34f5fcb568746fa5df..31132a93bbbbee251567fc1f84b427baf2ecb40a 100644 (file)
--- a/layers/core_validation.h
+++ b/layers/core_validation.h
@@ -424,11 +424,12 @@ typedef struct _PIPELINE_NODE {
     std::vector<VkVertexInputBindingDescription> vertexBindingDescriptions;
     std::vector<VkVertexInputAttributeDescription> vertexAttributeDescriptions;
     std::vector<VkPipelineColorBlendAttachmentState> 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 {