Jason Ekstrand <ja...@jlekstrand.net> writes: > On Aug 19, 2015 2:56 AM, "Francisco Jerez" <curroje...@riseup.net> wrote: >> >> Jason Ekstrand <ja...@jlekstrand.net> writes: >> >> > Previously, setting up image uniforms relied on being called after >> > fs_visitor::uniforms was set and with fs_visitor::uniforms not > allocating >> > space for it. This made sense in an ir_visitor world because the > visitor >> > assigns locations and uploads data as it walks through the variables. > In >> > NIR it also happened to work because nir_lower_io assumed zero space for >> > images. In the near future, we will be able to reserve space using >> > nir_lower_io and these invariants will be broken. >> > >> > This commit makes setup_image_uniforms take a pointer to the location in >> > the prog_data params array that indicates where the image uniforms > should >> > be stored. This way it can be called from anywhere in the shader setup. >> > >> > v2: Fix nir_setup_uniform to correctly correspond with this change >> > (rebase fail) >> > >> > Cc: Francisco Jerez <curroje...@riseup.net> >> > Cc: Kenneth Graunke <kenn...@whitecape.org> >> > --- >> > src/mesa/drivers/dri/i965/brw_fs.cpp | 12 ------------ >> > src/mesa/drivers/dri/i965/brw_fs.h | 3 --- >> > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 ++++-- >> > src/mesa/drivers/dri/i965/brw_shader.cpp | 28 > +++++++++++++++++++++------- >> > src/mesa/drivers/dri/i965/brw_shader.h | 5 ++--- >> > 5 files changed, 27 insertions(+), 27 deletions(-) >> > >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp > b/src/mesa/drivers/dri/i965/brw_fs.cpp >> > index 82cb499..d1550e6 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >> > @@ -942,18 +942,6 @@ fs_visitor::import_uniforms(fs_visitor *v) >> > this->param_size = v->param_size; >> > } >> > >> > -void >> > -fs_visitor::setup_vector_uniform_values(const gl_constant_value > *values, unsigned n) >> > -{ >> > - static const gl_constant_value zero = { 0 }; >> > - >> > - for (unsigned i = 0; i < n; ++i) >> > - stage_prog_data->param[uniforms++] = &values[i]; >> > - >> > - for (unsigned i = n; i < 4; ++i) >> > - stage_prog_data->param[uniforms++] = &zero; >> > -} >> > - >> > fs_reg * >> > fs_visitor::emit_fragcoord_interpolation(bool pixel_center_integer, >> > bool origin_upper_left) >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h > b/src/mesa/drivers/dri/i965/brw_fs.h >> > index 975183e..28fcfa3 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_fs.h >> > +++ b/src/mesa/drivers/dri/i965/brw_fs.h >> > @@ -291,9 +291,6 @@ public: >> > >> > struct brw_reg interp_reg(int location, int channel); >> > >> > - virtual void setup_vector_uniform_values(const gl_constant_value > *values, >> > - unsigned n); >> > - >> > int implied_mrf_writes(fs_inst *inst); >> > >> > virtual void dump_instructions(); >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> > index ce4153d..c8ea649 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> > @@ -244,10 +244,12 @@ fs_visitor::nir_setup_uniform(nir_variable *var) >> > * space for them here at the end of the parameter array. >> > */ >> > var->data.driver_location = uniforms; >> > - param_size[uniforms] = >> > + unsigned size = >> > BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1); >> > >> > - setup_image_uniform_values(storage); >> > + setup_image_uniform_values(stage_prog_data->param + uniforms, > storage); >> > + param_size[uniforms] = size; >> > + uniforms += size; >> > } 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 6b92806..b31ae9b 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp >> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp >> > @@ -1419,8 +1419,22 @@ > 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_vector_uniform_values(const gl_constant_value ***dst, >> > + const gl_constant_value *values, unsigned > n) >> > +{ >> > + static const gl_constant_value zero = { 0 }; >> > + >> > + for (unsigned i = 0; i < n; ++i) >> > + *((*dst)++) = &values[i]; >> > + >> > + for (unsigned i = n; i < 4; ++i) >> > + *((*dst)++) = &zero; >> > +} >> > + >> > void >> > -backend_shader::setup_image_uniform_values(const gl_uniform_storage > *storage) >> > +backend_shader::setup_image_uniform_values(const gl_constant_value > **prog_param, >> > + const gl_uniform_storage > *storage) >> > { >> > const unsigned stage = > _mesa_program_enum_to_shader_stage(prog->Target); >> > >> > @@ -1431,17 +1445,17 @@ > backend_shader::setup_image_uniform_values(const gl_uniform_storage > *storage) >> > /* Upload the brw_image_param structure. The order is expected > to match >> > * the BRW_IMAGE_PARAM_*_OFFSET defines. >> > */ >> > - setup_vector_uniform_values( >> > + setup_vector_uniform_values(&prog_param, >> > (const gl_constant_value *)¶m->surface_idx, 1); >> > - setup_vector_uniform_values( >> > + setup_vector_uniform_values(&prog_param, >> > (const gl_constant_value *)param->offset, 2); >> > - setup_vector_uniform_values( >> > + setup_vector_uniform_values(&prog_param, >> > (const gl_constant_value *)param->size, 3); >> > - setup_vector_uniform_values( >> > + setup_vector_uniform_values(&prog_param, >> > (const gl_constant_value *)param->stride, 4); >> > - setup_vector_uniform_values( >> > + setup_vector_uniform_values(&prog_param, >> > (const gl_constant_value *)param->tiling, 3); >> > - setup_vector_uniform_values( >> > + setup_vector_uniform_values(&prog_param, >> > (const gl_constant_value *)param->swizzling, 2); >> > >> > brw_mark_surface_used( >> > diff --git a/src/mesa/drivers/dri/i965/brw_shader.h > b/src/mesa/drivers/dri/i965/brw_shader.h >> > index 2cc97f2..2cd0a5a 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_shader.h >> > +++ b/src/mesa/drivers/dri/i965/brw_shader.h >> > @@ -270,9 +270,8 @@ public: >> > >> > virtual void invalidate_live_intervals() = 0; >> > >> > - virtual void setup_vector_uniform_values(const gl_constant_value > *values, >> > - unsigned n) = 0; >> > - void setup_image_uniform_values(const gl_uniform_storage *storage); >> > + void setup_image_uniform_values(const gl_constant_value > **prog_param, >> > + const gl_uniform_storage *storage); >> > }; >> > >> > uint32_t brw_texture_offset(int *offsets, unsigned num_components); >> > -- >> > 2.4.3 >> >> Did you notice that this makes >> backend_shader::setup_image_uniform_values() FS-specific? I think >> passing the base uniform index where it should be set up would do what >> you want and at the same time could be implemented in a backend-agnostic >> way. > > How is this FS-specific? setup_vector_uniform_values was exactly the same > code in both backends and the same as this static helper. They both have an > array of params. Am I missing something?
The VEC4 back-end additionally keeps track of the uniform vector sizes in the uniform_vector_size[] array.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev