[Mesa-dev] [PATCH] glsl: Allow precision mismatch on dead data with GLSL ES 1.00

[Ian Romanick]

NAK.

You mention varyings in the commit message, but then you quote spec text
about uniforms.  This bugged me, so I actually went and looked at the
GLSL ES 1.00 specification.

Section 4.3.5 (Varying) of the GLSL ES 1.00 specification says:

    The precision of varying variables does not need to match.

SO... for GLSL ES 1.00 we should not even emit a warning for varyings
with mismatched precision.  The above spec quotation should appear in
the patch that does this.  We should also add a piglit test and,
frankly, a CTS test!  We've been rejecting completely valid shaders for
over a year now, and the CTS should have caught that.

If there are actually shaders in the wild that expect mismatched
precision on uniforms in GLSL ES 1.00 shaders, we should deal with that
in a separate patch.

On 09/26/2017 01:35 AM, Tomasz Figa wrote:
> Commit 259fc505454ea6a67aeacf6cdebf1398d9947759 added linker error for
> mismatching uniform precision, as required by GLES 3.0 specification and
> conformance test-suite.
> 
> Several Android applications, including Forge of Empires, have shaders
> which violate this rule, on a dead varying that will be eliminated.
> The problem affects a big number of applications using Cocos2D engine
> and other GLES implementations accept this, this poses a serious
> application compatibility issue.
> 
> Starting from GLSL ES 3.0, declarations with conflicting precision
> qualifiers are explicitly prohibited. However GLSL ES 1.00 does not
> clearly specify the behavior, except that
> 
>   "Uniforms are defined to behave as if they are using the same storage in
>   the vertex and fragment processors and may be implemented this way.
>   If uniforms are used in both the vertex and fragment shaders, developers
>   should be warned if the precisions are different. Conversion of
>   precision should never be implicit."
> 
> The word "used" is not clear in this context and might refer to
>  1) declared (same as GLES 3.x)
>  2) referred after post-processing, or
>  3) linked after all optimizations are done.
> 
> Looking at existing applications, 2) or 3) seems to be widely adopted.
> To avoid compatibility issues, turn the error into a warning if GLSL ES
> version is lower than 3.0 and the data is dead in at least one of the
> shaders.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97532
> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> ---
>  src/compiler/glsl/linker.cpp | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index f352c5385ca..ff2e6b7a0d3 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -1121,10 +1121,16 @@ cross_validate_globals(struct gl_shader_program *prog,
>           if (prog->IsES && (prog->data->Version != 310 ||
>                              !var->get_interface_type()) &&
>               existing->data.precision != var->data.precision) {
> -            linker_error(prog, "declarations for %s `%s` have "
> -                         "mismatching precision qualifiers\n",
> -                         mode_string(var), var->name);
> -            return;
> +            if ((existing->data.used && var->data.used) || prog->data->Version >= 300) {
> +               linker_error(prog, "declarations for %s `%s` have "
> +                            "mismatching precision qualifiers\n",
> +                            mode_string(var), var->name);
> +               return;
> +            } else {
> +               linker_warning(prog, "declarations for %s `%s` have "
> +                              "mismatching precision qualifiers\n",
> +                              mode_string(var), var->name);
> +            }
>           }
>        } else
>           variables->add_variable(var);
>