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

2014-06-11 Thread Tapani Pälli
On 06/12/2014 09:44 AM, Petri Latvala wrote:
> 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 
>> ---
>>   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() ?

yes indeed, will fix

>
>> +
>> +  /* 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?
>

Nice catch and proposal, will change this.

>> +
>> +  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,
>> +  

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

2014-06-11 Thread 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 
---
  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.");
+  

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

2014-06-04 Thread Tapani Pälli
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 
---
 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;
+
+  /* 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;
+  }
+
+  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);
-- 
1.8.3.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev