[Mesa-dev] [PATCH v2] glsl: fix assignment of multiple scalar and vecs to matrices.

[Samuel Iglesias Gonsalvez]

When a vec has more elements than row components in a matrix, the
code could end up failing an assert inside assign_to_matrix_column().

This patch makes sure that when there is still room in the matrix for
more elements (but in other columns of the matrix), the data is actually
assigned.

This patch fixes the following dEQP test:

  dEQP-GLES3.functional.shaders.conversions.matrix_combine.float_bvec4_ivec2_bool_to_mat4x2_vertex
  dEQP-GLES3.functional.shaders.conversions.matrix_combine.float_bvec4_ivec2_bool_to_mat4x2_fragment

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
---

v2:
   - Improve the patch following Ben's comments.

 src/glsl/ast_function.cpp | 110 +++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 61 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 918be69..0010ffe 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -1370,71 +1370,59 @@ emit_inline_matrix_constructor(const glsl_type *type,
    } else {
       const unsigned cols = type->matrix_columns;
       const unsigned rows = type->vector_elements;
+      unsigned remaining_slots = rows * cols;
       unsigned col_idx = 0;
       unsigned row_idx = 0;
 
       foreach_in_list(ir_rvalue, rhs, parameters) {
-	 const unsigned components_remaining_this_column = rows - row_idx;
-	 unsigned rhs_components = rhs->type->components();
-	 unsigned rhs_base = 0;
-
-	 /* Since the parameter might be used in the RHS of two assignments,
-	  * generate a temporary and copy the paramter there.
-	  */
-	 ir_variable *rhs_var =
-	    new(ctx) ir_variable(rhs->type, "mat_ctor_vec", ir_var_temporary);
-	 instructions->push_tail(rhs_var);
-
-	 ir_dereference *rhs_var_ref =
-	    new(ctx) ir_dereference_variable(rhs_var);
-	 ir_instruction *inst = new(ctx) ir_assignment(rhs_var_ref, rhs, NULL);
-	 instructions->push_tail(inst);
-
-	 /* Assign the current parameter to as many components of the matrix
-	  * as it will fill.
-	  *
-	  * NOTE: A single vector parameter can span two matrix columns.  A
-	  * single vec4, for example, can completely fill a mat2.
-	  */
-	 if (rhs_components >= components_remaining_this_column) {
-	    const unsigned count = MIN2(rhs_components,
-					components_remaining_this_column);
-
-	    rhs_var_ref = new(ctx) ir_dereference_variable(rhs_var);
-
-	    ir_instruction *inst = assign_to_matrix_column(var, col_idx,
-							   row_idx,
-							   rhs_var_ref, 0,
-							   count, ctx);
-	    instructions->push_tail(inst);
-
-	    rhs_base = count;
-
-	    col_idx++;
-	    row_idx = 0;
-	 }
-
-	 /* If there is data left in the parameter and components left to be
-	  * set in the destination, emit another assignment.  It is possible
-	  * that the assignment could be of a vec4 to the last element of the
-	  * matrix.  In this case col_idx==cols, but there is still data
-	  * left in the source parameter.  Obviously, don't emit an assignment
-	  * to data outside the destination matrix.
-	  */
-	 if ((col_idx < cols) && (rhs_base < rhs_components)) {
-	    const unsigned count = rhs_components - rhs_base;
-
-	    rhs_var_ref = new(ctx) ir_dereference_variable(rhs_var);
-
-	    ir_instruction *inst = assign_to_matrix_column(var, col_idx,
-							   row_idx,
-							   rhs_var_ref,
-							   rhs_base,
-							   count, ctx);
-	    instructions->push_tail(inst);
-
-	    row_idx += count;
-	 }
+         unsigned rhs_components = rhs->type->components();
+         unsigned rhs_base = 0;
+
+         if (remaining_slots == 0)
+            break;
+
+         /* Since the parameter might be used in the RHS of two assignments,
+          * generate a temporary and copy the paramter there.
+          */
+         ir_variable *rhs_var =
+            new(ctx) ir_variable(rhs->type, "mat_ctor_vec", ir_var_temporary);
+         instructions->push_tail(rhs_var);
+
+         ir_dereference *rhs_var_ref =
+            new(ctx) ir_dereference_variable(rhs_var);
+         ir_instruction *inst = new(ctx) ir_assignment(rhs_var_ref, rhs, NULL);
+         instructions->push_tail(inst);
+
+         do {
+            /* Assign the current parameter to as many components of the matrix
+             * as it will fill.
+             *
+             * NOTE: A single vector parameter can span two matrix columns.  A
+             * single vec4, for example, can completely fill a mat2.
+             */
+            unsigned count = MIN2(rows - row_idx,
+                                  rhs_components - rhs_base);
+
+            rhs_var_ref = new(ctx) ir_dereference_variable(rhs_var);
+            ir_instruction *inst = assign_to_matrix_column(var, col_idx,
+                                                         row_idx,
+                                                         rhs_var_ref,
+                                                         rhs_base,
+                                                         count, ctx);
+            instructions->push_tail(inst);
+            rhs_base += count;
+            row_idx += count;
+            remaining_slots -= count;
+
+            /* Sometimes, there is still data left in the parameters and
+             * components left to be set in the destination but in other
+             * column.
+             */
+            if (row_idx >= rows) {
+               row_idx = 0;
+               col_idx++;
+            }
+         } while(remaining_slots > 0 && rhs_base < rhs_components);
       }
    }
 
-- 
2.1.0