[Mesa-dev] [PATCH 01/28] glsl: only add outward facing varyings to resourse list for SSO

[Timothy Arceri]

On Thu, 2016-01-07 at 16:43 -0800, Ian Romanick wrote:
> On 12/28/2015 09:00 PM, Timothy Arceri wrote:
> > An SSO program can have multiple stages and we only want to add the
> > externally
> > facing varyings. The current code was adding both the packed inputs
> > and outputs
> > for the first and last stage of each program.
> 
> Using just SSO and ARB_program_interface_query, is this problem
> externally visible?

I assume it would be.

>   If it is externally visible:
> 
> 1. Is there a piglit test?

Not testing the SSO issue directly, patch 23 goes on to allow this to
be called regardless of SSO as it's needed for vertex inputs/fragment
outputs that use the component layout qualifier to do packing and that
does have a piglit test for this code path.

> 
> 2. Should this fix be tagged for stable?

I'll write up a piglit just to be sure and tag it if it fixes a
problem.


> 
> > ---
> >  src/glsl/linker.cpp | 17 ++++++++++-------
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> > index c7e6976..d11c404 100644
> > --- a/src/glsl/linker.cpp
> > +++ b/src/glsl/linker.cpp
> > @@ -3433,7 +3433,7 @@ add_interface_variables(struct
> > gl_shader_program *shProg,
> >  }
> >  
> >  static bool
> > -add_packed_varyings(struct gl_shader_program *shProg, int stage)
> > +add_packed_varyings(struct gl_shader_program *shProg, int stage,
> > GLenum type)
> >  {
> >     struct gl_shader *sh = shProg->_LinkedShaders[stage];
> >     GLenum iface;
> > @@ -3454,10 +3454,13 @@ add_packed_varyings(struct
> > gl_shader_program *shProg, int stage)
> >           default:
> >              unreachable("unexpected type");
> >           }
> > -         if (!add_program_resource(shProg, iface, var,
> > -                                   build_stageref(shProg, var
> > ->name,
> > -                                                  var
> > ->data.mode)))
> > -            return false;
> > +
> > +         if (type == iface) {
> > +            if (!add_program_resource(shProg, iface, var,
> > +                                      build_stageref(shProg, var
> > ->name,
> > +                                                     var
> > ->data.mode)))
> > +               return false;
> > +         }
> >        }
> >     }
> >     return true;
> > @@ -3724,9 +3727,9 @@ build_program_resource_list(struct
> > gl_shader_program *shProg)
> >  
> >     /* Program interface needs to expose varyings in case of SSO.
> > */
> >     if (shProg->SeparateShader) {
> > -      if (!add_packed_varyings(shProg, input_stage))
> > +      if (!add_packed_varyings(shProg, input_stage,
> > GL_PROGRAM_INPUT))
> >           return;
> > -      if (!add_packed_varyings(shProg, output_stage))
> > +      if (!add_packed_varyings(shProg, output_stage,
> > GL_PROGRAM_OUTPUT))
> >           return;
> >     }
> >  
> g
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev