[Mesa-stable] [PATCH v2] radv: fix passing clip/cull distances from VS to PS
Dylan Baker
dylan at pnwbakers.com
Tue Sep 4 16:16:09 UTC 2018
Quoting Samuel Pitoiset (2018-08-29 13:56:17)
> CTS doesn't test input clip/cull distances for the fragment
> shader stage, which explains why this was totally broken. I
> wrote a simple test locally that works now.
>
> This fixes a crash with GTA V and DXVK.
>
> Note that we are exporting unused parameters from the vertex
> shader now, but this can't be optimized easily because we don't
> keep the fragment shader info...
>
> Cc: mesa-stable at lists.freedesktop.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107477
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
> src/amd/vulkan/radv_nir_to_llvm.c | 30 +++++++++++++++++++++++++++++-
> src/amd/vulkan/radv_pipeline.c | 16 ++++++++++++++++
> src/amd/vulkan/radv_shader.h | 1 +
> src/amd/vulkan/radv_shader_info.c | 4 ++++
> 4 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c
> index 4940e3230f..d7cd8cc069 100644
> --- a/src/amd/vulkan/radv_nir_to_llvm.c
> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> @@ -2098,9 +2098,10 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
> int idx = variable->data.location;
> unsigned attrib_count = glsl_count_attribute_slots(variable->type, false);
> LLVMValueRef interp = NULL;
> + uint64_t mask;
>
> variable->data.driver_location = idx * 4;
> - ctx->input_mask |= ((1ull << attrib_count) - 1) << variable->data.location;
> + mask = ((1ull << attrib_count) - 1) << variable->data.location;
>
> if (glsl_get_base_type(glsl_without_array(variable->type)) == GLSL_TYPE_FLOAT) {
> unsigned interp_type;
> @@ -2121,6 +2122,15 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
> for (unsigned i = 0; i < attrib_count; ++i)
> ctx->inputs[ac_llvm_reg_index_soa(idx + i, 0)] = interp;
>
> + if (idx == VARYING_SLOT_CLIP_DIST0) {
> + /* Do not account for the number of components inside the array
> + * of clip/cull distances because this might wrongly set other
> + * bits like primitive ID or layer.
> + */
> + mask = 1ull << VARYING_SLOT_CLIP_DIST0;
> + }
> +
> + ctx->input_mask |= mask;
> }
>
> static void
> @@ -2187,6 +2197,17 @@ handle_fs_inputs(struct radv_shader_context *ctx,
> if (LLVMIsUndef(interp_param))
> ctx->shader_info->fs.flat_shaded_mask |= 1u << index;
> ++index;
> + } else if (i == VARYING_SLOT_CLIP_DIST0) {
> + int length = ctx->shader_info->info.ps.num_input_clips_culls;
> +
> + for (unsigned j = 0; j < length; j += 4) {
> + inputs = ctx->inputs + ac_llvm_reg_index_soa(i, j);
> +
> + interp_param = *inputs;
> + interp_fs_input(ctx, index, interp_param,
> + ctx->abi.prim_mask, inputs);
> + ++index;
> + }
> } else if (i == VARYING_SLOT_POS) {
> for(int i = 0; i < 3; ++i)
> inputs[i] = ctx->abi.frag_pos[i];
> @@ -2482,6 +2503,13 @@ handle_vs_outputs_post(struct radv_shader_context *ctx,
> memcpy(&pos_args[target - V_008DFC_SQ_EXP_POS],
> &args, sizeof(args));
>
> + /* Export the clip/cull distances values to the next stage. */
> + radv_export_param(ctx, param_count, &slots[0], 0xf);
> + outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0] = param_count++;
> + if (ctx->num_output_clips + ctx->num_output_culls > 4) {
> + radv_export_param(ctx, param_count, &slots[4], 0xf);
> + outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1] = param_count++;
> + }
> }
>
> LLVMValueRef pos_values[4] = {ctx->ac.f32_0, ctx->ac.f32_0, ctx->ac.f32_0, ctx->ac.f32_1};
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
> index e63c481d1e..0303642d7e 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -3052,6 +3052,22 @@ radv_pipeline_generate_ps_inputs(struct radeon_cmdbuf *cs,
> ps_offset++;
> }
>
> + if (ps->info.info.ps.num_input_clips_culls) {
> + unsigned vs_offset;
> +
> + vs_offset = outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0];
> + if (vs_offset != AC_EXP_PARAM_UNDEFINED) {
> + ps_input_cntl[ps_offset] = offset_to_ps_input(vs_offset, true);
> + ++ps_offset;
> + }
> +
> + vs_offset = outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1];
> + if (vs_offset != AC_EXP_PARAM_UNDEFINED) {
> + ps_input_cntl[ps_offset] = offset_to_ps_input(vs_offset, true);
> + ++ps_offset;
> + }
> + }
> +
> for (unsigned i = 0; i < 32 && (1u << i) <= ps->info.fs.input_mask; ++i) {
> unsigned vs_offset;
> bool flat_shade;
> diff --git a/src/amd/vulkan/radv_shader.h b/src/amd/vulkan/radv_shader.h
> index 03760b689c..897e2fc5e1 100644
> --- a/src/amd/vulkan/radv_shader.h
> +++ b/src/amd/vulkan/radv_shader.h
> @@ -174,6 +174,7 @@ struct radv_shader_info {
> bool has_pcoord;
> bool prim_id_input;
> bool layer_input;
> + uint8_t num_input_clips_culls;
> } ps;
> struct {
> bool uses_grid_size;
> diff --git a/src/amd/vulkan/radv_shader_info.c b/src/amd/vulkan/radv_shader_info.c
> index 8026cca46c..a45c847c46 100644
> --- a/src/amd/vulkan/radv_shader_info.c
> +++ b/src/amd/vulkan/radv_shader_info.c
> @@ -341,6 +341,7 @@ static void
> gather_info_input_decl_ps(const nir_shader *nir, const nir_variable *var,
> struct radv_shader_info *info)
> {
> + unsigned attrib_count = glsl_count_attribute_slots(var->type, false);
> const struct glsl_type *type = glsl_without_array(var->type);
> int idx = var->data.location;
>
> @@ -354,6 +355,9 @@ gather_info_input_decl_ps(const nir_shader *nir, const nir_variable *var,
> case VARYING_SLOT_LAYER:
> info->ps.layer_input = true;
> break;
> + case VARYING_SLOT_CLIP_DIST0:
> + info->ps.num_input_clips_culls = attrib_count;
> + break;
> default:
> break;
> }
> --
> 2.18.0
>
Hi Samuel,
There was a very minor conflict when I pulled this into 18.1, I'm confident that
my resolution is correct, but please verify that:
https://gitlab.freedesktop.org/mesa/mesa/commit/1ec6ba931bc35b5aef1fae7d9377a035d53eee2a
looks correct to you.
Thanks,
Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20180904/e31d29ac/attachment.sig>
More information about the mesa-stable
mailing list