[Mesa-dev] [PATCH] i965: skip varyings without slot

[Juan A. Suarez Romero]

On Tue, 2017-06-27 at 09:29 +1000, Timothy Arceri wrote:
> On 16/06/17 18:12, Juan A. Suarez Romero wrote:
> 
> > Commit 00620782c9 (i965: use nir_shader_gather_info() over
> > do_set_program_inouts()) changed how we compute the outputs written.
> > 
> > In the previous version it was using the IR declared outputs, while in
> > the new one it uses NIR to parse the instructions that write outputs.
> > 
> > Thus, if the shader has declared some output that is not written later
> > in the code, like this:
> > 
> > ~~~
> > struct S {
> >      vec4 a;
> >      vec4 b;
> >      vec4 c;
> > };
> > 
> > layout (xfb_offset = sizeof_type) out S s;
> > 
> > void main()
> > {
> > 
> >      s.a = vec4(1.0, 0.0, 0.0, 1.0);
> >      s.c = vec4(0.0, 1.0, 0.0, 1.0);
> > }
> > ~~~
> > 
> > The former version computing 3 outputs written (s.a, s.b and s.c), while
> > the new version only counts 2 (s.a and s.c).
> > 
> > This means that with the new version, then could be varyings in the VUE
> > map that do not have an slot assigned (s.b), that must be skipped.
> > 
> > This fixes KHR-GL45.enhanced_layouts.xfb_capture_struct.
> > ---
> >   src/mesa/drivers/dri/i965/genX_state_upload.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > index a5ad2ca..573f0e3 100644
> > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > @@ -3102,9 +3102,10 @@ genX(upload_3dstate_so_decl_list)(struct brw_context *brw,
> >         const unsigned stream_id = output->StreamId;
> >         assert(stream_id < MAX_VERTEX_STREAMS);
> >   
> > -      buffer_mask[stream_id] |= 1 << buffer;
> > +      if (vue_map->varying_to_slot[varying] == -1)
> > +	      continue;
> >   
> > -      assert(vue_map->varying_to_slot[varying] >= 0);
> > +      buffer_mask[stream_id] |= 1 << buffer;
> >   
> 
> My feeling is we should try to avoid adding it to the VUE map in the 
> first place rather than trying to work around it.
> 

It isn't in the VUE map. That's the reason to skip it.

Maybe you mean not adding it in the linked_xfb_info?

	J.A.



> Is it not possible to do that instead?
> 
> 
> >         /* Mesa doesn't store entries for gl_SkipComponents in the Outputs[]
> >          * array.  Instead, it simply increments DstOffset for the following
> 
>