Combining more Dynamic State for upcoming header #8593
Conversation
|
CI Vulkan-ValidationLayers build queued with queue ID 264425. |
spencer-lunarg
changed the title
|
CI Vulkan-ValidationLayers build # 17593 running. |
| (!last_bound_state.pipeline_state || last_bound_state.pipeline_state->IsDynamic(CB_DYNAMIC_STATE_PRIMITIVE_TOPOLOGY)) | ||
| ? cb_state.dynamic_state_value.primitive_topology | ||
| : last_bound_state.pipeline_state->topology_at_rasterizer; | ||
| if (topology == VK_PRIMITIVE_TOPOLOGY_PATCH_LIST) { |
There was a problem hiding this comment.
Is this check necessary? It is the only valid topology and everything else should already be caught before this point
There was a problem hiding this comment.
the spec say
| RETURN_IF_SKIP(InitBasicShaderObject()); | ||
| VkPhysicalDeviceFeatures features; | ||
| GetPhysicalDeviceFeatures(&features); | ||
| if (features.tessellationShader == VK_FALSE) { |
There was a problem hiding this comment.
Why not use AddRequiredFeature
There was a problem hiding this comment.
because I just copy and pasted your test 😆
let me update both
| const VkShaderEXT shaders[] = {vertShader.handle(), tescShader.handle(), teseShader.handle(), VK_NULL_HANDLE, | ||
| fragShader.handle()}; | ||
| vk::CmdBindShadersEXT(m_commandBuffer->handle(), 5u, stages, shaders); | ||
| // valid because this is not VK_PRIMITIVE_TOPOLOGY_PATCH_LIST |
There was a problem hiding this comment.
I think there should be a VUID against this, because it doesn't make sense to draw with tessellation shaders and use anything but VK_PRIMITIVE_TOPOLOGY_PATCH_LIST
There was a problem hiding this comment.
I think the topology patch has some quirks to get out... I am going to pull that commit out and make after getting the other ones in
|
CI Vulkan-ValidationLayers build # 17593 failed. |
The following were approved and will be merged in for the next headers
This combines them accordingly