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

[Juan A. Suarez Romero]

On Fri, 2017-06-16 at 10:12 +0200, 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.


Gently ping if someone can review it.

Thanks in advance!


	J.A.

> ---
>  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;
>  
>        /* Mesa doesn't store entries for gl_SkipComponents in the Outputs[]
>         * array.  Instead, it simply increments DstOffset for the following