Bug 104668

Summary: dEQP-GLES31.functional.shaders.linkage.uniform.block.differing_precision regression
Product: Mesa Reporter: Mark Janes <mark.a.janes>
Component: Drivers/DRI/i965Assignee: Juan A. Suarez <jasuarez>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: lemody
Version: gitKeywords: bisected, regression
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 104757    

Description Mark Janes 2018-01-16 23:53:26 UTC
bisected to:
Author:     Juan A. Suarez Romero <jasuarez@igalia.com>
glsl/linker: link-error using the same name in unnamed block and outside

According with OpenGL GLSL 4.20 spec, section 4.3.9, page 57:

   "It is a link-time error if any particular shader interface
    contains:
      - two different blocks, each having no instance name, and each
        having a member of the same name, or
      - a variable outside a block, and a block with no instance name,
        where the variable has the same name as a member in the block."

This means that it is a link error if for example we have a vertex
shader with the following definition.

  "layout(location=0) uniform Data { float a; float b; };"

and a fragment shader with:

  "uniform float a;"

As in both cases we refer to both uniforms as "a", and thus using
glGetUniformLocation() wouldn't know which one we mean.

This fixes KHR-GL*.shaders.uniform_block.common.name_matching.

v2: add fixed tests (Tapani)

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
------------------------------------------------------------------
Standard Output

Expecting shader compilation and program linking to succeed. Resulting program will not be executed.
ERROR: expected shaders to compile and link properly, but failed to link.

------------------------------------------------------------------

Since the commit message indicates a similar fixed test, it may be possible that dEQP has a bug which has not been fixed on AOSP/master
Comment 1 Juan A. Suarez 2018-01-26 11:12:55 UTC
The problem is related with precision qualifiers.

In the offending commit, I was using get_interface_type() to check if two members with the same name belongs to the same block. If not, then we are breaking the spec.

The thing is that when we have two equal blocks, just with different precision qualifiers, like:

In VS: uniform Data { highp float a };
In FS: uniform Data { mediump float a };


get_interface_type() return different values, and thus we return an error because we assume that we have the same variable name in two different blocks.


But according to GLES spec, the precision qualifier is ignored to determine if two storge blocks are the same or not. That is, in the above case both uniform blocks are the same.

So here either get_interface_type() should return the same value, or work as it is and perform a check to ignore the type.
Comment 2 Juan A. Suarez 2018-01-30 15:14:25 UTC
Submitted a patch for review that fixes this issue:

https://patchwork.freedesktop.org/series/37349/
Comment 3 Juan A. Suarez 2018-02-05 17:17:04 UTC
commit 4195eed961ccfe404ae81b9112189fc93a254ded
Author: Juan A. Suarez Romero <jasuarez@igalia.com>
Date:   Mon Feb 5 17:38:39 2018 +0100

    glsl/linker: check same name is not used in block and outside
    
    According with OpenGL GLSL 3.20 spec, section 4.3.9:
    
      "It is a link-time error if any particular shader interface
       contains:
         - two different blocks, each having no instance name, and each
           having a member of the same name, or
         - a variable outside a block, and a block with no instance name,
           where the variable has the same name as a member in the block."
    
    This fixes a previous commit 9b894c8 ("glsl/linker: link-error using the
    same name in unnamed block and outside") that covered this case, but
    did not take in account that precision qualifiers are ignored when
    comparing blocks with no instance name.
    
    With this commit, the original tests
    KHR-GL*.shaders.uniform_block.common.name_matching keep fixed, and also
    dEQP-GLES31.functional.shaders.linkage.uniform.block.differing_precision
    regression is fixed, which was broken by previous commit.
    
    v2: use helper varibles (Matteo Bruni)
    
    Fixes: 9b894c8 ("glsl/linker: link-error using the same name in unnamed block and outside")
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104668
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104777
    CC: Mark Janes <mark.a.janes@intel.com>
    CC: "18.0" <mesa-stable@lists.freedesktop.org>
    Tested-by: Matteo Bruni <matteo.mystral@gmail.com>
    Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
    Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>

 src/compiler/glsl/linker.cpp | 53 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 23 deletions(-)

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.