I've sent a smaller fix for this bug, will save this change for an upcoming AoA patch series.
On Wed, 2015-06-17 at 22:24 +1000, Timothy Arceri wrote: > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
