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

[Krzysztof Cybulski]

W dniu 10.01.2017 o 18:48, Jason Ekstrand pisze:

Hi,

Can we add workaround to drirc and enable this only when needed?

Best Regards
Krzysztof Cybulski

> I'll be honest, I'm not a fan... Given that D3D10 has one defined 
> behavior, D3D9 has another, and GL doesn't specify, I don't really 
> think we should be making a global change to all drivers to do the 
> D3D9 behavior just to fix one app.  Sure, other apps probably have the 
> same bug, but are we going to have apps that expect the D3D10 behavior 
> that we've now explicitly made not work?
>
> If we're going to hack around an app bug, I would really rather see it 
> behind a driconf option rather than a global change to driver 
> behavior.  Even better, it'd be cool if we could see the app get 
> fixed. (Yes, I know that's not likely).
>
> On Tue, Jan 10, 2017 at 1:24 AM, Samuel Pitoiset 
> <samuel.pitoiset at gmail.com <mailto:samuel.pitoiset at gmail.com>> wrote:
>
>
>
>     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
>             <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)
>
>              /** @} */
>
>
>
>     _______________________________________________
>     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>
>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170111/7915ec02/attachment-0001.html>