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")) \


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to