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_arc...@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: i...@freedesktop.org > Cc: mesa-sta...@lists.freedesktop.org > Signed-off-by: Kenneth Graunke <kenn...@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; _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
