layers: Drawstate verify viewport and scissor state at PSO creation time
authorTobin Ehlis <tobin@lunarg.com>
Thu, 1 Oct 2015 17:15:13 +0000 (11:15 -0600)
committerTobin Ehlis <tobin@lunarg.com>
Fri, 2 Oct 2015 17:15:24 +0000 (11:15 -0600)
Make sure that viewportCount and scissorCount match
If viewport is not dynamic, make sure viewport state is supplied and if viewportCount is non-zero, that viewport data is non-NULL.
If scissor is not dynamic, make sure scissor state is supplied and if scissorCount is non-zero, that scissor data is non-NULL.

Added tests to hit these new cases.

layers/draw_state.cpp
layers/draw_state.h
layers/vk_validation_layer_details.md

index c88fa1b..85d921a 100644 (file)
@@ -420,13 +420,13 @@ static VkBool32 verifyPipelineCreateState(const VkDevice device, const PIPELINE_
     VkBool32 skipCall = VK_FALSE;
     // VS is required
     if (!(pPipeline->active_shaders & VK_SHADER_STAGE_VERTEX_BIT)) {
-        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
+        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
                 "Invalid Pipeline CreateInfo State: Vtx Shader required");
     }
     // Either both or neither TC/TE shaders should be defined
     if (((pPipeline->active_shaders & VK_SHADER_STAGE_TESS_CONTROL_BIT) == 0) !=
          ((pPipeline->active_shaders & VK_SHADER_STAGE_TESS_EVALUATION_BIT) == 0) ) {
-        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
+        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
                 "Invalid Pipeline CreateInfo State: TE and TC shaders must be included or excluded as a pair");
     }
     // Compute shaders should be specified independent of Gfx shaders
@@ -434,27 +434,70 @@ static VkBool32 verifyPipelineCreateState(const VkDevice device, const PIPELINE_
         (pPipeline->active_shaders & (VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_TESS_CONTROL_BIT |
                                      VK_SHADER_STAGE_TESS_EVALUATION_BIT | VK_SHADER_STAGE_GEOMETRY_BIT |
                                      VK_SHADER_STAGE_FRAGMENT_BIT))) {
-        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
+        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
                 "Invalid Pipeline CreateInfo State: Do not specify Compute Shader for Gfx Pipeline");
     }
     // VK_PRIMITIVE_TOPOLOGY_PATCH primitive topology is only valid for tessellation pipelines.
     // Mismatching primitive topology and tessellation fails graphics pipeline creation.
     if (pPipeline->active_shaders & (VK_SHADER_STAGE_TESS_CONTROL_BIT | VK_SHADER_STAGE_TESS_EVALUATION_BIT) &&
         (pPipeline->iaStateCI.topology != VK_PRIMITIVE_TOPOLOGY_PATCH)) {
-        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
+        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
                 "Invalid Pipeline CreateInfo State: VK_PRIMITIVE_TOPOLOGY_PATCH must be set as IA topology for tessellation pipelines");
     }
     if (pPipeline->iaStateCI.topology == VK_PRIMITIVE_TOPOLOGY_PATCH) {
         if (~pPipeline->active_shaders & VK_SHADER_STAGE_TESS_CONTROL_BIT) {
-            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
+            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
                 "Invalid Pipeline CreateInfo State: VK_PRIMITIVE_TOPOLOGY_PATCH primitive topology is only valid for tessellation pipelines");
         }
         if (!pPipeline->tessStateCI.patchControlPoints || (pPipeline->tessStateCI.patchControlPoints > 32)) {
-            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
+            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, "DS",
                     "Invalid Pipeline CreateInfo State: VK_PRIMITIVE_TOPOLOGY_PATCH primitive topology used with patchControlPoints value %u."
                     " patchControlPoints should be >0 and <=32.", pPipeline->tessStateCI.patchControlPoints);
         }
     }
+    // viewport and scissor counts should always match
+    // NOTE : Even if these are flagged as dynamic, counts need to be set correctly for shader compiler
+    //  TODO : Verify spec agrees with this check
+    if (pPipeline->vpStateCI.scissorCount != pPipeline->vpStateCI.viewportCount) {
+        skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
+                "Gfx Pipeline viewport count (%u) must match scissor count (%u).", pPipeline->vpStateCI.viewportCount, pPipeline->vpStateCI.scissorCount);
+    }
+    // If viewport or scissor are not dynamic, then verify that data is appropriate for count
+    VkBool32 dynViewport = VK_FALSE;
+    VkBool32 dynScissor = VK_FALSE;
+    if (pPipeline->graphicsPipelineCI.pDynamicState) {
+        for (uint32_t i=0; i<pPipeline->graphicsPipelineCI.pDynamicState->dynamicStateCount; i++) {
+            switch(pPipeline->graphicsPipelineCI.pDynamicState->pDynamicStates[i]) {
+                case VK_DYNAMIC_STATE_VIEWPORT:
+                    dynViewport = VK_TRUE;
+                    break;
+                case VK_DYNAMIC_STATE_SCISSOR:
+                    dynScissor = VK_TRUE;
+                    break;
+                default:
+                    break;
+            }
+        }
+    }
+    if (!dynViewport) {
+        if (!pPipeline->graphicsPipelineCI.pViewportState) {
+            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
+                "Gfx Pipeline viewport is not set as dynamic state and pViewportState is null. Must either set viewport as dynamic, or include viewport state in PSO.");
+        } else if (pPipeline->graphicsPipelineCI.pViewportState->viewportCount && !pPipeline->graphicsPipelineCI.pViewportState->pViewports) {
+            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
+                "Gfx Pipeline viewportCount is %u, but pViewports is NULL. For non-zero viewportCount, you must either include pViewports data, or include viewport in pDynamicState and set it with vkCmdSetViewport().", pPipeline->graphicsPipelineCI.pViewportState->viewportCount);
+        }
+    }
+    if (!dynScissor) {
+        if (!pPipeline->graphicsPipelineCI.pViewportState) {
+            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
+                "Gfx Pipeline scissor is not set as dynamic state and pViewportState is null. Must either set scissor as dynamic, or include viewport state in PSO.");
+        } else if (pPipeline->graphicsPipelineCI.pViewportState->scissorCount && !pPipeline->graphicsPipelineCI.pViewportState->pScissors) {
+            skipCall |= log_msg(mdd(device), VK_DBG_REPORT_ERROR_BIT, (VkDbgObjectType) 0, 0, 0, DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH, "DS",
+                "Gfx Pipeline scissorCount is %u, but pScissors is NULL. For non-zero scissorCount, you must either include pScissors data, or include scissor in pDynamicState and set it with vkCmdSetScissor().", pPipeline->graphicsPipelineCI.pViewportState->scissorCount);
+        }
+
+    }
     return skipCall;
 }
 // Init the pipeline mapping info based on pipeline create info LL tree
@@ -2067,7 +2110,7 @@ VK_LAYER_EXPORT void VKAPI vkCmdSetViewport(
             loader_platform_thread_lock_mutex(&globalLock);
             pCB->status |= CBSTATUS_VIEWPORT_SET;
             pCB->viewports.resize(viewportCount);
-            memcpy(pCB->viewports.data(), pViewports, viewportCount);
+            memcpy(pCB->viewports.data(), pViewports, viewportCount * sizeof(VkViewport));
             loader_platform_thread_unlock_mutex(&globalLock);
         } else {
             skipCall |= report_error_no_cb_begin(cmdBuffer, "vkCmdSetViewport()");
@@ -2091,7 +2134,7 @@ VK_LAYER_EXPORT void VKAPI vkCmdSetScissor(
             loader_platform_thread_lock_mutex(&globalLock);
             pCB->status |= CBSTATUS_SCISSOR_SET;
             pCB->scissors.resize(scissorCount);
-            memcpy(pCB->scissors.data(), pScissors, scissorCount);
+            memcpy(pCB->scissors.data(), pScissors, scissorCount * sizeof(VkRect2D));
             loader_platform_thread_unlock_mutex(&globalLock);
         } else {
             skipCall |= report_error_no_cb_begin(cmdBuffer, "vkCmdSetScissor()");
index 4399bd8..e42c5e4 100644 (file)
@@ -67,6 +67,7 @@ typedef enum _DRAW_STATE_ERROR
     DRAWSTATE_DESCRIPTOR_SET_NOT_UPDATED,       // DescriptorSet bound but it was never updated. This is a warning code.
     DRAWSTATE_CLEAR_CMD_BEFORE_DRAW,            // Clear cmd issued before any Draw in CmdBuffer, should use RenderPass Ops instead
     DRAWSTATE_BEGIN_CB_INVALID_STATE,           // Primary/Secondary CB created with mismatched FB/RP information
+    DRAWSTATE_VIEWPORT_SCISSOR_MISMATCH,        // Count for viewports and scissors mismatch and/or state doesn't match count
     DRAWSTATE_INVALID_EXTENSION,
 } DRAW_STATE_ERROR;
 
index 9180e42..90fa9e1 100644 (file)
@@ -46,6 +46,7 @@ The DrawState layer tracks state leading into Draw cmds. This includes the Pipel
 | DescriptorSet Updated | Warn user if DescriptorSet bound that was never updated | DESCRIPTOR_SET_NOT_UPDATED | vkCmdBindDescriptorSets | DescriptorSetNotUpdated | NA |
 | Correct Clear Use | Warn user if CmdClear for Color or DepthStencil issued to Cmd Buffer prior to a Draw Cmd. RenderPass LOAD_OP_CLEAR is preferred in this case. | CLEAR_CMD_BEFORE_DRAW | vkCmdClearColorImage vkCmdClearDepthStencilImage | ClearCmdNoDraw | NA |
 | Index Buffer Binding | Verify that an index buffer is bound at the point when an indexed draw is attempted. | INDEX_BUFFER_NOT_BOUND | vkCmdDrawIndexed vkCmdDrawIndexedIndirect | TODO | Implement validation test |
+| Viewport and Scissors match | In PSO viewportCount and scissorCount must match. Also for each count that is non-zero, there corresponding data array ptr should be non-NULL. | VIEWPORT_SCISSOR_MISMATCH | vkCreateGraphicsPipelines vkCmdSetViewport vkCmdSetScissor | TODO | Implement validation test |
 | NA | Enum used for informational messages | NONE | | NA | None |
 | NA | Enum used for errors in the layer itself. This does not indicate an app issue, but instead a bug in the layer. | INTERNAL_ERROR | | NA | None |
 | NA | Enum used when Drawstate attempts to allocate memory for its own internal use and is unable to. | OUT_OF_MEMORY | | NA | None |