[Mesa-dev] [PATCH] mesa: store full array type in gl_uniform_storage
Timothy Arceri
t_arceri at yahoo.com.au
Wed Jun 17 05:24:22 PDT 2015
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;
More information about the mesa-dev
mailing list