[Mesa-dev] [PATCH] glsl: Use M_PI_* macros.

[Ian Romanick]

On 04/07/2014 10:28 AM, Aaron Watry wrote:
> On Mon, Apr 7, 2014 at 12:19 PM, Matt Turner <mattst88 at gmail.com> wrote:
>> Notice our multiple values for M_PI_2, which rounded ...32 up to
>> ...4 and ...5.
>> ---
>> The float casts are ugly. I tried to define M_PI_2f using the
>> preprocessor -- something like
>>    #define M_PI_2f M_PI_2##f
>> but no luck.
>>
>>  src/glsl/builtin_functions.cpp | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
>> index 26ea923..62bf154 100644
>> --- a/src/glsl/builtin_functions.cpp
>> +++ b/src/glsl/builtin_functions.cpp
>> @@ -2538,11 +2538,11 @@ ir_expression *
>>  builtin_builder::asin_expr(ir_variable *x)
>>  {
>>     return mul(sign(x),
>> -              sub(imm(1.5707964f),
>> +              sub(imm((float)M_PI_2),
>>                    mul(sqrt(sub(imm(1.0f), abs(x))),
>> -                      add(imm(1.5707964f),
>> +                      add(imm((float)M_PI_2),
>>                            mul(abs(x),
>> -                              add(imm(-0.21460183f),
>> +                              add(imm((float)(1.0 - M_PI_4)),
> 
> 1 - M_PI_4 comes out to 0.21460 (positive), not -0.21460.  You
> probably want to switch the operand order in the subtraction here
> (unless this change was intentional).

There are a couple asin() tests, so it seems like this should have
caused at least one of them to fail... Matt, did you run all of piglit?
 If this didn't cause an existing test to fail, we should add a new test.

> --Aaron
> 
>>                                    mul(abs(x),
>>                                        add(imm(0.086566724f),
>>                                            mul(abs(x), imm(-0.03102955f))))))))));
>> @@ -2586,7 +2586,7 @@ builtin_builder::_acos(const glsl_type *type)
>>     ir_variable *x = in_var(type, "x");
>>     MAKE_SIG(type, always_available, 1, x);
>>
>> -   body.emit(ret(sub(imm(1.5707964f), asin_expr(x))));
>> +   body.emit(ret(sub(imm((float)M_PI_2), asin_expr(x))));
>>
>>     return sig;
>>  }
>> @@ -2623,13 +2623,13 @@ builtin_builder::_atan2(const glsl_type *type)
>>        ir_if *inner_if = new(mem_ctx) ir_if(less(x, imm(0.0f)));
>>        inner_if->then_instructions.push_tail(
>>           if_tree(gequal(y, imm(0.0f)),
>> -                 assign(r, add(r, imm(3.141593f))),
>> -                 assign(r, sub(r, imm(3.141593f)))));
>> +                 assign(r, add(r, imm((float)M_PI))),
>> +                 assign(r, sub(r, imm((float)M_PI)))));
>>        outer_then.emit(inner_if);
>>
>>        /* Else... */
>>        outer_if->else_instructions.push_tail(
>> -         assign(r, mul(sign(y), imm(1.5707965f))));
>> +         assign(r, mul(sign(y), imm((float)M_PI_2))));
>>
>>        body.emit(outer_if);
>>
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev