Mesa (master): mesa: fix GLSL program objects with more than 16 samplers combined

Marek Olšák mareko at kemper.freedesktop.org
Tue May 28 11:05:41 UTC 2013


Module: Mesa
Branch: master
Commit: d4a06d77f5898726e2453ef32795a2183c033c05
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=d4a06d77f5898726e2453ef32795a2183c033c05

Author: Marek Olšák <maraeo at gmail.com>
Date:   Mon May 13 15:46:49 2013 +0200

mesa: fix GLSL program objects with more than 16 samplers combined

The problem is the sampler units are allocated from the same pool for all
shader stages, so if a vertex shader uses 12 samplers (0..11), the fragment
shader samplers start at index 12, leaving only 4 sampler units
for the fragment shader. The main cause is probably the fact that samplers
(texture unit -> sampler unit mapping, etc.) are tracked globally
for an entire program object.

This commit adapts the GLSL linker and core Mesa such that the sampler units
are assigned to sampler uniforms for each shader stage separately
(if a sampler uniform is used in all shader stages, it may occupy a different
sampler unit in each, and vice versa, an i-th sampler unit may refer to
a different sampler uniform in each shader stage), and the sampler-specific
variables are moved from gl_shader_program to gl_shader.

This doesn't require any driver changes, and it fixes piglit/max-samplers
for gallium and classic swrast. It also works with any number of shader
stages.

v2: - converted tabs to spaces
    - added an assertion to _mesa_get_sampler_uniform_value

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
Reviewed-by: Eric Anholt <eric at anholt.net>

---

 src/glsl/ir_uniform.h                            |   27 +++--
 src/glsl/link_uniform_initializers.cpp           |   25 ++++-
 src/glsl/link_uniforms.cpp                       |  126 +++++++++++-----------
 src/glsl/tests/set_uniform_initializer_tests.cpp |   10 ++-
 src/mesa/main/mtypes.h                           |   22 ++--
 src/mesa/main/uniform_query.cpp                  |   24 ++--
 src/mesa/main/uniforms.c                         |   11 ++-
 src/mesa/program/ir_to_mesa.cpp                  |   15 ++-
 src/mesa/program/sampler.cpp                     |   20 +++-
 9 files changed, 167 insertions(+), 113 deletions(-)

diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h
index 30e6f26..8198c48 100644
--- a/src/glsl/ir_uniform.h
+++ b/src/glsl/ir_uniform.h
@@ -99,15 +99,24 @@ struct gl_uniform_storage {
     */
    bool initialized;
 
-   /**
-    * Base sampler index
-    *
-    * If \c ::base_type is \c GLSL_TYPE_SAMPLER, this represents the index of
-    * this sampler.  If \c ::array_elements is not zero, the array will use
-    * sampler indexes \c ::sampler through \c ::sampler + \c ::array_elements
-    * - 1, inclusive.
-    */
-   uint8_t sampler;
+   struct {
+      /**
+       * Base sampler index
+       *
+       * If \c ::base_type is \c GLSL_TYPE_SAMPLER, this represents the index
+       * of this sampler.  If \c ::array_elements is not zero, the array will
+       * use sampler indices \c ::sampler through \c ::sampler +
+       * \c ::array_elements - 1, inclusive.
+       *
+       * Note that the index may be different in each shader stage.
+       */
+      uint8_t index;
+
+      /**
+       * Whether this sampler is used in this shader stage.
+       */
+      bool active;
+   } sampler[MESA_SHADER_TYPES];
 
    /**
     * Storage used by the driver for the uniform
diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp
index 836a360..54d9bf1 100644
--- a/src/glsl/link_uniform_initializers.cpp
+++ b/src/glsl/link_uniform_initializers.cpp
@@ -138,8 +138,16 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog,
       }
 
       if (base_type == GLSL_TYPE_SAMPLER) {
-	 for (unsigned int i = 0; i < storage->array_elements; i++) {
-	    prog->SamplerUnits[storage->sampler + i] = storage->storage[i].i;
+         for (int sh = 0; sh < MESA_SHADER_TYPES; sh++) {
+            gl_shader *shader = prog->_LinkedShaders[sh];
+
+            if (shader && storage->sampler[sh].active) {
+               for (unsigned i = 0; i < storage->array_elements; i++) {
+                  unsigned index = storage->sampler[sh].index + i;
+
+                  shader->SamplerUnits[index] = storage->storage[i].i;
+               }
+            }
 	 }
       }
    } else {
@@ -148,8 +156,17 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog,
 			       val->type->base_type,
 			       val->type->components());
 
-      if (storage->type->is_sampler())
-	 prog->SamplerUnits[storage->sampler] = storage->storage[0].i;
+      if (storage->type->is_sampler()) {
+         for (int sh = 0; sh < MESA_SHADER_TYPES; sh++) {
+            gl_shader *shader = prog->_LinkedShaders[sh];
+
+            if (shader && storage->sampler[sh].active) {
+               unsigned index = storage->sampler[sh].index;
+
+               shader->SamplerUnits[index] = storage->storage[0].i;
+            }
+         }
+      }
    }
 
    storage->initialized = true;
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index d457e4d..04218be 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -263,15 +263,19 @@ public:
    parcel_out_uniform_storage(struct string_to_uint_map *map,
 			      struct gl_uniform_storage *uniforms,
 			      union gl_constant_value *values)
-      : map(map), uniforms(uniforms), next_sampler(0), values(values)
+      : map(map), uniforms(uniforms), values(values)
    {
-      memset(this->targets, 0, sizeof(this->targets));
    }
 
-   void start_shader()
+   void start_shader(gl_shader_type shader_type)
    {
+      assert(shader_type < MESA_SHADER_TYPES);
+      this->shader_type = shader_type;
+
       this->shader_samplers_used = 0;
       this->shader_shadow_samplers = 0;
+      this->next_sampler = 0;
+      memset(this->targets, 0, sizeof(this->targets));
    }
 
    void set_and_process(struct gl_shader_program *prog,
@@ -335,8 +339,37 @@ public:
    int ubo_block_index;
    int ubo_byte_offset;
    bool ubo_row_major;
+   gl_shader_type shader_type;
 
 private:
+   void handle_samplers(const glsl_type *base_type,
+                        struct gl_uniform_storage *uniform)
+   {
+      if (base_type->is_sampler()) {
+         uniform->sampler[shader_type].index = this->next_sampler;
+         uniform->sampler[shader_type].active = true;
+
+         /* Increment the sampler by 1 for non-arrays and by the number of
+          * array elements for arrays.
+          */
+         this->next_sampler +=
+               MAX2(1, uniform->array_elements);
+
+         const gl_texture_index target = base_type->sampler_index();
+         const unsigned shadow = base_type->sampler_shadow;
+         for (unsigned i = uniform->sampler[shader_type].index;
+              i < MIN2(this->next_sampler, MAX_SAMPLERS);
+              i++) {
+            this->targets[i] = target;
+            this->shader_samplers_used |= 1U << i;
+            this->shader_shadow_samplers |= shadow << i;
+         }
+      } else {
+         uniform->sampler[shader_type].index = ~0;
+         uniform->sampler[shader_type].active = false;
+      }
+   }
+
    virtual void visit_field(const glsl_type *type, const char *name,
                             bool row_major)
    {
@@ -354,31 +387,6 @@ private:
       if (!found)
 	 return;
 
-      /* 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 (this->uniforms[id].storage != NULL) {
-	 /* If the uniform already has storage set from another shader stage,
-	  * mark the samplers used for this shader stage.
-	  */
-	 if (type->contains_sampler()) {
-	    const unsigned count = MAX2(1, this->uniforms[id].array_elements);
-	    const unsigned shadow = (type->is_array())
-	       ? type->fields.array->sampler_shadow : type->sampler_shadow;
-
-	    for (unsigned i = 0; i < count; i++) {
-	       const unsigned s = this->uniforms[id].sampler + i;
-
-	       this->shader_samplers_used |= 1U << s;
-	       this->shader_shadow_samplers |= shadow << s;
-	    }
-	 }
-
-	 return;
-      }
-
       const glsl_type *base_type;
       if (type->is_array()) {
 	 this->uniforms[id].array_elements = type->length;
@@ -388,26 +396,16 @@ private:
 	 base_type = type;
       }
 
