On 01/14/2016 01:49 PM, Timothy Arceri wrote:
On Thu, 2016-01-14 at 13:02 +0200, Tapani Pälli wrote:
If shader declares uniform explicit location in one stage but
implicit in
another, explicit location should be used. Patch marks implicit
uniforms
as explicit if they were explicit in another stage. This makes sure
that
we don't treat them implicit later when assigning locations.


I think it would be better to fix the code in cross_validate_globals()
it seems it is halfway doing this already but it only copies the
explicit location from later stages into the first uniform it found,
and not the otherway around

True, I'll try moving the copying there.

I think you just need to check if the existing uniform is explicit and
copy the location to the currently matched var if its not explicit and
it should fix this bug.

Right, do the same as this patch but no additional map required, nice!


Fixes following CTS test:
    ES31-CTS.explicit_uniform_location.uniform-loc-implicit-in-some
-stages3

Signed-off-by: Tapani Pälli <[email protected]>
---
  src/glsl/linker.cpp | 36 +++++++++++++++++++++++++++++++++++-
  1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 564c471..70e701c 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -3131,11 +3131,33 @@ check_explicit_uniform_locations(struct
gl_context *ctx,
      */
     string_to_uint_map *uniform_map = new string_to_uint_map;

-   if (!uniform_map) {
+   /* This map is used to validate that uniforms with explicit
location are
+    * given the same explicit location properly across shader stages
which
+    * reference the same uniform without explicit location
qualifier.
+    */
+   string_to_uint_map *explicit_map = new string_to_uint_map;
+
+   if (!uniform_map || !explicit_map) {
        linker_error(prog, "Out of memory during linking.\n");
        return;
     }

+   /* A pass to generate hash that contains all uniforms where
location
+    * has been set explicitly.
+    */
+   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+      struct gl_shader *sh = prog->_LinkedShaders[i];
+      if (!sh)
+         continue;
+      foreach_in_list(ir_instruction, node, sh->ir) {
+         ir_variable *var = node->as_variable();
+         if (!var || var->data.mode != ir_var_uniform ||
+             !var->data.explicit_location)
+            continue;
+         explicit_map->put(var->data.location, var->name);
+      }
+   }
+
     unsigned entries_total = 0;
     for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
        struct gl_shader *sh = prog->_LinkedShaders[i];
@@ -3148,6 +3170,17 @@ check_explicit_uniform_locations(struct
gl_context *ctx,
           if (!var || var->data.mode != ir_var_uniform)
              continue;

+         /* Check if this uniform with implicit location was marked
explicit
+          * by any shader stage. If so, mark it explicit in this
stage too to
+          * make sure later processing does not treat it as implicit
one.
+          */
+         unsigned hash_loc;
+         if (!var->data.explicit_location &&
+             explicit_map->get(hash_loc, var->name)) {
+            var->data.explicit_location = 1;
+            var->data.location = hash_loc;
+         }
+
           entries_total += var->type->uniform_locations();

           if (var->data.explicit_location) {
@@ -3173,6 +3206,7 @@ check_explicit_uniform_locations(struct
gl_context *ctx,
                     ctx->Const.MaxUserAssignableUniformLocations);
     }
     delete uniform_map;
+   delete explicit_map;
  }

  static bool
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to