[Mesa-dev] [PATCH] glsl: uniform sampler should occupy 1 location

[Ian Romanick]

I'm not sure this is correct, and I think we need a more complex fix.
As Dave points out, bindless will make it even more complex.

In a non-bindless, static-sampler-array-indexing world, applications
assume that samplers will use zero uniform locations.  The sampler
information is baked into the instructions, and it isn't represented
anywhere else in GPU memory.  As a result, I would not be surprised to
see previously working applications fail to link (due to using too many
uniforms) with this change.

It seems that the only time a sampler needs non-zero space is when it is
part of any array samplers (or array of structures containing samplers,
etc.) or is bindless.  In the array cases, it is probably only necessary
when the index is dynamic.  I think the array splitting and structure
splitting passes may make that moot.

The 82921 bug seems to be an orthognal issue.  There is a difference
between the API visible locations assigned to a uniform and the GPU
visible locations where the uniform is stored.  My guess is that we're
using the same accounting for both in places where we should not.

On 08/21/2014 04:25 PM, Micael Dias wrote:
> ---
> If samplers occupy zero locations we can run into a lot of issues. See #82921.
> I briefly tested this with my own code (which was previously crashing and
> misbehaving) and also ran other apps and everything seems to work fine.
> I'm not used to contributing code in this fashion, so please forgive me if I'm
> making some mistake. Thanks.
> 
>  src/glsl/glsl_types.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 66e9b13..cc05193 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -691,6 +691,8 @@ glsl_type::uniform_locations() const
>        return size;
>     case GLSL_TYPE_ARRAY:
>        return this->length * this->fields.array->uniform_locations();
> +   case GLSL_TYPE_SAMPLER:
> +      return 1;
>     default:
>        break;
>     }
>