From: Lina Versace Date: Thu, 3 Aug 2023 21:23:27 +0000 (-0700) Subject: venus: Do pipeline fixes for VK_EXT_graphics_pipeline_library X-Git-Tag: upstream/23.3.3~611 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a65ac274acf250a8f4a5abb030c46b572c3552f4;p=platform%2Fupstream%2Fmesa.git venus: Do pipeline fixes for VK_EXT_graphics_pipeline_library Signed-off-by: Lina Versace Reviewed-by: Yiwei Zhang Part-of: --- diff --git a/src/virtio/vulkan/vn_pipeline.c b/src/virtio/vulkan/vn_pipeline.c index 524cbd0..94afb32 100644 --- a/src/virtio/vulkan/vn_pipeline.c +++ b/src/virtio/vulkan/vn_pipeline.c @@ -33,18 +33,28 @@ struct vn_graphics_pipeline_create_info_fields { * order. */ struct { + /** VkGraphicsPipelineCreateInfo::pStages */ + bool shader_stages : 1; /** VkGraphicsPipelineCreateInfo::pVertexInputState */ bool vertex_input_state : 1; + /** VkGraphicsPipelineCreateInfo::pInputAssemblyState */ + bool input_assembly_state : 1; /** VkGraphicsPipelineCreateInfo::pTessellationState */ bool tessellation_state : 1; /** VkGraphicsPipelineCreateInfo::pViewportState */ bool viewport_state : 1; + /** VkGraphicsPipelineCreateInfo::pRasterizationState */ + bool rasterization_state : 1; /** VkGraphicsPipelineCreateInfo::pMultisampleState */ bool multisample_state : 1; /** VkGraphicsPipelineCreateInfo::pDepthStencilState */ bool depth_stencil_state : 1; /** VkGraphicsPipelineCreateInfo::pColorBlendState */ bool color_blend_state : 1; + /** VkGraphicsPipelineCreateInfo::layout */ + bool pipeline_layout : 1; + /** VkGraphicsPipelineCreateInfo::renderPass */ + bool render_pass : 1; /** VkGraphicsPipelineCreateInfo::basePipelineHandle */ bool base_pipeline_handle : 1; @@ -52,6 +62,9 @@ struct vn_graphics_pipeline_create_info_fields { bool viewport_state_viewports : 1; /** VkPipelineViewportStateCreateInfo::pScissors */ bool viewport_state_scissors : 1; + + /** VkPipelineRenderingCreateInfo, all format fields */ + bool rendering_info_formats : 1; }; }; }; @@ -62,6 +75,107 @@ static_assert( "vn_graphics_pipeline_create_info_fields::mask is too small"); /** + * Typesafe bitmask for VkGraphicsPipelineLibraryFlagsEXT. Named members + * reduce long lines. + * + * From the Vulkan 1.3.215 spec: + * + * The state required for a graphics pipeline is divided into vertex input + * state, pre-rasterization shader state, fragment shader state, and + * fragment output state. + */ +struct vn_graphics_pipeline_library_state { + union { + VkGraphicsPipelineLibraryFlagsEXT mask; + + struct { + /** VK_GRAPHICS_PIPELINE_LIBRARY_VERTEX_INPUT_INTERFACE_BIT_EXT */ + bool vertex_input : 1; + /** VK_GRAPHICS_PIPELINE_LIBRARY_PRE_RASTERIZATION_SHADERS_BIT_EXT */ + bool pre_raster_shaders : 1; + /** VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_SHADER_BIT_EXT */ + bool fragment_shader : 1; + /** VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_OUTPUT_INTERFACE_BIT_EXT */ + bool fragment_output : 1; + }; + }; +}; + +/** + * Compact bitmask for the subset of graphics VkDynamicState that + * venus needs to track. Named members reduce long lines. + * + * We want a *compact* bitmask because enum VkDynamicState has large gaps due + * to extensions. + */ +struct vn_graphics_dynamic_state { + union { + uint32_t mask; + + struct { + /** VK_DYNAMIC_STATE_VERTEX_INPUT_EXT **/ + bool vertex_input : 1; + /** VK_DYNAMIC_STATE_VIEWPORT */ + bool viewport : 1; + /** VK_DYNAMIC_STATE_VIEWPORT_WITH_COUNT */ + bool viewport_with_count : 1; + /** VK_DYNAMIC_STATE_SCISSOR */ + bool scissor : 1; + /** VK_DYNAMIC_STATE_SCISSOR_WITH_COUNT */ + bool scissor_with_count : 1; + /** VK_DYNAMIC_STATE_RASTERIZER_DISCARD_ENABLE */ + bool rasterizer_discard_enable : 1; + }; + }; +}; + +/** + * Graphics pipeline state that Venus tracks to determine which fixes are + * required in the VkGraphicsPipelineCreateInfo pNext chain. + * + * This is the pipeline's fully linked state. That is, it includes the state + * provided directly in VkGraphicsPipelineCreateInfo and the state provided + * indirectly in VkPipelineLibraryCreateInfoKHR. + */ +struct vn_graphics_pipeline_state { + /** The GPL state subsets that the pipeline provides. */ + struct vn_graphics_pipeline_library_state gpl; + + struct vn_graphics_dynamic_state dynamic; + VkShaderStageFlags shader_stages; + + struct vn_render_pass_state { + /** + * The attachment aspects accessed by the pipeline. + * + * Valid if and only if VK_IMAGE_ASPECT_METADATA_BIT is unset. + * + * In a complete pipeline, this must be valid (and may be empty). In + * a pipeline library, this may be invalid. We initialize this to be + * invalid, and it remains invalid until we read the attachment info in + * the VkGraphicsPipelineCreateInfo chain. + * + * The app provides the attachment info in + * VkGraphicsPipelineCreateInfo::renderPass or + * VkPipelineRenderingCreateInfo, but the validity of that info depends + * on VkGraphicsPipelineLibraryFlagsEXT. + */ + VkImageAspectFlags attachment_aspects; + } render_pass; + + /** VkPipelineRasterizationStateCreateInfo::rasterizerDiscardEnable + * + * Valid if and only if gpl.pre_raster_shaders is set. + */ + bool rasterizer_discard_enable; +}; + +struct vn_graphics_pipeline { + struct vn_pipeline base; + struct vn_graphics_pipeline_state state; +}; + +/** * Description of fixes needed for a single VkGraphicsPipelineCreateInfo * pNext chain. */ @@ -360,6 +474,14 @@ vn_MergePipelineCaches(VkDevice device, /* pipeline commands */ +static struct vn_graphics_pipeline * +vn_graphics_pipeline_from_handle(VkPipeline pipeline_h) +{ + struct vn_pipeline *p = vn_pipeline_from_handle(pipeline_h); + assert(p->type == VN_PIPELINE_TYPE_GRAPHICS); + return (struct vn_graphics_pipeline *)p; +} + static bool vn_create_pipeline_handles(struct vn_device *dev, enum vn_pipeline_type type, @@ -367,9 +489,20 @@ vn_create_pipeline_handles(struct vn_device *dev, VkPipeline *pipeline_handles, const VkAllocationCallbacks *alloc) { + size_t pipeline_size; + + switch (type) { + case VN_PIPELINE_TYPE_GRAPHICS: + pipeline_size = sizeof(struct vn_graphics_pipeline); + break; + case VN_PIPELINE_TYPE_COMPUTE: + pipeline_size = sizeof(struct vn_pipeline); + break; + } + for (uint32_t i = 0; i < pipeline_count; i++) { struct vn_pipeline *pipeline = - vk_zalloc(alloc, sizeof(*pipeline), VN_DEFAULT_ALIGN, + vk_zalloc(alloc, pipeline_size, VN_DEFAULT_ALIGN, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT); if (!pipeline) { @@ -441,233 +574,614 @@ vn_graphics_pipeline_fix_tmp_alloc(const VkAllocationCallbacks *alloc, return tmp; } +/** + * Update \a gpl with the VkGraphicsPipelineLibraryFlagsEXT that the pipeline + * provides directly (without linking). The spec says that the pipeline always + * provides flags, but may do it implicitly. + * + * From the Vulkan 1.3.251 spec: + * + * If this structure [VkGraphicsPipelineLibraryCreateInfoEXT] is + * omitted, and either VkGraphicsPipelineCreateInfo::flags includes + * VK_PIPELINE_CREATE_LIBRARY_BIT_KHR or the + * VkGraphicsPipelineCreateInfo::pNext chain includes + * a VkPipelineLibraryCreateInfoKHR structure with a libraryCount + * greater than 0, it is as if flags is 0. Otherwise if this + * structure is omitted, it is as if flags includes all possible subsets + * of the graphics pipeline (i.e. a complete graphics pipeline). + */ static void -vn_find_graphics_pipeline_create_info_fixes( +vn_graphics_pipeline_library_state_update( const VkGraphicsPipelineCreateInfo *info, - struct vn_graphics_pipeline_fix_desc *fix_desc) + struct vn_graphics_pipeline_library_state *restrict gpl) { - memset(fix_desc, 0, sizeof(*fix_desc)); - - const VkPipelineRenderingCreateInfo *rendering_info = - vk_find_struct_const(info, PIPELINE_RENDERING_CREATE_INFO); - - VkShaderStageFlags stages = 0; - for (uint32_t j = 0; j < info->stageCount; j++) { - stages |= info->pStages[j].stage; - } - - /* VkDynamicState */ - struct { - bool rasterizer_discard_enable; - bool viewport; - bool viewport_with_count; - bool scissor; - bool scissor_with_count; - bool vertex_input; - } has_dynamic_state = { 0 }; - - if (info->pDynamicState) { - for (uint32_t j = 0; j < info->pDynamicState->dynamicStateCount; j++) { - switch (info->pDynamicState->pDynamicStates[j]) { - case VK_DYNAMIC_STATE_RASTERIZER_DISCARD_ENABLE: - has_dynamic_state.rasterizer_discard_enable = true; - break; - case VK_DYNAMIC_STATE_VIEWPORT: - has_dynamic_state.viewport = true; - break; - case VK_DYNAMIC_STATE_VIEWPORT_WITH_COUNT: - has_dynamic_state.viewport_with_count = true; - break; - case VK_DYNAMIC_STATE_SCISSOR: - has_dynamic_state.scissor = true; - break; - case VK_DYNAMIC_STATE_SCISSOR_WITH_COUNT: - has_dynamic_state.scissor_with_count = true; - break; - case VK_DYNAMIC_STATE_VERTEX_INPUT_EXT: - has_dynamic_state.vertex_input = true; - break; - default: - break; - } - } + const VkGraphicsPipelineLibraryCreateInfoEXT *gpl_info = + vk_find_struct_const(info->pNext, + GRAPHICS_PIPELINE_LIBRARY_CREATE_INFO_EXT); + const VkPipelineLibraryCreateInfoKHR *lib_info = + vk_find_struct_const(info->pNext, PIPELINE_LIBRARY_CREATE_INFO_KHR); + const uint32_t lib_count = lib_info ? lib_info->libraryCount : 0; + + if (gpl_info) { + gpl->mask |= gpl_info->flags; + } else if ((info->flags & VK_PIPELINE_CREATE_LIBRARY_BIT_KHR) || + lib_count > 0) { + gpl->mask |= 0; + } else { + gpl->mask |= + VK_GRAPHICS_PIPELINE_LIBRARY_VERTEX_INPUT_INTERFACE_BIT_EXT | + VK_GRAPHICS_PIPELINE_LIBRARY_PRE_RASTERIZATION_SHADERS_BIT_EXT | + VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_SHADER_BIT_EXT | + VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_OUTPUT_INTERFACE_BIT_EXT; } +} - const struct vn_render_pass *pass = - vn_render_pass_from_handle(info->renderPass); +/** + * Update \a dynamic with the VkDynamicState that the pipeline provides + * directly (without linking). + * + * \a direct_gpl The VkGraphicsPipelineLibraryFlagsEXT that the pipeline sets + * directly (without linking). + */ +static void +vn_graphics_dynamic_state_update( + const VkGraphicsPipelineCreateInfo *info, + struct vn_graphics_pipeline_library_state direct_gpl, + struct vn_graphics_dynamic_state *restrict dynamic) +{ + const VkPipelineDynamicStateCreateInfo *dyn_info = info->pDynamicState; + if (!dyn_info) + return; - const struct vn_subpass *subpass = NULL; - if (pass) - subpass = &pass->subpasses[info->subpass]; + struct vn_graphics_dynamic_state raw = { 0 }; - /* TODO: Ignore VkPipelineRenderingCreateInfo when not using dynamic - * rendering. This requires either a deep rewrite of - * VkGraphicsPipelineCreateInfo::pNext or a fix in the generated - * protocol code. - * - * The Vulkan spec (1.3.223) says about VkPipelineRenderingCreateInfo: - * If a graphics pipeline is created with a valid VkRenderPass, - * parameters of this structure are ignored. - */ - const bool has_dynamic_rendering = !pass && rendering_info; + for (uint32_t i = 0; i < dyn_info->dynamicStateCount; i++) { + switch (dyn_info->pDynamicStates[i]) { + case VK_DYNAMIC_STATE_VERTEX_INPUT_EXT: + raw.vertex_input = true; + break; + case VK_DYNAMIC_STATE_VIEWPORT: + raw.viewport = true; + break; + case VK_DYNAMIC_STATE_VIEWPORT_WITH_COUNT: + raw.viewport_with_count = true; + break; + case VK_DYNAMIC_STATE_SCISSOR: + raw.scissor = true; + break; + case VK_DYNAMIC_STATE_SCISSOR_WITH_COUNT: + raw.scissor_with_count = true; + break; + case VK_DYNAMIC_STATE_RASTERIZER_DISCARD_ENABLE: + raw.rasterizer_discard_enable = true; + break; + default: + break; + } + } - /* For each pipeline state category, we define a bool. + /* We must ignore VkDynamicState unrelated to the + * VkGraphicsPipelineLibraryFlagsEXT that the pipeline provides directly + * (without linking). * - * The Vulkan spec (1.3.223) says: - * The state required for a graphics pipeline is divided into vertex - * input state, pre-rasterization shader state, fragment shader - * state, and fragment output state. + * [Vulkan 1.3.252] + * Dynamic state values set via pDynamicState must be ignored if the + * state they correspond to is not otherwise statically set by one of + * the state subsets used to create the pipeline. * - * Without VK_EXT_graphics_pipeline_library, most states are - * unconditionally included in the pipeline. Despite that, we still - * reference the state bools in the ignore rules because (a) it makes - * the ignore condition easier to validate against the text of the - * relevant VUs; and (b) it makes it easier to enable - * VK_EXT_graphics_pipeline_library because we won't need to carefully - * revisit the text of each VU to untangle the missing pipeline state - * bools. + * In general, we must update dynamic state bits with `|=` rather than `=` + * because multiple GPL state subsets can enable the same dynamic state. + * + * [Vulkan 1.3.252] + * Any linked library that has dynamic state enabled that same dynamic + * state must also be enabled in all the other linked libraries to which + * that dynamic state applies. */ + if (direct_gpl.vertex_input) { + dynamic->vertex_input |= raw.vertex_input; + } + if (direct_gpl.pre_raster_shaders) { + dynamic->viewport |= raw.viewport; + dynamic->viewport_with_count |= raw.viewport_with_count; + dynamic->scissor |= raw.scissor; + dynamic->scissor_with_count |= raw.scissor_with_count; + dynamic->rasterizer_discard_enable |= raw.rasterizer_discard_enable; + } +} - /* The spec does not assign a name to this state. We define it just to - * deduplicate code. +/** + * Update \a shader_stages with the VkShaderStageFlags that the pipeline + * provides directly (without linking). + * + * \a direct_gpl The VkGraphicsPipelineLibraryFlagsEXT that the pipeline sets + * directly (without linking). + */ +static void +vn_graphics_shader_stages_update( + const VkGraphicsPipelineCreateInfo *info, + struct vn_graphics_pipeline_library_state direct_gpl, + struct vn_graphics_pipeline_create_info_fields *restrict valid, + VkShaderStageFlags *restrict shader_stages) +{ + /* From the Vulkan 1.3.251 spec: * - * The Vulkan spec (1.3.223) says: - * If the value of [...]rasterizerDiscardEnable in the - * pre-rasterization shader state is VK_FALSE or the - * VK_DYNAMIC_STATE_RASTERIZER_DISCARD_ENABLE dynamic state is - * enabled fragment shader state and fragment output interface state - * is included in a complete graphics pipeline. + * VUID-VkGraphicsPipelineCreateInfo-flags-06640 + * + * If VkGraphicsPipelineLibraryCreateInfoEXT::flags includes + * VK_GRAPHICS_PIPELINE_LIBRARY_PRE_RASTERIZATION_SHADERS_BIT_EXT or + * VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_SHADER_BIT_EXT, pStages must be + * a valid pointer to an array of stageCount valid + * VkPipelineShaderStageCreateInfo structures */ - const bool has_raster_state = - has_dynamic_state.rasterizer_discard_enable || - (info->pRasterizationState && - info->pRasterizationState->rasterizerDiscardEnable == VK_FALSE); + if (!direct_gpl.pre_raster_shaders && !direct_gpl.fragment_shader) + return; - /* VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_SHADER_BIT_EXT */ - const bool has_fragment_shader_state = has_raster_state; + valid->shader_stages = true; - /* VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_OUTPUT_INTERFACE_BIT_EXT */ - const bool has_fragment_output_state = has_raster_state; + for (uint32_t i = 0; i < info->stageCount; i++) { + /* We do not need to ignore the stages irrelevant to the GPL flags. + * The following VUs require the app to provide only relevant stages. + * + * VUID-VkGraphicsPipelineCreateInfo-pStages-06894 + * VUID-VkGraphicsPipelineCreateInfo-pStages-06895 + * VUID-VkGraphicsPipelineCreateInfo-pStages-06896 + */ + *shader_stages |= info->pStages[i].stage; + } +} - /* Ignore pTessellationState? - * VUID-VkGraphicsPipelineCreateInfo-pStages-00731 +/** + * Update the render pass state with the state that the pipeline provides + * directly (without linking). + * + * \a direct_gpl The VkGraphicsPipelineLibraryFlagsEXT that the pipeline sets + * directly (without linking). + */ +static void +vn_render_pass_state_update( + const VkGraphicsPipelineCreateInfo *info, + struct vn_graphics_pipeline_library_state direct_gpl, + struct vn_graphics_pipeline_create_info_fields *restrict valid, + struct vn_render_pass_state *restrict state) +{ + /* We must set validity before early returns, to ensure we don't erase + * valid info during fixup. We must not erase valid info because, even if + * we don't read it, the host driver may read it. */ - if (info->pTessellationState && - (!(stages & VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT) || - !(stages & VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT))) { - fix_desc->erase.tessellation_state = true; - } - /* Ignore pViewportState? - * VUID-VkGraphicsPipelineCreateInfo-rasterizerDiscardEnable-00750 - * VUID-VkGraphicsPipelineCreateInfo-pViewportState-04892 + /* XXX: Should this ignore the render pass for some state subsets when + * rasterization is statically disabled? The spec suggests "yes" and "no". */ - if (info->pViewportState && !has_raster_state) { - fix_desc->erase.viewport_state = true; - } - /* Ignore pViewports? - * VUID-VkGraphicsPipelineCreateInfo-pDynamicStates-04130 + /* VUID-VkGraphicsPipelineCreateInfo-flags-06643 * - * If viewportCount is 0, then venus encoder will ignore pViewports and we - * do not need to erase it. + * If VkGraphicsPipelineLibraryCreateInfoEXT::flags includes + * VK_GRAPHICS_PIPELINE_LIBRARY_PRE_RASTERIZATION_SHADERS_BIT_EXT, or + * VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_SHADER_BIT_EXT, + * VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_OUTPUT_INTERFACE_BIT_EXT, and + * renderPass is not VK_NULL_HANDLE, renderPass must be a valid + * VkRenderPass handle */ - if (!fix_desc->erase.viewport_state && info->pViewportState && - info->pViewportState->pViewports && - info->pViewportState->viewportCount) { - const bool has_dynamic_viewport = - has_dynamic_state.viewport || has_dynamic_state.viewport_with_count; - - if (has_dynamic_viewport) { - fix_desc->erase.viewport_state_viewports = true; - } - } + valid->render_pass |= direct_gpl.pre_raster_shaders || + direct_gpl.fragment_shader || + direct_gpl.fragment_output; - /* Ignore pScissors? - * VUID-VkGraphicsPipelineCreateInfo-pDynamicStates-04131 + /* VUID-VkGraphicsPipelineCreateInfo-renderPass-06579 * - * If scissorCount is 0, then venus encoder will ignore pScissors and we - * do not need to erase it. + * If the pipeline requires fragment output interface state, and renderPass + * is VK_NULL_HANDLE, and + * VkPipelineRenderingCreateInfo::colorAttachmentCount is not 0, + * VkPipelineRenderingCreateInfo::pColorAttachmentFormats must be a valid + * pointer to an array of colorAttachmentCount valid VkFormat values + * + * VUID-VkGraphicsPipelineCreateInfo-renderPass-06580 + * + * If the pipeline requires fragment output interface state, and renderPass + * is VK_NULL_HANDLE, each element of + * VkPipelineRenderingCreateInfo::pColorAttachmentFormats must be a valid + * VkFormat value */ - if (!fix_desc->erase.viewport_state && info->pViewportState && - info->pViewportState->pScissors && - info->pViewportState->scissorCount) { - const bool has_dynamic_scissor = - has_dynamic_state.scissor || has_dynamic_state.scissor_with_count; - if (has_dynamic_scissor) { - fix_desc->erase.viewport_state_scissors = true; + valid->rendering_info_formats |= + direct_gpl.fragment_output && !info->renderPass; + + if (state->attachment_aspects != VK_IMAGE_ASPECT_METADATA_BIT) { + /* We have previously collected the pipeline's attachment aspects. We + * do not need to inspect the attachment info again because VUs ensure + * that all valid render pass info used to create the pipeline and its + * linked pipelines are compatible. Ignored info is not required to be + * compatible across linked pipeline libraries. An example of ignored + * info is VkPipelineRenderingCreateInfo::pColorAttachmentFormats + * without + * VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_OUTPUT_INTERFACE_BIT_EXT. + * + * VUID-VkGraphicsPipelineCreateInfo-renderpass-06625 + * VUID-VkGraphicsPipelineCreateInfo-pLibraries-06628 + */ + return; + } + + if (valid->render_pass && info->renderPass) { + struct vn_render_pass *pass = + vn_render_pass_from_handle(info->renderPass); + state->attachment_aspects = + pass->subpasses[info->subpass].attachment_aspects; + return; + } + + if (valid->rendering_info_formats) { + state->attachment_aspects = 0; + + /* From the Vulkan 1.3.255 spec: + * + * When a pipeline is created without a VkRenderPass, if this + * structure [VkPipelineRenderingCreateInfo] is present in the pNext + * chain of VkGraphicsPipelineCreateInfo, it specifies the view mask + * and format of attachments used for rendering. If this structure + * is not specified, and the pipeline does not include + * a VkRenderPass, viewMask and colorAttachmentCount are 0, and + * depthAttachmentFormat and stencilAttachmentFormat are + * VK_FORMAT_UNDEFINED. If a graphics pipeline is created with + * a valid VkRenderPass, parameters of this structure are ignored. + * + * However, other spec text clearly states that the format members of + * VkPipelineRenderingCreateInfo are ignored unless the pipeline + * provides fragment output interface state directly (without linking). + */ + const VkPipelineRenderingCreateInfo *r_info = + vk_find_struct_const(info->pNext, PIPELINE_RENDERING_CREATE_INFO); + + if (r_info) { + for (uint32_t i = 0; i < r_info->colorAttachmentCount; i++) { + if (r_info->pColorAttachmentFormats[i]) { + state->attachment_aspects |= VK_IMAGE_ASPECT_COLOR_BIT; + break; + } + } + if (r_info->depthAttachmentFormat) + state->attachment_aspects |= VK_IMAGE_ASPECT_DEPTH_BIT; + if (r_info->stencilAttachmentFormat) + state->attachment_aspects |= VK_IMAGE_ASPECT_STENCIL_BIT; } + + return; } - /* Ignore pMultisampleState? - * VUID-VkGraphicsPipelineCreateInfo-rasterizerDiscardEnable-00751 + /* Aspects remain invalid. */ + assert(state->attachment_aspects == VK_IMAGE_ASPECT_METADATA_BIT); +} + +static void +vn_graphics_pipeline_state_merge( + struct vn_graphics_pipeline_state *restrict dst, + const struct vn_graphics_pipeline_state *restrict src) +{ + /* The Vulkan 1.3.251 spec says: + * VUID-VkGraphicsPipelineCreateInfo-pLibraries-06611 + * + * Any pipeline libraries included via + * VkPipelineLibraryCreateInfoKHR::pLibraries must not include any state + * subset already defined by this structure or defined by any other + * pipeline library in VkPipelineLibraryCreateInfoKHR::pLibraries */ - if (info->pMultisampleState && !has_fragment_output_state) { - fix_desc->erase.multisample_state = true; + assert(!(dst->gpl.mask & src->gpl.mask)); + + dst->gpl.mask |= src->gpl.mask; + dst->dynamic.mask |= src->dynamic.mask; + dst->shader_stages |= src->shader_stages; + + VkImageAspectFlags src_aspects = src->render_pass.attachment_aspects; + VkImageAspectFlags *dst_aspects = &dst->render_pass.attachment_aspects; + + if (src_aspects != VK_IMAGE_ASPECT_METADATA_BIT) { + if (*dst_aspects != VK_IMAGE_ASPECT_METADATA_BIT) { + /* All linked pipelines must have compatible render pass info. */ + assert(*dst_aspects == src_aspects); + } else { + *dst_aspects = src_aspects; + } } - /* Ignore pDepthStencilState? */ - if (info->pDepthStencilState) { - const bool has_static_attachment = - subpass && - (subpass->attachment_aspects & - (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)); + if (dst->gpl.pre_raster_shaders) + dst->rasterizer_discard_enable = src->rasterizer_discard_enable; +} - /* VUID-VkGraphicsPipelineCreateInfo-renderPass-06043 */ - bool require_state = has_fragment_shader_state && has_static_attachment; +/** + * Fill \a state by reading the VkGraphicsPipelineCreateInfo pNext chain, + * including any linked pipeline libraries. Return in \a out_fix_desc + * a description of required fixes to the VkGraphicsPipelineCreateInfo chain. + * + * \pre state is zero-filled + * + * The logic for choosing which struct members to ignore, and which members + * have valid values, is derived from the Vulkan spec sections for + * VkGraphicsPipelineCreateInfo, VkGraphicsPipelineLibraryCreateInfoEXT, and + * VkPipelineLibraryCreateInfoKHR. As of Vulkan 1.3.255, the spec text and VUs + * still contain inconsistencies regarding the validity of struct members, so + * read it carefully. Many of the VUs were written before + * VK_EXT_graphics_pipeline_library and never updated. (Lina's advice: Focus + * primarily on understanding the non-VU text, and use VUs to verify your + * comprehension). + */ +static void +vn_graphics_pipeline_state_fill( + const VkGraphicsPipelineCreateInfo *info, + struct vn_graphics_pipeline_state *restrict state, + struct vn_graphics_pipeline_fix_desc *out_fix_desc) +{ + /* Assume that state is already zero-filled. + * + * Invalidate attachment_aspects. + */ + state->render_pass.attachment_aspects = VK_IMAGE_ASPECT_METADATA_BIT; - if (!require_state) { - const bool has_dynamic_attachment = - has_dynamic_rendering && - (rendering_info->depthAttachmentFormat != VK_FORMAT_UNDEFINED || - rendering_info->stencilAttachmentFormat != VK_FORMAT_UNDEFINED); + const VkPipelineRenderingCreateInfo *rendering_info = + vk_find_struct_const(info->pNext, PIPELINE_RENDERING_CREATE_INFO); + const VkPipelineLibraryCreateInfoKHR *lib_info = + vk_find_struct_const(info->pNext, PIPELINE_LIBRARY_CREATE_INFO_KHR); + const uint32_t lib_count = lib_info ? lib_info->libraryCount : 0; - /* VUID-VkGraphicsPipelineCreateInfo-renderPass-06053 */ - require_state = has_fragment_shader_state && - has_fragment_output_state && has_dynamic_attachment; - } + /* This tracks which fields have valid values in the + * VkGraphicsPipelineCreateInfo pNext chain. + * + * We initially assume that all fields are invalid. We flip fields from + * invalid to valid as we dig through the pNext chain. + * + * A single field may be updated at multiple locations, therefore we update + * with `|=` instead of `=`. + * + * If `valid.foo` is set, then foo has a valid value if foo exists in the + * pNext chain. Even though NULL is not a valid pointer, NULL is considered + * a valid *value* for a pointer-typed variable. Same for VK_NULL_HANDLE + * and Vulkan handle-typed variables. + * + * Conversely, if `valid.foo` remains false at the end of this function, + * then the Vulkan spec permits foo to have any value. If foo has a pointer + * type, it may be an invalid pointer. If foo has a Vulkan handle type, it + * may be an invalid handle. + */ + struct vn_graphics_pipeline_create_info_fields valid = { 0 }; - fix_desc->erase.depth_stencil_state = !require_state; + /* Merge the linked pipeline libraries. */ + for (uint32_t i = 0; i < lib_count; i++) { + struct vn_graphics_pipeline *p = + vn_graphics_pipeline_from_handle(lib_info->pLibraries[i]); + vn_graphics_pipeline_state_merge(state, &p->state); } - /* Ignore pColorBlendState? */ - if (info->pColorBlendState) { - const bool has_static_attachment = - subpass && (subpass->attachment_aspects & VK_IMAGE_ASPECT_COLOR_BIT); + /* The VkGraphicsPipelineLibraryFlagsEXT that this pipeline provides + * directly (without linking). + */ + struct vn_graphics_pipeline_library_state direct_gpl = { 0 }; + vn_graphics_pipeline_library_state_update(info, &direct_gpl); - /* VUID-VkGraphicsPipelineCreateInfo-renderPass-06044 */ - bool require_state = has_fragment_output_state && has_static_attachment; + /* From the Vulkan 1.3.251 spec: + * VUID-VkGraphicsPipelineCreateInfo-pLibraries-06611 + * + * Any pipeline libraries included via + * VkPipelineLibraryCreateInfoKHR::pLibraries must not include any state + * subset already defined by this structure or defined by any other + * pipeline library in VkPipelineLibraryCreateInfoKHR::pLibraries + */ + assert(!(direct_gpl.mask & state->gpl.mask)); - if (!require_state) { - const bool has_dynamic_attachment = - has_dynamic_rendering && rendering_info->colorAttachmentCount; + /* Collect orthogonal state that is common to multiple GPL state subsets. */ + vn_graphics_dynamic_state_update(info, direct_gpl, &state->dynamic); + vn_graphics_shader_stages_update(info, direct_gpl, &valid, + &state->shader_stages); + vn_render_pass_state_update(info, direct_gpl, &valid, &state->render_pass); - /* VUID-VkGraphicsPipelineCreateInfo-renderPass-06054 */ - require_state = has_fragment_output_state && has_dynamic_attachment; + /* Collect remaining pre-raster shaders state. + * + * Of the remaining state, we must first collect the pre-raster shaders + * state because it influences how the other state is collected. + */ + if (direct_gpl.pre_raster_shaders) { + valid.tessellation_state |= + (bool)(state->shader_stages & + (VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT | + VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT)); + valid.rasterization_state = true; + valid.pipeline_layout = true; + + state->rasterizer_discard_enable = + info->pRasterizationState->rasterizerDiscardEnable; + + const bool is_raster_statically_disabled = + !state->dynamic.rasterizer_discard_enable && + state->rasterizer_discard_enable; + + if (!is_raster_statically_disabled) { + /* TODO(VK_EXT_extended_dynamic_state3): pViewportState may be + * invalid. + */ + valid.viewport_state = true; + + valid.viewport_state_viewports = + !state->dynamic.viewport && !state->dynamic.viewport_with_count; + + valid.viewport_state_scissors = + !state->dynamic.scissor && !state->dynamic.scissor_with_count; } - fix_desc->erase.color_blend_state = !require_state; + /* Defer setting the flag until all its state is filled. */ + state->gpl.pre_raster_shaders = true; } - /* Ignore pVertexInputState? - * The Vulkan spec (1.3.264) says: - * VK_DYNAMIC_STATE_VERTEX_INPUT_EXT specifies that the - * pVertexInputState state will be ignored and must be set dynamically - * with vkCmdSetVertexInputEXT before any drawing commands + /* Collect remaining vertex input interface state. + * + * TODO(VK_EXT_mesh_shader): Update. */ - if (info->pVertexInputState && has_dynamic_state.vertex_input) { - fix_desc->erase.vertex_input_state = true; + if (direct_gpl.vertex_input) { + const bool may_have_vertex_shader = + !state->gpl.pre_raster_shaders || + (state->shader_stages & VK_SHADER_STAGE_VERTEX_BIT); + + valid.vertex_input_state |= + may_have_vertex_shader && !state->dynamic.vertex_input; + + valid.input_assembly_state |= may_have_vertex_shader; + + /* Defer setting the flag until all its state is filled. */ + state->gpl.vertex_input = true; } - /* Ignore basePipelineHandle? - * VUID-VkGraphicsPipelineCreateInfo-flags-00722 - * VUID-VkGraphicsPipelineCreateInfo-flags-00724 - * VUID-VkGraphicsPipelineCreateInfo-flags-00725 + /* Does this pipeline have rasterization statically disabled? If disabled, + * then this pipeline does not directly provide fragment shader state nor + * fragment output state. + * + * About fragment shader state, the Vulkan 1.3.254 spec says: + * + * If a pipeline specifies pre-rasterization state either directly or by + * including it as a pipeline library and rasterizerDiscardEnable is set + * to VK_FALSE or VK_DYNAMIC_STATE_RASTERIZER_DISCARD_ENABLE is used, + * this state must be specified to create a complete graphics pipeline. + * + * If a pipeline includes + * VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_SHADER_BIT_EXT in + * VkGraphicsPipelineLibraryCreateInfoEXT::flags either explicitly or as + * a default, and either the conditions requiring this state for + * a complete graphics pipeline are met or this pipeline does not + * specify pre-rasterization state in any way, that pipeline must + * specify this state directly. + * + * About fragment output state, the Vulkan 1.3.254 spec says the same, but + * with VK_GRAPHICS_PIPELINE_LIBRARY_FRAGMENT_OUTPUT_INTERFACE_BIT_EXT. */ - if (info->basePipelineHandle != VK_NULL_HANDLE && - !(info->flags & VK_PIPELINE_CREATE_DERIVATIVE_BIT)) { - fix_desc->erase.base_pipeline_handle = true; + const bool is_raster_statically_disabled = + state->gpl.pre_raster_shaders && + !state->dynamic.rasterizer_discard_enable && + state->rasterizer_discard_enable; + + /* Collect remaining fragment shader state. */ + if (direct_gpl.fragment_shader) { + if (!is_raster_statically_disabled) { + /* Validity of pMultisampleState is easy here. + * + * VUID-VkGraphicsPipelineCreateInfo-pMultisampleState-06629 + * + * If the pipeline requires fragment shader state + * pMultisampleState must be NULL or a valid pointer to a valid + * VkPipelineMultisampleStateCreateInfo structure + */ + valid.multisample_state = true; + + /* TODO(VK_EXT_extended_dynamic_state3): pSampleMask may be invalid. */ + + if ((state->render_pass.attachment_aspects & + (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))) { + valid.depth_stencil_state = true; + } else if (state->render_pass.attachment_aspects == + VK_IMAGE_ASPECT_METADATA_BIT && + (info->flags & VK_PIPELINE_CREATE_LIBRARY_BIT_KHR)) { + /* The app has not yet provided render pass info, neither directly + * in this VkGraphicsPipelineCreateInfo nor in any linked pipeline + * libraries. Therefore we do not know if the final complete + * pipeline will have any depth or stencil attachments. If the + * final complete pipeline does have depth or stencil attachments, + * then the pipeline will use + * VkPipelineDepthStencilStateCreateInfo. Therefore, we must not + * ignore it. + */ + valid.depth_stencil_state = true; + } + + valid.pipeline_layout = true; + } + + /* Defer setting the flag until all its state is filled. */ + state->gpl.fragment_shader = true; } + + /* Collect remaining fragment output interface state. */ + if (direct_gpl.fragment_output) { + if (!is_raster_statically_disabled) { + /* Validity of pMultisampleState is easy here. + * + * VUID-VkGraphicsPipelineCreateInfo-rasterizerDiscardEnable-00751 + * + * If the pipeline requires fragment output interface state, + * pMultisampleState must be a valid pointer to a valid + * VkPipelineMultisampleStateCreateInfo structure + */ + valid.multisample_state = true; + + /* TODO(VK_EXT_extended_dynamic_state3): pSampleMask may be invalid */ + + valid.color_blend_state |= + (bool)(state->render_pass.attachment_aspects & + VK_IMAGE_ASPECT_COLOR_BIT); + valid.depth_stencil_state |= + (bool)(state->render_pass.attachment_aspects & + (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)); + } + + /* Defer setting the flag until all its state is filled. */ + state->gpl.fragment_output = true; + } + + *out_fix_desc = (struct vn_graphics_pipeline_fix_desc) { + .erase = { + /* clang-format off + * + * Format this to resemble a table. (clang-format transforms it into + * a difficult-to-read wall of text). + */ + .shader_stages = + !valid.shader_stages && + info->pStages, + .vertex_input_state = + !valid.vertex_input_state && + info->pVertexInputState, + .input_assembly_state = + !valid.input_assembly_state && + info->pInputAssemblyState, + .tessellation_state = + !valid.tessellation_state && + info->pTessellationState, + .viewport_state = + !valid.viewport_state && + info->pViewportState, + .viewport_state_viewports = + !valid.viewport_state_viewports && + valid.viewport_state && + info->pViewportState && + info->pViewportState->pViewports && + info->pViewportState->viewportCount, + .viewport_state_scissors = + !valid.viewport_state_scissors && + valid.viewport_state && + info->pViewportState && + info->pViewportState->pScissors && + info->pViewportState->scissorCount, + .rasterization_state = + !valid.rasterization_state && + info->pRasterizationState, + .multisample_state = + !valid.multisample_state && + info->pMultisampleState, + .depth_stencil_state = + !valid.depth_stencil_state && + info->pDepthStencilState, + .color_blend_state = + !valid.color_blend_state && + info->pColorBlendState, + .pipeline_layout = + !valid.pipeline_layout && + info->layout, + .render_pass = + !valid.render_pass && + info->renderPass, + .base_pipeline_handle = + !valid.base_pipeline_handle && + info->basePipelineHandle, + .rendering_info_formats = + !valid.rendering_info_formats && + rendering_info && + rendering_info->pColorAttachmentFormats && + rendering_info->colorAttachmentCount, + /* clang-format on */ + }, + }; } static const VkGraphicsPipelineCreateInfo * @@ -706,41 +1220,62 @@ vn_fix_graphics_pipeline_create_infos( continue; } + /* VkGraphicsPipelineCreateInfo */ + if (fix_descs[i].erase.shader_stages) { + fix_tmp->infos[i].stageCount = 0; + fix_tmp->infos[i].pStages = NULL; + } + if (fix_descs[i].erase.vertex_input_state) + fix_tmp->infos[i].pVertexInputState = NULL; + if (fix_descs[i].erase.input_assembly_state) + fix_tmp->infos[i].pInputAssemblyState = NULL; if (fix_descs[i].erase.tessellation_state) fix_tmp->infos[i].pTessellationState = NULL; - if (fix_descs[i].erase.viewport_state) fix_tmp->infos[i].pViewportState = NULL; - - if (fix_tmp->infos[i].pViewportState) { - if (fix_descs[i].erase.viewport_state_viewports || - fix_descs[i].erase.viewport_state_scissors) { - fix_tmp->viewport_state_infos[i] = *infos[i].pViewportState; - fix_tmp->infos[i].pViewportState = - &fix_tmp->viewport_state_infos[i]; - } - - if (fix_descs[i].erase.viewport_state_viewports) - fix_tmp->viewport_state_infos[i].pViewports = NULL; - - if (fix_descs[i].erase.viewport_state_scissors) - fix_tmp->viewport_state_infos[i].pScissors = NULL; - } - + if (fix_descs[i].erase.rasterization_state) + fix_tmp->infos[i].pRasterizationState = NULL; if (fix_descs[i].erase.multisample_state) fix_tmp->infos[i].pMultisampleState = NULL; - if (fix_descs[i].erase.depth_stencil_state) fix_tmp->infos[i].pDepthStencilState = NULL; - if (fix_descs[i].erase.color_blend_state) fix_tmp->infos[i].pColorBlendState = NULL; - - if (fix_descs[i].erase.vertex_input_state) - fix_tmp->infos[i].pVertexInputState = NULL; - + if (fix_descs[i].erase.pipeline_layout) + fix_tmp->infos[i].layout = VK_NULL_HANDLE; if (fix_descs[i].erase.base_pipeline_handle) fix_tmp->infos[i].basePipelineHandle = VK_NULL_HANDLE; + + /* VkPipelineViewportStateCreateInfo */ + if (fix_descs[i].erase.viewport_state_viewports || + fix_descs[i].erase.viewport_state_scissors) { + /* Swap original pViewportState with temporary state. */ + fix_tmp->viewport_state_infos[i] = *infos[i].pViewportState; + fix_tmp->infos[i].pViewportState = &fix_tmp->viewport_state_infos[i]; + + if (fix_descs[i].erase.viewport_state_viewports) + fix_tmp->viewport_state_infos[i].pViewports = NULL; + if (fix_descs[i].erase.viewport_state_scissors) + fix_tmp->viewport_state_infos[i].pScissors = NULL; + } + + /* VkPipelineRenderingCreateInfo */ + if (fix_descs[i].erase.rendering_info_formats) { + /* All format fields are invalid, but the only field that must be + * erased is pColorAttachmentFormats because the other + * fields are merely VkFormat values. Encoding invalid pointers is + * unsafe; encoding invalid VkFormat values is not unsafe. + * + * However, the fix is difficult because it requires a deep rewrite + * of the pNext chain. + * + * TODO: Fix invalid + * VkPipelineRenderingCreateInfo::pColorAttachmentFormats. + */ + vn_log(dev->instance, + "venus may encode array from invalid pointer " + "VkPipelineRenderingCreateInfo::pColorAttachmentFormats"); + } } *out_fix_tmp = fix_tmp; @@ -787,29 +1322,30 @@ vn_CreateGraphicsPipelines(VkDevice device, bool want_sync = false; VkResult result; + memset(pPipelines, 0, sizeof(*pPipelines) * createInfoCount); + + if (!vn_create_pipeline_handles(dev, VN_PIPELINE_TYPE_GRAPHICS, + createInfoCount, pPipelines, alloc)) { + return vn_error(dev->instance, VK_ERROR_OUT_OF_HOST_MEMORY); + } + STACK_ARRAY(struct vn_graphics_pipeline_fix_desc, fix_descs, createInfoCount); struct vn_graphics_pipeline_fix_tmp *fix_tmp = NULL; - memset(pPipelines, 0, sizeof(*pPipelines) * createInfoCount); - - VN_TRACE_BEGIN("find_fixes"); + VN_TRACE_BEGIN("fill_states"); for (uint32_t i = 0; i < createInfoCount; i++) { - vn_find_graphics_pipeline_create_info_fixes(&pCreateInfos[i], - &fix_descs[i]); + struct vn_graphics_pipeline *pipeline = + vn_graphics_pipeline_from_handle(pPipelines[i]); + vn_graphics_pipeline_state_fill(&pCreateInfos[i], &pipeline->state, + &fix_descs[i]); } VN_TRACE_END(); pCreateInfos = vn_fix_graphics_pipeline_create_infos( dev, createInfoCount, pCreateInfos, fix_descs, &fix_tmp, alloc); if (!pCreateInfos) { - STACK_ARRAY_FINISH(fix_descs); - return vn_error(dev->instance, VK_ERROR_OUT_OF_HOST_MEMORY); - } - - if (!vn_create_pipeline_handles(dev, VN_PIPELINE_TYPE_GRAPHICS, - createInfoCount, pPipelines, alloc)) { - vk_free(alloc, fix_tmp); + vn_destroy_failed_pipelines(dev, createInfoCount, pPipelines, alloc); STACK_ARRAY_FINISH(fix_descs); return vn_error(dev->instance, VK_ERROR_OUT_OF_HOST_MEMORY); }