[Mesa-dev] [PATCH] glsl: always do sqrt(abs()) and inversesqrt(abs())

[Samuel Pitoiset]

On 01/09/2017 10:03 PM, Roland Scheidegger wrote:
> Am 06.01.2017 um 10:42 schrieb Samuel Pitoiset:
>> D3D always computes the absolute value while GLSL says that the
> That should probably say "d3d9" - it is completely wrong for d3d10 and
> later (which have it to be defined as a guaranteed NaN).
> (Otherwise, I'm still not quite convinced it's the right thing to do
> this always, but meh...)

I'm not familiar with D3D, thanks for noticing. Fixed locally.

>
> Roland
>
>
>> result of inversesqrt() is undefined if x <= 0 (and undefined if
>> x < 0 for sqrt()). But some apps rely on this specific behaviour
>> which is not clearly defined by OpenGL.
>>
>> Computing the absolute value before sqrt()/inversesqrt() will
>> prevent that, especially for apps which have been ported from D3D.
>> Note that closed drivers seem to also use that quirk.
>>
>> This gets rid of the NaN values in the "Spec Ops: The Line" game
>> as well as the black squares with radeonsi. Note that Nouveau is
>> not affected by this bug because we already take the absolute value
>> when translating from TGSI to nv50/ir.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97338
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>  src/compiler/glsl/builtin_functions.cpp | 22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
>> index 797af08b6c..f816f2ff7d 100644
>> --- a/src/compiler/glsl/builtin_functions.cpp
>> +++ b/src/compiler/glsl/builtin_functions.cpp
>> @@ -3623,12 +3623,30 @@ builtin_builder::_pow(const glsl_type *type)
>>     return binop(always_available, ir_binop_pow, type, type, type);
>>  }
>>
>> +ir_function_signature *
>> +builtin_builder::_sqrt(builtin_available_predicate avail,
>> +                       const glsl_type *type)
>> +{
>> +   ir_variable *x = in_var(type, "x");
>> +   MAKE_SIG(type, avail, 1, x);
>> +   body.emit(ret(expr(ir_unop_sqrt, abs(x))));
>> +   return sig;
>> +}
>> +
>> +ir_function_signature *
>> +builtin_builder::_inversesqrt(builtin_available_predicate avail,
>> +                              const glsl_type *type)
>> +{
>> +   ir_variable *x = in_var(type, "x");
>> +   MAKE_SIG(type, avail, 1, x);
>> +   body.emit(ret(expr(ir_unop_rsq, abs(x))));
>> +   return sig;
>> +}
>> +
>>  UNOP(exp,         ir_unop_exp,  always_available)
>>  UNOP(log,         ir_unop_log,  always_available)
>>  UNOP(exp2,        ir_unop_exp2, always_available)
>>  UNOP(log2,        ir_unop_log2, always_available)
>> -UNOPA(sqrt,        ir_unop_sqrt)
>> -UNOPA(inversesqrt, ir_unop_rsq)
>>
>>  /** @} */
>>
>>
>