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

[Jakob Bornecrantz]

So I have encountered bug that should be fixed by this. In my case its
a happy little accident that I can use GL_ARB_gpu_shader5 in a
compatibility context (since I want the textureGatherOffset
functions). So while one part of me is happy that the bug exist.

Cheers, Jakob.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Ilia Mirkin]

Urgh, just realized that I also need to teach glcpp about this stuff,
otherwise we can end up with e.g. GL_ARB_gpu_shader5 defined but not
enableable in a compat context. Also a discussion with Jakob on IRC
pointed out that we currently allow GL_ARB_gpu_shader5 to be enabled
in compat contexts, but with this patch, this will no longer be
allowed. IMHO this is correct, but just wanted to point it out. Will
send a v3 that accounts for glcpp later in the week probably.

On Tue, Jun 21, 2016 at 2:24 PM, Ilia Mirkin  wrote:
> Ping? I got a R-b from Eric Engestrom (thanks!) but I was hoping some
> more experienced Mesa contributors could give this a look and see if
> this makes sense or if I'm still missing some crucial bits.
>
> On Mon, Jun 13, 2016 at 11:43 PM, 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 
>> ---
>>
>> v1 -> v2:
>>  - use a final enum to obtain number of extensions
>>  - move calculation of the gl version to be once per shader, for better reuse
>>  - bake GL version into the "supported_versions" struct
>>  - while we're at it, fix supported_versions size, it was off by 1 since ES 
>> 3.20
>>"support" was added.
>>
>>  src/Makefile.am  |   1 +
>>  src/compiler/SConscript.glsl |   2 +
>>  src/compiler/glsl/glsl_parser_extras.cpp | 244 
>> +++
>>  src/compiler/glsl/glsl_parser_extras.h   |   4 +-
>>  src/mesa/Android.libmesa_glsl_utils.mk   |   2 +
>>  src/mesa/Makefile.sources|   1 +
>>  src/mesa/main/extensions.c   |  33 +
>>  src/mesa/main/extensions.h   |   1 +
>>  src/mesa/main/extensions_table.c |  51 +++
>>  9 files changed, 190 insertions(+), 149 deletions(-)
>>  create mode 100644 src/mesa/main/extensions_table.c
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 32372da..d38f7c4 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -114,6 +114,7 @@ AM_CPPFLAGS = \
>>  noinst_LTLIBRARIES = libglsl_util.la
>>
>>  libglsl_util_la_SOURCES = \
>> +   mesa/main/extensions_table.c \
>> mesa/main/imports.c \
>> mesa/program/prog_hash_table.c \
>> mesa/program/symbol_table.c \
>> diff --git a/src/compiler/SConscript.glsl b/src/compiler/SConscript.glsl
>> index 4252ce1..31d8f6d 100644
>> --- a/src/compiler/SConscript.glsl
>> +++ b/src/compiler/SConscript.glsl
>> @@ -70,6 +70,7 @@ if env['msvc']:
>>  # Copy these files to avoid generation object files into src/mesa/program
>>  env.Prepend(CPPPATH = ['#src/mesa/main'])
>>  env.Command('glsl/imports.c', '#src/mesa/main/imports.c', Copy('$TARGET', 
>> '$SOURCE'))
>> +env.Command('glsl/extensions_table.c', '#src/mesa/main/extensions_table.c', 
>> Copy('$TARGET', '$SOURCE'))
>>  # Copy these files to avoid generation object files into src/mesa/program
>>  env.Prepend(CPPPATH = ['#src/mesa/program'])
>>  env.Command('glsl/prog_hash_table.c', 
>> '#src/mesa/program/prog_hash_table.c', Copy('$TARGET', '$SOURCE'))
>> @@ -79,6 +80,7 @@ env.Command('glsl/dummy_errors.c', 
>> '#src/mesa/program/dummy_errors.c', Copy('$TA
>>  compiler_objs = env.StaticObject(source_lists['GLSL_COMPILER_CXX_FILES'])
>>
>>  mesa_objs = env.StaticObject([
>> +'glsl/extensions_table.c',
>>  'glsl/imports.c',
>>  'glsl/prog_hash_table.c',
>>  'glsl/symbol_table.c',
>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
>> b/src/compiler/glsl/glsl_parser_extras.cpp
>> index ce2c3e8..0e9f152 100644
>> --- a/src/compiler/glsl/glsl_parser_extras.cpp
>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
>> @@ -50,6 +50,8 @@ glsl_compute_version_string(void *mem_ctx, bool is_es, 
>> unsigned version)
>>
>>  static const unsigned known_desktop_glsl_versions[] =
>> { 110, 120, 130, 140, 150, 330, 400, 410, 420, 430, 440, 450 };
>> +static const unsigned known_desktop_gl_versions[] =
>> +   {  20,  21,  30,  31,  32,  33,  40,  41,  42,  43,  44,  45 };
>>
>>
>>  _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
>> @@ -74,6 +76,7 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
>> gl_context *_ctx,
>> /* Set default language version and extensions */
>> 

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

[Ilia Mirkin]

Ping? I got a R-b from Eric Engestrom (thanks!) but I was hoping some
more experienced Mesa contributors could give this a look and see if
this makes sense or if I'm still missing some crucial bits.

On Mon, Jun 13, 2016 at 11:43 PM, 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 
> ---
>
> v1 -> v2:
>  - use a final enum to obtain number of extensions
>  - move calculation of the gl version to be once per shader, for better reuse
>  - bake GL version into the "supported_versions" struct
>  - while we're at it, fix supported_versions size, it was off by 1 since ES 
> 3.20
>"support" was added.
>
>  src/Makefile.am  |   1 +
>  src/compiler/SConscript.glsl |   2 +
>  src/compiler/glsl/glsl_parser_extras.cpp | 244 
> +++
>  src/compiler/glsl/glsl_parser_extras.h   |   4 +-
>  src/mesa/Android.libmesa_glsl_utils.mk   |   2 +
>  src/mesa/Makefile.sources|   1 +
>  src/mesa/main/extensions.c   |  33 +
>  src/mesa/main/extensions.h   |   1 +
>  src/mesa/main/extensions_table.c |  51 +++
>  9 files changed, 190 insertions(+), 149 deletions(-)
>  create mode 100644 src/mesa/main/extensions_table.c
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 32372da..d38f7c4 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -114,6 +114,7 @@ AM_CPPFLAGS = \
>  noinst_LTLIBRARIES = libglsl_util.la
>
>  libglsl_util_la_SOURCES = \
> +   mesa/main/extensions_table.c \
> mesa/main/imports.c \
> mesa/program/prog_hash_table.c \
> mesa/program/symbol_table.c \
> diff --git a/src/compiler/SConscript.glsl b/src/compiler/SConscript.glsl
> index 4252ce1..31d8f6d 100644
> --- a/src/compiler/SConscript.glsl
> +++ b/src/compiler/SConscript.glsl
> @@ -70,6 +70,7 @@ if env['msvc']:
>  # Copy these files to avoid generation object files into src/mesa/program
>  env.Prepend(CPPPATH = ['#src/mesa/main'])
>  env.Command('glsl/imports.c', '#src/mesa/main/imports.c', Copy('$TARGET', 
> '$SOURCE'))
> +env.Command('glsl/extensions_table.c', '#src/mesa/main/extensions_table.c', 
> Copy('$TARGET', '$SOURCE'))
>  # Copy these files to avoid generation object files into src/mesa/program
>  env.Prepend(CPPPATH = ['#src/mesa/program'])
>  env.Command('glsl/prog_hash_table.c', '#src/mesa/program/prog_hash_table.c', 
> Copy('$TARGET', '$SOURCE'))
> @@ -79,6 +80,7 @@ env.Command('glsl/dummy_errors.c', 
> '#src/mesa/program/dummy_errors.c', Copy('$TA
>  compiler_objs = env.StaticObject(source_lists['GLSL_COMPILER_CXX_FILES'])
>
>  mesa_objs = env.StaticObject([
> +'glsl/extensions_table.c',
>  'glsl/imports.c',
>  'glsl/prog_hash_table.c',
>  'glsl/symbol_table.c',
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index ce2c3e8..0e9f152 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -50,6 +50,8 @@ glsl_compute_version_string(void *mem_ctx, bool is_es, 
> unsigned version)
>
>  static const unsigned known_desktop_glsl_versions[] =
> { 110, 120, 130, 140, 150, 330, 400, 410, 420, 430, 440, 450 };
> +static const unsigned known_desktop_gl_versions[] =
> +   {  20,  21,  30,  31,  32,  33,  40,  41,  42,  43,  44,  45 };
>
>
>  _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
> @@ -74,6 +76,7 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
> gl_context *_ctx,
> /* Set default language version and extensions */
> this->language_version = 110;
> this->forced_language_version = ctx->Const.ForceGLSLVersion;
> +   this->gl_version = 20;
> this->es_shader = false;
> this->ARB_texture_rectangle_enable = true;
>
> @@ -194,9 +197,9 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
> gl_context *_ctx,
> this->subroutine_types = NULL;
>
> /* supported_versions should be large enough to support the known desktop
> -* GLSL versions plus 3 GLES versions (ES 1.00, ES 3.00, and ES 3.10))
> +* GLSL versions plus 4 GLES versions (ES 1.00, ES 3.00, ES 3.10, ES 3.20)
>  */
> -   STATIC_ASSERT((ARRAY_SIZE(known_desktop_glsl_versions) + 3) ==
> 

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

[Eric Engestrom]

On Mon, Jun 13, 2016 at 11:43:57PM -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 
> ---
> 
> v1 -> v2:
>  - use a final enum to obtain number of extensions
>  - move calculation of the gl version to be once per shader, for better reuse
>  - bake GL version into the "supported_versions" struct
>  - while we're at it, fix supported_versions size, it was off by 1 since ES 
> 3.20
>"support" was added.
> 

Looks all good to me :)
Reviewed-by: Eric Engestrom 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Ilia Mirkin]

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 
---

v1 -> v2:
 - use a final enum to obtain number of extensions
 - move calculation of the gl version to be once per shader, for better reuse
 - bake GL version into the "supported_versions" struct
 - while we're at it, fix supported_versions size, it was off by 1 since ES 3.20
   "support" was added.

 src/Makefile.am  |   1 +
 src/compiler/SConscript.glsl |   2 +
 src/compiler/glsl/glsl_parser_extras.cpp | 244 +++
 src/compiler/glsl/glsl_parser_extras.h   |   4 +-
 src/mesa/Android.libmesa_glsl_utils.mk   |   2 +
 src/mesa/Makefile.sources|   1 +
 src/mesa/main/extensions.c   |  33 +
 src/mesa/main/extensions.h   |   1 +
 src/mesa/main/extensions_table.c |  51 +++
 9 files changed, 190 insertions(+), 149 deletions(-)
 create mode 100644 src/mesa/main/extensions_table.c

diff --git a/src/Makefile.am b/src/Makefile.am
index 32372da..d38f7c4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -114,6 +114,7 @@ AM_CPPFLAGS = \
 noinst_LTLIBRARIES = libglsl_util.la
 
 libglsl_util_la_SOURCES = \
+   mesa/main/extensions_table.c \
mesa/main/imports.c \
mesa/program/prog_hash_table.c \
mesa/program/symbol_table.c \
diff --git a/src/compiler/SConscript.glsl b/src/compiler/SConscript.glsl
index 4252ce1..31d8f6d 100644
--- a/src/compiler/SConscript.glsl
+++ b/src/compiler/SConscript.glsl
@@ -70,6 +70,7 @@ if env['msvc']:
 # Copy these files to avoid generation object files into src/mesa/program
 env.Prepend(CPPPATH = ['#src/mesa/main'])
 env.Command('glsl/imports.c', '#src/mesa/main/imports.c', Copy('$TARGET', 
'$SOURCE'))
+env.Command('glsl/extensions_table.c', '#src/mesa/main/extensions_table.c', 
Copy('$TARGET', '$SOURCE'))
 # Copy these files to avoid generation object files into src/mesa/program
 env.Prepend(CPPPATH = ['#src/mesa/program'])
 env.Command('glsl/prog_hash_table.c', '#src/mesa/program/prog_hash_table.c', 
Copy('$TARGET', '$SOURCE'))
@@ -79,6 +80,7 @@ env.Command('glsl/dummy_errors.c', 
'#src/mesa/program/dummy_errors.c', Copy('$TA
 compiler_objs = env.StaticObject(source_lists['GLSL_COMPILER_CXX_FILES'])
 
 mesa_objs = env.StaticObject([
+'glsl/extensions_table.c',
 'glsl/imports.c',
 'glsl/prog_hash_table.c',
 'glsl/symbol_table.c',
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index ce2c3e8..0e9f152 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -50,6 +50,8 @@ glsl_compute_version_string(void *mem_ctx, bool is_es, 
unsigned version)
 
 static const unsigned known_desktop_glsl_versions[] =
{ 110, 120, 130, 140, 150, 330, 400, 410, 420, 430, 440, 450 };
+static const unsigned known_desktop_gl_versions[] =
+   {  20,  21,  30,  31,  32,  33,  40,  41,  42,  43,  44,  45 };
 
 
 _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
@@ -74,6 +76,7 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
gl_context *_ctx,
/* Set default language version and extensions */
this->language_version = 110;
this->forced_language_version = ctx->Const.ForceGLSLVersion;
+   this->gl_version = 20;
this->es_shader = false;
this->ARB_texture_rectangle_enable = true;
 
@@ -194,9 +197,9 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
gl_context *_ctx,
this->subroutine_types = NULL;
 
/* supported_versions should be large enough to support the known desktop
-* GLSL versions plus 3 GLES versions (ES 1.00, ES 3.00, and ES 3.10))
+* GLSL versions plus 4 GLES versions (ES 1.00, ES 3.00, ES 3.10, ES 3.20)
 */
-   STATIC_ASSERT((ARRAY_SIZE(known_desktop_glsl_versions) + 3) ==
+   STATIC_ASSERT((ARRAY_SIZE(known_desktop_glsl_versions) + 4) ==
  ARRAY_SIZE(this->supported_versions));
 
/* Populate the list of supported GLSL versions */
@@ -211,6 +214,8 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
gl_context *_ctx,
  if (known_desktop_glsl_versions[i] <= ctx->Const.GLSLVersion) {
 this->supported_versions[this->num_supported_versions].ver
= known_desktop_glsl_versions[i];
+