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

[Timothy Arceri]

Looks right to me. However for code clarity it would probably make sense to
move:

m.num_components = MAX2(m.num_components, (to + 1));

to under the second update_rhs_swizzle() call as this is the only place still
using it.

Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>

On Wed, 2015-07-22 at 21:39 -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.
> 
> Cc: idr at freedesktop.org
> Cc: mesa-stable at lists.freedesktop.org
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/glsl/ir.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 8fb7ca4..7350dfa 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -95,6 +95,8 @@ ir_assignment::set_lhs(ir_rvalue *lhs)
>  
>  	 write_mask |= (((this->write_mask >> i) & 1) << c);
>  	 update_rhs_swizzle(rhs_swiz, i, c);
> +         assert(rhs_swiz.num_components <= swiz->val->type
> ->vector_elements);
> +         rhs_swiz.num_components = swiz->val->type->vector_elements;
>        }
>  
>        this->write_mask = write_mask;