[Mesa-dev] [PATCH 3/3] r600: Scale interger valued texture border colors to float

[Roland Scheidegger]

Am 01.07.2018 um 19:32 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. For some reason 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.
> 
> 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*
> Signed-off-by: Gert Wollny <gw.fossdev at gmail.com>
> ---
>  src/gallium/drivers/r600/evergreen_state.c | 66 ++++++++++++++++++++++++++++--
>  1 file changed, 63 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index b6a01622e4..34ebc73af5 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
> @@ -2405,14 +2405,67 @@ 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);
>  }
>  
> +/* It seems that the border color is passed in the texture format, but
> + * the hardware interprets the color always as signed or unsigned floating
> + * point value (depending on the sign of texture values). So convert the
> + * color values here. This works for channel sizes < 32 bit. It seems, however,
> + * that for 32 bit component integer textures the border color is ignored and
> + * always set to zero.
> + */
> +static void evergreen_convert_border_color(union pipe_color_union *in,
I have given up on fixing border colors long ago (for instance it's
completely FUBAR if texture swizzling is used and cannot be fixed up in
this case due to the essentially random mapping of channels, which
includes assigning the same channel of border color to multiple channels
of the results wrongly depending on the exact swizzle).
In any case, it's not surprising for pure integers it wouldn't work
neither (since it's a feature not supported by d3d11, and IME what's not
supported by d3d11 near certainly is broken if it's supposed to even
work in the first place on eg).

+														 union pipe_color_union *out,
> +														 enum pipe_format format)
> +{
> +	const struct util_format_description *d;
> +	bool handle_separate = false;
> +
> +	if (util_format_is_float(format)) {
> +		memcpy(out->f, in->f, 4 * sizeof(float));
> +		return;
> +	}
> +
> +	d = util_format_description(format);
> +	for (int i = 0; i < d->nr_channels && !handle_separate; ++i) {
> +
> +		if (d->channel[i].pure_integer) {
> +			int cs = d->channel[i].size;
> +			if (d->channel[i].type == UTIL_FORMAT_TYPE_SIGNED)
> +            out->f[i] = (float)((double)(in->i[i]) / ((1ul << (cs - 1)) - 1));
> +			else if (d->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED)
> +				out->f[i] = (float)((double)(in->ui[i]) / ((1ul << cs) - 1));
> +
> +		} else if (d->channel[i].type != UTIL_FORMAT_TYPE_VOID || i == 0) {
> +			handle_separate = true;
> +      } else {
whitespace looks off.


> +			/* Assume this is a X channel and just clear border color component */
> +			out->f[i] = 0;
> +      }
> +	}
> +
> +	if (handle_separate)  {
> +		switch (format) {
> +      case PIPE_FORMAT_X24S8_UINT:
> +		case PIPE_FORMAT_X32_S8X24_UINT:
whitespace looks off again.

I'm thinking all the logic could be simplified if you'd assume there
can't be formats with mixed pure integers and without, since only the
formats with pure ints needs workarounds (as written it actually looks
like it would change things for all other non-float formats too, which
isn't actually true).
The only exception are the d/s formats (and even then they shouldn't
have both pure and non-pure values in there, only voids can be mixed
with both).

Doesn't that result in piglit result changes? (Although there's lot of
failures in there due to texture swizzling)

Roland



> +			out->f[0] = (float)(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,
> +														struct r600_textures_info *texinfo,
>  				unsigned resource_id_base,
>  				unsigned border_index_reg,
>  				unsigned pkt_flags)
>  {
>  	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;
> @@ -2432,7 +2485,7 @@ static void evergreen_emit_sampler_states(struct r600_context *rctx,
>  		 */
>  		struct r600_pipe_sampler_view	*rview = texinfo->views.views[i];
>  		if (rview) {
> -			rstate->tex_sampler_words[0] &= C_03C000_Z_FILTER;
> +                        rstate->tex_sampler_words[0] &= C_03C000_Z_FILTER;
Unintentional whitespace change?

>  			enum pipe_texture_target target = rview->base.texture->target;
>  			if (target == PIPE_TEXTURE_2D_ARRAY ||
>  				target == PIPE_TEXTURE_CUBE_ARRAY ||
> @@ -2442,6 +2495,13 @@ static void evergreen_emit_sampler_states(struct r600_context *rctx,
>  			} else {
>  				rstate->tex_sampler_words[2] &= C_03C008_TRUNCATE_COORD;
>  			}
> +
> +			if (rstate->border_color_use) {
> +				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);
> @@ -2451,7 +2511,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;
>