[Mesa-dev] [PATCH 4/5] glsl: Fix conversions in array constructors

[Chad Versace]

On 07/29/2011 01:57 PM, Kenneth Graunke wrote:
> On 07/27/2011 10:55 PM, Chad Versace wrote:
>> Array constructors obey narrower conversion rules than other constructors
>> [1] --- they use the implicit conversion rules [2] instead of the scalar
>> constructor conversions [3].  But process_array_constructor() was
>> incorrectly applying the broader rules.
>>
>> To fix this, first check (with glsl_type::can_be_implicitly_converted_to)
>> if an implicit conversion is legal before performing the conversion.
>>
>> [1] GLSL 1.50 spec, Section 5.4.4 Array Constructors, page 52 (58 of pdf)
>> [2] GLSL 1.50 spec, Section 4.1.10 Implicit Conversions, page 25 (31 of pdf)
>> [3] GLSL 1.50 spec, Section 5.4.1 Conversion, page 48 (54 of pdf)
>>
>> Fixes Piglit tests:
>>     spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bool-float.vert
>>     spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bvec*-vec*.vert
>>
>> Note: This is a candidate for the 7.10 and 7.11 branches.
>> CC: Ian Romanick <ian.d.romanick at intel.com>
>> CC: Kenneth Graunke <kenneth at whitecape.org>
>> Signed-off-by: Chad Versace <chad at chad-versace.us>
>> ---
>>  src/glsl/ast_function.cpp |   12 ++++++++++--
>>  1 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
>> index bdb73f4..9bee3eb 100644
>> --- a/src/glsl/ast_function.cpp
>> +++ b/src/glsl/ast_function.cpp
>> @@ -442,13 +442,21 @@ process_array_constructor(exec_list *instructions,
>>        ir_rvalue *ir = (ir_rvalue *) n;
>>        ir_rvalue *result = ir;
>>  
>> -      /* Apply implicit conversions (not the scalar constructor rules!) */
>> +      /* Apply implicit conversions (not the scalar constructor rules!). See
>> +       * the spec quote above. */
>>        if (constructor_type->element_type()->is_float()) {
>>  	 const glsl_type *desired_type =
>>  	    glsl_type::get_instance(GLSL_TYPE_FLOAT,
>>  				    ir->type->vector_elements,
>>  				    ir->type->matrix_columns);
>> -	 result = convert_component(ir, desired_type);
>> +	 if (result->type->can_implicitly_convert_to(desired_type)) {
>> +	    /* Even though convert_component() implements the scalar
>> +	     * conversion rules, not the implicit conversion rules, its safe
> 
> * constructor rules (not the implicit conversion rules), it's safe
> 
>> +	     * to use it here because we already checked that the implicit
>> +	     * conversion is legal.
>> +	     */
>> +	    result = convert_component(ir, desired_type);
>> +	 }
>>        }
>>  
>>        if (result->type != constructor_type->element_type()) {
> 
> Otherwise...
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks.

Will fix the comment mishap.

-- 
Chad Versace
chad at chad-versace.us