[Mesa-dev] [PATCH v2 1/2] r600: correct texture offset for array index lookup

[Roland Scheidegger]

Am 29.06.2018 um 16:35 schrieb Gert Wollny:
> For texture array lookup the slice index is evaluated according to
>   idx = floor(z + 0.5)
> 
> This patch implements the first part by adding 0.5 to the according
> texture coordinate when appropriate.
> 
> Fixes multi-sample tests out of:
>   dEQP-GLES3.functional.texture.shadow.2d_array.*
>   dEQP-GLES3.functional.texture.vertex.2d_array.*
>   dEQP-GLES3.functional.texture.filtering.2d_array.*
> (In the multi-sample case the rounding accuracy is not tested.)
> 
> v2: - Don't apply texture offset correction for GATHER*O (corrects piglit
>       failures reported by Dave Airlie)
>     - unconditionally set the texture offset to 1 (=0.5) because the shader
>       can't set an offset for the array index (Roland Scheidegger)
>     - Add Fixes comment to commit message
> 
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
>  src/gallium/drivers/r600/r600_shader.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> index c466a48262..4d17b3d875 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -7456,6 +7456,7 @@ static int tgsi_tex(struct r600_shader_ctx *ctx)
>  	int8_t offset_x = 0, offset_y = 0, offset_z = 0;
>  	boolean has_txq_cube_array_z = false;
>  	unsigned sampler_index_mode;
> +	int *array_index_offset = NULL;
>  
>  	if (inst->Instruction.Opcode == TGSI_OPCODE_TXQ &&
>  	    ((inst->Texture.Texture == TGSI_TEXTURE_CUBE_ARRAY ||
> @@ -8411,18 +8412,33 @@ static int tgsi_tex(struct r600_shader_ctx *ctx)
>  		    opcode == FETCH_OP_SAMPLE_C_LB) {
>  			/* the array index is read from Y */
>  			tex.coord_type_y = 0;
> +			array_index_offset = &tex.offset_y;
>  		} else {
>  			/* the array index is read from Z */
>  			tex.coord_type_z = 0;
>  			tex.src_sel_z = tex.src_sel_y;
> +			array_index_offset = &tex.offset_z;
> +
>  		}
>  	} else if (inst->Texture.Texture == TGSI_TEXTURE_2D_ARRAY ||
>  		   inst->Texture.Texture == TGSI_TEXTURE_SHADOW2D_ARRAY ||
>  		   ((inst->Texture.Texture == TGSI_TEXTURE_CUBE_ARRAY ||
>  		    inst->Texture.Texture == TGSI_TEXTURE_SHADOWCUBE_ARRAY) &&
> -		    (ctx->bc->chip_class >= EVERGREEN)))
> +		    (ctx->bc->chip_class >= EVERGREEN))) {
>  		/* the array index is read from Z */
>  		tex.coord_type_z = 0;
> +		array_index_offset = &tex.offset_z;
> +	}
> +
> +	/* We have array access, the coordinates are not int and we use the
> +	 * offset registers -> add 0.5 to the array index to adjust it according
> +	 * to floor(z + 0.5). The floor opretaion is set as TRUNC in the texture
operation


So for gather4_O (where you'd need to do it differently) it must not be
done, but for gather4 it has to be done?
Still doesn't make all that much sense to me. The hw may be weird but I
don't think it would be that weird?
But anyway, as long as it doesn't regress anything,
Acked-by: Roland Scheidegger <sroland at vmware.com>


> +	 * state.
> +	 */
> +	if (array_index_offset && opcode != FETCH_OP_LD &&
> +	    opcode != FETCH_OP_GATHER4_C_O && opcode != FETCH_OP_GATHER4_O) {
> +		*array_index_offset = 1;
> +	}
>  
>  	/* mask unused source components */
>  	if (opcode == FETCH_OP_SAMPLE || opcode == FETCH_OP_GATHER4) {
>