<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 10, 2017 at 9:48 AM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div>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?<br><br></div>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).<br></div></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 10, 2017 at 1:24 AM, Samuel Pitoiset <span dir="ltr"><<a href="mailto:samuel.pitoiset@gmail.com" target="_blank">samuel.pitoiset@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><br>
<br>
On 01/09/2017 10:03 PM, Roland Scheidegger wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Am 06.01.2017 um 10:42 schrieb Samuel Pitoiset:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
D3D always computes the absolute value while GLSL says that the<br>
</blockquote>
That should probably say "d3d9" - it is completely wrong for d3d10 and<br>
later (which have it to be defined as a guaranteed NaN).<br>
(Otherwise, I'm still not quite convinced it's the right thing to do<br>
this always, but meh...)<br>
</blockquote>
<br></span>
I'm not familiar with D3D, thanks for noticing. Fixed locally.<div class="m_-8456992462311867174HOEnZb"><div class="m_-8456992462311867174h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Roland<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
result of inversesqrt() is undefined if x <= 0 (and undefined if<br>
x < 0 for sqrt()). But some apps rely on this specific behaviour<br>
which is not clearly defined by OpenGL.<br>
<br>
Computing the absolute value before sqrt()/inversesqrt() will<br>
prevent that, especially for apps which have been ported from D3D.<br>
Note that closed drivers seem to also use that quirk.<br>
<br>
This gets rid of the NaN values in the "Spec Ops: The Line" game<br>
as well as the black squares with radeonsi. Note that Nouveau is<br>
not affected by this bug because we already take the absolute value<br>
when translating from TGSI to nv50/ir.<br>
<br>
Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=97338" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/s<wbr>how_bug.cgi?id=97338</a><br>
<br>
Signed-off-by: Samuel Pitoiset <<a href="mailto:samuel.pitoiset@gmail.com" target="_blank">samuel.pitoiset@gmail.com</a>><br>
---<br>
src/compiler/glsl/builtin_fun<wbr>ctions.cpp | 22 ++++++++++++++++++++--<br>
1 file changed, 20 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/compiler/glsl/builtin_fu<wbr>nctions.cpp b/src/compiler/glsl/builtin_fu<wbr>nctions.cpp<br>
index 797af08b6c..f816f2ff7d 100644<br>
--- a/src/compiler/glsl/builtin_fu<wbr>nctions.cpp<br>
+++ b/src/compiler/glsl/builtin_fu<wbr>nctions.cpp<br>
@@ -3623,12 +3623,30 @@ builtin_builder::_pow(const glsl_type *type)<br>
return binop(always_available, ir_binop_pow, type, type, type);<br>
}<br>
<br>
+ir_function_signature *<br>
+builtin_builder::_sqrt(builti<wbr>n_available_predicate avail,<br>
+ const glsl_type *type)<br>
+{<br>
+ ir_variable *x = in_var(type, "x");<br>
+ MAKE_SIG(type, avail, 1, x);<br>
+ body.emit(ret(expr(ir_unop_sq<wbr>rt, abs(x))));<br>
+ return sig;<br>
+}<br>
+<br>
+ir_function_signature *<br>
+builtin_builder::_inversesqrt<wbr>(builtin_available_predicate avail,<br>
+ const glsl_type *type)<br>
+{<br>
+ ir_variable *x = in_var(type, "x");<br>
+ MAKE_SIG(type, avail, 1, x);<br>
+ body.emit(ret(expr(ir_unop_rs<wbr>q, abs(x))));<br>
+ return sig;<br>
+}<br>
+<br>
UNOP(exp, ir_unop_exp, always_available)<br>
UNOP(log, ir_unop_log, always_available)<br>
UNOP(exp2, ir_unop_exp2, always_available)<br>
UNOP(log2, ir_unop_log2, always_available)<br>
-UNOPA(sqrt, ir_unop_sqrt)<br>
-UNOPA(inversesqrt, ir_unop_rsq)<br>
<br>
/** @} */<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div><div class="HOEnZb"><div class="h5">
</div></div></blockquote></div><br></div></div>