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 fixed all the places in the code that made use of the
structure that I changed. 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).

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Signed-off-by: Martin Peres <martin.peres@linux.intel.com>

diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h
index 21b5d05c11a03c5d7882552fbda462bb4c6b5415..e1b80147788f32d98bce78df9a0a38c8616deff8 100644 (file)
--- 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 69073841ea4af80f33aa0a5fe7fdb03523621017..204acfa22b2d133003be2547be1103de00bcf45b 100644 (file)
--- 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 2c928e1445bd09dfda2aaaffd7249eb47fb6a64b..11ae06f9bfba523748ab708d0449b26def20e83a 100644 (file)
--- 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++) {
-      assert(uniforms[i].storage != NULL);
+   for (unsigned i = 0; i < num_uniforms; i++) {
+      assert(uniforms[i].storage != NULL || uniforms[i].builtin);
    }
 
    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 99e0a388bb4a55ba7b82cb30c335be9ce3e27d3d..99783800b7f0f18635a7ccc0f114ef32df3d9596 100644 (file)
--- 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 a109c4e92d275ff484e08b6c9b82f80b453683be..00db61e409be188e766f3005a0cd0cfac3514185 100644 (file)
--- 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 d3fdeb3a844e2386fe36b12f11d8534e4a32fd70..91227d9487af5fc6957cad3317d9579476778809 100644 (file)
--- 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_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 56a2278a2dc4448b58f2d9a8cfffbe06916d2f10..5d3501c60ba0f348385cbd5eaf2ec54bb8833f92 100644 (file)
--- 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 d3347ecce38848859aecdb6c7f24f5f5261dc948..242d007b3559b3721fa2f1bcf91575231a6ccfd1 100644 (file)
--- 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 737f0be6d626e509128a70bb7b4d2235fc7ba52d..0aa607653d19d317793e0474640e184a8ee4ed16 100644 (file)
--- 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 a04b28711f780ef5db51098d422c0e16d5ac8088..6d8e6e23e9ce31b242ea9918dcb41075d47ed1fa 100644 (file)
--- 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 e428960362df2a489d74d1d95b9e855676d347e1..110a18e1e2c427e26cfac5e0924b90890273d678 100644 (file)
--- 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 728bd1bac101d6adf187555d59413ecc520475fc..cab5083e81b38e2babecbe98e091624a8bc32cff 100644 (file)
--- 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 this
+    * function should already have returned NULL, 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 37597247904b77ffe9e25c427b3047550e9fa2ae..50b86ada37adacfcf8abf137d6924aea2fe37e88 100644 (file)
--- 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 488f6ead20147704b96b143020cc8d97b0323768..8b43582c14b4fcb6b8f1b9ef0047cbee5ea6a14d 100644 (file)
--- 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,