[Mesa-dev] [PATCH] glsl: only match gl_FragData and not gl_SecondaryFragDataEXT

[Kenneth Graunke]

On Tuesday, June 21, 2016 3:01:16 PM PDT Ilia Mirkin wrote:
> There's special logic around finding gl_FragData. It latches onto any
> array with FRAG_RESULT_DATA0. However gl_SecondaryFragDataEXT[], added
> by GL_EXT_blend_func_extended, fits those parameters as well. The real
> frag data array should have index 0 though, so we can use that to
> distinguish them.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96617
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: "11.1 11.2 12.0" <mesa-stable at lists.freedesktop.org>
> ---
> 
> TBH I don't fully understand all this code. However this makes a hacked-up piglit test pass. This should go through a full testing cycle before being pushed to make sure it doesn't break anything in the process.
> 
>  src/compiler/glsl/opt_dead_builtin_varyings.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compiler/glsl/opt_dead_builtin_varyings.cpp b/src/compiler/glsl/opt_dead_builtin_varyings.cpp
> index 37bcbcc..0f9fdb3 100644
> --- a/src/compiler/glsl/opt_dead_builtin_varyings.cpp
> +++ b/src/compiler/glsl/opt_dead_builtin_varyings.cpp
> @@ -88,7 +88,9 @@ public:
>        if (!var || var->data.mode != this->mode || !var->type->is_array())
>           return visit_continue;
>  
> -      if (this->find_frag_outputs && var->data.location == FRAG_RESULT_DATA0) {
> +      /* Only match gl_FragData[], not gl_SecondaryFragDataEXT[] */
> +      if (this->find_frag_outputs && var->data.location == FRAG_RESULT_DATA0 &&
> +          var->data.index == 0) {
>           this->fragdata_array = var;
>  
>           ir_constant *index = ir->array_index->as_constant();
> 

This looks good to me.  This code is trying to break up the
gl_FragData[] array and throw away unused components.  Things will go
very poorly if we recognize the wrong variable.

It would make sense to update the pass to also modify
gl_SecondaryFragDataEXT[] in the same way as it handles gl_FragData[].
However, Ryan's code for that hardcodes the array to length 1 anyway
(as no driver supports more than 1 RT with dual color blending), so
it's kind of a moot point.  We'd just need to update it if some driver
ever gained support for MRT + dual color blending.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160621/38c2715e/attachment.sig>