[Mesa-dev] [PATCH] glsl: Fix a bug where LHS swizzles of swizzles were too small.
t_arceri at yahoo.com.au
Thu Jul 23 02:38:28 PDT 2015
Looks right to me. However for code clarity it would probably make sense to
m.num_components = MAX2(m.num_components, (to + 1));
to under the second update_rhs_swizzle() call as this is the only place still
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-.
> 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
> + rhs_swiz.num_components = swiz->val->type->vector_elements;
> this->write_mask = write_mask;
More information about the mesa-dev