[Mesa-dev] [PATCH 11/11] i965: Use nir_lower_tex for texture coordinate lowering

Iago Toral itoral at igalia.com
Mon Nov 16 02:33:44 PST 2015


On Wed, 2015-11-11 at 17:26 -0800, Jason Ekstrand wrote:
> Previously, we had a rescale_texcoords helper in the FS backend for
> handling rescaling of texture coordinates.  Now that we can do variants in
> NIR, we can use nir_lower_tex to do the rescaling for us.  This allows us
> to delete the i965-specific code and gives us proper TEXTURE_RECTANGLE and
> GL_CLAMP handling in vertex and geometry shaders.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp              |   2 +
>  src/mesa/drivers/dri/i965/brw_fs.h                |   3 -
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp          |   4 +-
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp      | 125 ----------------------
>  src/mesa/drivers/dri/i965/brw_nir.c               |  23 ++++
>  src/mesa/drivers/dri/i965/brw_nir.h               |   6 ++
>  src/mesa/drivers/dri/i965/brw_vec4.cpp            |   2 +
>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp |   2 +
>  8 files changed, 36 insertions(+), 131 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index b8713ab..c56cafe 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -5468,6 +5468,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
>                 char **error_str)
>  {
>     nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex, true);

This looks like it is part of the post-processing process. In fact, you
call this right before brw_postprocess_nir() for every stage. Why not
just add a key parameter to brw_postprocess_nir() and call
brw_nir_apply_sampler_key from there instead?

Either way,
Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

