I've created a new piglit test to confirm this fixes a bug in _mesa_sampler_uniforms_pipeline_are_valid()
http://lists.freedesktop.org/archives/piglit/2015-June/016270.html I'll update the commit message to: "Previously only the type of a single array element was stored. _mesa_sampler_uniforms_pipeline_are_valid() was expecting to get the array type so this fixes a bug with validating arrays of samplers in SSO. This change will also useful for implementing arrays of arrays support in glGetUniformLocation()." I guess this could also be a stable candidate. On Mon, 2015-06-08 at 18:58 +1000, Timothy Arceri wrote: > Previously only the type of a single array element > was stored. > > _mesa_sampler_uniforms_pipeline_are_valid() was expecting to get the array > type so this probably fixes a bug there. > However the main reason for doing this is to use the array type for > implementing arrays of arrays in glGetUniformLocation() in an upcoming > patch series. > --- > src/glsl/ir_uniform.h | 5 +- > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +- > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 +- > src/mesa/main/shader_query.cpp | 2 +- > src/mesa/main/uniform_query.cpp | 64 > ++++++++++++++------------ > src/mesa/program/ir_to_mesa.cpp | 7 +-- > 6 files changed, 46 insertions(+), 40 deletions(-) > > diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h > index e1b8014..07dd3c3 100644 > --- a/src/glsl/ir_uniform.h > +++ b/src/glsl/ir_uniform.h > @@ -91,9 +91,8 @@ struct gl_opaque_uniform_index { > > struct gl_uniform_storage { > char *name; > - /** Type of this uniform data stored. > - * > - * In the case of an array, it's the type of a single array element. > + /** > + * Type of this uniform > */ > const struct glsl_type *type; > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > index 5d3501c..6b669c2 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > @@ -220,6 +220,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var) > unsigned index = var->data.driver_location; > for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) { > struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u]; > + const glsl_type *type = storage->type->without_array(); > > if (storage->builtin) > continue; > @@ -231,7 +232,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var) > continue; > } > > - unsigned slots = storage->type->component_slots(); > + unsigned slots = type->component_slots(); > if (storage->array_elements) > slots *= storage->array_elements; > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > index 242d007..e5874ca 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > @@ -685,6 +685,7 @@ vec4_visitor::setup_uniform_values(ir_variable *ir) > */ > for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) { > struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u]; > + const glsl_type *type = storage->type->without_array(); > > if (storage->builtin) > continue; > @@ -698,11 +699,11 @@ vec4_visitor::setup_uniform_values(ir_variable *ir) > > gl_constant_value *components = storage->storage; > unsigned vector_count = (MAX2(storage->array_elements, 1) * > - storage->type->matrix_columns); > + type->matrix_columns); > > for (unsigned s = 0; s < vector_count; s++) { > assert(uniforms < uniform_array_size); > - uniform_vector_size[uniforms] = storage->type->vector_elements; > + uniform_vector_size[uniforms] = type->vector_elements; > > int i; > for (i = 0; i < uniform_vector_size[uniforms]; i++) { > diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp > index a6246a3..807a95c 100644 > --- a/src/mesa/main/shader_query.cpp > +++ b/src/mesa/main/shader_query.cpp > @@ -953,7 +953,7 @@ _mesa_program_resource_prop(struct gl_shader_program > *shProg, > case GL_TYPE: > switch (res->Type) { > case GL_UNIFORM: > - *val = RESOURCE_UNI(res)->type->gl_type; > + *val = RESOURCE_UNI(res)->type->without_array()->gl_type; > return 1; > case GL_PROGRAM_INPUT: > case GL_PROGRAM_OUTPUT: > diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp > index cab5083..c8b0b58 100644 > --- a/src/mesa/main/uniform_query.cpp > +++ b/src/mesa/main/uniform_query.cpp > @@ -320,9 +320,10 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint > program, GLint location, > return; > } > > + const glsl_type *uni_type = uni->type->without_array(); > { > - unsigned elements = (uni->type->is_sampler()) > - ? 1 : uni->type->components(); > + unsigned elements = (uni_type->is_sampler()) > + ? 1 : uni_type->components(); > > /* Calculate the source base address *BEFORE* modifying elements to > * account for the size of the user's buffer. > @@ -348,14 +349,14 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint > program, GLint location, > * just memcpy the data. If the types are not compatible, perform a > * slower convert-and-copy process. > */ > - if (returnType == uni->type->base_type > + if (returnType == uni_type->base_type > || ((returnType == GLSL_TYPE_INT > || returnType == GLSL_TYPE_UINT) > && > - (uni->type->base_type == GLSL_TYPE_INT > - || uni->type->base_type == GLSL_TYPE_UINT > - || uni->type->base_type == GLSL_TYPE_SAMPLER > - || uni->type->base_type == GLSL_TYPE_IMAGE))) { > + (uni_type->base_type == GLSL_TYPE_INT > + || uni_type->base_type == GLSL_TYPE_UINT > + || uni_type->base_type == GLSL_TYPE_SAMPLER > + || uni_type->base_type == GLSL_TYPE_IMAGE))) { > memcpy(paramsOut, src, bytes); > } else { > union gl_constant_value *const dst = > @@ -368,7 +369,7 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, > GLint location, > for (unsigned i = 0; i < elements; i++) { > switch (returnType) { > case GLSL_TYPE_FLOAT: > - switch (uni->type->base_type) { > + switch (uni_type->base_type) { > case GLSL_TYPE_UINT: > dst[i].f = (float) src[i].u; > break; > @@ -388,7 +389,7 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, > GLint location, > > case GLSL_TYPE_INT: > case GLSL_TYPE_UINT: > - switch (uni->type->base_type) { > + switch (uni_type->base_type) { > case GLSL_TYPE_FLOAT: > /* While the GL 3.2 core spec doesn't explicitly > * state how conversion of float uniforms to integer > @@ -519,9 +520,10 @@ _mesa_propagate_uniforms_to_driver_storage(struct > gl_uniform_storage *uni, > > /* vector_elements and matrix_columns can be 0 for samplers. > */ > - const unsigned components = MAX2(1, uni->type->vector_elements); > - const unsigned vectors = MAX2(1, uni->type->matrix_columns); > - const int dmul = uni->type->base_type == GLSL_TYPE_DOUBLE ? 2 : 1; > + const glsl_type *uni_type = uni->type->without_array(); > + const unsigned components = MAX2(1, uni_type->vector_elements); > + const unsigned vectors = MAX2(1, uni_type->matrix_columns); > + const int dmul = uni_type->base_type == GLSL_TYPE_DOUBLE ? 2 : 1; > > /* Store the data in the driver's requested type in the driver's storage > * areas. > @@ -649,7 +651,8 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > if (uni == NULL) > return; > > - if (uni->type->is_matrix()) { > + const glsl_type *uni_type = uni->type->without_array(); > + if (uni_type->is_matrix()) { > /* Can't set matrix uniforms (like mat4) with glUniform */ > _mesa_error(ctx, GL_INVALID_OPERATION, > "glUniform%u(uniform \"%s\"@%d is matrix)", > @@ -659,8 +662,8 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > > /* Verify that the types are compatible. > */ > - const unsigned components = uni->type->is_sampler() > - ? 1 : uni->type->vector_elements; > + const unsigned components = uni_type->is_sampler() > + ? 1 : uni_type->vector_elements; > > if (components != src_components) { > /* glUniformN() must match float/vecN type */ > @@ -672,7 +675,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > } > > bool match; > - switch (uni->type->base_type) { > + switch (uni_type->base_type) { > case GLSL_TYPE_BOOL: > match = (basicType != GLSL_TYPE_DOUBLE); > break; > @@ -681,7 +684,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > match = (basicType == GLSL_TYPE_INT); > break; > default: > - match = (basicType == uni->type->base_type); > + match = (basicType == uni_type->base_type); > break; > } > > @@ -689,7 +692,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > _mesa_error(ctx, GL_INVALID_OPERATION, > "glUniform%u(\"%s\"@%d is %s, not %s)", > src_components, uni->name, location, > - glsl_type_name(uni->type->base_type), > + glsl_type_name(uni_type->base_type), > glsl_type_name(basicType)); > return; > } > @@ -716,7 +719,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > * Based on that, when an invalid sampler is specified, we generate a > * GL_INVALID_VALUE error and ignore the command. > */ > - if (uni->type->is_sampler()) { > + if (uni_type->is_sampler()) { > for (int i = 0; i < count; i++) { > const unsigned texUnit = ((unsigned *) values)[i]; > > @@ -731,7 +734,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > } > } > > - if (uni->type->is_image()) { > + if (uni_type->is_image()) { > for (int i = 0; i < count; i++) { > const int unit = ((GLint *) values)[i]; > > @@ -764,7 +767,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > > /* Store the data in the "actual type" backing storage for the uniform. > */ > - if (!uni->type->is_boolean()) { > + if (!uni_type->is_boolean()) { > memcpy(&uni->storage[size_mul * components * offset], values, > sizeof(uni->storage[0]) * components * count * size_mul); > } else { > @@ -789,7 +792,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > /* If the uniform is a sampler, do the extra magic necessary to propagate > * the changes through. > */ > - if (uni->type->is_sampler()) { > + if (uni_type->is_sampler()) { > bool flushed = false; > for (int i = 0; i < MESA_SHADER_STAGES; i++) { > struct gl_shader *const sh = shProg->_LinkedShaders[i]; > @@ -840,7 +843,7 @@ _mesa_uniform(struct gl_context *ctx, struct > gl_shader_program *shProg, > /* If the uniform is an image, update the mapping from image > * uniforms to image units present in the shader data structure. > */ > - if (uni->type->is_image()) { > + if (uni_type->is_image()) { > for (int i = 0; i < MESA_SHADER_STAGES; i++) { > if (uni->image[i].active) { > struct gl_shader *sh = shProg->_LinkedShaders[i]; > @@ -874,10 +877,12 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct > gl_shader_program *shProg, > struct gl_uniform_storage *const uni = > validate_uniform_parameters(ctx, shProg, location, count, > &offset, "glUniformMatrix"); > + > if (uni == NULL) > return; > > - if (!uni->type->is_matrix()) { > + const glsl_type *uni_type = uni->type->without_array(); > + if (!uni_type->is_matrix()) { > _mesa_error(ctx, GL_INVALID_OPERATION, > "glUniformMatrix(non-matrix uniform)"); > return; > @@ -886,9 +891,9 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct > gl_shader_program *shProg, > assert(type == GL_FLOAT || type == GL_DOUBLE); > size_mul = type == GL_DOUBLE ? 2 : 1; > > - assert(!uni->type->is_sampler()); > - vectors = uni->type->matrix_columns; > - components = uni->type->vector_elements; > + assert(!uni_type->is_sampler()); > + vectors = uni_type->matrix_columns; > + components = uni_type->vector_elements; > > /* Verify that the types are compatible. This is greatly simplified for > * matrices because they can only have a float base type. > @@ -911,7 +916,7 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct > gl_shader_program *shProg, > } > > if (unlikely(ctx->_Shader->Flags & GLSL_UNIFORMS)) { > - log_uniform(values, uni->type->base_type, components, vectors, count, > + log_uniform(values, uni_type->base_type, components, vectors, count, > bool(transpose), shProg, location, uni); > } > > @@ -1101,8 +1106,7 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct > gl_pipeline_object *pipeline) > for (unsigned i = 0; i < shProg[idx]->NumUniformStorage; i++) { > const struct gl_uniform_storage *const storage = > &shProg[idx]->UniformStorage[i]; > - const glsl_type *const t = (storage->type->is_array()) > - ? storage->type->fields.array : storage->type; > + const glsl_type *const t = storage->type->without_array(); > > if (!t->is_sampler()) > continue; > diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp > index 514bb93..582d714 100644 > --- a/src/mesa/program/ir_to_mesa.cpp > +++ b/src/mesa/program/ir_to_mesa.cpp > @@ -2417,8 +2417,9 @@ _mesa_associate_uniform_storage(struct gl_context *ctx, > enum gl_uniform_driver_format format = uniform_native; > > unsigned columns = 0; > + const glsl_type *type = storage->type->without_array(); > int dmul = 4 * sizeof(float); > - switch (storage->type->base_type) { > + switch (type->base_type) { > case GLSL_TYPE_UINT: > assert(ctx->Const.NativeIntegers); > format = uniform_native; > @@ -2431,12 +2432,12 @@ _mesa_associate_uniform_storage(struct gl_context > *ctx, > break; > > case GLSL_TYPE_DOUBLE: > - if (storage->type->vector_elements > 2) > + if (type->vector_elements > 2) > dmul *= 2; > /* fallthrough */ > case GLSL_TYPE_FLOAT: > format = uniform_native; > - columns = storage->type->matrix_columns; > + columns = type->matrix_columns; > break; > case GLSL_TYPE_BOOL: > format = uniform_native; _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
