From a27508babf63d50aea75883a3702979193c23683 Mon Sep 17 00:00:00 2001 From: Mark Young Date: Thu, 31 Mar 2016 14:56:43 -0600 Subject: [PATCH] layers: Add fix for lx172 to catch invalid line widths. Validate line wdith in both vkCreateGraphicsPipelines and in vkCmdSetLineWidth. Also, add a warning in vkCmdSetLineWidth if the user calls it but doesn't enable dynamic line width. Also, updated demos to fix missing lineWidth setting as well. Change-Id: I62118da9cb5282fcc22b1506e9be2db82b5f4a40 --- demos/cube.c | 1 + demos/tri.c | 1 + layers/core_validation.cpp | 48 +++++++++- tests/layer_validation_tests.cpp | 194 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 241 insertions(+), 3 deletions(-) diff --git a/demos/cube.c b/demos/cube.c index e38dc6f..cd2c8f4 100644 --- a/demos/cube.c +++ b/demos/cube.c @@ -1555,6 +1555,7 @@ static void demo_prepare_pipeline(struct demo *demo) { rs.depthClampEnable = VK_FALSE; rs.rasterizerDiscardEnable = VK_FALSE; rs.depthBiasEnable = VK_FALSE; + rs.lineWidth = 1.0f; memset(&cb, 0, sizeof(cb)); cb.sType = VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO; diff --git a/demos/tri.c b/demos/tri.c index 95a8eec..bb7c6aa 100644 --- a/demos/tri.c +++ b/demos/tri.c @@ -1316,6 +1316,7 @@ static void demo_prepare_pipeline(struct demo *demo) { rs.depthClampEnable = VK_FALSE; rs.rasterizerDiscardEnable = VK_FALSE; rs.depthBiasEnable = VK_FALSE; + rs.lineWidth = 1.0f; memset(&cb, 0, sizeof(cb)); cb.sType = VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO; diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 9002006..f7c1622 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -2936,6 +2936,31 @@ static bool validate_and_update_draw_state(layer_data *my_data, GLOBAL_CB_NODE * return result; } +// Validate HW line width capabilities prior to setting requested line width. +static bool verifyLineWidth(layer_data *my_data, DRAW_STATE_ERROR dsError, const uint64_t &target, float lineWidth) { + bool skip_call = false; + + // First check to see if the physical device supports wide lines. + if ((VK_FALSE == my_data->phys_dev_properties.features.wideLines) && (1.0f != lineWidth)) { + skip_call |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, target, __LINE__, + dsError, "DS", "Attempt to set lineWidth to %f but physical device wideLines feature " + "not supported/enabled so lineWidth must be 1.0f!", + lineWidth); + } else { + // Otherwise, make sure the width falls in the valid range. + if ((my_data->phys_dev_properties.properties.limits.lineWidthRange[0] > lineWidth) || + (my_data->phys_dev_properties.properties.limits.lineWidthRange[1] < lineWidth)) { + skip_call |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, target, + __LINE__, dsError, "DS", "Attempt to set lineWidth to %f but physical device limits line width " + "to between [%f, %f]!", + lineWidth, my_data->phys_dev_properties.properties.limits.lineWidthRange[0], + my_data->phys_dev_properties.properties.limits.lineWidthRange[1]); + } + } + + return skip_call; +} + // Verify that create state for a pipeline is valid static bool verifyPipelineCreateState(layer_data *my_data, const VkDevice device, std::vector pPipelines, int pipelineIndex) { @@ -3093,6 +3118,13 @@ static bool verifyPipelineCreateState(layer_data *my_data, const VkDevice device pPipeline->graphicsPipelineCI.pTessellationState->patchControlPoints); } } + // If a rasterization state is provided, make sure that the line width conforms to the HW. + if (pPipeline->graphicsPipelineCI.pRasterizationState) { + if (!isDynamic(pPipeline, VK_DYNAMIC_STATE_LINE_WIDTH)) { + skipCall |= verifyLineWidth(my_data, DRAWSTATE_INVALID_PIPELINE_CREATE_STATE, reinterpret_cast(pPipeline), + pPipeline->graphicsPipelineCI.pRasterizationState->lineWidth); + } + } // Viewport state must be included if rasterization is enabled. // If the viewport state is included, the viewport and scissor counts should always match. // NOTE : Even if these are flagged as dynamic, counts need to be set correctly for shader compiler @@ -6948,16 +6980,26 @@ vkCmdSetScissor(VkCommandBuffer commandBuffer, uint32_t firstScissor, uint32_t s } VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkCmdSetLineWidth(VkCommandBuffer commandBuffer, float lineWidth) { - bool skipCall = false; + bool skip_call = false; layer_data *dev_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map); std::unique_lock lock(global_lock); GLOBAL_CB_NODE *pCB = getCBNode(dev_data, commandBuffer); if (pCB) { - skipCall |= addCmd(dev_data, pCB, CMD_SETLINEWIDTHSTATE, "vkCmdSetLineWidth()"); + skip_call |= addCmd(dev_data, pCB, CMD_SETLINEWIDTHSTATE, "vkCmdSetLineWidth()"); pCB->status |= CBSTATUS_LINE_WIDTH_SET; + + PIPELINE_NODE *pPipeTrav = getPipeline(dev_data, pCB->lastBound[VK_PIPELINE_BIND_POINT_GRAPHICS].pipeline); + if (pPipeTrav != NULL && !isDynamic(pPipeTrav, VK_DYNAMIC_STATE_LINE_WIDTH)) { + skip_call |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_WARNING_BIT_EXT, (VkDebugReportObjectTypeEXT)0, + reinterpret_cast(commandBuffer), __LINE__, DRAWSTATE_INVALID_SET, "DS", + "vkCmdSetLineWidth called but pipeline was created without VK_DYNAMIC_STATE_LINE_WIDTH" + "flag. This is undefined behavior and could be ignored."); + } else { + skip_call |= verifyLineWidth(dev_data, DRAWSTATE_INVALID_SET, reinterpret_cast(commandBuffer), lineWidth); + } } lock.unlock(); - if (!skipCall) + if (!skip_call) dev_data->device_dispatch_table->CmdSetLineWidth(commandBuffer, lineWidth); } diff --git a/tests/layer_validation_tests.cpp b/tests/layer_validation_tests.cpp index 655a455..3f627b9 100644 --- a/tests/layer_validation_tests.cpp +++ b/tests/layer_validation_tests.cpp @@ -403,6 +403,7 @@ void VkLayerTest::VKTriangleTest(const char *vertShaderText, rs_state.sType = VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO; rs_state.depthBiasEnable = VK_TRUE; + rs_state.lineWidth = 1.0f; pipelineobj.SetRasterization(&rs_state); } // Viewport and scissors must stay in synch or other errors will occur than @@ -4068,6 +4069,199 @@ TEST_F(VkLayerTest, PSOScissorCountWithoutDataAndDynViewportMismatch) { vkDestroyDescriptorPool(m_device->device(), ds_pool, NULL); } +TEST_F(VkLayerTest, PSOLineWidthInvalid) { + VkResult err; + + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, + "Attempt to set lineWidth to 0"); + + ASSERT_NO_FATAL_FAILURE(InitState()); + ASSERT_NO_FATAL_FAILURE(InitRenderTarget()); + + VkDescriptorPoolSize ds_type_count = {}; + ds_type_count.type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + ds_type_count.descriptorCount = 1; + + VkDescriptorPoolCreateInfo ds_pool_ci = {}; + ds_pool_ci.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO; + ds_pool_ci.maxSets = 1; + ds_pool_ci.poolSizeCount = 1; + ds_pool_ci.pPoolSizes = &ds_type_count; + + VkDescriptorPool ds_pool; + err = + vkCreateDescriptorPool(m_device->device(), &ds_pool_ci, NULL, &ds_pool); + ASSERT_VK_SUCCESS(err); + + VkDescriptorSetLayoutBinding dsl_binding = {}; + dsl_binding.binding = 0; + dsl_binding.descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + dsl_binding.descriptorCount = 1; + dsl_binding.stageFlags = VK_SHADER_STAGE_ALL; + + VkDescriptorSetLayoutCreateInfo ds_layout_ci = {}; + ds_layout_ci.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO; + ds_layout_ci.bindingCount = 1; + ds_layout_ci.pBindings = &dsl_binding; + + VkDescriptorSetLayout ds_layout; + err = vkCreateDescriptorSetLayout(m_device->device(), &ds_layout_ci, NULL, + &ds_layout); + ASSERT_VK_SUCCESS(err); + + VkDescriptorSet descriptorSet; + VkDescriptorSetAllocateInfo alloc_info = {}; + alloc_info.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO; + alloc_info.descriptorSetCount = 1; + alloc_info.descriptorPool = ds_pool; + alloc_info.pSetLayouts = &ds_layout; + err = vkAllocateDescriptorSets(m_device->device(), &alloc_info, + &descriptorSet); + ASSERT_VK_SUCCESS(err); + + VkPipelineLayoutCreateInfo pipeline_layout_ci = {}; + pipeline_layout_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO; + pipeline_layout_ci.setLayoutCount = 1; + pipeline_layout_ci.pSetLayouts = &ds_layout; + + VkPipelineLayout pipeline_layout; + err = vkCreatePipelineLayout(m_device->device(), &pipeline_layout_ci, NULL, + &pipeline_layout); + ASSERT_VK_SUCCESS(err); + + VkPipelineViewportStateCreateInfo vp_state_ci = {}; + vp_state_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_VIEWPORT_STATE_CREATE_INFO; + vp_state_ci.scissorCount = 1; + vp_state_ci.pScissors = NULL; + vp_state_ci.viewportCount = 1; + vp_state_ci.pViewports = NULL; + + VkDynamicState dynamic_states[3] = {VK_DYNAMIC_STATE_VIEWPORT, + VK_DYNAMIC_STATE_SCISSOR, + VK_DYNAMIC_STATE_LINE_WIDTH}; + // Set scissor as dynamic to avoid that error + VkPipelineDynamicStateCreateInfo dyn_state_ci = {}; + dyn_state_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_DYNAMIC_STATE_CREATE_INFO; + dyn_state_ci.dynamicStateCount = 2; + dyn_state_ci.pDynamicStates = dynamic_states; + + VkPipelineShaderStageCreateInfo shaderStages[2]; + memset(&shaderStages, 0, 2 * sizeof(VkPipelineShaderStageCreateInfo)); + + VkShaderObj vs(m_device, bindStateVertShaderText, + VK_SHADER_STAGE_VERTEX_BIT, this); + VkShaderObj fs(m_device, bindStateFragShaderText, + VK_SHADER_STAGE_FRAGMENT_BIT, + this); // TODO - We shouldn't need a fragment shader + // but add it to be able to run on more devices + shaderStages[0] = vs.GetStageCreateInfo(); + shaderStages[1] = fs.GetStageCreateInfo(); + + VkPipelineVertexInputStateCreateInfo vi_ci = {}; + vi_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_VERTEX_INPUT_STATE_CREATE_INFO; + vi_ci.pNext = nullptr; + vi_ci.vertexBindingDescriptionCount = 0; + vi_ci.pVertexBindingDescriptions = nullptr; + vi_ci.vertexAttributeDescriptionCount = 0; + vi_ci.pVertexAttributeDescriptions = nullptr; + + VkPipelineInputAssemblyStateCreateInfo ia_ci = {}; + ia_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_INPUT_ASSEMBLY_STATE_CREATE_INFO; + ia_ci.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_STRIP; + + VkPipelineRasterizationStateCreateInfo rs_ci = {}; + rs_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO; + rs_ci.pNext = nullptr; + + // Check too low (line width of 0.0f). + rs_ci.lineWidth = 0.0f; + + VkPipelineColorBlendAttachmentState att = {}; + att.blendEnable = VK_FALSE; + att.colorWriteMask = 0xf; + + VkPipelineColorBlendStateCreateInfo cb_ci = {}; + cb_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO; + cb_ci.pNext = nullptr; + cb_ci.attachmentCount = 1; + cb_ci.pAttachments = &att; + + VkGraphicsPipelineCreateInfo gp_ci = {}; + gp_ci.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO; + gp_ci.stageCount = 2; + gp_ci.pStages = shaderStages; + gp_ci.pVertexInputState = &vi_ci; + gp_ci.pInputAssemblyState = &ia_ci; + gp_ci.pViewportState = &vp_state_ci; + gp_ci.pRasterizationState = &rs_ci; + gp_ci.pColorBlendState = &cb_ci; + gp_ci.pDynamicState = &dyn_state_ci; + gp_ci.flags = VK_PIPELINE_CREATE_DISABLE_OPTIMIZATION_BIT; + gp_ci.layout = pipeline_layout; + gp_ci.renderPass = renderPass(); + + VkPipelineCacheCreateInfo pc_ci = {}; + pc_ci.sType = VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO; + + VkPipeline pipeline; + VkPipelineCache pipelineCache; + + err = + vkCreatePipelineCache(m_device->device(), &pc_ci, NULL, &pipelineCache); + ASSERT_VK_SUCCESS(err); + err = vkCreateGraphicsPipelines(m_device->device(), pipelineCache, 1, + &gp_ci, NULL, &pipeline); + + m_errorMonitor->VerifyFound(); + + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, + "Attempt to set lineWidth to 65536"); + + // Check too high (line width of 65536.0f). + rs_ci.lineWidth = 65536.0f; + + err = + vkCreatePipelineCache(m_device->device(), &pc_ci, NULL, &pipelineCache); + ASSERT_VK_SUCCESS(err); + err = vkCreateGraphicsPipelines(m_device->device(), pipelineCache, 1, + &gp_ci, NULL, &pipeline); + + m_errorMonitor->VerifyFound(); + + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, + "Attempt to set lineWidth to 0"); + + dyn_state_ci.dynamicStateCount = 3; + + rs_ci.lineWidth = 1.0f; + + err = + vkCreatePipelineCache(m_device->device(), &pc_ci, NULL, &pipelineCache); + ASSERT_VK_SUCCESS(err); + err = vkCreateGraphicsPipelines(m_device->device(), pipelineCache, 1, + &gp_ci, NULL, &pipeline); + BeginCommandBuffer(); + vkCmdBindPipeline(m_commandBuffer->GetBufferHandle(), + VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline); + + // Check too low with dynamic setting. + vkCmdSetLineWidth(m_commandBuffer->GetBufferHandle(), 0.0f); + m_errorMonitor->VerifyFound(); + + m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, + "Attempt to set lineWidth to 65536"); + + // Check too high with dynamic setting. + vkCmdSetLineWidth(m_commandBuffer->GetBufferHandle(), 65536.0f); + m_errorMonitor->VerifyFound(); + EndCommandBuffer(); + + vkDestroyPipelineCache(m_device->device(), pipelineCache, NULL); + vkDestroyPipelineLayout(m_device->device(), pipeline_layout, NULL); + vkDestroyDescriptorSetLayout(m_device->device(), ds_layout, NULL); + vkDestroyDescriptorPool(m_device->device(), ds_pool, NULL); +} + TEST_F(VkLayerTest, NullRenderPass) { // Bind a NULL RenderPass m_errorMonitor->SetDesiredFailureMsg( -- 2.7.4