On 05/21/2014 05:43 PM, Tapani wrote:
On 05/21/2014 01:27 AM, Ian Romanick wrote:On 05/19/2014 10:08 PM, Tapani wrote:Currently, it should only bee matrices and structures containing matrices that need different treatment. I think dvec3 and dvec4 (when we add supportOn 05/19/2014 08:18 PM, Ian Romanick wrote:On 04/09/2014 02:56 AM, Tapani Pälli wrote:Patch adds a preprocessor define for the extension and stores explicitlocation data for uniforms during AST->HIR conversion. It also sets layout token to be available when having the extension in place.Signed-off-by: Tapani Pälli <tapani.pa...@intel.com> --- src/glsl/ast_to_hir.cpp | 37 +++++++++++++++++++++++++++++++++++++ src/glsl/glcpp/glcpp-parse.y | 3 +++ src/glsl/glsl_lexer.ll | 1 + src/glsl/glsl_parser_extras.h | 14 ++++++++++++++ 4 files changed, 55 insertions(+) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 8d55ee3..7431ad7 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -2170,6 +2170,43 @@ validate_explicit_location(const struct ast_type_qualifier *qual, { bool fail = false; + /* Checks for GL_ARB_explicit_uniform_location. */ + if (qual->flags.q.uniform) { +Extra blank line.oops+ 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;I think that over counts for this purpose, and we can blame confusingnomenclature. component_slots for a mat4 is 4, so a mat4 uniform counts4*4 against the GL_MAX_VERTEX_UNIFORM_COMPONENTS limit. However, it only has one "location" (as returned by glGetUniformLocation), so it only counts 1 against the GL_MAX_UNIFORM_LOCATIONS limit.I see, I was considering structs and arrays when writing this part and forgot about matrix. I assume matrix is the only special case here though? Everything else gets correct location count value via component_slots().for doubles) will also take multiple slots.Ok, I'll add additional helper to glsl_type for this as it is needed when assigning locations too.+ + /* 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 anyshader * stage can be assigned explicit locations. The checking here associatesdiff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.yindex f28d853..6d42138 100644 --- a/src/glsl/glcpp/glcpp-parse.y +++ b/src/glsl/glcpp/glcpp-parse.y @@ -2087,6 +2087,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 7602351..83f0b6d 100644 --- a/src/glsl/glsl_lexer.ll +++ b/src/glsl/glsl_lexer.ll @@ -393,6 +393,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 c53c583..20879a0 100644 --- a/src/glsl/glsl_parser_extras.h +++ b/src/glsl/glsl_parser_extras.h @@ -152,6 +152,20 @@ struct _mesa_glsl_parse_state { return true; } + bool check_explicit_uniform_location_allowed(YYLTYPE *locp,+ const ir_variable *var)+ { + /* Requires OpenGL 3.3 or ARB_explicit_attrib_location. */ + if (ctx->Version < 33 && !ctx->Extensions.ARB_explicit_attrib_location) {+ _mesa_glsl_error(locp, this, "%s explicit location requires "+ "GL_ARB_explicit_attrib_location extension " + "or OpenGL 3.3", mode_string(var));Many copy-and-paste bugs. :) Explicit uniform locations aren't added until 4.3.It may look copy-paste but the specification states that "Requires OpenGL 3.3 or ARB_explicit_attrib_location": https://www.opengl.org/registry/specs/ARB/explicit_uniform_location.txt Using 4.3 capable driver will pass this check correctly.Oh right, because it relies on 3.3 or ARB_explicit_attib_location to add the layout keyword. Some comments explaining that this isn't a copy-and-paste bug will prevent the next person from also thinking that it is. :) But this code should check the version (and extension) bits set in the shader, not what's enabled in the context. How about: bool check_explicit_attrib_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; }Sure, this is fine by me. I'll send new patches soon.
Or maybe fine with some changes since my piglit tests won't pass with this change (for those explicit attrib location is not available for some reason (!)), will take a look.
+ return false; + } + + return true; + } + bool has_explicit_attrib_location() const {return ARB_explicit_attrib_location_enable || is_version(330,300);_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
