From 602ec1e504a7cbe584bcdf27682660a0cc42ff09 Mon Sep 17 00:00:00 2001 From: Martin Peres Date: Thu, 21 May 2015 15:51:09 +0300 Subject: [PATCH] mesa: reference built-in uniforms into gl_uniform_storage This change introduces a new field in gl_uniform_storage to explicitely say that a uniform is built-in. In the case where it is, no storage is defined to make it clear that it is read-only from the mesa side. I think I fixed all the places in the code that made use of the structure that I changed. Hopefully, any place making a wrong assumption and using the storage straight away will just crash. This patch seems to implement the path of least resistance towards listing built-in uniforms in GL_ACTIVE_UNIFORM (and other APIs). Signed-off-by: Martin Peres --- src/glsl/ir_uniform.h | 5 +++ src/glsl/link_uniform_initializers.cpp | 4 +- src/glsl/link_uniforms.cpp | 53 +++++++++++------------- src/glsl/linker.cpp | 6 +-- src/glsl/standalone_scaffolding.cpp | 2 +- src/glsl/tests/set_uniform_initializer_tests.cpp | 4 +- src/mesa/drivers/dri/i965/brw_fs.cpp | 5 ++- src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 5 ++- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 ++- src/mesa/main/mtypes.h | 2 +- src/mesa/main/shaderapi.c | 4 +- src/mesa/main/shaderobj.c | 4 +- src/mesa/main/uniform_query.cpp | 15 ++++++- src/mesa/program/ir_to_mesa.cpp | 9 +++- src/mesa/state_tracker/st_draw.c | 2 +- 15 files changed, 76 insertions(+), 49 deletions(-) diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h index 21b5d05..e1b8014 100644 --- a/src/glsl/ir_uniform.h +++ b/src/glsl/ir_uniform.h @@ -181,6 +181,11 @@ struct gl_uniform_storage { * via the API. */ bool hidden; + + /** + * This is a built-in uniform that should not be modified through any gl API. + */ + bool builtin; }; #ifdef __cplusplus diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp index 6907384..204acfa 100644 --- a/src/glsl/link_uniform_initializers.cpp +++ b/src/glsl/link_uniform_initializers.cpp @@ -103,7 +103,7 @@ void set_sampler_binding(gl_shader_program *prog, const char *name, int binding) { struct gl_uniform_storage *const storage = - get_storage(prog->UniformStorage, prog->NumUserUniformStorage, name); + get_storage(prog->UniformStorage, prog->NumUniformStorage, name); if (storage == NULL) { assert(storage != NULL); @@ -193,7 +193,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, struct gl_uniform_storage *const storage = get_storage(prog->UniformStorage, - prog->NumUserUniformStorage, + prog->NumUniformStorage, name); if (storage == NULL) { assert(storage != NULL); diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp index 2c928e1..ef365b1 100644 --- a/src/glsl/link_uniforms.cpp +++ b/src/glsl/link_uniforms.cpp @@ -589,12 +589,13 @@ private: handle_samplers(base_type, &this->uniforms[id]); handle_images(base_type, &this->uniforms[id]); - /* If there is already storage associated with this uniform, it means - * that it was set while processing an earlier shader stage. For - * example, we may be processing the uniform in the fragment shader, but - * the uniform was already processed in the vertex shader. + /* If there is already storage associated with this uniform or if the + * uniform is set as builtin, it means that it was set while processing + * an earlier shader stage. For example, we may be processing the + * uniform in the fragment shader, but the uniform was already processed + * in the vertex shader. */ - if (this->uniforms[id].storage != NULL) { + if (this->uniforms[id].storage != NULL || this->uniforms[id].builtin) { return; } @@ -619,10 +620,15 @@ private: this->uniforms[id].initialized = 0; this->uniforms[id].num_driver_storage = 0; this->uniforms[id].driver_storage = NULL; - this->uniforms[id].storage = this->values; this->uniforms[id].atomic_buffer_index = -1; this->uniforms[id].hidden = current_var->data.how_declared == ir_var_hidden; + this->uniforms[id].builtin = is_gl_identifier(name); + + /* Do not assign storage if the uniform is builtin */ + if (!this->uniforms[id].builtin) + this->uniforms[id].storage = this->values; + if (this->ubo_block_index != -1) { this->uniforms[id].block_index = this->ubo_block_index; @@ -894,7 +900,7 @@ link_assign_uniform_locations(struct gl_shader_program *prog, { ralloc_free(prog->UniformStorage); prog->UniformStorage = NULL; - prog->NumUserUniformStorage = 0; + prog->NumUniformStorage = 0; if (prog->UniformHash != NULL) { prog->UniformHash->clear(); @@ -940,14 +946,6 @@ link_assign_uniform_locations(struct gl_shader_program *prog, if ((var == NULL) || (var->data.mode != ir_var_uniform)) continue; - /* FINISHME: Update code to process built-in uniforms! - */ - if (is_gl_identifier(var->name)) { - uniform_size.num_shader_uniform_components += - var->type->component_slots(); - continue; - } - uniform_size.process(var); } @@ -962,16 +960,16 @@ link_assign_uniform_locations(struct gl_shader_program *prog, } } - const unsigned num_user_uniforms = uniform_size.num_active_uniforms; + const unsigned num_uniforms = uniform_size.num_active_uniforms; const unsigned num_data_slots = uniform_size.num_values; /* On the outside chance that there were no uniforms, bail out. */ - if (num_user_uniforms == 0) + if (num_uniforms == 0) return; struct gl_uniform_storage *uniforms = - rzalloc_array(prog, struct gl_uniform_storage, num_user_uniforms); + rzalloc_array(prog, struct gl_uniform_storage, num_uniforms); union gl_constant_value *data = rzalloc_array(uniforms, union gl_constant_value, num_data_slots); #ifndef NDEBUG @@ -992,11 +990,6 @@ link_assign_uniform_locations(struct gl_shader_program *prog, if ((var == NULL) || (var->data.mode != ir_var_uniform)) continue; - /* FINISHME: Update code to process built-in uniforms! - */ - if (is_gl_identifier(var->name)) - continue; - parcel.set_and_process(prog, var); } @@ -1009,10 +1002,10 @@ link_assign_uniform_locations(struct gl_shader_program *prog, } const unsigned hidden_uniforms = - move_hidden_uniforms_to_end(prog, uniforms, num_user_uniforms); + move_hidden_uniforms_to_end(prog, uniforms, num_uniforms); /* Reserve all the explicit locations of the active uniforms. */ - for (unsigned i = 0; i < num_user_uniforms; i++) { + for (unsigned i = 0; i < num_uniforms; i++) { if (uniforms[i].remap_location != UNMAPPED_UNIFORM_LOC) { /* How many new entries for this uniform? */ const unsigned entries = MAX2(1, uniforms[i].array_elements); @@ -1028,7 +1021,11 @@ link_assign_uniform_locations(struct gl_shader_program *prog, } /* Reserve locations for rest of the uniforms. */ - for (unsigned i = 0; i < num_user_uniforms; i++) { + for (unsigned i = 0; i < num_uniforms; i++) { + + /* Built-in uniforms should not get any location. */ + if (uniforms[i].builtin) + continue; /* Explicit ones have been set already. */ if (uniforms[i].remap_location != UNMAPPED_UNIFORM_LOC) @@ -1055,14 +1052,14 @@ link_assign_uniform_locations(struct gl_shader_program *prog, } #ifndef NDEBUG - for (unsigned i = 0; i < num_user_uniforms; i++) { + for (unsigned i = 0; i < num_uniforms; i++) { assert(uniforms[i].storage != NULL); } assert(parcel.values == data_end); #endif - prog->NumUserUniformStorage = num_user_uniforms; + prog->NumUniformStorage = num_uniforms; prog->NumHiddenUniforms = hidden_uniforms; prog->UniformStorage = uniforms; diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index ecdc025..8b92a39 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1400,8 +1400,8 @@ link_fs_input_layout_qualifiers(struct gl_shader_program *prog, "layout qualifiers for gl_FragCoord\n"); } - /* Update the linked shader state.  Note that uses_gl_fragcoord should - * accumulate the results.  The other values should replace.  If there + /* Update the linked shader state. Note that uses_gl_fragcoord should + * accumulate the results. The other values should replace. If there * are multiple redeclarations, all the fields except uses_gl_fragcoord * are already known to be the same. */ @@ -2693,7 +2693,7 @@ build_program_resource_list(struct gl_context *ctx, } /* Add uniforms from uniform storage. */ - for (unsigned i = 0; i < shProg->NumUserUniformStorage; i++) { + for (unsigned i = 0; i < shProg->NumUniformStorage; i++) { /* Do not add uniforms internally used by Mesa. */ if (shProg->UniformStorage[i].hidden) continue; diff --git a/src/glsl/standalone_scaffolding.cpp b/src/glsl/standalone_scaffolding.cpp index a109c4e..00db61e 100644 --- a/src/glsl/standalone_scaffolding.cpp +++ b/src/glsl/standalone_scaffolding.cpp @@ -89,7 +89,7 @@ _mesa_clear_shader_program_data(struct gl_shader_program *shProg) { unsigned i; - shProg->NumUserUniformStorage = 0; + shProg->NumUniformStorage = 0; shProg->UniformStorage = NULL; shProg->NumUniformRemapTable = 0; shProg->UniformRemapTable = NULL; diff --git a/src/glsl/tests/set_uniform_initializer_tests.cpp b/src/glsl/tests/set_uniform_initializer_tests.cpp index d3fdeb3..91227d9 100644 --- a/src/glsl/tests/set_uniform_initializer_tests.cpp +++ b/src/glsl/tests/set_uniform_initializer_tests.cpp @@ -110,7 +110,7 @@ establish_uniform_storage(struct gl_shader_program *prog, unsigned num_storage, prog->UniformStorage = rzalloc_array(prog, struct gl_uniform_storage, num_storage); - prog->NumUserUniformStorage = num_storage; + prog->NumUniformStorage = num_storage; prog->UniformStorage[index_to_set].name = (char *) name; prog->UniformStorage[index_to_set].type = type; @@ -155,7 +155,7 @@ establish_uniform_storage(struct gl_shader_program *prog, unsigned num_storage, static void verify_initialization(struct gl_shader_program *prog, unsigned actual_index) { - for (unsigned i = 0; i < prog->NumUserUniformStorage; i++) { + for (unsigned i = 0; i < prog->NumUniformStorage; i++) { if (i == actual_index) { EXPECT_TRUE(prog->UniformStorage[actual_index].initialized); } else { diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 9b3186b..37c56e9 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -1181,9 +1181,12 @@ fs_visitor::setup_uniform_values(ir_variable *ir) * with our name, or the prefix of a component that starts with our name. */ unsigned params_before = uniforms; - for (unsigned u = 0; u < shader_prog->NumUserUniformStorage; u++) { + for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) { struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u]; + if (storage->builtin) + continue; + if (strncmp(ir->name, storage->name, namelen) != 0 || (storage->name[namelen] != 0 && storage->name[namelen] != '.' && diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp index 5dd8363..9c00b8a 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp @@ -218,9 +218,12 @@ fs_visitor::nir_setup_uniform(nir_variable *var) * our name. */ unsigned index = var->data.driver_location; - for (unsigned u = 0; u < shader_prog->NumUserUniformStorage; u++) { + for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) { struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u]; + if (storage->builtin) + continue; + if (strncmp(var->name, storage->name, namelen) != 0 || (storage->name[namelen] != 0 && storage->name[namelen] != '.' && diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp index 5a60fe4..6d118d2 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -683,9 +683,12 @@ vec4_visitor::setup_uniform_values(ir_variable *ir) * order we'd walk the type, so walk the list of storage and find anything * with our name, or the prefix of a component that starts with our name. */ - for (unsigned u = 0; u < shader_prog->NumUserUniformStorage; u++) { + for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) { struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u]; + if (storage->builtin) + continue; + if (strncmp(ir->name, storage->name, namelen) != 0 || (storage->name[namelen] != 0 && storage->name[namelen] != '.' && diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 8342517..3d791b3 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2728,7 +2728,7 @@ struct gl_shader_program } Comp; /* post-link info: */ - unsigned NumUserUniformStorage; + unsigned NumUniformStorage; unsigned NumHiddenUniforms; struct gl_uniform_storage *UniformStorage; diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c index a04b287..6d8e6e2 100644 --- a/src/mesa/main/shaderapi.c +++ b/src/mesa/main/shaderapi.c @@ -569,13 +569,13 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname, *params = _mesa_longest_attribute_name_length(shProg); return; case GL_ACTIVE_UNIFORMS: - *params = shProg->NumUserUniformStorage - shProg->NumHiddenUniforms; + *params = shProg->NumUniformStorage - shProg->NumHiddenUniforms; return; case GL_ACTIVE_UNIFORM_MAX_LENGTH: { unsigned i; GLint max_len = 0; const unsigned num_uniforms = - shProg->NumUserUniformStorage - shProg->NumHiddenUniforms; + shProg->NumUniformStorage - shProg->NumHiddenUniforms; for (i = 0; i < num_uniforms; i++) { /* Add one for the terminating NUL character for a non-array, and diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c index e428960..110a18e 100644 --- a/src/mesa/main/shaderobj.c +++ b/src/mesa/main/shaderobj.c @@ -282,10 +282,10 @@ _mesa_clear_shader_program_data(struct gl_shader_program *shProg) unsigned i; if (shProg->UniformStorage) { - for (i = 0; i < shProg->NumUserUniformStorage; ++i) + for (i = 0; i < shProg->NumUniformStorage; ++i) _mesa_uniform_detach_all_driver_storage(&shProg->UniformStorage[i]); ralloc_free(shProg->UniformStorage); - shProg->NumUserUniformStorage = 0; + shProg->NumUniformStorage = 0; shProg->UniformStorage = NULL; } diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp index 728bd1b..c5c830b 100644 --- a/src/mesa/main/uniform_query.cpp +++ b/src/mesa/main/uniform_query.cpp @@ -237,6 +237,13 @@ validate_uniform_parameters(struct gl_context *ctx, struct gl_uniform_storage *const uni = shProg->UniformRemapTable[location]; + /* Even though no location is assigned to a built-in uniform and it should + * already have aborted, this test makes it explicit that we are not allowing + * to update the value of a built-in. + */ + if (uni->builtin) + return NULL; + if (uni->array_elements == 0) { if (count > 1) { _mesa_error(ctx, GL_INVALID_OPERATION, @@ -1028,6 +1035,10 @@ _mesa_get_uniform_location(struct gl_shader_program *shProg, if (!found) return GL_INVALID_INDEX; + /* If the uniform is built-in, fail. */ + if (shProg->UniformStorage[location].builtin) + return GL_INVALID_INDEX; + /* If the uniform is an array, fail if the index is out of bounds. * (A negative index is caught above.) This also fails if the uniform * is not an array, but the user is trying to index it, because @@ -1047,7 +1058,7 @@ _mesa_sampler_uniforms_are_valid(const struct gl_shader_program *shProg, char *errMsg, size_t errMsgLength) { /* Shader does not have samplers. */ - if (shProg->NumUserUniformStorage == 0) + if (shProg->NumUniformStorage == 0) return true; if (!shProg->SamplersValidated) { @@ -1087,7 +1098,7 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct gl_pipeline_object *pipeline) if (!shProg[idx]) continue; - for (unsigned i = 0; i < shProg[idx]->NumUserUniformStorage; i++) { + 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()) diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp index 3dcb537..4a8a8ea 100644 --- a/src/mesa/program/ir_to_mesa.cpp +++ b/src/mesa/program/ir_to_mesa.cpp @@ -2406,9 +2406,14 @@ _mesa_associate_uniform_storage(struct gl_context *ctx, if (!found) continue; + struct gl_uniform_storage *storage = + &shader_program->UniformStorage[location]; + + /* Do not associate any uniform storage to built-in uniforms */ + if (!storage->builtin) + continue; + if (location != last_location) { - struct gl_uniform_storage *storage = - &shader_program->UniformStorage[location]; enum gl_uniform_driver_format format = uniform_native; unsigned columns = 0; diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c index 488f6ea..8b43582 100644 --- a/src/mesa/state_tracker/st_draw.c +++ b/src/mesa/state_tracker/st_draw.c @@ -141,7 +141,7 @@ check_uniforms(struct gl_context *ctx) if (shProg[j] == NULL || !shProg[j]->LinkStatus) continue; - for (i = 0; i < shProg[j]->NumUserUniformStorage; i++) { + for (i = 0; i < shProg[j]->NumUniformStorage; i++) { const struct gl_uniform_storage *u = &shProg[j]->UniformStorage[i]; if (!u->initialized) { _mesa_warning(ctx, -- 2.4.1