From 7e0616ecc58a037572b9657af21075f3cb0655fe Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Mon, 27 May 2024 11:22:02 +0200 Subject: [PATCH] v3dv: only flag 'shader writes point size' if the shader actually writes it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the shader writes point size, then the compiler needs to ensure it writes it in the appropriate vpm output slot and also clamp its value to expected limits. This is why we have the per_vertex_point_size in the shader key, so it doesn't really make sense to set this if the shader doesn't write point size. If the shader record flags that the shader writes point size then the hardware will use the shader written value to override point size state (set with the POINT_SIZE packet), so again, we really only want to set this in the shader state record if the shader actually writes its value. While we could also limit this to point primitives, since these are the only primitives where point size has an effect, this is not really required, and skipping this allows us to use the same shader with any primitive type (otherwise we would have to compile 2 different shaders). Finally, this change makes the vertex shader setup for point size match the one we had been doing for geometry shaders, so it makes both stages behave consistently regarding point size behavior. Reviewed-by: Alejandro Piñeiro Part-of: --- src/broadcom/vulkan/v3dv_pipeline.c | 12 ++---------- src/broadcom/vulkan/v3dvx_pipeline.c | 4 +++- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index 6f0434ab255..0df505f7255 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -1299,16 +1299,8 @@ pipeline_populate_v3d_vs_key(struct v3d_vs_key *key, struct v3dv_pipeline *pipeline = p_stage->pipeline; - /* Vulkan specifies a point size per vertex, so true for if the prim are - * points, like on ES2) - */ - const VkPipelineInputAssemblyStateCreateInfo *ia_info = - pCreateInfo->pInputAssemblyState; - uint8_t topology = vk_to_mesa_prim[ia_info->topology]; - - /* FIXME: PRIM_POINTS is not enough, in gallium the full check is - * MESA_PRIM_POINTS && v3d->rasterizer->base.point_size_per_vertex */ - key->per_vertex_point_size = (topology == MESA_PRIM_POINTS); + key->per_vertex_point_size = + p_stage->nir->info.outputs_written & (1ull << VARYING_SLOT_PSIZ); key->is_coord = broadcom_shader_stage_is_binning(p_stage->stage); diff --git a/src/broadcom/vulkan/v3dvx_pipeline.c b/src/broadcom/vulkan/v3dvx_pipeline.c index 6c4e340f6af..51738a2aa4f 100644 --- a/src/broadcom/vulkan/v3dvx_pipeline.c +++ b/src/broadcom/vulkan/v3dvx_pipeline.c @@ -375,7 +375,9 @@ pack_shader_state_record(struct v3dv_pipeline *pipeline) bool point_size_in_shaded_vertex_data; if (!pipeline->has_gs) { - point_size_in_shaded_vertex_data = pipeline->topology == MESA_PRIM_POINTS; + struct v3d_vs_prog_data *prog_data_vs = + pipeline->shared_data->variants[BROADCOM_SHADER_VERTEX]->prog_data.vs; + point_size_in_shaded_vertex_data = prog_data_vs->writes_psiz; } else { struct v3d_gs_prog_data *prog_data_gs = pipeline->shared_data->variants[BROADCOM_SHADER_GEOMETRY]->prog_data.gs;