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 <ito...@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 = _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev