[Mesa-dev] [PATCH v2 04/31] glsl: make component_slots() returns 2 for samplers/images

[Nicolai Hähnle]

On 24.04.2017 12:35, Samuel Pitoiset wrote:
> Bindless samplers/images are 64-bit unsigned integers, which
> means they consume two components as specified by
> ARB_bindless_texture.
>
> It looks like we are not wasting uniform storage by changing
> this because default-block uniforms are not packed. So, if
> we use N uint uniforms, they occupy N * 16 bytes in the
> constant buffer. This is something that could be improved.
>
> Though, count_uniform_size needs to be adjusted to not count
> a sampler (or image) twice.
>
> As a side effect, this will probably break the cache if you
> have one because it will consider sampler/image types as
> two components.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/compiler/glsl/link_uniforms.cpp | 8 ++++++--
>  src/compiler/glsl_types.cpp         | 2 ++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
> index b462cb9d59..3331c85af4 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -340,9 +340,13 @@ private:
>        if (type->contains_subroutine()) {
>           this->num_shader_subroutines += values;
>        } else if (type->contains_sampler()) {
> -         this->num_shader_samplers += values;
> +         /* Old-style (or bound) samplers are counted as two components as
> +          * specified by ARB_bindless_texture. */

I think this comment is a bit confusing. All samplers are counted as two 
components by these changes.

Question: Are samplers allowed in structs? If yes, then this code will 
need fixing to account for it. Either way, it would be good to add a 
corresponding test to piglit (whether negative or positive).

I don't think the spec is clear on this. The general language on 
structures (Section 4.1.8 of GLSL 4.5) doesn't restrict the types in 
structs.

The introduction of section 4.1.7 of GLSL 4.5 (Opaque types) implicitly 
says opaque types are not allowed in structs. The problem is that 
ARB_bindless_texture is written against GLSL 4.0, and a general "Opaque 
types" section simply doesn't exist there. There is a list in the 
replacement "Samplers" section which is probably meant to be exhaustive, 
which would suggest that samplers in structs are forbidden.

Cheers,
Nicolai


> +         this->num_shader_samplers += values / 2;
>        } else if (type->contains_image()) {
> -         this->num_shader_images += values;
> +         /* Old-style (or bound) images are counted as two components as
> +          * specified by ARB_bindless_texture. */
> +         this->num_shader_images += values / 2;
>
>           /* As drivers are likely to represent image uniforms as
>            * scalar indices, count them against the limit of uniform
> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
> index 0480bef80e..df148cfd21 100644
> --- a/src/compiler/glsl_types.cpp
> +++ b/src/compiler/glsl_types.cpp
> @@ -1298,6 +1298,8 @@ glsl_type::component_slots() const
>
>     case GLSL_TYPE_SAMPLER:
>     case GLSL_TYPE_IMAGE:
> +      return 2;
> +
>     case GLSL_TYPE_SUBROUTINE:
>        return 1;
>
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.