On Wed, 2015-09-30 at 18:41 -0700, Jason Ekstrand wrote: > I tried to do this once before but Curro pointed out that having it in > backend_shader meant it could use the setup_vec4_uniform_values helper > which did different things in vec4 and fs. Now the setup_uniform_values > function differs only by an assert in the two backends so there's no real > good reason to be using it anymore. > --- > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +- > src/mesa/drivers/dri/i965/brw_shader.cpp | 52 > +++++++++++++++++++++----------- > src/mesa/drivers/dri/i965/brw_shader.h | 7 +++-- > 3 files changed, 42 insertions(+), 20 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > index 7a965cd..d33e452 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > @@ -236,7 +236,8 @@ fs_visitor::nir_setup_uniform(nir_variable *var) > } > > if (storage->type->is_image()) { > - setup_image_uniform_values(index, storage); > + brw_setup_image_uniform_values(stage, stage_prog_data, > + index, storage); > } else { > unsigned slots = storage->type->component_slots(); > if (storage->array_elements) > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp > b/src/mesa/drivers/dri/i965/brw_shader.cpp > index 72388ce..1d184a7 100644 > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp > @@ -1419,32 +1419,50 @@ > backend_shader::assign_common_binding_table_offsets(uint32_t > next_binding_table_ > /* prog_data->base.binding_table.size will be set by > brw_mark_surface_used. */ > } > > +static void > +setup_vec4_uniform_value(const gl_constant_value **params, > + const gl_constant_value *values, > + unsigned n) > +{ > + static const gl_constant_value zero = { 0 }; > + > + for (unsigned i = 0; i < n; ++i) > + params[i] = &values[i]; > + > + for (unsigned i = n; i < 4; ++i) > + params[i] = &zero; > +}
I actually liked the version that received an offset into params better, since that allows us to assert that we are not messing our writes to params. Not a big deal though, so either way: Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> > void > -backend_shader::setup_image_uniform_values(unsigned param_offset, > - const gl_uniform_storage *storage) > +brw_setup_image_uniform_values(gl_shader_stage stage, > + struct brw_stage_prog_data *stage_prog_data, > + unsigned param_start_index, > + const gl_uniform_storage *storage) > { > - const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target); > + const gl_constant_value **param = > + &stage_prog_data->param[param_start_index]; > > for (unsigned i = 0; i < MAX2(storage->array_elements, 1); i++) { > const unsigned image_idx = storage->image[stage].index + i; > - const brw_image_param *param = > &stage_prog_data->image_param[image_idx]; > + const brw_image_param *image_param = > + &stage_prog_data->image_param[image_idx]; > > /* Upload the brw_image_param structure. The order is expected to > match > * the BRW_IMAGE_PARAM_*_OFFSET defines. > */ > - setup_vec4_uniform_value(param_offset + > BRW_IMAGE_PARAM_SURFACE_IDX_OFFSET, > - (const gl_constant_value *)¶m->surface_idx, 1); > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_OFFSET_OFFSET, > - (const gl_constant_value *)param->offset, 2); > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_SIZE_OFFSET, > - (const gl_constant_value *)param->size, 3); > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_STRIDE_OFFSET, > - (const gl_constant_value *)param->stride, 4); > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_TILING_OFFSET, > - (const gl_constant_value *)param->tiling, 3); > - setup_vec4_uniform_value(param_offset + > BRW_IMAGE_PARAM_SWIZZLING_OFFSET, > - (const gl_constant_value *)param->swizzling, 2); > - param_offset += BRW_IMAGE_PARAM_SIZE; > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_SURFACE_IDX_OFFSET, > + (const gl_constant_value *)&image_param->surface_idx, 1); > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_OFFSET_OFFSET, > + (const gl_constant_value *)image_param->offset, 2); > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_SIZE_OFFSET, > + (const gl_constant_value *)image_param->size, 3); > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_STRIDE_OFFSET, > + (const gl_constant_value *)image_param->stride, 4); > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_TILING_OFFSET, > + (const gl_constant_value *)image_param->tiling, 3); > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_SWIZZLING_OFFSET, > + (const gl_constant_value *)image_param->swizzling, 2); > + param += BRW_IMAGE_PARAM_SIZE; > > brw_mark_surface_used( > stage_prog_data, > diff --git a/src/mesa/drivers/dri/i965/brw_shader.h > b/src/mesa/drivers/dri/i965/brw_shader.h > index ccccf4d..ee6afce 100644 > --- a/src/mesa/drivers/dri/i965/brw_shader.h > +++ b/src/mesa/drivers/dri/i965/brw_shader.h > @@ -273,12 +273,15 @@ public: > virtual void setup_vec4_uniform_value(unsigned param_offset, > const gl_constant_value *values, > unsigned n) = 0; > - void setup_image_uniform_values(unsigned param_offset, > - const gl_uniform_storage *storage); > }; > > uint32_t brw_texture_offset(int *offsets, unsigned num_components); > > +void brw_setup_image_uniform_values(gl_shader_stage stage, > + struct brw_stage_prog_data > *stage_prog_data, > + unsigned param_start_index, > + const gl_uniform_storage *storage); > + > #endif /* __cplusplus */ > > enum brw_reg_type brw_type_for_base_type(const struct glsl_type *type); _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev