[Mesa-dev] [PATCH] Revert "mesa: stop assigning unused storage for non-bindless opaque types"

[Timothy Arceri]

On 01/08/17 06:59, Samuel Pitoiset wrote:
> This reverts commit fcbb93e860246375d03f280f927f79d3645a8988 and
> also  commit 7c5b204e38d8cae70f5bf26e7223da5bc448bb5c to avoid
> compilation errors.
> 
> Basically, the parameter indexes look wrong when a non-bindless
> sampler is declared inside a nested struct (because it is skipped).
> I think it's safer to just restore the previous behaviour which is
> here since ages and also because the initial attempt is only a
> little performance improvement.

I've got a proper fix. Just doing some final testing, will send soon.

> 
> This fixes a regression with
> ES2-CTS.functional.shaders.struct.uniform.sampler_nested*.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101983
> Cc: 17.2 <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>   src/mesa/program/ir_to_mesa.cpp | 56 +++++++++++++++++++++++++++++++++--------
>   1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index ac12b59d07..775211cefb 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -2409,8 +2409,10 @@ namespace {
>   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_stage shader_type)
> +      : shader_program(shader_program), params(params), idx(-1),
> +        shader_type(shader_type)
>      {
>         /* empty */
>      }
> @@ -2433,6 +2435,7 @@ private:
>      struct gl_program_parameter_list *params;
>      int idx;
>      ir_variable *var;
> +   gl_shader_stage shader_type;
>   };
>   
>   } /* anonymous namespace */
> @@ -2444,18 +2447,49 @@ add_uniform_to_shader::visit_field(const glsl_type *type, const char *name,
>                                      const enum glsl_interface_packing,
>                                      bool /* last_field */)
>   {
> -   /* opaque types don't use storage in the param list unless they are
> -    * bindless samplers or images.
> -    */
> -   if (type->contains_opaque() && !var->data.bindless)
> +   /* atomics don't get real storage */
> +   if (type->contains_atomic())
>         return;
>   
> -   assert(_mesa_lookup_parameter_index(params, name) < 0);
> +   gl_register_file file;
> +   if (type->without_array()->is_sampler() && !var->data.bindless) {
> +      file = PROGRAM_SAMPLER;
> +   } else {
> +      file = PROGRAM_UNIFORM;
> +   }
> +
> +   int index = _mesa_lookup_parameter_index(params, name);
> +   if (index < 0) {
> +      unsigned size = type_size(type) * 4;
> +
> +      index = _mesa_add_parameter(params, file, name, size, type->gl_type,
> +				  NULL, NULL);
>   
> -   unsigned size = type_size(type) * 4;
> +      /* Sampler uniform values are stored in prog->SamplerUnits,
> +       * and the entry in that array is selected by this index we
> +       * store in ParameterValues[].
> +       */
> +      if (file == PROGRAM_SAMPLER) {
> +	 unsigned location;
> +	 const bool found =
> +	    this->shader_program->UniformHash->get(location,
> +						   params->Parameters[index].Name);
> +	 assert(found);
> +
> +	 if (!found)
> +	    return;
> +
> +	 struct gl_uniform_storage *storage =
> +            &this->shader_program->data->UniformStorage[location];
>   
> -   int index = _mesa_add_parameter(params, PROGRAM_UNIFORM, name, size,
> -                                   type->gl_type, NULL, NULL);
> +         assert(storage->type->is_sampler() &&
> +                storage->opaque[shader_type].active);
> +
> +	 for (unsigned int j = 0; j < size / 4; j++)
> +            params->ParameterValues[index + j][0].f =
> +               storage->opaque[shader_type].index + j;
> +      }
> +   }
>   
>      /* The first part of the uniform that's processed determines the base
>       * location of the whole uniform (for structures).
> @@ -2479,7 +2513,7 @@ _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, sh->Stage);
>   
>      foreach_in_list(ir_instruction, node, sh->ir) {
>         ir_variable *var = node->as_variable();
>