From 5019b21f0ebd154fcc53f7851aef85c748bb1d24 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Thu, 28 Nov 2024 12:38:23 -0500 Subject: [PATCH] zink: fix gl_PrimitiveID reads with quads Zink emulates quads with a GS, which imposes requirements for gl_PrimitiveID. Handle them here. Previously Zink went out of spec. Fixes spec@glsl-1.50@execution@primitive-id-no-gs-quads and spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Antonino Maniscalco Fixes: e2220ee55e4 ("zink: filled quad emulation gs generation function") Closes: #12214 Part-of: (cherry picked from commit 23601d6632bde5c57a353699bf2505ffca45c1c5) Conflicts: src/gallium/drivers/zink/ci/zink-anv-adl-fails.txt src/gallium/drivers/zink/ci/zink-lvp-fails.txt src/gallium/drivers/zink/ci/zink-radv-vangogh-fails.txt src/gallium/drivers/zink/ci/zink-venus-lvp-fails.txt --- .pick_status.json | 2 +- .../drivers/zink/ci/zink-anv-adl-fails.txt | 2 -- .../drivers/zink/ci/zink-anv-tgl-fails.txt | 3 -- .../drivers/zink/ci/zink-lvp-fails.txt | 4 --- .../drivers/zink/ci/zink-nvk-ga106-fails.txt | 2 -- .../zink/ci/zink-radv-navi10-fails.txt | 1 - .../zink/ci/zink-radv-navi31-fails.txt | 1 - .../zink/ci/zink-radv-polaris10-fails.txt | 1 - .../zink/ci/zink-radv-vangogh-fails.txt | 1 - .../drivers/zink/ci/zink-tu-a750-fails.txt | 2 -- .../drivers/zink/ci/zink-venus-lvp-fails.txt | 3 -- src/gallium/drivers/zink/zink_compiler.c | 34 ++++++++++++++++++- 12 files changed, 34 insertions(+), 22 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 4d7dbe0c1e3..c5a8311544a 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -774,7 +774,7 @@ "description": "zink: fix gl_PrimitiveID reads with quads", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "e2220ee55e40ec5e1ef0d8f74ff6e7d7bb5db16a", "notes": null diff --git a/src/gallium/drivers/zink/ci/zink-anv-adl-fails.txt b/src/gallium/drivers/zink/ci/zink-anv-adl-fails.txt index 83a41c197e0..0e288119387 100644 --- a/src/gallium/drivers/zink/ci/zink-anv-adl-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-anv-adl-fails.txt @@ -634,7 +634,6 @@ dEQP-GLES3.functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_revers dEQP-GLES3.functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_src_x,Fail dEQP-GLES3.functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_src_x,Fail wayland-dEQP-EGL.functional.resize.surface_size.shrink,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quads,Fail dEQP-GLES3.functional.fbo.multiview.samples_1,Fail spec@arb_gpu_shader_fp64@uniform_buffers@fs-ubo-load.indirect.3,Fail dEQP-GLES3.functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_src_dst_y,Fail @@ -682,4 +681,3 @@ spec@ext_framebuffer_multisample@interpolation 8 centroid-deriv-disabled,Fail wayland-dEQP-EGL.functional.resize.surface_size.grow,Fail dEQP-GLES3.functional.fbo.blit.rect.nearest_consistency_out_of_bounds_mag_reverse_dst_x,Fail dEQP-GLES3.functional.fbo.blit.rect.nearest_consistency_out_of_bounds_min_reverse_dst_x,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail diff --git a/src/gallium/drivers/zink/ci/zink-anv-tgl-fails.txt b/src/gallium/drivers/zink/ci/zink-anv-tgl-fails.txt index 75a77b8ce06..93bd8664b4c 100644 --- a/src/gallium/drivers/zink/ci/zink-anv-tgl-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-anv-tgl-fails.txt @@ -558,9 +558,6 @@ spec@ext_transform_feedback@tessellation triangle_fan flat_first,Fail spec@ext_transform_feedback@tessellation quad_strip wireframe,Fail spec@ext_transform_feedback@tessellation quads wireframe,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quads,Fail - spec@khr_texture_compression_astc@miptree-gl srgb-fp,Fail spec@khr_texture_compression_astc@miptree-gl srgb-fp@sRGB decode full precision,Fail spec@khr_texture_compression_astc@miptree-gles srgb-fp,Fail diff --git a/src/gallium/drivers/zink/ci/zink-lvp-fails.txt b/src/gallium/drivers/zink/ci/zink-lvp-fails.txt index 3446fcbcd7d..8e2b357ab7a 100644 --- a/src/gallium/drivers/zink/ci/zink-lvp-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-lvp-fails.txt @@ -123,10 +123,6 @@ spec@!opengl 1.0@rasterpos,Fail spec@!opengl 1.0@rasterpos@glsl_vs_gs_linked,Fail spec@!opengl 1.0@rasterpos@glsl_vs_tes_linked,Fail -spec@arb_tessellation_shader@execution@gs-primitiveid-instanced,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quads,Fail - spec@ext_transform_feedback@tessellation quads wireframe,Fail # Debian 12 CI update, see https://gitlab.freedesktop.org/mesa/mesa/-/issues/9072 diff --git a/src/gallium/drivers/zink/ci/zink-nvk-ga106-fails.txt b/src/gallium/drivers/zink/ci/zink-nvk-ga106-fails.txt index 3ccc9616fd9..44fcce635b7 100644 --- a/src/gallium/drivers/zink/ci/zink-nvk-ga106-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-nvk-ga106-fails.txt @@ -461,8 +461,6 @@ spec@ext_transform_feedback@immediate-reuse-uniform-buffer,Fail spec@ext_transform_feedback@tessellation quad_strip wireframe,Fail spec@ext_transform_feedback@tessellation quads wireframe,Fail spec@glsl-1.10@execution@samplers@glsl-fs-lots-of-tex,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quads,Fail spec@glsl-es-3.00@execution@built-in-functions@fs-packhalf2x16,Fail spec@glsl-es-3.00@execution@built-in-functions@vs-packhalf2x16,Fail spec@khr_texture_compression_astc@miptree-gl srgb-fp,Fail diff --git a/src/gallium/drivers/zink/ci/zink-radv-navi10-fails.txt b/src/gallium/drivers/zink/ci/zink-radv-navi10-fails.txt index 27e88c99c03..ac08a9dc0f0 100644 --- a/src/gallium/drivers/zink/ci/zink-radv-navi10-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-radv-navi10-fails.txt @@ -44,7 +44,6 @@ spec@glsl-1.10@execution@samplers@glsl-fs-shadow2d-clamp-z,Fail spec@glsl-1.20@compiler@invalid-vec4-array-to-vec3-array-conversion.vert,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail spec@glsl-es-3.00@execution@built-in-functions@fs-packhalf2x16,Fail spec@glsl-es-3.00@execution@built-in-functions@vs-packhalf2x16,Fail spec@khr_texture_compression_astc@miptree-gles srgb-fp,Fail diff --git a/src/gallium/drivers/zink/ci/zink-radv-navi31-fails.txt b/src/gallium/drivers/zink/ci/zink-radv-navi31-fails.txt index 600d253fc5d..a8f1e23a2cc 100644 --- a/src/gallium/drivers/zink/ci/zink-radv-navi31-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-radv-navi31-fails.txt @@ -44,7 +44,6 @@ spec@glsl-1.10@execution@samplers@glsl-fs-shadow2d-clamp-z,Fail spec@glsl-1.20@compiler@invalid-vec4-array-to-vec3-array-conversion.vert,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail spec@glsl-es-3.00@execution@built-in-functions@fs-packhalf2x16,Fail spec@glsl-es-3.00@execution@built-in-functions@vs-packhalf2x16,Fail spec@khr_texture_compression_astc@miptree-gles srgb-fp,Fail diff --git a/src/gallium/drivers/zink/ci/zink-radv-polaris10-fails.txt b/src/gallium/drivers/zink/ci/zink-radv-polaris10-fails.txt index 66dd138c47d..e77ae129ce4 100644 --- a/src/gallium/drivers/zink/ci/zink-radv-polaris10-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-radv-polaris10-fails.txt @@ -56,7 +56,6 @@ spec@glsl-1.10@execution@samplers@glsl-fs-shadow2d-clamp-z,Fail spec@glsl-1.20@compiler@invalid-vec4-array-to-vec3-array-conversion.vert,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail spec@glsl-es-3.00@execution@built-in-functions@fs-packhalf2x16,Fail spec@glsl-es-3.00@execution@built-in-functions@vs-packhalf2x16,Fail spec@khr_texture_compression_astc@miptree-gles srgb-fp,Fail diff --git a/src/gallium/drivers/zink/ci/zink-radv-vangogh-fails.txt b/src/gallium/drivers/zink/ci/zink-radv-vangogh-fails.txt index 61c00e3942d..95c019e0860 100644 --- a/src/gallium/drivers/zink/ci/zink-radv-vangogh-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-radv-vangogh-fails.txt @@ -43,7 +43,6 @@ spec@glsl-1.10@execution@samplers@glsl-fs-shadow2d-clamp-z,Fail spec@glsl-1.20@compiler@invalid-vec4-array-to-vec3-array-conversion.vert,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail spec@glsl-es-3.00@execution@built-in-functions@fs-packhalf2x16,Fail spec@glsl-es-3.00@execution@built-in-functions@vs-packhalf2x16,Fail spec@khr_texture_compression_astc@miptree-gles srgb-fp,Fail diff --git a/src/gallium/drivers/zink/ci/zink-tu-a750-fails.txt b/src/gallium/drivers/zink/ci/zink-tu-a750-fails.txt index 5213e153513..8aebd8fbc63 100644 --- a/src/gallium/drivers/zink/ci/zink-tu-a750-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-tu-a750-fails.txt @@ -493,8 +493,6 @@ spec@glsl-1.50@built-in constants@gl_MaxFragmentInputComponents,Fail spec@glsl-1.50@execution@geometry@point-size-out,Fail spec@glsl-1.50@execution@geometry@primitive-id-restart gl_line_strip_adjacency other,Crash spec@glsl-1.50@execution@interface-blocks-complex-vs-fs,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quads,Fail spec@glsl-1.50@execution@redeclare-pervertex-out-subset-gs,Fail spec@glsl-1.50@execution@variable-indexing@gs-output-array-vec4-index-wr,Fail spec@glsl-3.30@built-in constants,Fail diff --git a/src/gallium/drivers/zink/ci/zink-venus-lvp-fails.txt b/src/gallium/drivers/zink/ci/zink-venus-lvp-fails.txt index 7424fab4055..cddef3bd34b 100644 --- a/src/gallium/drivers/zink/ci/zink-venus-lvp-fails.txt +++ b/src/gallium/drivers/zink/ci/zink-venus-lvp-fails.txt @@ -142,9 +142,6 @@ spec@arb_arrays_of_arrays@execution@image_store@basic-imagestore-mixed-const-non spec@arb_arrays_of_arrays@execution@image_store@basic-imagestore-mixed-const-non-const-uniform-index2,Fail spec@arb_arrays_of_arrays@execution@image_store@basic-imagestore-non-const-uniform-index,Fail spec@arb_gpu_shader_fp64@execution@conversion,Fail -spec@arb_tessellation_shader@execution@gs-primitiveid-instanced,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quad-strip,Fail -spec@glsl-1.50@execution@primitive-id-no-gs-quads,Fail spec@glsl-4.00@execution@conversion,Fail spec@ext_transform_feedback@tessellation quads wireframe,Fail diff --git a/src/gallium/drivers/zink/zink_compiler.c b/src/gallium/drivers/zink/zink_compiler.c index d962f4d26d8..ee5221003f6 100644 --- a/src/gallium/drivers/zink/zink_compiler.c +++ b/src/gallium/drivers/zink/zink_compiler.c @@ -22,6 +22,7 @@ */ #include "nir_opcodes.h" +#include "shader_enums.h" #include "zink_context.h" #include "zink_compiler.h" #include "zink_descriptors.h" @@ -1096,6 +1097,8 @@ zink_create_quads_emulation_gs(const nir_shader_compiler_options *options, /* Create input/output variables. */ nir_foreach_shader_out_variable(var, prev_stage) { assert(!var->data.patch); + assert(var->data.location != VARYING_SLOT_PRIMITIVE_ID && + "not a VS output"); /* input vars can't be created for those */ if (var->data.location == VARYING_SLOT_LAYER || @@ -1132,6 +1135,30 @@ zink_create_quads_emulation_gs(const nir_shader_compiler_options *options, out_vars[num_vars++] = out; } + /* When a geometry shader is not used, a fragment shader may read primitive + * ID and get an implicit value without the vertex shader writing an ID. This + * case needs to work even when we inject a GS internally. + * + * However, if a geometry shader precedes a fragment shader that reads + * primitive ID, Vulkan requires that the geometry shader write primitive ID. + * To handle this case correctly, we must write primitive ID, copying the + * fixed-function gl_PrimitiveIDIn input which matches what the fragment + * shader will expect. + * + * If the fragment shader doesn't read primitive ID, this copy will likely be + * optimized out at link-time by the Vulkan driver. Unless this is + * non-monolithic -- in which case we don't know whether the fragment shader + * will read primitive ID either. In both cases, the right thing for Zink + * to do is copy primitive ID unconditionally. + */ + in_vars[num_vars] = nir_create_variable_with_location( + nir, nir_var_shader_in, VARYING_SLOT_PRIMITIVE_ID, glsl_int_type()); + + out_vars[num_vars] = nir_create_variable_with_location( + nir, nir_var_shader_out, VARYING_SLOT_PRIMITIVE_ID, glsl_int_type()); + + num_vars++; + int mapping_first[] = {0, 1, 2, 0, 2, 3}; int mapping_last[] = {0, 1, 3, 1, 2, 3}; nir_def *last_pv_vert_def = nir_load_provoking_last(&b); @@ -1146,7 +1173,12 @@ zink_create_quads_emulation_gs(const nir_shader_compiler_options *options, if (in_vars[j]->data.location == VARYING_SLOT_EDGE) { continue; } - nir_deref_instr *in_value = nir_build_deref_array(&b, nir_build_deref_var(&b, in_vars[j]), idx); + + /* gl_PrimitiveIDIn is not arrayed, all other inputs are */ + nir_deref_instr *in_value = nir_build_deref_var(&b, in_vars[j]); + if (in_vars[j]->data.location != VARYING_SLOT_PRIMITIVE_ID) + in_value = nir_build_deref_array(&b, in_value, idx); + copy_vars(&b, nir_build_deref_var(&b, out_vars[j]), in_value); } nir_emit_vertex(&b, 0);