[Mesa-dev] [PATCH] glsl: fix constructing a vector from a matrix

[Tapani Pälli]

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

On 05/26/2015 03:32 PM, Martin Peres wrote:
> Without this patch, the following constructs (not an extensive list)
> would crash mesa:
>
> - mat2 foo = mat2(1); vec4 bar = vec4(foo);
> - mat3 foo = mat3(1); vec4 bar = vec4(foo);
> - mat3 foo = mat3(1); ivec4 bar = ivec4(foo);
>
> The first case is explicitely allowed by the GLSL spec, as seen on
> page 101 of the GLSL 4.40 spec:
>
> 	"vec4(mat2) // the vec4 is column 0 followed by column 1"
>
> The other cases are implicitely allowed also.
>
> The actual changes are quite minimal. We first split each column of
> the matrix to a list of vectors and then use them to initialize the
> vector. An additional check to make sure that we are not trying to
> copy 0 elements of a vector fix the (i)vec4(mat3) case as the last
> vector (3rd column) is not needed at all.
>
> Signed-off-by: Martin Peres <martin.peres at linux.intel.com>
> ---
>   src/glsl/ast_function.cpp | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 1e77124..92e26bf 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -993,11 +993,15 @@ emit_inline_vector_constructor(const glsl_type *type,
>      ir_variable *var = new(ctx) ir_variable(type, "vec_ctor", ir_var_temporary);
>      instructions->push_tail(var);
>
> -   /* There are two kinds of vector constructors.
> +   /* There are three kinds of vector constructors.
>       *
>       *  - Construct a vector from a single scalar by replicating that scalar to
>       *    all components of the vector.
>       *
> +    *  - Construct a vector from at least a matrix. This case should already
> +    *    have been taken care of in ast_function_expression::hir by breaking
> +    *    down the matrix into a series of column vectors.
> +    *
>       *  - Construct a vector from an arbirary combination of vectors and
>       *    scalars.  The components of the constructor parameters are assigned
>       *    to the vector in order until the vector is full.
> @@ -1091,6 +1095,14 @@ emit_inline_vector_constructor(const glsl_type *type,
>   	    rhs_components = lhs_components - base_component;
>   	 }
>
> +	 /* If we do not have any components left to copy, break out of the
> +	  * loop. This can happen when initializing a vec4 with a mat3 as the
> +	  * mat3 would have been broken into a series of column vectors.
> +	  */
> +	 if (rhs_components == 0) {
> +	    break;
> +	 }
> +
>   	 const ir_constant *const c = param->as_constant();
>   	 if (c == NULL) {
>   	    /* Mask of fields to be written in the assignment.
> @@ -1681,11 +1693,11 @@ ast_function_expression::hir(exec_list *instructions,
>   	 return ir_rvalue::error_value(ctx);
>         }
>
> -      /* Later, we cast each parameter to the same base type as the
> -       * constructor.  Since there are no non-floating point matrices, we
> -       * need to break them up into a series of column vectors.
> +      /* Matrices can never be consumed as is by any constructor but matrix
> +       * constructors. If the constructor type is not matrix, always break the
> +       * matrix up into a series of column vectors.
>          */
> -      if (constructor_type->base_type != GLSL_TYPE_FLOAT) {
> +      if (!constructor_type->is_matrix()) {
>   	 foreach_in_list_safe(ir_rvalue, matrix, &actual_parameters) {
>   	    if (!matrix->type->is_matrix())
>   	       continue;
>