On Tue, 2015-11-17 at 19:02 +0000, Emil Velikov wrote:
> On 14 November 2015 at 13:42, Timothy Arceri <t_arc...@yahoo.com.au> wrote:
> > From: Timothy Arceri <timothy.arc...@collabora.com>
> >
> > This patch replaces the old interger constant qualifiers with either
> > the new ast_layout_expression type if the qualifier requires merging
> > or ast_expression if the qualifier can't have mulitple declarations
> > or if all but the newest qualifier is simply ignored.
> >
> > We also update the process_qualifier_constant() helper to be
> > similar to the one in the ast_layout_expression class, but in
> > this case it will be used to process the ast_expression qualifiers.
> >
> > Global shader layout qualifier validation is moved out of the parser
> > in this change as we now need to evaluate any constant expression
> > before doing the validation.
> > ---
> > src/glsl/ast.h | 33 +++++------
> > src/glsl/ast_to_hir.cpp | 126 ++++++++++++++++++++++++++++-------
> > -----
> > src/glsl/ast_type.cpp | 69 ++++++++--------------
> > src/glsl/glsl_parser.yy | 87 +++++++++------------------
> > src/glsl/glsl_parser_extras.cpp | 44 ++++++++++++--
> > 5 files changed, 195 insertions(+), 164 deletions(-)
> >
>
> > --- a/src/glsl/ast_to_hir.cpp
> > +++ b/src/glsl/ast_to_hir.cpp
>
> > @@ -4079,9 +4113,18 @@ ast_declarator_list::hir(exec_list *instructions,
> > */
> > if (decl_type && decl_type->contains_atomic()) {
> > if (type->qualifier.flags.q.explicit_binding &&
> > - type->qualifier.flags.q.explicit_offset)
> > - state->atomic_counter_offsets[type->qualifier.binding] =
> > - type->qualifier.offset;
> > + type->qualifier.flags.q.explicit_offset) {
> > + unsigned qual_binding;
> > + unsigned qual_offset;
> > + if (process_qualifier_constant(state, &loc, "binding",
> > + type->qualifier.binding,
> > + &qual_binding)
> > + && process_qualifier_constant(state, &loc, "offset",
> Nitpick: Please leave the && trailing on the previous line.
>
> > --- a/src/glsl/glsl_parser.yy
> > +++ b/src/glsl/glsl_parser.yy
>
> > /* Layout qualifiers for tessellation control shaders. */
> > if (match_layout_qualifier("vertices", $1, state) == 0) {
> > $$.flags.q.vertices = 1;
> > -
> > - if ($3 <= 0) {
> > - _mesa_glsl_error(& @3, state,
> > - "invalid vertices (%d) specified", $3);
> > - YYERROR;
> > - } else if ($3 > (int)state->Const.MaxPatchVertices) {
> > - _mesa_glsl_error(& @3, state,
> > - "vertices (%d) exceeds "
> > - "GL_MAX_PATCH_VERTICES", $3);
> > - YYERROR;
> > - } else {
> > - $$.vertices = $3;
> > - if (!state->ARB_tessellation_shader_enable &&
> > - !state->is_version(400, 0)) {
> > - _mesa_glsl_error(& @1, state,
> > - "vertices qualifier requires GLSL 4.00 or
> > "
> > - "ARB_tessellation_shader");
> > - }
> > + $$.vertices = new(ctx) ast_layout_expression(@1, $3);
> > + if (!state->ARB_tessellation_shader_enable &&
> > + !state->is_version(400, 0)) {
> > + _mesa_glsl_error(& @1, state,
> > + "vertices qualifier requires GLSL 4.00 or "
> > + "ARB_tessellation_shader");
> > }
> > }
> >
> > diff --git a/src/glsl/glsl_parser_extras.cpp
> > b/src/glsl/glsl_parser_extras.cpp
> > index 1678d89..2f870fc 100644
> > --- a/src/glsl/glsl_parser_extras.cpp
> > +++ b/src/glsl/glsl_parser_extras.cpp
> > @@ -1644,8 +1644,20 @@ set_shader_inout_layout(struct gl_shader *shader,
> > switch (shader->Stage) {
> > case MESA_SHADER_TESS_CTRL:
> > shader->TessCtrl.VerticesOut = 0;
> > - if (state->tcs_output_vertices_specified)
> > - shader->TessCtrl.VerticesOut = state->out_qualifier->vertices;
> > + if (state->tcs_output_vertices_specified) {
> > + unsigned vertices;
> > + if (state->out_qualifier->vertices->
> > + process_qualifier_constant(state, "vertices", &vertices,
> > + true)) {
> The existing code considering 0 an invalid amount, which afaict that
> is incorrect. Splitting that into a separate patch is an overkill,
> although mentioning it in the commit message is a (almost) must have
> imho.
Nope this is my bug thanks for spotting :) I'll fix this locally. I'm sure I
double checked all these before sending out. In this case I'm sure I changed
this value, maybe I changed it to the wrong value.
>
> -Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
