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

[Matt Turner]

On Mon, Apr 7, 2014 at 11:39 AM, Ian Romanick <idr at freedesktop.org> wrote:
> 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?

Nope, I didn't. My bad.