On Tue, Jan 10, 2017 at 9:48 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> 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). > As a quick addendum, this won't actually hurt our performance (at least not on modern hardware) because we can do source modifiers on SQRT and RSQ. > On Tue, Jan 10, 2017 at 1:24 AM, Samuel Pitoiset < > samuel.pitoi...@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 >>>> >>>> Signed-off-by: Samuel Pitoiset <samuel.pitoi...@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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