>     brw_postprocess_nir(shader, compiler->devinfo, true);
>  
>     /* key->alpha_test_func means simulating alpha testing via discards,
> @@ -5628,6 +5629,7 @@ brw_compile_cs(const struct brw_compiler *compiler, void *log_data,
>                 char **error_str)
>  {
>     nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex, true);
>     brw_postprocess_nir(shader, compiler->devinfo, true);
>  
>     prog_data->local_size[0] = shader->info.cs.local_size[0];
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 2dfcab1..8a181d7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -217,8 +217,6 @@ public:
>     void emit_interpolation_setup_gen4();
>     void emit_interpolation_setup_gen6();
>     void compute_sample_position(fs_reg dst, fs_reg int_sample_pos);
> -   fs_reg rescale_texcoord(fs_reg coordinate, int coord_components,
> -                           bool is_rect, uint32_t sampler);
>     void emit_texture(ir_texture_opcode op,
>                       const glsl_type *dest_type,
>                       fs_reg coordinate, int components,
> @@ -229,7 +227,6 @@ public:
>                       fs_reg mcs,
>                       int gather_component,
>                       bool is_cube_array,
> -                     bool is_rect,
>                       uint32_t sampler,
>                       fs_reg sampler_reg);
>     fs_reg emit_mcs_fetch(const fs_reg &coordinate, unsigned components,
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 02b9f5b..3d83d7c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -2411,8 +2411,6 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
>  
>     int gather_component = instr->component;
>  
> -   bool is_rect = instr->sampler_dim == GLSL_SAMPLER_DIM_RECT;
> -
>     bool is_cube_array = instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE &&
>                          instr->is_array;
>  
> @@ -2549,7 +2547,7 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
>     emit_texture(op, dest_type, coordinate, instr->coord_components,
>                  shadow_comparitor, lod, lod2, lod_components, sample_index,
>                  tex_offset, mcs, gather_component,
> -                is_cube_array, is_rect, sampler, sampler_reg);
> +                is_cube_array, sampler, sampler_reg);
>  
>     fs_reg dest = get_nir_dest(instr->dest);
>     dest.type = this->result.type;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 213c912..faf304c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -79,122 +79,6 @@ fs_visitor::emit_vs_system_value(int location)
>     return reg;
>  }
>  
> -fs_reg
> -fs_visitor::rescale_texcoord(fs_reg coordinate, int coord_components,
> -                             bool is_rect, uint32_t sampler)
> -{
> -   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 (is_rect &&
> -       (devinfo->gen < 6 ||
> -        (devinfo->gen >= 6 && (key_tex->gl_clamp_mask[0] & (1 << sampler) ||
> -                               key_tex->gl_clamp_mask[1] & (1 << sampler))))) {
> -      struct gl_program_parameter_list *params = prog->Parameters;
> -
> -
> -      /* FINISHME: We're failing to recompile our programs when the sampler is
> -       * updated.  This only matters for the texture rectangle scale
> -       * parameters (pre-gen6, or gen6+ with GL_CLAMP).
> -       */
> -      int tokens[STATE_LENGTH] = {
> -	 STATE_INTERNAL,
> -	 STATE_TEXRECT_SCALE,
> -	 prog->SamplerUnits[sampler],
> -	 0,
> -	 0
> -      };
> -
> -      no16("rectangle scale uniform setup not supported on SIMD16\n");
> -      if (dispatch_width == 16) {
> -	 return coordinate;
> -      }
> -
> -      GLuint index = _mesa_add_state_reference(params,
> -					       (gl_state_index *)tokens);
> -      /* Try to find existing copies of the texrect scale uniforms. */
> -      for (unsigned i = 0; i < uniforms; i++) {
> -         if (stage_prog_data->param[i] ==
> -             &prog->Parameters->ParameterValues[index][0]) {
> -            scale_x = fs_reg(UNIFORM, i);
> -            scale_y = fs_reg(UNIFORM, i + 1);
> -            break;
> -         }
> -      }
> -
> -      /* If we didn't already set them up, do so now. */
> -      if (scale_x.file == BAD_FILE) {
> -         scale_x = fs_reg(UNIFORM, uniforms);
> -         scale_y = fs_reg(UNIFORM, uniforms + 1);
> -
> -         stage_prog_data->param[uniforms++] =
> -            &prog->Parameters->ParameterValues[index][0];
> -         stage_prog_data->param[uniforms++] =
> -            &prog->Parameters->ParameterValues[index][1];
> -      }
> -   }
> -
> -   /* 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 (devinfo->gen < 6 && is_rect) {
> -      fs_reg dst = fs_reg(GRF, alloc.allocate(coord_components));
> -      fs_reg src = coordinate;
> -      coordinate = dst;
> -
> -      bld.MUL(dst, src, scale_x);
> -      dst = offset(dst, bld, 1);
> -      src = offset(src, bld, 1);
> -      bld.MUL(dst, src, scale_y);
> -   } 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,
> -       * not [0, 1] like the default case below.
> -       */
> -      needs_gl_clamp = false;
> -
> -      for (int i = 0; i < 2; i++) {
> -	 if (key_tex->gl_clamp_mask[i] & (1 << sampler)) {
> -	    fs_reg chan = coordinate;
> -	    chan = offset(chan, bld, i);
> -
> -            set_condmod(BRW_CONDITIONAL_GE,
> -                        bld.emit(BRW_OPCODE_SEL, chan, chan, fs_reg(0.0f)));
> -
> -	    /* Our parameter comes in as 1.0/width or 1.0/height,
> -	     * because that's what people normally want for doing
> -	     * texture rectangle handling.  We need width or height
> -	     * for clamping, but we don't care enough to make a new
> -	     * parameter type, so just invert back.
> -	     */
> -	    fs_reg limit = vgrf(glsl_type::float_type);
> -            bld.MOV(limit, i == 0 ? scale_x : scale_y);
> -            bld.emit(SHADER_OPCODE_RCP, limit, limit);
> -
> -            set_condmod(BRW_CONDITIONAL_L,
> -                        bld.emit(BRW_OPCODE_SEL, chan, chan, limit));
> -	 }
> -      }
> -   }
> -
> -   if (coord_components > 0 && needs_gl_clamp) {
> -      for (int i = 0; i < MIN2(coord_components, 3); i++) {
> -	 if (key_tex->gl_clamp_mask[i] & (1 << sampler)) {
> -	    fs_reg chan = coordinate;
> -	    chan = offset(chan, bld, i);
> -            set_saturate(true, bld.MOV(chan, chan));
> -	 }
> -      }
> -   }
> -   return coordinate;
> -}
> -
>  /* Sample from the MCS surface attached to this multisample texture. */
>  fs_reg
>  fs_visitor::emit_mcs_fetch(const fs_reg &coordinate, unsigned components,
> @@ -227,7 +111,6 @@ fs_visitor::emit_texture(ir_texture_opcode op,
>                           fs_reg mcs,
>                           int gather_component,
>                           bool is_cube_array,
> -                         bool is_rect,
>                           uint32_t sampler,
>                           fs_reg sampler_reg)
>  {
> @@ -259,14 +142,6 @@ fs_visitor::emit_texture(ir_texture_opcode op,
>        lod = fs_reg(0u);
>     }
>  
> -   if (coordinate.file != BAD_FILE) {
> -      /* FINISHME: Texture coordinate rescaling doesn't work with non-constant
> -       * samplers.  This should only be a problem with GL_CLAMP on Gen7.
> -       */
> -      coordinate = rescale_texcoord(coordinate, coord_components, is_rect,
> -                                    sampler);
> -   }
> -
>     /* Writemasking doesn't eliminate channels on SIMD8 texture
>      * samples, so don't worry about them.
>      */
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
> index 693b9cd..3999a60 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -395,6 +395,29 @@ brw_create_nir(struct brw_context *brw,
>     return nir;
>  }
>  
> +void
> +brw_nir_apply_sampler_key(nir_shader *nir,
> +                          const struct brw_device_info *devinfo,
> +                          const struct brw_sampler_prog_key_data *key_tex,
> +                          bool is_scalar)
> +{
> +   nir_lower_tex_options tex_options = { 0 };
> +
> +   /* Iron Lake and prior require lowering of all rectangle textures */
> +   if (devinfo->gen < 6)
> +      tex_options.lower_rect = true;
> +
> +   /* Prior to Broadwell, our hardware can't actually do GL_CLAMP */
> +   if (devinfo->gen < 8) {
> +      tex_options.saturate_s = key_tex->gl_clamp_mask[0];
> +      tex_options.saturate_t = key_tex->gl_clamp_mask[1];
> +      tex_options.saturate_r = key_tex->gl_clamp_mask[2];
> +   }
> +
> +   if (nir_lower_tex(nir, &tex_options))
> +      nir_optimize(nir, is_scalar);
> +}
> +
>  enum brw_reg_type
>  brw_type_for_nir_type(nir_alu_type type)
>  {
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.h b/src/mesa/drivers/dri/i965/brw_nir.h
> index 1cdfe00..a399992 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.h
> +++ b/src/mesa/drivers/dri/i965/brw_nir.h
> @@ -90,6 +90,12 @@ void brw_postprocess_nir(nir_shader *nir,
>                           const struct brw_device_info *devinfo,
>                           bool is_scalar);
>  
> +
> +void brw_nir_apply_sampler_key(nir_shader *nir,
> +                               const struct brw_device_info *devinfo,
> +                               const struct brw_sampler_prog_key_data *key,
> +                               bool is_scalar);
> +
>  enum brw_reg_type brw_type_for_nir_type(nir_alu_type type);
>  
>  enum glsl_base_type brw_glsl_base_type_for_nir_type(nir_alu_type type);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 9f75bb6..d207180 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -2036,6 +2036,8 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data,
>                 char **error_str)
>  {
>     nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex,
> +                             compiler->scalar_vs);
>     brw_postprocess_nir(shader, compiler->devinfo, compiler->scalar_vs);
>  
>     const unsigned *assembly = NULL;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> index 92b15d9..4caf54e 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> @@ -616,6 +616,8 @@ brw_compile_gs(const struct brw_compiler *compiler, void *log_data,
>     c.key = *key;
>  
>     nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex,
> +                             compiler->scalar_gs);
>     brw_postprocess_nir(shader, compiler->devinfo, compiler->scalar_gs);
>  
>     prog_data->include_primitive_id =




More information about the mesa-dev mailing list