[Mesa-dev] [PATCH 2/2] glsl: never enable unsupported extensions (bug 38015)

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/15/2011 04:26 PM, Paul Berry wrote:
> For some extensions, _mesa_glsl_process_extension() only checked
> whether the extension was supported *after* enabling it.  Modified the
> logic to always check first, and only enable supported extensions.
> 
> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38015 and piglit
> test glsl-link-bug38015.

This looks mostly good.  It ended up being more code than I had
expected, but I think it's all necessary.  I think it would be better to
make the default state of unsupported be true (instead of false).  That
should cut the total amount of code.  That will help when people need to
cut-and-paste for new extensions.

The first patch looks fine as-is, and I'll push it shortly.

> ---
>  src/glsl/glsl_parser_extras.cpp |  117 +++++++++++++++++++++++----------------
>  1 files changed, 70 insertions(+), 47 deletions(-)
> 
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index d9aa300..afd8679 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -211,73 +211,96 @@ _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp,
>  	 state->ARB_draw_buffers_warn = (ext_mode == extension_warn);
>        }
>     } else if (strcmp(name, "GL_ARB_draw_instanced") == 0) {
> -      state->ARB_draw_instanced_enable = (ext_mode != extension_disable);
> -      state->ARB_draw_instanced_warn = (ext_mode == extension_warn);
> -
>        /* This extension is only supported in vertex shaders.
>         */
> -      unsupported = (state->target != vertex_shader)
> -	 ||  !state->extensions->ARB_draw_instanced;
> +      if ((state->target != vertex_shader)
> +          ||  !state->extensions->ARB_draw_instanced) {
> +         unsupported = true;
> +      } else {
> +         state->ARB_draw_instanced_enable = (ext_mode != extension_disable);
> +         state->ARB_draw_instanced_warn = (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_ARB_explicit_attrib_location") == 0) {
> -      state->ARB_explicit_attrib_location_enable =
> -	 (ext_mode != extension_disable);
> -      state->ARB_explicit_attrib_location_warn =
> -	 (ext_mode == extension_warn);
> -
> -      unsupported = !state->extensions->ARB_explicit_attrib_location;
> +      if (!state->extensions->ARB_explicit_attrib_location) {
> +         unsupported = true;
> +      } else {
> +         state->ARB_explicit_attrib_location_enable =
> +            (ext_mode != extension_disable);
> +         state->ARB_explicit_attrib_location_warn =
> +            (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_ARB_fragment_coord_conventions") == 0) {
> -      state->ARB_fragment_coord_conventions_enable =
> -	 (ext_mode != extension_disable);
> -      state->ARB_fragment_coord_conventions_warn =
> -	 (ext_mode == extension_warn);
> -
> -      unsupported = !state->extensions->ARB_fragment_coord_conventions;
> +      if (!state->extensions->ARB_fragment_coord_conventions) {
> +         unsupported = true;
> +      } else {
> +         state->ARB_fragment_coord_conventions_enable =
> +            (ext_mode != extension_disable);
> +         state->ARB_fragment_coord_conventions_warn =
> +            (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_ARB_texture_rectangle") == 0) {
>        state->ARB_texture_rectangle_enable = (ext_mode != extension_disable);
>        state->ARB_texture_rectangle_warn = (ext_mode == extension_warn);
>     } else if (strcmp(name, "GL_EXT_texture_array") == 0) {
> -      state->EXT_texture_array_enable = (ext_mode != extension_disable);
> -      state->EXT_texture_array_warn = (ext_mode == extension_warn);
> -
> -      unsupported = !state->extensions->EXT_texture_array;
> +      if (!state->extensions->EXT_texture_array) {
> +         unsupported = true;
> +      } else {
> +         state->EXT_texture_array_enable = (ext_mode != extension_disable);
> +         state->EXT_texture_array_warn = (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_ARB_shader_texture_lod") == 0) {
> -      /* Force ARB_texture_rectangle to be on so sampler2DRects are defined */
> -      state->ARB_texture_rectangle_enable = true;
> -
> -      state->ARB_shader_texture_lod_enable = (ext_mode != extension_disable);
> -      state->ARB_shader_texture_lod_warn = (ext_mode == extension_warn);
> +      if (!state->extensions->ARB_shader_texture_lod) {
> +         unsupported = true;
> +      } else {
> +         /* Force ARB_texture_rectangle to be on so sampler2DRects are defined
> +          */
> +         state->ARB_texture_rectangle_enable = true;
>  
> -      unsupported = !state->extensions->ARB_shader_texture_lod;
> +         state->ARB_shader_texture_lod_enable
> +            = (ext_mode != extension_disable);
> +         state->ARB_shader_texture_lod_warn = (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_ARB_shader_stencil_export") == 0) {
> -      state->ARB_shader_stencil_export_enable = (ext_mode != extension_disable);
> -      state->ARB_shader_stencil_export_warn = (ext_mode == extension_warn);
> -
>        /* This extension is only supported in fragment shaders.
>         */
> -      unsupported = (state->target != fragment_shader)
> -	 || !state->extensions->ARB_shader_stencil_export;
> +      if ((state->target != fragment_shader)
> +          || !state->extensions->ARB_shader_stencil_export) {
> +         unsupported = true;
> +      } else {
> +         state->ARB_shader_stencil_export_enable
> +            = (ext_mode != extension_disable);
> +         state->ARB_shader_stencil_export_warn = (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_AMD_conservative_depth") == 0) {
> -      /* The AMD_conservative spec does not forbid requiring the extension in
> -       * the vertex shader.
> -       */
> -      state->AMD_conservative_depth_enable = (ext_mode != extension_disable);
> -      state->AMD_conservative_depth_warn = (ext_mode == extension_warn);
> -      unsupported = !state->extensions->AMD_conservative_depth;
> +      if (!state->extensions->AMD_conservative_depth) {
> +         unsupported = true;
> +      } else {
> +         /* The AMD_conservative spec does not forbid requiring the extension
> +          * in the vertex shader.
> +          */
> +         state->AMD_conservative_depth_enable = (ext_mode != extension_disable);
> +         state->AMD_conservative_depth_warn = (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_AMD_shader_stencil_export") == 0) {
> -      state->AMD_shader_stencil_export_enable = (ext_mode != extension_disable);
> -      state->AMD_shader_stencil_export_warn = (ext_mode == extension_warn);
> -
>        /* This extension is only supported in fragment shaders.
>         * Both the ARB and AMD variants share the same ARB flag
>         * in gl_extensions.
>         */
> -      unsupported = (state->target != fragment_shader)
> -	 || !state->extensions->ARB_shader_stencil_export;
> +      if ((state->target != fragment_shader)
> +          || !state->extensions->ARB_shader_stencil_export) {
> +         unsupported = true;
> +      } else {
> +         state->AMD_shader_stencil_export_enable
> +            = (ext_mode != extension_disable);
> +         state->AMD_shader_stencil_export_warn = (ext_mode == extension_warn);
> +      }
>     } else if (strcmp(name, "GL_OES_texture_3D") == 0 && state->es_shader) {
> -      state->OES_texture_3D_enable = (ext_mode != extension_disable);
> -      state->OES_texture_3D_warn = (ext_mode == extension_warn);
> -
> -      unsupported = !state->extensions->EXT_texture3D;
> +      if (!state->extensions->EXT_texture3D) {
> +         unsupported = true;
> +      } else {
> +         state->OES_texture_3D_enable = (ext_mode != extension_disable);
> +         state->OES_texture_3D_warn = (ext_mode == extension_warn);
> +      }
>     } else {
>        unsupported = true;
>     }

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk4Dir4ACgkQX1gOwKyEAw+negCZAVfX5aPiFBNFw5/IvseQLnE6
BrcAmgKpGwmZuHq5aOq6b8k2R0T1KxO3
=kXMW
-----END PGP SIGNATURE-----