Bug 60737 - In GLSL ES, a missing FS precision qualifier does not generate an error
Summary: In GLSL ES, a missing FS precision qualifier does not generate an error
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: unspecified
Hardware: Other All
: low normal
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-12 16:16 UTC by Paul Berry
Modified: 2013-08-27 22:37 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Paul Berry 2013-02-12 16:16:18 UTC
The following text appears in the GLSL ES 1.00 spec, section 4.5.3 ("Default Precision Qualifiers"), and the GLSL ES 3.00 spec, section 4.5.4 ("Default Precision Qualifiers"):

"The fragment language has no default precision qualifier for floating point types. Hence for float, floating point vector and matrix variable declarations, either the declaration must include a precision qualifier or the default float precision must have been previously declared."

Mesa doesn't appear to enforce this.
Comment 1 Paul Berry 2013-02-12 21:20:14 UTC
Ok, this turned out to be more complex than I thought (don't they always?).  In addition to the problem described above:

1. The spec says that default precision statements are alowed on sampler types.  Mesa doesn't respect this.

2. Only the sampler types sampler2D and samplerCube have a default precision defined.  The remaining sampler types require precision statements in *all* shaders.  (However, in my experiments with the proprietary nVidia driver for Linux, it appears not to enforce this).

3. The spec says that default precision statements are scoped, which means that this bug won't be trivial to fix (the code we write to keep track of which types the shader has declared a default precision for will have to be scope-aware).

Out of all of these problems, only problem 1 seems necessary to fix in the short run, since it is the only problem that would cause Mesa to reject an otherwise-correct shader.  I have a patch out on the mailing list to fix it (http://lists.freedesktop.org/archives/mesa-dev/2013-February/034481.html).  Once that patch lands I'll downgrade the priority of this bug appropriately.
Comment 2 Paul Berry 2013-02-14 19:22:52 UTC
(In reply to comment #1)
> Out of all of these problems, only problem 1 seems necessary to fix in the
> short run, since it is the only problem that would cause Mesa to reject an
> otherwise-correct shader.  I have a patch out on the mailing list to fix it
> (http://lists.freedesktop.org/archives/mesa-dev/2013-February/034481.html). 
> Once that patch lands I'll downgrade the priority of this bug appropriately.

Ok, that patch has landed:

commit d5948f2f5e37d1abc0d433ddf43407d87b2d1227
Author: Paul Berry <stereotype441@gmail.com>
Date:   Tue Feb 12 12:36:41 2013 -0800

    glsl: Allow default precision qualifiers to be set for sampler types.
    
    From GLSL ES 3.00 section 4.5.4 ("Default Precision Qualifiers"):
    
        "The precision statement
    
            precision precision-qualifier type;
    
        can be used to establish a default precision qualifier. The type
        field can be either int or float or any of the sampler types, and
        the precision-qualifier can be lowp, mediump, or highp."
    
    GLSL ES 1.00 has similar language.  GLSL 1.30 doesn't allow precision
    qualifiers on sampler types, but this seems like an oversight (since
    the intention of including these in GLSL 1.30 is to allow
    compatibility with ES shaders).
    
    Previously, Mesa followed GLSL 1.30 and only allowed default precision
    qualifiers to be set for float and int.  This patch makes it follow
    GLSL ES rules in all cases.
    
    Fixes Piglit tests default-precision-sampler.{vert,frag}.
    
    Partially addresses https://bugs.freedesktop.org/show_bug.cgi?id=60737.
    
    NOTE: This is a candidate for stable branches.
    
    Reviewed-by: Eric Anholt <eric@anholt.net>

The remaining problems are lower priority, hence I'm lowering the priority of this bug.
Comment 3 Ian Romanick 2013-08-27 22:37:38 UTC
This should be fixed by:

commit cabd45773b58d6aa48202da1cdd8cf1a6b856c53
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Fri Aug 9 15:17:18 2013 -0700

    glsl: Track existence of default float precision in GLSL ES fragment shaders
    
    This is required by the spec, and it's a bit tricky because the default
    precision is scoped.  As a result, I'm slightly abusing the symbol
    table.
    
    Fixes piglit no-default-float-precision.frag tests and the piglit
    default-precision-nested-scope-0[1234].frag tests that are currently on
    the piglit mailing list for review.
    
    On IRC I got confirmation from cwabbot that ARM (Mali T6xx and T400)
    enforces this requirement and from kusma that NVIDIA (Tegra2) enforces
    this requirement.  We should be safe from regressing shipping
    applications.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Cc: "9.2" <mesa-stable@lists.freedesktop.org>


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.