[Mesa-dev] [PATCH 03/10] i965/fs: Refactor rectangle/GL_CLAMP texture coordinate adjustment.

[Kenneth Graunke]

On 09/22/2012 02:04 PM, Eric Anholt wrote:
> We'll want to reuse this for ARB_fp handling.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.h           |    2 ++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   44 +++++++++++++++-----------
>  2 files changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index e0dd720..5134857 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -307,6 +307,8 @@ public:
>     void emit_interpolation_setup_gen4();
>     void emit_interpolation_setup_gen6();
>     fs_reg emit_texcoord(ir_texture *ir, int sampler, int texunit);
> +   fs_reg rescale_texcoord(ir_texture *ir, fs_reg coordinate,
> +                           bool is_rect, int sampler, int texunit);
>     fs_inst *emit_texture_gen4(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>  			      fs_reg shadow_comp, fs_reg lod, fs_reg lod2);
>     fs_inst *emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index da09538..5bd7238 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -1159,32 +1159,19 @@ fs_visitor::emit_texture_gen7(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>     return inst;
>  }
>  
> -/**
> - * Emit code to produce the coordinates for a texture lookup.
> - *
> - * Returns the fs_reg containing the texture coordinate (as opposed to
> - * setting this->result).
> - */
>  fs_reg
> -fs_visitor::emit_texcoord(ir_texture *ir, int sampler, int texunit)
> +fs_visitor::rescale_texcoord(ir_texture *ir, fs_reg coordinate,
> +                             bool is_rect, int sampler, int texunit)
>  {

Do you actually want to pass an ir_texture* here?  All you actually need
is to pass ir->coordinate->type, which might be a little cleaner for
your new code.  Not sure.

>     fs_inst *inst = NULL;
> -
> -   if (!ir->coordinate)
> -      return fs_reg(); /* Return the default BAD_FILE register. */
> -
> -   ir->coordinate->accept(this);
> -   fs_reg coordinate = this->result;
> -
>     bool needs_gl_clamp = true;
> -
>     fs_reg scale_x, scale_y;
>  
>     /* The 965 requires the EU to do the normalization of GL rectangle
>      * texture coordinates.  We use the program parameter state
>      * tracking to get the scaling factor.
>      */
> -   if (ir->sampler->type->sampler_dimensionality == GLSL_SAMPLER_DIM_RECT &&
> +   if (is_rect &&
>         (intel->gen < 6 ||
>  	(intel->gen >= 6 && (c->key.tex.gl_clamp_mask[0] & (1 << sampler) ||
>  			     c->key.tex.gl_clamp_mask[1] & (1 << sampler))))) {
> @@ -1220,8 +1207,7 @@ fs_visitor::emit_texcoord(ir_texture *ir, int sampler, int texunit)
>      * texture coordinates.  We use the program parameter state
>      * tracking to get the scaling factor.
>      */
> -   if (intel->gen < 6 &&
> -       ir->sampler->type->sampler_dimensionality == GLSL_SAMPLER_DIM_RECT) {
> +   if (intel->gen < 6 && is_rect) {
>        fs_reg dst = fs_reg(this, ir->coordinate->type);
>        fs_reg src = coordinate;
>        coordinate = dst;
> @@ -1230,7 +1216,7 @@ fs_visitor::emit_texcoord(ir_texture *ir, int sampler, int texunit)
>        dst.reg_offset++;
>        src.reg_offset++;
>        emit(BRW_OPCODE_MUL, dst, src, scale_y);
> -   } else if (ir->sampler->type->sampler_dimensionality == GLSL_SAMPLER_DIM_RECT) {
> +   } else if (is_rect) {
>        /* On gen6+, the sampler handles the rectangle coordinates
>         * natively, without needing rescaling.  But that means we have
>         * to do GL_CLAMP clamping at the [0, width], [0, height] scale,
> @@ -1277,6 +1263,26 @@ fs_visitor::emit_texcoord(ir_texture *ir, int sampler, int texunit)
>     return coordinate;
>  }
>  
> +/**
> + * Emit code to produce the coordinates for a texture lookup.
> + *
> + * Returns the fs_reg containing the texture coordinate (as opposed to
> + * setting this->result).
> + */
> +fs_reg
> +fs_visitor::emit_texcoord(ir_texture *ir, int sampler, int texunit)
> +{
> +   if (!ir->coordinate)
> +      return fs_reg(); /* Return the default BAD_FILE register. */
> +
> +   ir->coordinate->accept(this);
> +
> +   return rescale_texcoord(ir, this->result,
> +                           ir->sampler->type->sampler_dimensionality ==
> +                           GLSL_SAMPLER_DIM_RECT,
> +                           sampler, texunit);
> +}
> +
>  void
>  fs_visitor::visit(ir_texture *ir)
>  {
> 

The rectangle/GL_CLAMP coordinate adjustment was the entire reason I
refactored this code into emit_texcoord(), and I feel like
emit_texcoord() is rather pointless after your change.  Can we just fold
it back into visit()?