[Mesa-dev] [PATCH] glsl: improve accuracy of atan()

[Erik Faye-Lund]

On Thu, Sep 25, 2014 at 4:54 PM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
> On Wed, Sep 24, 2014 at 1:35 PM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>> Hm. Don't I need to expand this last immediate to a vector of
>> type->components() size as well?
>>
>> If so, this patch should go on top:
>>
>> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
>> index 1820dd5..bfa46eb 100644
>> --- a/src/glsl/builtin_functions.cpp
>> +++ b/src/glsl/builtin_functions.cpp
>> @@ -2749,13 +2749,11 @@ builtin_builder::do_atan(ir_factory &body,
>> const glsl_type *type, ir_variable *r
>>     /* range-reduction fixup */
>>     body.emit(assign(tmp, add(tmp,
>>                               csel(greater(abs(y_over_x),
>> -                                          swizzle(imm(1.0f),
>> -                                                  SWIZZLE_XXXX,
>> -                                                  type->components())),
>> +                                          imm(1.0f, type->components())),
>>                                    add(mul(tmp,
>>                                            imm(-2.0f)),
>>                                        imm(M_PI_2f)),
>> -                                  imm(0.0f)))));
>> +                                  imm(0.0f, type->components())))));
>>
>>     /* sign fixup */
>>     body.emit(assign(res, mul(tmp, sign(y_over_x))));
>
> Ugh, it seems things fail piglit's shaders/glsl-fs-atan-1 pretty badly
> on swrast.
>
> ../../src/mesa/program/ir_to_mesa.cpp:1426: virtual void
> {anonymous}::ir_to_mesa_visitor::visit(ir_expression*): Assertion
> `!"not supported"' failed.
>
> Ugh, yeah. We don't seem to support ir_trip_csel on swrast, and
> there's no attempt to lower it unless it's condition is constant 0 or
> 1 (in opt_algebraic.cpp). So yeah, this regresses there, but looking
> at the other builtins, so does anything that uses gentype mix(gentype,
> gentype, genBtype) etc. However, that's at least blocked by a GLSL
> v130 check, which swrast does not support.
>
> So, I've got some more work to do :/

OK, that was easier than I thought:

diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
index bfa46eb..c126b60 100644
--- a/src/glsl/builtin_functions.cpp
+++ b/src/glsl/builtin_functions.cpp
@@ -2748,12 +2748,11 @@ builtin_builder::do_atan(ir_factory &body,
const glsl_type *type, ir_variable *r

    /* range-reduction fixup */
    body.emit(assign(tmp, add(tmp,
-                             csel(greater(abs(y_over_x),
-                                          imm(1.0f, type->components())),
+                             mul(b2f(greater(abs(y_over_x),
+                                          imm(1.0f, type->components()))),
                                   add(mul(tmp,
                                           imm(-2.0f)),
-                                      imm(M_PI_2f)),
-                                  imm(0.0f, type->components())))));
+                                      imm(M_PI_2f))))));

    /* sign fixup */
    body.emit(assign(res, mul(tmp, sign(y_over_x))));