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

[Paul Berry]

On 12 December 2013 08:32, Francisco Jerez <currojerez at riseup.net> wrote:

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

Argh, that's frustrating.  There's no good reason to have two enums.  I'll
send out a patch series that consolidates them into one.


>
>
> >
> >> +   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/20131216/f44ea29e/attachment.html>