[Mesa-dev] [PATCH] glsl: reuse main extension table to appropriate restrict extensions

[Eric Engestrom]

On Sun, Jun 12, 2016 at 07:23:41PM -0400, Ilia Mirkin wrote:
> Previously we were only restricting based on ES/non-ES-ness and whether
> the overall enable bit had been flipped on. However we have been adding
> more fine-grained restrictions, such as based on compat profiles, as
> well as specific ES versions. Most of the time this doesn't matter, but
> it can create awkward situations and duplication of logic.
> 
> Here we separate the main extension table into a separate object file,
> linked to the glsl compiler, which makes use of it with a custom
> function which takes the ES-ness of the shader into account (thus
> allowing desktop shaders to properly use ES extensions that would
> otherwise have been disallowed.)
> 
> The effect of this change should be nil in most cases.
> 
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---

[snip]

> diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
> index fa50cb6..3dc9c50 100644
> --- a/src/mesa/main/extensions.c
> +++ b/src/mesa/main/extensions.c
> @@ -49,25 +49,15 @@ static char *extra_extensions = NULL;
>  #define o(x) offsetof(struct gl_extensions, x)
>  
>  
> -/**
> - * \brief Table of supported OpenGL extensions for all API's.
> - */
> -const struct mesa_extension _mesa_extension_table[] = {
> +static bool extension_table_size[] = {
>  #define EXT(name_str, driver_cap, gll_ver, glc_ver, gles_ver, gles2_ver, yyyy) \
> -        { .name = "GL_" #name_str, .offset = o(driver_cap), \
> -          .version = { \
> -            [API_OPENGL_COMPAT] = gll_ver, \
> -            [API_OPENGL_CORE]   = glc_ver, \
> -            [API_OPENGLES]      = gles_ver, \
> -            [API_OPENGLES2]     = gles2_ver, \
> -           }, \
> -           .year = yyyy \
> -        },
> +   0,
> +
>  #include "extensions_table.h"
>  #undef EXT
>  };
>  
> -static bool disabled_extensions[ARRAY_SIZE(_mesa_extension_table)];
> +static bool disabled_extensions[ARRAY_SIZE(extension_table_size)];

/me thinks `extension_table_size` should contain the extension table size...

Maybe this?

	static bool extension_table_for_size[] = {
		/* ... */
	};
	static enum { extension_table_size = ARRAY_SIZE(extension_table_for_size) };
	static bool disabled_extensions[extension_table_size];

>  
>  /**
>   * Given an extension name, lookup up the corresponding member of struct
> @@ -85,7 +75,7 @@ name_to_index(const char* name)
>     if (name == 0)
>        return -1;
>  
> -   for (i = 0; i < ARRAY_SIZE(_mesa_extension_table); ++i) {
> +   for (i = 0; i < ARRAY_SIZE(extension_table_size); ++i) {

This (and following) is now simply:

	for (i = 0; i < extension_table_size; ++i) {

>        if (strcmp(name, _mesa_extension_table[i].name) == 0)
>  	 return i;
>     }

[snip]

> diff --git a/src/mesa/main/extensions_table.c b/src/mesa/main/extensions_table.c
> new file mode 100644
> index 0000000..1e37fbc
> --- /dev/null
> +++ b/src/mesa/main/extensions_table.c
> @@ -0,0 +1,51 @@
> +/*
> + * Mesa 3-D graphics library
> + *
> + * Copyright (C) 1999-2008  Brian Paul   All Rights Reserved.
> + * Copyright (C) 2009  VMware, Inc.  All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included
> + * in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "main/mtypes.h"
> +#include "main/extensions.h"
> +
> +/**
> + * Given a member \c x of struct gl_extensions, return offset of
> + * \c x in bytes.
> + */
> +#define o(x) offsetof(struct gl_extensions, x)
> +
> +/**
> + * \brief Table of supported OpenGL extensions for all API's.
> + */
> +const struct mesa_extension _mesa_extension_table[] = {
> +#define EXT(name_str, driver_cap, gll_ver, glc_ver, gles_ver, gles2_ver, yyyy) \
> +        { .name = "GL_" #name_str, .offset = o(driver_cap), \
> +          .version = { \
> +            [API_OPENGL_COMPAT] = gll_ver, \
> +            [API_OPENGL_CORE]   = glc_ver, \
> +            [API_OPENGLES]      = gles_ver, \
> +            [API_OPENGLES2]     = gles2_ver, \
> +           }, \
> +           .year = yyyy \
> +        },
> +#include "extensions_table.h"
> +#undef EXT

I'm not sure this macro is useful in the first place, but:

	#undef o


With these issues fixed:
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

> +};
> -- 
> 2.7.3