[Mesa-stable] [Mesa-dev] [PATCH v2] glsl: Fix a bug where LHS swizzles of swizzles were too small.

[Timothy Arceri]

On Tue, 2015-07-28 at 16:34 -0700, Kenneth Graunke wrote:
> A simple shader such as
> 
>    vec4 color;
>    color.xy.x = 1.0;
> 
> would cause ir_assignment::set_lhs() to generate bogus IR:
> 
>    (swiz xy (swiz x (constant float (1.0))))
> 
> We were setting the number of components of each new RHS swizzle based
> on the highest channel used in the LHS swizzle.  So, .xy.y would
> generate (swiz xy (swiz xx ...)), while .xy.x would break.
> 
> Our existing Piglit test happened to use .xzy.z, which worked, since
> 'z' is the third component, resulting in an xxx swizzle.
> 
> This patch sets the number of swizzle components based on the size of
> the LHS swizzle's inner value, so we always have the correct number
> at each step.
> 
> Fixes new Piglit tests glsl-vs-swizzle-swizzle-lhs-[23].
> Fixes ir_validate assertions in in Metro 2033 Redux.
> 
> v2: Move num_components updating completely out of update_rhs_swizzle
>     (suggested by Timothy Arceri).  Simplify.
> 
> Cc: mesa-stable at lists.freedesktop.org
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au> [v1]
> ---
>  src/glsl/ir.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Hey Tim, does this look better?

Looks good to me Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>

> 
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 3a40926..724861b 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -63,8 +63,6 @@ update_rhs_swizzle(ir_swizzle_mask &m, unsigned from, 
> unsigned to)
>     case 3: m.w = from; break;
>     default: assert(!"Should not get here.");
>     }
> -
> -   m.num_components = MAX2(m.num_components, (to + 1));
>  }
>  
>  void
> @@ -95,6 +93,7 @@ ir_assignment::set_lhs(ir_rvalue *lhs)
>  
>  	 write_mask |= (((this->write_mask >> i) & 1) << c);
>  	 update_rhs_swizzle(rhs_swiz, i, c);
> +         rhs_swiz.num_components = swiz->val->type->vector_elements;
>        }
>  
>        this->write_mask = write_mask;
> @@ -114,6 +113,7 @@ ir_assignment::set_lhs(ir_rvalue *lhs)
>  	 if (write_mask & (1 << i))
>  	    update_rhs_swizzle(rhs_swiz, i, rhs_chan++);
>        }
> +      rhs_swiz.num_components = rhs_chan;
>        this->rhs = new(mem_ctx) ir_swizzle(this->rhs, rhs_swiz);
>     }
>