[Mesa-dev] [PATCH] glsl: simplify setting of image access qualifiers

[Iago Toral]

On Thu, 2016-02-04 at 12:03 +1100, Timothy Arceri wrote:
> Cc: Francisco Jerez <currojerez at riseup.net>
> ---
>  src/compiler/glsl/link_uniforms.cpp | 73 ++++++++-----------------------------
>  1 file changed, 16 insertions(+), 57 deletions(-)
> 
> diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
> index da39941..fc46ba2 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -504,6 +504,7 @@ public:
>  			ir_variable *var)
>     {
>        current_var = var;
> +      this->prog = prog;

No reason to assign this every time we find a variable, we should do
this in the constructor of the parcel object instead.

With that change,
Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

>        field_counter = 0;
>        this->record_next_sampler = new string_to_uint_map;
>  
> @@ -651,6 +652,16 @@ private:
>           uniform->opaque[shader_type].index = this->next_image;
>           uniform->opaque[shader_type].active = true;
>  
> +         /* Set image access qualifiers */
> +         const GLenum access =
> +            (current_var->data.image_read_only ? GL_READ_ONLY :
> +             current_var->data.image_write_only ? GL_WRITE_ONLY :
> +                GL_READ_WRITE);
> +
> +         for (unsigned j = 0; j < MAX2(1, uniform->array_elements); ++j)
> +            prog->_LinkedShaders[shader_type]->
> +               ImageAccess[this->next_image + j] = access;
> +
>           /* Increment the image index by 1 for non-arrays and by the
>            * number of array elements for arrays.
>            */
> @@ -875,6 +886,11 @@ private:
>      */
>     ir_variable *current_var;
>  
> +   /**
> +    * Current program being processed.
> +    */
> +   struct gl_shader_program *prog;
> +
>     /* Used to store the explicit location from current_var so that we can
>      * reuse the location field for storing the uniform slot id.
>      */
> @@ -1020,40 +1036,6 @@ link_update_uniform_buffer_variables(struct gl_shader *shader)
>     }
>  }
>  
> -static void
> -link_set_image_access_qualifiers(struct gl_shader_program *prog,
> -                                 gl_shader *sh, unsigned shader_stage,
> -                                 ir_variable *var, const glsl_type *type,
> -                                 char **name, size_t name_length)
> -{
> -   /* Handle arrays of arrays */
> -   if (type->is_array() && type->fields.array->is_array()) {
> -      for (unsigned i = 0; i < type->length; i++) {
> -	 size_t new_length = name_length;
> -
> -	 /* Append the subscript to the current variable name */
> -	 ralloc_asprintf_rewrite_tail(name, &new_length, "[%u]", i);
> -
> -         link_set_image_access_qualifiers(prog, sh, shader_stage, var,
> -                                          type->fields.array, name,
> -                                          new_length);
> -      }
> -   } else {
> -      unsigned id = 0;
> -      bool found = prog->UniformHash->get(id, *name);
> -      assert(found);
> -      (void) found;
> -      const gl_uniform_storage *storage = &prog->UniformStorage[id];
> -      const unsigned index = storage->opaque[shader_stage].index;
> -      const GLenum access = (var->data.image_read_only ? GL_READ_ONLY :
> -                             var->data.image_write_only ? GL_WRITE_ONLY :
> -                             GL_READ_WRITE);
> -
> -      for (unsigned j = 0; j < MAX2(1, storage->array_elements); ++j)
> -         sh->ImageAccess[index + j] = access;
> -   }
> -}
> -
>  /**
>   * Combine the hidden uniform hash map with the uniform hash map so that the
>   * hidden uniforms will be given indicies at the end of the uniform storage
> @@ -1314,29 +1296,6 @@ link_assign_uniform_locations(struct gl_shader_program *prog,
>     prog->NumHiddenUniforms = hidden_uniforms;
>     prog->UniformStorage = uniforms;
>  
> -   /**
> -    * Scan the program for image uniforms and store image unit access
> -    * information into the gl_shader data structure.
> -    */
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> -      gl_shader *sh = prog->_LinkedShaders[i];
> -
> -      if (sh == NULL)
> -	 continue;
> -
> -      foreach_in_list(ir_instruction, node, sh->ir) {
> -	 ir_variable *var = node->as_variable();
> -
> -         if (var && var->data.mode == ir_var_uniform &&
> -             var->type->contains_image()) {
> -            char *name_copy = ralloc_strdup(NULL, var->name);
> -            link_set_image_access_qualifiers(prog, sh, i, var, var->type,
> -                                             &name_copy, strlen(var->name));
> -            ralloc_free(name_copy);
> -         }
> -      }
> -   }
> -
>     link_set_uniform_initializers(prog, boolean_true);
>  
>     return;