[Mesa-dev] [PATCH] mesa: store full array type in gl_uniform_storage
Timothy Arceri
t_arceri at yahoo.com.au
Wed Jun 17 15:25:39 PDT 2015
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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list