[Mesa-dev] [PATCH v2 4/5] glsl: Stop exposing compatibility-only builtin vars in the core profile.
Kenneth Graunke
kenneth at whitecape.org
Fri Jul 12 18:53:22 PDT 2013
On 07/12/2013 06:25 PM, Paul Berry wrote:
> GL 3.2 and GLSL 1.50 specify that several builtin variables
> (e.g. gl_FrontColor) are available only in the compatibility profile.
> GL 3.1 and GLSL 1.40 make a similar stipulation (except phrased in
> terms of ARB_compatibility).
>
> Previous versions of GLSL make no such stipulation. However, since GL
> versions 3.1 and above are allowed to be mixed with older versions of
> GLSL, we need to avoid exposing those compatibility-only builtin
> variables for older GLSL versions too. Otherwise we open up a
> loophole to allow compatibility-only features to be used in a core
> context.
>
> In the process I've moved the logic for determining whether
> compatibility-only GLSL features are active into
> _mesa_glsl_parse_state, so that we'll be able to use it to switch
> other GLSL features on and off (this will be used in the next patch,
> which fixes ftransform()).
> ---
> src/glsl/builtin_variables.cpp | 19 +++++--------------
> src/glsl/glsl_parser_extras.h | 26 ++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index 1e88b6a..ac54d41 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -342,14 +342,6 @@ private:
> exec_list * const instructions;
> struct _mesa_glsl_parse_state * const state;
> glsl_symbol_table * const symtab;
> -
> - /**
> - * True if compatibility-profile-only variables should be included. (In
> - * desktop GL, these are always included when the GLSL version is 1.30 and
> - * or below).
> - */
> - const bool compatibility;
> -
> const glsl_type * const bool_t;
> const glsl_type * const int_t;
> const glsl_type * const float_t;
> @@ -364,7 +356,6 @@ private:
> builtin_variable_generator::builtin_variable_generator(
> exec_list *instructions, struct _mesa_glsl_parse_state *state)
> : instructions(instructions), state(state), symtab(state->symbols),
> - compatibility(!state->is_version(140, 100)),
> bool_t(glsl_type::bool_type), int_t(glsl_type::int_type),
> float_t(glsl_type::float_type), vec2_t(glsl_type::vec2_type),
> vec3_t(glsl_type::vec3_type), vec4_t(glsl_type::vec4_type),
> @@ -534,7 +525,7 @@ builtin_variable_generator::generate_constants()
> add_const("gl_MaxVaryingComponents", state->Const.MaxVaryingFloats);
> }
>
> - if (compatibility) {
> + if (state->compatibility()) {
> /* Note: gl_MaxLights stopped being listed as an explicit constant in
> * GLSL 1.30, however it continues to be referred to (as a minimum size
> * for compatibility-mode uniforms) all the way up through GLSL 4.30, so
> @@ -568,7 +559,7 @@ builtin_variable_generator::generate_uniforms()
> add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
> add_uniform(array(vec4_t, VARYING_SLOT_MAX), "gl_CurrentAttribFragMESA");
>
> - if (compatibility) {
> + if (state->compatibility()) {
> add_uniform(mat4_t, "gl_ModelViewMatrix");
> add_uniform(mat4_t, "gl_ProjectionMatrix");
> add_uniform(mat4_t, "gl_ModelViewProjectionMatrix");
> @@ -650,7 +641,7 @@ builtin_variable_generator::generate_vs_special_vars()
> add_system_value(SYSTEM_VALUE_INSTANCE_ID, int_t, "gl_InstanceID");
> if (state->AMD_vertex_shader_layer_enable)
> add_output(VARYING_SLOT_LAYER, int_t, "gl_Layer");
> - if (compatibility) {
> + if (state->compatibility()) {
> add_input(VERT_ATTRIB_POS, vec4_t, "gl_Vertex");
> add_input(VERT_ATTRIB_NORMAL, vec3_t, "gl_Normal");
> add_input(VERT_ATTRIB_COLOR0, vec4_t, "gl_Color");
> @@ -706,7 +697,7 @@ builtin_variable_generator::generate_fs_special_vars()
> * 1.30, and were relegated to the compatibility profile in GLSL 4.20.
> * They were removed from GLSL ES 3.00.
> */
> - if (compatibility || !state->is_version(420, 300)) {
> + if (state->compatibility() || !state->is_version(420, 300)) {
> add_output(FRAG_RESULT_COLOR, vec4_t, "gl_FragColor");
> add_output(FRAG_RESULT_DATA0,
> array(vec4_t, state->Const.MaxDrawBuffers), "gl_FragData");
> @@ -780,7 +771,7 @@ builtin_variable_generator::generate_varyings()
> "gl_ClipDistance");
> }
>
> - if (compatibility) {
> + if (state->compatibility()) {
> ADD_VARYING(VARYING_SLOT_TEX0, array(vec4_t, 0), "gl_TexCoord");
> ADD_VARYING(VARYING_SLOT_FOGC, float_t, "gl_FogFragCoord");
> if (state->target == fragment_shader) {
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index ad8b063..7ff7faa 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -139,6 +139,32 @@ struct _mesa_glsl_parse_state {
> void process_version_directive(YYLTYPE *locp, int version,
> const char *ident);
>
> + /**
> + * Determine whether compatibility-profile-only features should be included.
> + */
> + bool compatibility() const
> + {
> + if (this->es_shader)
> + return false;
> +
> + /* Mesa doesn't support compatibility-only features in GLSL 1.40+.
> + * Note: under normal operation this if test isn't be necessary, since
> + * Mesa will reject GLSL 1.40+ shaders when using a compatibility
> + * profile. However, at compile time, we compile built-ins for all GLSL
> + * versions using a compatibility profile. So this test avoids problems
> + * at compile time.
> + */
> + if (this->is_version(140, 0))
> + return false;
Since you already dismissed ES shaders above, maybe simplify this to:
if (this->language_version >= 140)
Either way, patches 2-5 are:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> + /* GLSL versions prior to 1.40 don't have a notion of compatibility-only
> + * features, however it's still possible to compile pre-GLSL-1.40
> + * shaders in a GL-3.1+ context, so to avoid a loophole, switch off
> + * compatibility-only features if the core profile is in use.
> + */
> + return this->ctx->API != API_OPENGL_CORE;
> + }
> +
> struct gl_context *const ctx;
> void *scanner;
> exec_list translation_unit;
>
More information about the mesa-dev
mailing list