[Mesa-dev] [PATCH v2] glsl: parser changes for GL_ARB_explicit_uniform_location

[Petri Latvala]

On 06/05/2014 08:08 AM, Tapani Pälli wrote:
> Patch adds a preprocessor define for the extension and stores explicit
> location data for uniforms during AST->HIR conversion. It also sets
> layout token to be available when having the extension in place.
>
> v2: change parser check to require GLSL 330 or enabling
>      GL_ARB_explicit_attrib_location (Ian)
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>   src/glsl/ast_to_hir.cpp       | 36 ++++++++++++++++++++++++++++++++++++
>   src/glsl/glcpp/glcpp-parse.y  |  3 +++
>   src/glsl/glsl_lexer.ll        |  1 +
>   src/glsl/glsl_parser_extras.h | 15 +++++++++++++++
>   4 files changed, 55 insertions(+)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index d1c77f1..04452b8 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2182,6 +2182,42 @@ validate_explicit_location(const struct ast_type_qualifier *qual,
>   {
>      bool fail = false;
>   
> +   /* Checks for GL_ARB_explicit_uniform_location. */
> +   if (qual->flags.q.uniform) {
> +      if (!state->check_explicit_uniform_location_allowed(loc, var))
> +         return;
> +
> +      const struct gl_context *const ctx = state->ctx;
> +      unsigned max_loc = qual->location + var->type->component_slots() - 1;

var->type->uniform_locations() ?


> +
> +      /* ARB_explicit_uniform_location specification states:
> +       *
> +       *     "The explicitly defined locations and the generated locations
> +       *     must be in the range of 0 to MAX_UNIFORM_LOCATIONS minus one."
> +       *
> +       *     "Valid locations for default-block uniform variable locations
> +       *     are in the range of 0 to the implementation-defined maximum
> +       *     number of uniform locations."
> +       */
> +      if (qual->location < 0) {
> +         _mesa_glsl_error(loc, state,
> +                          "explicit location < 0 for uniform %s", var->name);
> +         return;
> +      }
> +
> +      if (max_loc >= ctx->Const.MaxUserAssignableUniformLocations) {
> +         _mesa_glsl_error(loc, state, "location qualifier for uniform %s "
> +                          ">= MAX_UNIFORM_LOCATIONS (%u)",
> +                          var->name,
> +                          ctx->Const.MaxUserAssignableUniformLocations);
> +         return;
> +      }

This error message might be too confusing.

If MAX_UNIFORM_LOCATIONS were 6:

uniform layout(location=3) float x[10];

Error: Location qualifier for uniform x >= MAX_UNIFORM_LOCATIONS(6)

"But 3 is not >= 6!" says the confused developer.

Maybe "location qualifier or automatically assigned location" or something?


> +
> +      var->data.explicit_location = true;
> +      var->data.location = qual->location;
> +      return;
> +   }
> +
>      /* Between GL_ARB_explicit_attrib_location an
>       * GL_ARB_separate_shader_objects, the inputs and outputs of any shader
>       * stage can be assigned explicit locations.  The checking here associates
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 9887583..dacb954 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -2089,6 +2089,9 @@ _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t versio
>   	      if (extensions->ARB_explicit_attrib_location)
>   	         add_builtin_define(parser, "GL_ARB_explicit_attrib_location", 1);
>   
> +	      if (extensions->ARB_explicit_uniform_location)
> +	         add_builtin_define(parser, "GL_ARB_explicit_uniform_location", 1);
> +
>   	      if (extensions->ARB_shader_texture_lod)
>   	         add_builtin_define(parser, "GL_ARB_shader_texture_lod", 1);
>   
> diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
> index 6c3f9b6..db7b1d1 100644
> --- a/src/glsl/glsl_lexer.ll
> +++ b/src/glsl/glsl_lexer.ll
> @@ -396,6 +396,7 @@ layout		{
>   		      || yyextra->AMD_conservative_depth_enable
>   		      || yyextra->ARB_conservative_depth_enable
>   		      || yyextra->ARB_explicit_attrib_location_enable
> +		      || yyextra->ARB_explicit_uniform_location_enable
>                         || yyextra->has_separate_shader_objects()
>   		      || yyextra->ARB_uniform_buffer_object_enable
>   		      || yyextra->ARB_fragment_coord_conventions_enable
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index e26460e..b73f816 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -151,6 +151,21 @@ struct _mesa_glsl_parse_state {
>         return true;
>      }
>   
> +   bool check_explicit_uniform_location_allowed(YYLTYPE *locp,
> +                                                const ir_variable *var)
> +   {
> +      if (!this->has_explicit_attrib_location() ||
> +          !this->ARB_explicit_uniform_location_enable) {
> +         _mesa_glsl_error(locp, this,
> +                          "uniform explicit location requires "
> +                          "GL_ARB_explicit_uniform_location and either "
> +                          "GL_ARB_explicit_attrib_location or GLSL 330.");
> +         return false;
> +      }
> +
> +      return true;
> +   }
> +
>      bool has_explicit_attrib_location() const
>      {
>         return ARB_explicit_attrib_location_enable || is_version(330, 300);