On Sun, Aug 28, 2016 at 7:10 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
> ---
>  src/compiler/glsl/glsl_parser_extras.cpp | 57 
> +++++++++++++++++++++++---------
>  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>  src/mesa/main/extensions_table.h         |  2 ++
>  src/mesa/main/mtypes.h                   |  1 +
>  4 files changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index b33cd3a..a44f014 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -523,6 +523,11 @@ struct _mesa_glsl_extension {
>     const char *name;
>
>     /**
> +    * Whether this extension is a part of AEP
> +    */
> +   bool aep;

In reviewing this patch, I developed some doubts whether this is the
right approach. Looking at the list of extensions in the AEP spec,
it's difficult to compare with our code to confirm that they're all
included.

Take OES_sample_shading for instance. It's enabled if
OES_sample_variables is. OES_sample_variables is also in AEP, so using
EXT_AEP seems fine, but it takes a bit of analysis.

Or take EXT_copy_image. It's enabled based on OES_copy_image. Does
that mean that OES_copy_image should use EXT_AEP even though it's not
part of AEP?

I wonder if handling AEP in version.c like we do with the ver_3_0
variables and the list of extensions would be better.

> +
> +   /**
>      * Predicate that checks whether the relevant extension is available for
>      * this context.
>      */
> @@ -565,9 +570,14 @@ has_##name_str(const struct gl_context *ctx, gl_api api, 
> uint8_t version) \
>  #undef EXT
>
>  #define EXT(NAME)                                           \
> -   { "GL_" #NAME, has_##NAME,                         \
> -         &_mesa_glsl_parse_state::NAME##_enable,            \
> -         &_mesa_glsl_parse_state::NAME##_warn }
> +   { "GL_" #NAME, false, has_##NAME,                        \
> +     &_mesa_glsl_parse_state::NAME##_enable,                \
> +     &_mesa_glsl_parse_state::NAME##_warn }
> +
> +#define EXT_AEP(NAME)                                       \
> +   { "GL_" #NAME, true, has_##NAME,                         \
> +     &_mesa_glsl_parse_state::NAME##_enable,                \
> +     &_mesa_glsl_parse_state::NAME##_warn }
>
>  /**
>   * Table of extensions that can be enabled/disabled within a shader,
> @@ -623,7 +633,7 @@ static const _mesa_glsl_extension 
> _mesa_glsl_supported_extensions[] = {
>
>     /* KHR extensions go here, sorted alphabetically.
>      */
> -   EXT(KHR_blend_equation_advanced),
> +   EXT_AEP(KHR_blend_equation_advanced),
>
>     /* OES extensions go here, sorted alphabetically.
>      */
> @@ -632,17 +642,17 @@ static const _mesa_glsl_extension 
> _mesa_glsl_supported_extensions[] = {
>     EXT(OES_geometry_shader),
>     EXT(OES_gpu_shader5),
>     EXT(OES_primitive_bounding_box),
> -   EXT(OES_sample_variables),
> -   EXT(OES_shader_image_atomic),
> +   EXT_AEP(OES_sample_variables),
> +   EXT_AEP(OES_shader_image_atomic),
>     EXT(OES_shader_io_blocks),
> -   EXT(OES_shader_multisample_interpolation),
> +   EXT_AEP(OES_shader_multisample_interpolation),
>     EXT(OES_standard_derivatives),
>     EXT(OES_tessellation_point_size),
>     EXT(OES_tessellation_shader),
>     EXT(OES_texture_3D),
>     EXT(OES_texture_buffer),
>     EXT(OES_texture_cube_map_array),
> -   EXT(OES_texture_storage_multisample_2d_array),
> +   EXT_AEP(OES_texture_storage_multisample_2d_array),
>
>     /* All other extensions go here, sorted alphabetically.
>      */
> @@ -651,23 +661,24 @@ static const _mesa_glsl_extension 
> _mesa_glsl_supported_extensions[] = {
>     EXT(AMD_shader_trinary_minmax),
>     EXT(AMD_vertex_shader_layer),
>     EXT(AMD_vertex_shader_viewport_index),
> +   EXT(ANDROID_extension_pack_es31a),
>     EXT(EXT_blend_func_extended),
>     EXT(EXT_draw_buffers),
>     EXT(EXT_clip_cull_distance),
>     EXT(EXT_geometry_point_size),
> -   EXT(EXT_geometry_shader),
> -   EXT(EXT_gpu_shader5),
> -   EXT(EXT_primitive_bounding_box),
> +   EXT_AEP(EXT_geometry_shader),
> +   EXT_AEP(EXT_gpu_shader5),
> +   EXT_AEP(EXT_primitive_bounding_box),
>     EXT(EXT_separate_shader_objects),
>     EXT(EXT_shader_framebuffer_fetch),
>     EXT(EXT_shader_integer_mix),
> -   EXT(EXT_shader_io_blocks),
> +   EXT_AEP(EXT_shader_io_blocks),
>     EXT(EXT_shader_samples_identical),
>     EXT(EXT_tessellation_point_size),
> -   EXT(EXT_tessellation_shader),
> +   EXT_AEP(EXT_tessellation_shader),
>     EXT(EXT_texture_array),
> -   EXT(EXT_texture_buffer),
> -   EXT(EXT_texture_cube_map_array),
> +   EXT_AEP(EXT_texture_buffer),
> +   EXT_AEP(EXT_texture_cube_map_array),
>     EXT(MESA_shader_integer_functions),

I think we're missing:

KHR_texture_compression_astc_ldr
OES_texture_stencil8
EXT_copy_image
EXT_draw_buffers_indexed
EXT_texture_border_clamp
EXT_texture_sRGB_decode

>  };
>
> @@ -713,7 +724,6 @@ static const _mesa_glsl_extension *find_extension(const 
> char *name)
>     return NULL;
>  }
>
> -
>  bool
>  _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp,
>                              const char *behavior_string, YYLTYPE 
> *behavior_locp,
> @@ -768,6 +778,21 @@ _mesa_glsl_process_extension(const char *name, YYLTYPE 
> *name_locp,
>        const _mesa_glsl_extension *extension = find_extension(name);
>        if (extension && extension->compatible_with_state(state, api, 
> gl_version)) {
>           extension->set_flags(state, behavior);
> +         if (extension->available_pred == has_ANDROID_extension_pack_es31a) {
> +            for (unsigned i = 0;
> +                 i < ARRAY_SIZE(_mesa_glsl_supported_extensions); ++i) {
> +               const _mesa_glsl_extension *extension =
> +                  &_mesa_glsl_supported_extensions[i];
> +
> +               if (!extension->aep)
> +                  continue;
> +               // AEP should not be enabled if all of the sub-extensions 
> can't
> +               // also be enabled. This is not the proper layer to do such
> +               // error-checking though.

Please use /* */ comments, and leave a newline between code and the
start of the multiline comment.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to