Bug 104777 - Attaching multiple shader objects for the same stage to a GLSL program triggers a linker error
Summary: Attaching multiple shader objects for the same stage to a GLSL program trigge...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: All All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-24 22:34 UTC by Matteo Bruni
Modified: 2018-02-05 17:17 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Matteo Bruni 2018-01-24 22:34:36 UTC
Specifically, I get errors like:

"error: declarations for shader output `gl_BackSecondaryColor` are in gl_PerVertex and gl_PerVertex"

and obviously neither gl_BackSecondaryColor or gl_PerVertex are actually redefined in the shaders, those are just the GLSL builtin variables.
FWIW this breaks d3d8 and d3d9 support in Wine since we can use two GLSL vertex shader objects for those (where the additional vertex shader "rearranges" the outputs of the application's vertex shader).

It's a recent regression, bisection says:

9b894c88a688904860d0f37221b78fbcb168f216 is the first bad commit
commit 9b894c88a688904860d0f37221b78fbcb168f216
Author: Juan A. Suarez Romero <jasuarez@igalia.com>
Date:   Tue Jan 16 19:42:35 2018 +0100

    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>

I haven't spent much time debugging the affected code yet but it looks like get_interface_type() returns different glsl_type values for builtins coming from different shader objects. Not sure how to fix it either.
Comment 1 Matteo Bruni 2018-01-24 23:38:33 UTC
Looking for the testcase mentioned in the regression commit I stumbled upon bug 104668. It sounds like it's the same issue.
Comment 2 Juan A. Suarez 2018-01-30 15:13:58 UTC
I've sent a patch for review that I think should fix this issue.

https://patchwork.freedesktop.org/series/37349/
Comment 3 Juan A. Suarez 2018-02-05 17:17:43 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.