[Mesa-dev] [PATCH] r600: Scale integer valued texture border colors to float (v2)

[Roland Scheidegger]

Am 24.07.2018 um 21:27 schrieb Gert Wollny:
> It seems the hardware always expects floating point border color values
> [0,1] for unsigned, and [-1,1] for signed texture component, regardless
> of pixel type, but the border colors are passed according to texture
> component type. Hence, before submitting the border color, convert and
> scale it these ranges accordingly.
> 
> This doesn't seem to work for textures with 32 bit integer components
> though, here, it seems that the border color is always set to zero,
> regardless of the BORDER_COLOR_TYPE state set in Q_TEX_SAMPLER_WORD0_0.
> 
> v2: Simplyfy logic as suggested by Roland Schneidegger
> 
> Fixes:
>   dEQP-GLES31.functional.texture.border_clamp.formats.compressed*
>   dEQP-GLES31.functional.texture.border_clamp.formats.r* (non 32 bit integer)
>   dEQP-GLES31.functional.texture.border_clamp.per_axis_wrap_mode.texture_2d*
>  and a number of piglits out of 
>   piglit run gpu -t texture -t gather -t formats 
> 
> Signed-off-by: Gert Wollny <gw.fossdev at gmail.com>
> ---
>  src/gallium/drivers/r600/evergreen_state.c | 45 +++++++++++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index d100cfab1d..e7207a500b 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
> @@ -2402,6 +2402,37 @@ static void evergreen_emit_cs_sampler_views(struct r600_context *rctx, struct r6
>  	                             EG_FETCH_CONSTANTS_OFFSET_CS + R600_MAX_CONST_BUFFERS, RADEON_CP_PACKET3_COMPUTE_MODE);
>  }
>  
> +static void evergreen_convert_border_color(union pipe_color_union *in,
> +                                           union pipe_color_union *out,
> +                                           enum pipe_format format)
> +{
> +	if (util_format_is_pure_integer(format) &&
> +		 !util_format_is_depth_or_stencil(format)) {
> +		const struct util_format_description *d = util_format_description(format);
> +
> +		for (int i = 0; i < d->nr_channels; ++i) {
> +			int cs = d->channel[i].size;
> +			if (d->channel[i].type == UTIL_FORMAT_TYPE_SIGNED)
> +				out->f[i] = (double)(in->i[i]) / ((1ul << (cs - 1)) - 1 );
> +			else if (d->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED)
> +				out->f[i] = (double)(in->ui[i]) / ((1ul << cs) - 1 );
> +			else
> +				out->f[i] = 0;
> +		}
> +
> +	} else {
> +		switch (format) {
> +      case PIPE_FORMAT_X24S8_UINT:
> +      case PIPE_FORMAT_X32_S8X24_UINT:
whitespace.

Looks alright to me (though as mentioned, I consider trying to fix up
border color a futile attempt on eg :-)).
btw I suppose it would apply to pre-eg chips as well? Not that I'd
suggest you fix it up there too ;-).

Reviewed-by: Roland Scheidegger <sroland at vmware.com>

> +			out->f[0] = (double)(in->ui[0]) / 255.0;
> +			out->f[1] = out->f[2] = out->f[3] = 0.0f;
> +			break;
> +		default:
> +			memcpy(out->f, in->f, 4 * sizeof(float));
> +		}
> +	}
> +}
> +
>  static void evergreen_emit_sampler_states(struct r600_context *rctx,
>  				struct r600_textures_info *texinfo,
>  				unsigned resource_id_base,
> @@ -2410,6 +2441,8 @@ static void evergreen_emit_sampler_states(struct r600_context *rctx,
>  {
>  	struct radeon_cmdbuf *cs = rctx->b.gfx.cs;
>  	uint32_t dirty_mask = texinfo->states.dirty_mask;
> +	union pipe_color_union border_color = {{0,0,0,1}};
> +	union pipe_color_union *border_color_ptr = &border_color;
>  
>  	while (dirty_mask) {
>  		struct r600_pipe_sampler_state *rstate;
> @@ -2418,6 +2451,16 @@ static void evergreen_emit_sampler_states(struct r600_context *rctx,
>  		rstate = texinfo->states.states[i];
>  		assert(rstate);
>  
> +		if (rstate->border_color_use) {
> +			struct r600_pipe_sampler_view	*rview = texinfo->views.views[i];
> +			if (rview) {
> +				evergreen_convert_border_color(&rstate->border_color,
> +				                               &border_color, rview->base.format);
> +			} else {
> +				border_color_ptr = &rstate->border_color;
> +			}
> +		}
> +
>  		radeon_emit(cs, PKT3(PKT3_SET_SAMPLER, 3, 0) | pkt_flags);
>  		radeon_emit(cs, (resource_id_base + i) * 3);
>  		radeon_emit_array(cs, rstate->tex_sampler_words, 3);
> @@ -2425,7 +2468,7 @@ static void evergreen_emit_sampler_states(struct r600_context *rctx,
>  		if (rstate->border_color_use) {
>  			radeon_set_config_reg_seq(cs, border_index_reg, 5);
>  			radeon_emit(cs, i);
> -			radeon_emit_array(cs, rstate->border_color.ui, 4);
> +			radeon_emit_array(cs, border_color_ptr->ui, 4);
>  		}
>  	}
>  	texinfo->states.dirty_mask = 0;
>