[Mesa-dev] [PATCH 6/7] mesa/util: add allow_glsl_relaxed_es driconfig override
Eero Tamminen
eero.t.tamminen at intel.com
Fri Jun 8 14:07:31 UTC 2018
Hi,
On 08.06.2018 15:19, Timothy Arceri wrote:
> This relaxes a number of ES shader restrictions allowing shaders
> to follow more desktop GLSL like rules.
>
> This initial implementation relaxes the following:
>
> - allows linking ES shaders with desktop shaders
> - allows mismatching precision qualifiers
> - always enables standard derivative builtins
>
> These relaxations allow Google Earth VR shaders to compile.
The fixes should rather go to apps, than bug workarounds to Mesa.
Is bug reported to Google Earth VR maintainers and have they stated that
they aren't going to fix the issues?
If different shader stages have different precision in a variable that's
actually being used, that's a real bug in the application.
(There are lot of apps on Android that have mismatches on unused/dead
variables, but for those the mismatches don't matter in practice.)
- Eero
> ---
> src/compiler/glsl/builtin_functions.cpp | 3 ++-
> src/compiler/glsl/linker.cpp | 22 +++++++++++--------
> .../auxiliary/pipe-loader/driinfo_gallium.h | 1 +
> src/gallium/include/state_tracker/st_api.h | 1 +
> src/gallium/state_trackers/dri/dri_screen.c | 2 ++
> src/mesa/main/mtypes.h | 6 +++++
> src/mesa/state_tracker/st_extensions.c | 3 +++
> src/util/xmlpool/t_options.h | 5 +++++
> 8 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
> index e1ee9943172..de206b8fd71 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -446,7 +446,8 @@ fs_oes_derivatives(const _mesa_glsl_parse_state *state)
> {
> return state->stage == MESA_SHADER_FRAGMENT &&
> (state->is_version(110, 300) ||
> - state->OES_standard_derivatives_enable);
> + state->OES_standard_derivatives_enable ||
> + state->ctx->Const.AllowGLSLRelaxedES);
> }
>
> static bool
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index f060c5316fa..3159af181f1 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -894,7 +894,7 @@ validate_intrastage_arrays(struct gl_shader_program *prog,
> * Perform validation of global variables used across multiple shaders
> */
> static void
> -cross_validate_globals(struct gl_shader_program *prog,
> +cross_validate_globals(struct gl_context *ctx, struct gl_shader_program *prog,
> struct exec_list *ir, glsl_symbol_table *variables,
> bool uniforms_only)
> {
> @@ -1115,7 +1115,8 @@ cross_validate_globals(struct gl_shader_program *prog,
> /* Check the precision qualifier matches for uniform variables on
> * GLSL ES.
> */
> - if (prog->IsES && !var->get_interface_type() &&
> + if (!ctx->Const.AllowGLSLRelaxedES &&
> + prog->IsES && !var->get_interface_type() &&
> existing->data.precision != var->data.precision) {
> if ((existing->data.used && var->data.used) || prog->data->Version >= 300) {
> linker_error(prog, "declarations for %s `%s` have "
> @@ -1168,15 +1169,16 @@ cross_validate_globals(struct gl_shader_program *prog,
> * Perform validation of uniforms used across multiple shader stages
> */
> static void
> -cross_validate_uniforms(struct gl_shader_program *prog)
> +cross_validate_uniforms(struct gl_context *ctx,
> + struct gl_shader_program *prog)
> {
> glsl_symbol_table variables;
> for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> if (prog->_LinkedShaders[i] == NULL)
> continue;
>
> - cross_validate_globals(prog, prog->_LinkedShaders[i]->ir, &variables,
> - true);
> + cross_validate_globals(ctx, prog, prog->_LinkedShaders[i]->ir,
> + &variables, true);
> }
> }
>
> @@ -2202,7 +2204,8 @@ link_intrastage_shaders(void *mem_ctx,
> for (unsigned i = 0; i < num_shaders; i++) {
> if (shader_list[i] == NULL)
> continue;
> - cross_validate_globals(prog, shader_list[i]->ir, &variables, false);
> + cross_validate_globals(ctx, prog, shader_list[i]->ir, &variables,
> + false);
> }
>
> if (!prog->data->LinkStatus)
> @@ -4799,7 +4802,8 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> min_version = MIN2(min_version, prog->Shaders[i]->Version);
> max_version = MAX2(max_version, prog->Shaders[i]->Version);
>
> - if (prog->Shaders[i]->IsES != prog->Shaders[0]->IsES) {
> + if (!ctx->Const.AllowGLSLRelaxedES &&
> + prog->Shaders[i]->IsES != prog->Shaders[0]->IsES) {
> linker_error(prog, "all shaders must use same shading "
> "language version\n");
> goto done;
> @@ -4817,7 +4821,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> /* In desktop GLSL, different shader versions may be linked together. In
> * GLSL ES, all shader versions must be the same.
> */
> - if (prog->Shaders[0]->IsES && min_version != max_version) {
> + if (!ctx->Const.AllowGLSLRelaxedES && min_version != max_version) {
> linker_error(prog, "all shaders must use same shading "
> "language version\n");
> goto done;
> @@ -4943,7 +4947,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> * performed, then locations are assigned for uniforms, attributes, and
> * varyings.
> */
> - cross_validate_uniforms(prog);
> + cross_validate_uniforms(ctx, prog);
> if (!prog->data->LinkStatus)
> goto done;
>
> diff --git a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
> index b5c9dc0eb61..d7c1e041ed7 100644
> --- a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
> +++ b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
> @@ -25,6 +25,7 @@ DRI_CONF_SECTION_DEBUG
> DRI_CONF_ALLOW_GLSL_EXTENSION_DIRECTIVE_MIDSHADER("false")
> DRI_CONF_ALLOW_GLSL_SEMICOLON_AFTER_FUNCTION("false")
> DRI_CONF_ALLOW_GLSL_BUILTIN_CONST_EXPRESSION("false")
> + DRI_CONF_ALLOW_GLSL_RELAXED_ES("false")
> DRI_CONF_ALLOW_GLSL_BUILTIN_VARIABLE_REDECLARATION("false")
> DRI_CONF_ALLOW_GLSL_CROSS_STAGE_INTERPOLATION_MISMATCH("false")
> DRI_CONF_ALLOW_HIGHER_COMPAT_VERSION("false")
> diff --git a/src/gallium/include/state_tracker/st_api.h b/src/gallium/include/state_tracker/st_api.h
> index 033f2be8b26..07ec6592da5 100644
> --- a/src/gallium/include/state_tracker/st_api.h
> +++ b/src/gallium/include/state_tracker/st_api.h
> @@ -224,6 +224,7 @@ struct st_config_options
> boolean allow_glsl_extension_directive_midshader;
> boolean allow_glsl_semicolon_after_function;
> boolean allow_glsl_builtin_const_expression;
> + boolean allow_glsl_relaxed_es;
> boolean allow_glsl_builtin_variable_redeclaration;
> boolean allow_higher_compat_version;
> boolean glsl_zero_init;
> diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c
> index ddbd75aa85e..2256a193a10 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/dri_screen.c
> @@ -78,6 +78,8 @@ dri_fill_st_options(struct dri_screen *screen)
> driQueryOptionb(optionCache, "allow_glsl_semicolon_after_function");
> options->allow_glsl_builtin_const_expression =
> driQueryOptionb(optionCache, "allow_glsl_builtin_const_expression");
> + options->allow_glsl_relaxed_es =
> + driQueryOptionb(optionCache, "allow_glsl_relaxed_es");
> options->allow_glsl_builtin_variable_redeclaration =
> driQueryOptionb(optionCache, "allow_glsl_builtin_variable_redeclaration");
> options->allow_higher_compat_version =
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 8847f461048..c57c1c38894 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3723,6 +3723,12 @@ struct gl_constants
> */
> GLboolean AllowGLSLBuiltinConstantExpression;
>
> + /**
> + * Allow some relaxation of GLSL ES shader restrictions. This encompasses
> + * a number of relaxations to the ES shader rules.
> + */
> + GLboolean AllowGLSLRelaxedES;
> +
> /**
> * Allow GLSL built-in variables to be redeclared verbatim
> */
> diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
> index 26190394577..9bc3d18af2d 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -1139,6 +1139,9 @@ void st_init_extensions(struct pipe_screen *screen,
> if (options->allow_glsl_builtin_const_expression)
> consts->AllowGLSLBuiltinConstantExpression = GL_TRUE;
>
> + if (options->allow_glsl_relaxed_es)
> + consts->AllowGLSLRelaxedES = GL_TRUE;
> +
> consts->MinMapBufferAlignment =
> screen->get_param(screen, PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT);
>
> diff --git a/src/util/xmlpool/t_options.h b/src/util/xmlpool/t_options.h
> index e7054495e33..8e65c1f4e77 100644
> --- a/src/util/xmlpool/t_options.h
> +++ b/src/util/xmlpool/t_options.h
> @@ -125,6 +125,11 @@ DRI_CONF_OPT_BEGIN_B(allow_glsl_builtin_const_expression, def) \
> DRI_CONF_DESC(en,gettext("Allow builtins as part of constant expressions")) \
> DRI_CONF_OPT_END
>
> +#define DRI_CONF_ALLOW_GLSL_RELAXED_ES(def) \
> +DRI_CONF_OPT_BEGIN_B(allow_glsl_relaxed_es, def) \
> + DRI_CONF_DESC(en,gettext("Allow some relaxation of GLSL ES shader restrictions")) \
> +DRI_CONF_OPT_END
> +
> #define DRI_CONF_ALLOW_GLSL_BUILTIN_VARIABLE_REDECLARATION(def) \
> DRI_CONF_OPT_BEGIN_B(allow_glsl_builtin_variable_redeclaration, def) \
> DRI_CONF_DESC(en,gettext("Allow GLSL built-in variables to be redeclared verbatim")) \
>
More information about the mesa-dev
mailing list