[Mesa-stable] [PATCH 2/2] radeonsi: hard-code pixel center for interpolateAtSample without multisample buffers

Marek Olšák maraeo at gmail.com
Tue Sep 12 23:44:51 UTC 2017


For the series:

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Mon, Sep 11, 2017 at 5:11 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> The GLSL rules for interpolateAtSample are unfortunate:
>
>    "Returns the value of the input interpolant variable at
>     the location of sample number sample. If
>     multisample buffers are not available, the input
>     variable will be evaluated at the center of the pixel.
>     If sample sample does not exist, the position used to
>     interpolate the input variable is undefined."
>
> This fix will fallback to monolithic shader compilation when
> interpolateAtSample is used without multisampling.
>
> One alternative would be to always upload 16 sample positions,
> filling the buffer up with repetition when the actual number of
> samples is less, and then ANDing the sample ID with 0xf. However,
> that punishes all well-behaving users of interpolateAtSample,
> when in reality, only conformance tests should be affected by
> the issue.
>
> Fixes
> dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_sample.non_multisample_buffer.*
> ---
>  src/gallium/drivers/radeonsi/si_shader.c        | 28 ++++++++++++++++++++++++-
>  src/gallium/drivers/radeonsi/si_shader.h        |  3 +++
>  src/gallium/drivers/radeonsi/si_state_shaders.c |  3 +++
>  3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c
> index 85de2e407b4..d0af60856b0 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -3665,21 +3665,47 @@ static void interp_fetch_args(
>                 LLVMValueRef sample_id;
>                 LLVMValueRef halfval = LLVMConstReal(ctx->f32, 0.5f);
>
>                 /* fetch sample ID, then fetch its sample position,
>                  * and place into first two channels.
>                  */
>                 sample_id = lp_build_emit_fetch(bld_base,
>                                                 emit_data->inst, 1, TGSI_CHAN_X);
>                 sample_id = LLVMBuildBitCast(gallivm->builder, sample_id,
>                                              ctx->i32, "");
> -               sample_position = load_sample_position(ctx, sample_id);
> +
> +               /* Section 8.13.2 (Interpolation Functions) of the OpenGL Shading
> +                * Language 4.50 spec says about interpolateAtSample:
> +                *
> +                *    "Returns the value of the input interpolant variable at
> +                *     the location of sample number sample. If multisample
> +                *     buffers are not available, the input variable will be
> +                *     evaluated at the center of the pixel. If sample sample
> +                *     does not exist, the position used to interpolate the
> +                *     input variable is undefined."
> +                *
> +                * This means that sample_id values outside of the valid are
> +                * in fact valid input, and the usual mechanism for loading the
> +                * sample position doesn't work.
> +                */
> +               if (ctx->shader->key.mono.u.ps.interpolate_at_sample_force_center) {
> +                       LLVMValueRef center[4] = {
> +                               LLVMConstReal(ctx->f32, 0.5),
> +                               LLVMConstReal(ctx->f32, 0.5),
> +                               ctx->ac.f32_0,
> +                               ctx->ac.f32_0,
> +                       };
> +
> +                       sample_position = lp_build_gather_values(gallivm, center, 4);
> +               } else {
> +                       sample_position = load_sample_position(ctx, sample_id);
> +               }
>
>                 emit_data->args[0] = LLVMBuildExtractElement(gallivm->builder,
>                                                              sample_position,
>                                                              ctx->i32_0, "");
>
>                 emit_data->args[0] = LLVMBuildFSub(gallivm->builder, emit_data->args[0], halfval, "");
>                 emit_data->args[1] = LLVMBuildExtractElement(gallivm->builder,
>                                                              sample_position,
>                                                              ctx->i32_1, "");
>                 emit_data->args[1] = LLVMBuildFSub(gallivm->builder, emit_data->args[1], halfval, "");
> diff --git a/src/gallium/drivers/radeonsi/si_shader.h b/src/gallium/drivers/radeonsi/si_shader.h
> index be17cf462be..22bb56b0ece 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.h
> +++ b/src/gallium/drivers/radeonsi/si_shader.h
> @@ -508,20 +508,23 @@ struct si_shader_key {
>
>         /* Flags for monolithic compilation only. */
>         struct {
>                 /* One byte for every input: SI_FIX_FETCH_* enums. */
>                 uint8_t         vs_fix_fetch[SI_MAX_ATTRIBS];
>
>                 union {
>                         uint64_t        ff_tcs_inputs_to_copy; /* for fixed-func TCS */
>                         /* When PS needs PrimID and GS is disabled. */
>                         unsigned        vs_export_prim_id:1;
> +                       struct {
> +                               unsigned interpolate_at_sample_force_center:1;
> +                       } ps;
>                 } u;
>         } mono;
>
>         /* Optimization flags for asynchronous compilation only. */
>         struct {
>                 /* For HW VS (it can be VS, TES, GS) */
>                 uint64_t        kill_outputs; /* "get_unique_index" bits */
>                 unsigned        clip_disable:1;
>
>                 /* For shaders where monolithic variants have better code.
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index 6b63e69d91c..cbef8d8a646 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -1453,20 +1453,23 @@ static inline void si_shader_selector_key(struct pipe_context *ctx,
>                                  * of (i,j), which is the optimization here. */
>                                 key->part.ps.prolog.force_persp_center_interp =
>                                         sel->info.uses_persp_center +
>                                         sel->info.uses_persp_centroid +
>                                         sel->info.uses_persp_sample > 1;
>
>                                 key->part.ps.prolog.force_linear_center_interp =
>                                         sel->info.uses_linear_center +
>                                         sel->info.uses_linear_centroid +
>                                         sel->info.uses_linear_sample > 1;
> +
> +                               if (sel->info.opcode_count[TGSI_OPCODE_INTERP_SAMPLE])
> +                                       key->mono.u.ps.interpolate_at_sample_force_center = 1;
>                         }
>                 }
>
>                 key->part.ps.epilog.alpha_func = si_get_alpha_test_func(sctx);
>                 break;
>         }
>         default:
>                 assert(0);
>         }
>
> --
> 2.11.0
>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable


More information about the mesa-stable mailing list