[Mesa-dev] [PATCH 2/9] radeonsi: use better sample locations for 4x MSAA

[Nicolai Hähnle]

On 02.05.2018 06:13, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> Discovered by luck. Verified with the piglit MSAA accuracy test.
> It also shows that the worst case EQAA 16s4f results in very good 4x MSAA
> in the worst case.
> 
> Nine might not like these positions, but they are prettier to the eye and
> GL doesn't care.

Curious. Can you actually tell the difference? Anyway,

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>


> ---
>   src/gallium/drivers/radeonsi/si_state_msaa.c | 21 ++++++++++----------
>   1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_state_msaa.c b/src/gallium/drivers/radeonsi/si_state_msaa.c
> index 0f9e0fea1c7..b6504d53a2a 100644
> --- a/src/gallium/drivers/radeonsi/si_state_msaa.c
> +++ b/src/gallium/drivers/radeonsi/si_state_msaa.c
> @@ -85,64 +85,63 @@
>   /* 1x MSAA */
>   static const uint32_t sample_locs_1x =
>   	FILL_SREG( 0, 0,   0, 0,   0, 0,   0, 0); /* S1, S2, S3 fields are not used by 1x */
>   static const uint64_t centroid_priority_1x = 0x0000000000000000ull;
>   
>   /* 2x MSAA */
>   static const uint32_t sample_locs_2x =
>   	FILL_SREG(-4,-4,   4, 4,   0, 0,   0, 0); /* S2 & S3 fields are not used by 2x MSAA */
>   static const uint64_t centroid_priority_2x = 0x1010101010101010ull;
>   
> -/* 4x MSAA */
> -static const uint32_t sample_locs_4x =
> -	FILL_SREG(-2,-6,   2, 6,  -6, 2,   6,-2);
> -static const uint64_t centroid_priority_4x = 0x3210321032103210ull;
> -
>   /* 8x MSAA */
>   static const uint32_t sample_locs_8x[] = {
>   	FILL_SREG(-3,-5,   5, 1,  -5, 5,   7,-7),
>   	FILL_SREG(-7,-1,   3, 7,  -1, 3,   1,-3),
>   	FILL_SREG( 0, 0,   0, 0,   0, 0,   0, 0), /* S8, S9 etc. are not used by 8x */
>   	FILL_SREG( 0, 0,   0, 0,   0, 0,   0, 0),
>   };
>   static const uint64_t centroid_priority_8x = 0x3542017635420176ull;
>   
> -/* 16x MSAA */
> -static const uint32_t sample_locs_16x[] = {
> +/* 4x and 16x MSAA
> + * (the first 4 locations happen to be optimal for 4x MSAA, better than
> + *  the standard DX 4x locations)
> + */
> +static const uint32_t sample_locs_4x_16x[] = {
>   	FILL_SREG(-5,-2,   5, 3,  -2, 6,   3,-5),
>   	FILL_SREG(-7,-8,   1, 1,  -6, 4,   7,-4),
>   	FILL_SREG(-1,-3,   6, 7,  -3, 2,   0,-7),
>   	FILL_SREG(-4,-6,   2, 5,  -8, 0,   4,-1),
>   };
> +static const uint64_t centroid_priority_4x = 0x2310231023102310ull;
>   static const uint64_t centroid_priority_16x = 0x497ec6b231d0fa85ull;
>   
>   static void si_get_sample_position(struct pipe_context *ctx, unsigned sample_count,
>   				   unsigned sample_index, float *out_value)
>   {
>   	const uint32_t *sample_locs;
>   
>   	switch (sample_count) {
>   	case 1:
>   	default:
>   		sample_locs = &sample_locs_1x;
>   		break;
>   	case 2:
>   		sample_locs = &sample_locs_2x;
>   		break;
>   	case 4:
> -		sample_locs = &sample_locs_4x;
> +		sample_locs = sample_locs_4x_16x;
>   		break;
>   	case 8:
>   		sample_locs = sample_locs_8x;
>   		break;
>   	case 16:
> -		sample_locs = sample_locs_16x;
> +		sample_locs = sample_locs_4x_16x;
>   		break;
>   	}
>   
>   	out_value[0] = (GET_SX(sample_locs, sample_index) + 8) / 16.0f;
>   	out_value[1] = (GET_SY(sample_locs, sample_index) + 8) / 16.0f;
>   }
>   
>   static void si_emit_max_4_sample_locs(struct radeon_winsys_cs *cs,
>   				      uint64_t centroid_priority,
>   				      uint32_t sample_locs)
> @@ -176,27 +175,27 @@ void si_emit_sample_locations(struct radeon_winsys_cs *cs, int nr_samples)
>   {
>   	switch (nr_samples) {
>   	default:
>   	case 1:
>   		si_emit_max_4_sample_locs(cs, centroid_priority_1x, sample_locs_1x);
>   		break;
>   	case 2:
>   		si_emit_max_4_sample_locs(cs, centroid_priority_2x, sample_locs_2x);
>   		break;
>   	case 4:
> -		si_emit_max_4_sample_locs(cs, centroid_priority_4x, sample_locs_4x);
> +		si_emit_max_4_sample_locs(cs, centroid_priority_4x, sample_locs_4x_16x[0]);
>   		break;
>   	case 8:
>   		si_emit_max_16_sample_locs(cs, centroid_priority_8x, sample_locs_8x, 8);
>   		break;
>   	case 16:
> -		si_emit_max_16_sample_locs(cs, centroid_priority_16x, sample_locs_16x, 16);
> +		si_emit_max_16_sample_locs(cs, centroid_priority_16x, sample_locs_4x_16x, 16);
>   		break;
>   	}
>   }
>   
>   void si_init_msaa_functions(struct si_context *sctx)
>   {
>   	int i;
>   
>   	sctx->b.get_sample_position = si_get_sample_position;
>   
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.