[Mesa-dev] [PATCH 20/23] glsl/linker: Count and check image resources.

[Paul Berry]

On 26 November 2013 00:02, Francisco Jerez <currojerez at riseup.net> wrote:

> ---
>  src/glsl/link_uniforms.cpp | 13 +++++++++++-
>  src/glsl/linker.cpp        | 50
> ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 0a15739..c75a38c 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -240,7 +240,8 @@ class count_uniform_size : public
> program_resource_visitor {
>  public:
>     count_uniform_size(struct string_to_uint_map *map)
>        : num_active_uniforms(0), num_values(0), num_shader_samplers(0),
> -       num_shader_uniform_components(0), is_ubo_var(false), map(map)
> +        num_shader_images(0), num_shader_uniform_components(0),
> +        is_ubo_var(false), map(map)
>     {
>        /* empty */
>     }
> @@ -248,6 +249,7 @@ public:
>     void start_shader()
>     {
>        this->num_shader_samplers = 0;
> +      this->num_shader_images = 0;
>        this->num_shader_uniform_components = 0;
>     }
>
> @@ -277,6 +279,11 @@ public:
>     unsigned num_shader_samplers;
>
>     /**
> +    * Number of images used
> +    */
> +   unsigned num_shader_images;
> +
> +   /**
>      * Number of uniforms used in the current shader
>      */
>     unsigned num_shader_uniform_components;
> @@ -303,6 +310,9 @@ private:
>        if (type->contains_sampler()) {
>          this->num_shader_samplers +=
>             type->is_array() ? type->array_size() : 1;
> +      } else if (type->contains_image()) {
> +         this->num_shader_images += values;
> +         this->num_shader_uniform_components += values;
>

How come images contribute to num_shader_uniform_components but samplers
don't?  There should be a comment explaining this difference.


>        } else {
>          /* Accumulate the total number of uniform slots used by this
> shader.
>           * Note that samplers do not count against this limit because they
> @@ -782,6 +792,7 @@ link_assign_uniform_locations(struct gl_shader_program
> *prog)
>        }
>
>        sh->num_samplers = uniform_size.num_shader_samplers;
> +      sh->NumImages = uniform_size.num_shader_images;
>        sh->num_uniform_components =
> uniform_size.num_shader_uniform_components;
>
>        sh->num_combined_uniform_components = sh->num_uniform_components;
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index fac186a..86a0cae 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1980,6 +1980,55 @@ check_resources(struct gl_context *ctx, struct
> gl_shader_program *prog)
>     }
>  }
>
> +/**
> + * Validate shader image resources.
> + */
> +static void
> +check_image_resources(struct gl_context *ctx, struct gl_shader_program
> *prog)
> +{
> +   STATIC_ASSERT(MESA_SHADER_TYPES == 3);
> +   static const char *const shader_names[MESA_SHADER_TYPES] = {
> +      "vertex", "geometry", "fragment"
> +   };
>

Use _mesa_glsl_shader_target_name() instead of hardcoding this array.


> +   const unsigned max_images[MESA_SHADER_TYPES] = {
> +      ctx->Const.VertexProgram.MaxImageUniforms,
> +      ctx->Const.GeometryProgram.MaxImageUniforms,
> +      ctx->Const.FragmentProgram.MaxImageUniforms
> +   };
>

Rather than doing STATIC_ASSERT(MESA_SHADER_TYPES == 3), I think this would
be slightly cleaner, since it will be more obvious what needs fixing if the
assertion ever fires:

   const unsigned max_images[] = {
      ctx->Const.VertexProgram.MaxImageUniforms,
      ctx->Const.GeometryProgram.MaxImageUniforms,
      ctx->Const.FragmentProgram.MaxImageUniforms
   };
   STATIC_ASSERT(Elements(max_images) == MESA_SHADER_TYPES);


With those changes, the patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131211/c7d58aa4/attachment.html>