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

[Francisco Jerez]

Paul Berry <stereotype441 at gmail.com> writes:

> 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.
>

Neither of the _mesa_glsl_shader_target_name() overloads accept a
gl_shader_type as argument, the first one takes a GLenum, the second one
takes a _mesa_glsl_parser_targets enumerant -- Yes, the enum values
happen to match Mesa's gl_shader_type, but...

>
>> +   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 --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131212/c3f5287d/attachment.pgp>