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

[Samuel Pitoiset]

On 01/11/2017 07:33 PM, Jason Ekstrand wrote:
> One trivial request:  If we do land this patch, please include a link
> the the mailing list archives in the commit message so that we can
> easily track down this discussion if we ever need to in the future.

Sure, I was planning to do it. :-)

But it's unclear if we should or not push it.

>
> On Fri, Jan 6, 2017 at 1:42 AM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com <mailto:samuel.pitoiset at gmail.com>> wrote:
>
>     D3D always computes the absolute value while GLSL says that the
>     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
>     <https://bugs.freedesktop.org/show_bug.cgi?id=97338>
>
>     Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com
>     <mailto: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)
>
>      /** @} */
>
>     --
>     2.11.0
>
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>
>