On 05/19/2014 08:07 PM, Ian Romanick wrote:
On 04/09/2014 02:56 AM, Tapani Pälli wrote:
Patch refactors the existing uniform processing so explicit locations
are taken in to account during variable processing. These locations
are temporarily stored in gl_uniform_storage before actual locations
are set.

The 'remap_location' variable in gl_uniform_storage is changed to be
signed so that we can use 0 as a valid explicit location and '-1' as
identifier that no explicit location has been defined.

When locations are set, UniformRemapTable is first populated with
uniforms that have explicit location set (inactive and actives ones),
rest are put after explicit location slots.

Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
---
  src/glsl/ir_uniform.h      |  5 +++--
  src/glsl/link_uniforms.cpp | 56 +++++++++++++++++++++++++++++++++++++++++-----
  2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h
index 3508509..9dc4a8e 100644
--- a/src/glsl/ir_uniform.h
+++ b/src/glsl/ir_uniform.h
@@ -181,9 +181,10 @@ struct gl_uniform_storage {
/**
      * The 'base location' for this uniform in the uniform remap table. For
-    * arrays this is the first element in the array.
+    * arrays this is the first element in the array. It needs to be signed
+    * so that we can use 0 as valid location and -1 as initial value
      */
-   unsigned remap_location;
+   int remap_location;
You could use ~0u instead of -1, right?  A #define for the magic value
will also help.

Sure, I can move to using ~0u. Should be enough to never become a problem.

  };
#ifdef __cplusplus
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 29dc0b1..0f99082 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -387,6 +387,9 @@ public:
     void set_and_process(struct gl_shader_program *prog,
                        ir_variable *var)
     {
+      current_var = var;
+      field_counter = 0;
+
        ubo_block_index = -1;
        if (var->is_in_uniform_block()) {
           if (var->is_interface_instance() && var->type->is_array()) {
@@ -543,6 +546,22 @@ private:
           return;
        }
+ /* Assign explicit locations. */
+      if (current_var->data.explicit_location) {
+         /* Set sequential locations for struct fields. */
+         if (current_var->type->is_record()) {
I think you can accomplish the same thing with record_type != NULL.

ok, I can change

+            const unsigned entries = MAX2(1, 
this->uniforms[id].array_elements);
+            this->uniforms[id].remap_location =
+               current_var->data.location + field_counter;
+               field_counter += entries;
Weird indentation.

will fix

+         } else {
+            this->uniforms[id].remap_location = current_var->data.location;
+         }
+      } else {
+         /* Initialize to -1 to indicate that no explicit location is set */
+         this->uniforms[id].remap_location = -1;
+      }
+
        this->uniforms[id].name = ralloc_strdup(this->uniforms, name);
        this->uniforms[id].type = base_type;
        this->uniforms[id].initialized = 0;
@@ -598,6 +617,17 @@ public:
     gl_texture_index targets[MAX_SAMPLERS];
/**
+    * Current variable being processed.
+    */
+   ir_variable *current_var;
+
+   /**
+    * Field counter is used to take care that uniform structures
+    * with explicit locations get sequential locations.
+    */
+   unsigned field_counter;
+
+   /**
      * Mask of samplers used by the current shader stage.
      */
     unsigned shader_samplers_used;
@@ -799,10 +829,6 @@ link_assign_uniform_locations(struct gl_shader_program 
*prog)
     prog->UniformStorage = NULL;
     prog->NumUserUniformStorage = 0;
- ralloc_free(prog->UniformRemapTable);
-   prog->UniformRemapTable = NULL;
-   prog->NumUniformRemapTable = 0;
-
     if (prog->UniformHash != NULL) {
        prog->UniformHash->clear();
     } else {
@@ -915,9 +941,29 @@ link_assign_uniform_locations(struct gl_shader_program 
*prog)
               sizeof(prog->_LinkedShaders[i]->SamplerTargets));
     }
- /* Build the uniform remap table that is used to set/get uniform locations */
+   /* Reserve all the explicit locations of the active uniforms. */
+   for (unsigned i = 0; i < num_user_uniforms; i++) {
+      if (uniforms[i].remap_location != -1) {
+         /* How many new entries for this uniform? */
+         const unsigned entries = MAX2(1, uniforms[i].array_elements);
+
+         /* Set remap table entries point to correct gl_uniform_storage. */
+         for (unsigned j = 0; j < entries; j++) {
+            unsigned element_loc = uniforms[i].remap_location + j;
+            assert(prog->UniformRemapTable[element_loc] ==
+                   (gl_uniform_storage *) -1);
+            prog->UniformRemapTable[element_loc] = &uniforms[i];
+         }
+      }
+   }
+
+   /* Reserve locations for rest of the uniforms. */
     for (unsigned i = 0; i < num_user_uniforms; i++) {
+ /* Explicit ones have been set already. */
+      if (uniforms[i].remap_location != -1)
+         continue;
+
        /* how many new entries for this uniform? */
        const unsigned entries = MAX2(1, uniforms[i].array_elements);

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to