-      if (base_type->is_sampler()) {
-	 this->uniforms[id].sampler = this->next_sampler;
+      /* This assigns sampler uniforms to sampler units. */
+      handle_samplers(base_type, &this->uniforms[id]);
 
-	 /* Increment the sampler by 1 for non-arrays and by the number of
-	  * array elements for arrays.
-	  */
-	 this->next_sampler += MAX2(1, this->uniforms[id].array_elements);
-
-	 const gl_texture_index target = base_type->sampler_index();
-	 const unsigned shadow = base_type->sampler_shadow;
-	 for (unsigned i = this->uniforms[id].sampler
-		 ; i < MIN2(this->next_sampler, MAX_SAMPLERS)
-		 ; i++) {
-	    this->targets[i] = target;
-	    this->shader_samplers_used |= 1U << i;
-	    this->shader_shadow_samplers |= shadow << i;
-	 }
-
-      } else {
-	 this->uniforms[id].sampler = ~0;
+      /* 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 (this->uniforms[id].storage != NULL) {
+         return;
       }
 
       this->uniforms[id].name = ralloc_strdup(this->uniforms, name);
@@ -633,17 +631,6 @@ link_assign_uniform_locations(struct gl_shader_program *prog)
       prog->UniformHash = new string_to_uint_map;
    }
 
-   /* Uniforms that lack an initializer in the shader code have an initial
-    * value of zero.  This includes sampler uniforms.
-    *
-    * Page 24 (page 30 of the PDF) of the GLSL 1.20 spec says:
-    *
-    *     "The link time initial value is either the value of the variable's
-    *     initializer, if present, or 0 if no initializer is present. Sampler
-    *     types cannot have initializers."
-    */
-   memset(prog->SamplerUnits, 0, sizeof(prog->SamplerUnits));
-
    /* First pass: Count the uniform resources used by the user-defined
     * uniforms.  While this happens, each active uniform will have an index
     * assigned to it.
@@ -656,6 +643,18 @@ link_assign_uniform_locations(struct gl_shader_program *prog)
       if (prog->_LinkedShaders[i] == NULL)
 	 continue;
 
+      /* Uniforms that lack an initializer in the shader code have an initial
+       * value of zero.  This includes sampler uniforms.
+       *
+       * Page 24 (page 30 of the PDF) of the GLSL 1.20 spec says:
+       *
+       *     "The link time initial value is either the value of the variable's
+       *     initializer, if present, or 0 if no initializer is present. Sampler
+       *     types cannot have initializers."
+       */
+      memset(prog->_LinkedShaders[i]->SamplerUnits, 0,
+             sizeof(gl_shader::SamplerUnits));
+
       link_update_uniform_buffer_variables(prog->_LinkedShaders[i]);
 
       /* Reset various per-shader target counts.
@@ -706,9 +705,7 @@ link_assign_uniform_locations(struct gl_shader_program *prog)
       if (prog->_LinkedShaders[i] == NULL)
 	 continue;
 
-      /* Reset various per-shader target counts.
-       */
-      parcel.start_shader();
+      parcel.start_shader((gl_shader_type)i);
 
       foreach_list(node, prog->_LinkedShaders[i]->ir) {
 	 ir_variable *const var = ((ir_instruction *) node)->as_variable();
@@ -726,10 +723,11 @@ link_assign_uniform_locations(struct gl_shader_program *prog)
 
       prog->_LinkedShaders[i]->active_samplers = parcel.shader_samplers_used;
       prog->_LinkedShaders[i]->shadow_samplers = parcel.shader_shadow_samplers;
-   }
 
-   assert(sizeof(prog->SamplerTargets) == sizeof(parcel.targets));
-   memcpy(prog->SamplerTargets, parcel.targets, sizeof(prog->SamplerTargets));
+      assert(sizeof(gl_shader::SamplerTargets) == sizeof(parcel.targets));
+      memcpy(prog->_LinkedShaders[i]->SamplerTargets, parcel.targets,
+             sizeof(gl_shader::SamplerTargets));
+   }
 
 #ifndef NDEBUG
    for (unsigned i = 0; i < num_user_uniforms; i++) {
diff --git a/src/glsl/tests/set_uniform_initializer_tests.cpp b/src/glsl/tests/set_uniform_initializer_tests.cpp
index 55831f9..5c6d4a5 100644
--- a/src/glsl/tests/set_uniform_initializer_tests.cpp
+++ b/src/glsl/tests/set_uniform_initializer_tests.cpp
@@ -116,7 +116,10 @@ establish_uniform_storage(struct gl_shader_program *prog, unsigned num_storage,
    prog->UniformStorage[index_to_set].type = type;
    prog->UniformStorage[index_to_set].array_elements = array_size;
    prog->UniformStorage[index_to_set].initialized = false;
-   prog->UniformStorage[index_to_set].sampler = ~0;
+   for (int sh = 0; sh < MESA_SHADER_TYPES; sh++) {
+      prog->UniformStorage[index_to_set].sampler[sh].index = ~0;
+      prog->UniformStorage[index_to_set].sampler[sh].active = false;
+   }
    prog->UniformStorage[index_to_set].num_driver_storage = 0;
    prog->UniformStorage[index_to_set].driver_storage = NULL;
    prog->UniformStorage[index_to_set].storage =
@@ -134,7 +137,10 @@ establish_uniform_storage(struct gl_shader_program *prog, unsigned num_storage,
       prog->UniformStorage[i].type = glsl_type::void_type;
       prog->UniformStorage[i].array_elements = 0;
       prog->UniformStorage[i].initialized = false;
-      prog->UniformStorage[i].sampler = ~0;
+      for (int sh = 0; sh < MESA_SHADER_TYPES; sh++) {
+         prog->UniformStorage[i].sampler[sh].index = ~0;
+         prog->UniformStorage[i].sampler[sh].active = false;
+      }
       prog->UniformStorage[i].num_driver_storage = 0;
       prog->UniformStorage[i].driver_storage = NULL;
       prog->UniformStorage[i].storage = NULL;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 40ffb92..e0e1886 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2132,6 +2132,17 @@ struct gl_shader
    /*@}*/
 
    /**
+    * Map from sampler unit to texture unit (set by glUniform1i())
+    *
+    * A sampler unit is associated with each sampler uniform by the linker.
+    * The sampler unit associated with each uniform is stored in the
+    * \c gl_uniform_storage::sampler field.
+    */
+   GLubyte SamplerUnits[MAX_SAMPLERS];
+   /** Which texture target is being sampled (TEXTURE_1D/2D/3D/etc_INDEX) */
+   gl_texture_index SamplerTargets[MAX_SAMPLERS];
+
+   /**
     * Number of uniform components used by this shader.
     *
     * This field is only set post-linking.
@@ -2335,17 +2346,6 @@ struct gl_shader_program
     */
    struct string_to_uint_map *UniformHash;
 
-   /**
-    * Map from sampler unit to texture unit (set by glUniform1i())
-    *
-    * A sampler unit is associated with each sampler uniform by the linker.
-    * The sampler unit associated with each uniform is stored in the
-    * \c gl_uniform_storage::sampler field.
-    */
-   GLubyte SamplerUnits[MAX_SAMPLERS];
-   /** Which texture target is being sampled (TEXTURE_1D/2D/3D/etc_INDEX) */
-   gl_texture_index SamplerTargets[MAX_SAMPLERS];
-
    GLboolean LinkStatus;   /**< GL_LINK_STATUS */
    GLboolean Validated;
    GLboolean _Used;        /**< Ever used for drawing? */
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index b7f25e0..ec31049 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -778,24 +778,24 @@ _mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shProg,
    if (uni->type->is_sampler()) {
       int i;
 
-      for (i = 0; i < count; i++) {
-	 shProg->SamplerUnits[uni->sampler + offset + i] =
-	    ((unsigned *) values)[i];
-      }
-
       bool flushed = false;
       for (i = 0; i < MESA_SHADER_TYPES; i++) {
 	 struct gl_shader *const sh = shProg->_LinkedShaders[i];
+         int j;
 
-	 /* If the shader stage doesn't use any samplers, don't bother
-	  * checking if any samplers have changed.
+	 /* If the shader stage doesn't use the sampler uniform, skip this.
 	  */
-	 if (sh == NULL || sh->active_samplers == 0)
+	 if (sh == NULL || !uni->sampler[i].active)
 	    continue;
 
+         for (j = 0; j < count; j++) {
+            sh->SamplerUnits[uni->sampler[i].index + offset + j] =
+               ((unsigned *) values)[j];
+         }
+
 	 struct gl_program *const prog = sh->Program;
 
-	 assert(sizeof(prog->SamplerUnits) == sizeof(shProg->SamplerUnits));
+	 assert(sizeof(prog->SamplerUnits) == sizeof(sh->SamplerUnits));
 
 	 /* Determine if any of the samplers used by this shader stage have
 	  * been modified.
@@ -803,7 +803,7 @@ _mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shProg,
 	 bool changed = false;
 	 for (unsigned j = 0; j < Elements(prog->SamplerUnits); j++) {
 	    if ((sh->active_samplers & (1U << j)) != 0
-		&& (prog->SamplerUnits[j] != shProg->SamplerUnits[j])) {
+		&& (prog->SamplerUnits[j] != sh->SamplerUnits[j])) {
 	       changed = true;
 	       break;
 	    }
@@ -816,8 +816,8 @@ _mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shProg,
 	    }
 
 	    memcpy(prog->SamplerUnits,
-		   shProg->SamplerUnits,
-		   sizeof(shProg->SamplerUnits));
+		   sh->SamplerUnits,
+		   sizeof(sh->SamplerUnits));
 
 	    _mesa_update_shader_textures_used(shProg, prog);
             if (ctx->Driver.SamplerUniformChange)
diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
index f0d80f0..6d79df6 100644
--- a/src/mesa/main/uniforms.c
+++ b/src/mesa/main/uniforms.c
@@ -45,6 +45,7 @@
 #include "main/enums.h"
 #include "ir_uniform.h"
 #include "glsl_types.h"
+#include "program/program.h"
 
 /**
  * Update the vertex/fragment program's TexturesUsed array.
@@ -66,14 +67,18 @@ _mesa_update_shader_textures_used(struct gl_shader_program *shProg,
 				  struct gl_program *prog)
 {
    GLuint s;
+   struct gl_shader *shader =
+      shProg->_LinkedShaders[_mesa_program_target_to_index(prog->Target)];
 
-   memcpy(prog->SamplerUnits, shProg->SamplerUnits, sizeof(prog->SamplerUnits));
+   assert(shader);
+
+   memcpy(prog->SamplerUnits, shader->SamplerUnits, sizeof(prog->SamplerUnits));
    memset(prog->TexturesUsed, 0, sizeof(prog->TexturesUsed));
 
    for (s = 0; s < MAX_SAMPLERS; s++) {
       if (prog->SamplersUsed & (1 << s)) {
-         GLuint unit = shProg->SamplerUnits[s];
-         GLuint tgt = shProg->SamplerTargets[s];
+         GLuint unit = shader->SamplerUnits[s];
+         GLuint tgt = shader->SamplerTargets[s];
          assert(unit < Elements(prog->TexturesUsed));
          assert(tgt < NUM_TEXTURE_TARGETS);
          prog->TexturesUsed[unit] |= (1 << tgt);
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index b3b55a5..a5b6699 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -2404,8 +2404,10 @@ print_program(struct prog_instruction *mesa_instructions,
 class add_uniform_to_shader : public program_resource_visitor {
 public:
    add_uniform_to_shader(struct gl_shader_program *shader_program,
-			 struct gl_program_parameter_list *params)
-      : shader_program(shader_program), params(params), idx(-1)
+			 struct gl_program_parameter_list *params,
+                         gl_shader_type shader_type)
+      : shader_program(shader_program), params(params), idx(-1),
+        shader_type(shader_type)
    {
       /* empty */
    }
@@ -2425,6 +2427,7 @@ private:
    struct gl_shader_program *shader_program;
    struct gl_program_parameter_list *params;
    int idx;
+   gl_shader_type shader_type;
 };
 
 void
@@ -2471,8 +2474,11 @@ add_uniform_to_shader::visit_field(const glsl_type *type, const char *name,
 	 struct gl_uniform_storage *storage =
 	    &this->shader_program->UniformStorage[location];
 
+         assert(storage->sampler[shader_type].active);
+
 	 for (unsigned int j = 0; j < size / 4; j++)
-	    params->ParameterValues[index + j][0].f = storage->sampler + j;
+            params->ParameterValues[index + j][0].f =
+               storage->sampler[shader_type].index + j;
       }
    }
 
@@ -2498,7 +2504,8 @@ _mesa_generate_parameters_list_for_uniforms(struct gl_shader_program
 					    struct gl_program_parameter_list
 					    *params)
 {
-   add_uniform_to_shader add(shader_program, params);
+   add_uniform_to_shader add(shader_program, params,
+                             _mesa_shader_type_to_index(sh->Type));
 
    foreach_list(node, sh->ir) {
       ir_variable *var = ((ir_instruction *) node)->as_variable();
diff --git a/src/mesa/program/sampler.cpp b/src/mesa/program/sampler.cpp
index e3641aa..9b94127 100644
--- a/src/mesa/program/sampler.cpp
+++ b/src/mesa/program/sampler.cpp
@@ -34,6 +34,7 @@ extern "C" {
 #include "main/compiler.h"
 #include "main/mtypes.h"
 #include "program/prog_parameter.h"
+#include "program/program.h"
 }
 
 class get_sampler_name : public ir_hierarchical_visitor
@@ -102,14 +103,16 @@ public:
    ir_dereference *last;
 };
 
-extern "C" {
-int
+
+extern "C" int
 _mesa_get_sampler_uniform_value(class ir_dereference *sampler,
 				struct gl_shader_program *shader_program,
 				const struct gl_program *prog)
 {
    get_sampler_name getname(sampler, shader_program);
 
+   GLuint shader = _mesa_program_target_to_index(prog->Target);
+
    sampler->accept(&getname);
 
    unsigned location;
@@ -119,6 +122,15 @@ _mesa_get_sampler_uniform_value(class ir_dereference *sampler,
       return 0;
    }
 
-   return shader_program->UniformStorage[location].sampler + getname.offset;
-}
+   if (!shader_program->UniformStorage[location].sampler[shader].active) {
+      assert(0 && "cannot return a sampler");
+      linker_error(shader_program,
+		   "cannot return a sampler named %s, because it is not "
+                   "used in this shader stage. This is a driver bug.\n",
+                   getname.name);
+      return 0;
+   }
+
+   return shader_program->UniformStorage[location].sampler[shader].index +
+          getname.offset;
 }




More information about the mesa-commit mailing list