On 30 November 2012 11:52, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">From: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> Note that GLSL 1.00 is selected using "#version 100", so "#version 100 es" is prohibited. v2: Check for GLES3 before allowing '#version 300 es' </div><div class="im">v3: Make sure a correct language_version is set in _mesa_glsl_parse_state::process_version_directive. </div><div class="im">Signed-off-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> --- </div><div class="im">This fixes the invalid-version.vert recently posted to the piglit mailing list. A later patch adds (valid) assertions in _mesa_glsl_parse_state::process_version_directive that fail if language_version is bogus. I plan to post some unit tests for _mesa_glsl_parse_state::process_version_directive shortly. </div></blockquote><div> This fix looks good to me. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im"> src/glsl/glsl_parser.yy | 6 ++- src/glsl/glsl_parser_extras.cpp | 105 ++++++++++++++++++++++++++++++---------- </div> src/glsl/glsl_parser_extras.h | 3 +- 3 files changed, 86 insertions(+), 28 deletions(-) <div class="im"> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy index b15e1d1..d938765 100644 --- a/src/glsl/glsl_parser.yy +++ b/src/glsl/glsl_parser.yy @@ -261,8 +261,12 @@ version_statement: /* blank - no #version specified: defaults are already set */ | VERSION_TOK INTCONSTANT EOL { - state->process_version_directive(&@2, $2); + state->process_version_directive(&@2, $2, NULL); } + | VERSION_TOK INTCONSTANT any_identifier EOL + { + state->process_version_directive(&@2, $2, $3); + } ; pragma_statement: diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp </div>index cc33a07..9efeb12 100644 <div><div class="h5">--- a/src/glsl/glsl_parser_extras.cpp +++ b/src/glsl/glsl_parser_extras.cpp @@ -178,38 +178,72 @@ _mesa_glsl_parse_state::check_version(unsigned required_glsl_version, * Process a GLSL #version directive. * * \param version is the integer that follows the #version token. + * + * \param ident is a string identifier that follows the integer, if any is + * present. Otherwise NULL. */ void -_mesa_glsl_parse_state::process_version_directive(YYLTYPE *locp, int version) +_mesa_glsl_parse_state::process_version_directive(YYLTYPE *locp, int version, + const char *ident) { + bool es_token_present = false; + if (ident) { + if (strcmp(ident, "es") == 0) { + es_token_present = true; + } else { + _mesa_glsl_error(locp, this, + "Illegal text following version number\n"); + } + } + bool supported = false; - switch (version) { - case 100: + if (es_token_present) { this->es_shader = true; - supported = this->ctx->API == API_OPENGLES2 || - this->ctx->Extensions.ARB_ES2_compatibility; - break; - case 110: - case 120: - /* FINISHME: Once the OpenGL 3.0 'forward compatible' context or - * the OpenGL 3.2 Core context is supported, this logic will need - * change. Older versions of GLSL are no longer supported - * outside the compatibility contexts of 3.x. - */ - case 130: - case 140: - case 150: - case 330: - case 400: - case 410: - case 420: - supported = _mesa_is_desktop_gl(this->ctx) && - ((unsigned) version) <= this->ctx->Const.GLSLVersion; - break; - default: - supported = false; - break; + switch (version) { + case 100: + _mesa_glsl_error(locp, this, + "GLSL 1.00 ES should be selected using " + "`#version 100'\n"); + supported = this->ctx->API == API_OPENGLES2 || + this->ctx->Extensions.ARB_ES2_compatibility; + break; + case 300: + supported = _mesa_is_gles3(this->ctx) || + this->ctx->Extensions.ARB_ES3_compatibility; + break; + default: + supported = false; + break; + } + } else { + switch (version) { + case 100: + this->es_shader = true; + supported = this->ctx->API == API_OPENGLES2 || + this->ctx->Extensions.ARB_ES2_compatibility; + break; + case 110: + case 120: + /* FINISHME: Once the OpenGL 3.0 'forward compatible' context or + * the OpenGL 3.2 Core context is supported, this logic will need + * change. Older versions of GLSL are no longer supported + * outside the compatibility contexts of 3.x. + */ + case 130: + case 140: + case 150: + case 330: + case 400: + case 410: + case 420: + supported = _mesa_is_desktop_gl(this->ctx) && + ((unsigned) version) <= this->ctx->Const.GLSLVersion; + break; + default: + supported = false; + break; + } } this->language_version = version; </div></div><div class="im">@@ -219,6 +253,25 @@ _mesa_glsl_parse_state::process_version_directive(YYLTYPE *locp, int version) "Supported versions are: %s\n", this->get_version_string(), this->supported_version_string); + + /* On exit, the language_version must be set to a valid value. + * Later calls to _mesa_glsl_initialize_types will misbehave if + * the version is invalid. + */ + switch (this->ctx->API) { + case API_OPENGL: + case API_OPENGL_CORE: </div>+ this->language_version = this->ctx->Const.GLSLVersion; + break; + <div class="im HOEnZb">+ case API_OPENGLES: + assert(!"Should not get here."); + /* FALLTHROUGH */ + + case API_OPENGLES2: + this->language_version = 100; + break; + } } if (this->language_version >= 140) { </div><div class="HOEnZb"><div class="h5">diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h index 5ee7bc1..210f262 100644 --- a/src/glsl/glsl_parser_extras.h +++ b/src/glsl/glsl_parser_extras.h @@ -136,7 +136,8 @@ struct _mesa_glsl_parse_state { return check_version(130, 300, locp, "bit-wise operations are forbidden"); } - void process_version_directive(YYLTYPE *locp, int version); + void process_version_directive(YYLTYPE *locp, int version, + const char *ident); struct gl_context *const ctx; void *scanner; -- 1.7.11.7 </div></div></blockquote></div> </div>