<div dir="auto">Reviewed-by: Marek Olšák <<a href="mailto:marek.olsak@amd.com">marek.olsak@amd.com</a>><div dir="auto"><br></div><div dir="auto">Marek</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Jul 14, 2017 6:38 AM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The previous implementation of CLAMP() allowed NaN to pass through<br>
unscathed, by failing both comparisons.  NaN isn't exactly a value<br>
between MIN and MAX, which can break the assumptions of many callers.<br>
<br>
This patch changes CLAMP to convert NaN to MIN, arbitrarily.  Callers<br>
that need NaN to be handled in a specific manner should probably open<br>
code something, or use a macro specifically designed to do that.<br>
<br>
Section 2.3.4.1 of the OpenGL 4.5 spec says:<br>
<br>
   "Any representable floating-point value is legal as input to a GL<br>
    command that requires floating-point data. The result of providing a<br>
    value that is not a floating-point number to such a command is<br>
    unspecified, but must not lead to GL interruption or termination.<br>
    In IEEE arithmetic, for example, providing a negative zero or a<br>
    denormalized number to a GL command yields predictable results,<br>
    while providing a NaN or an infinity yields unspecified results."<br>
<br>
While CLAMP may apply to more than just GL inputs, it seems reasonable<br>
to follow those rules, and allow MIN as an "unspecified result".<br>
<br>
This prevents assertion failures in i965 when running the games<br>
"XCOM: Enemy Unknown" and "XCOM: Enemy Within", which call<br>
<br>
   glTexEnv(GL_TEXTURE_FILTER_<wbr>CONTROL_EXT, GL_TEXTURE_LOD_BIAS_EXT,<br>
            -nan(0x7ffff3));<br>
<br>
presumably unintentionally.  i965 clamps the LOD bias to be in range,<br>
and asserts that it's in the proper range when converting to fixed<br>
point.  NaN is not, so it crashed.  We'd like to at least avoid that.<br>
<br>
Cc: Marek Olšák <<a href="mailto:maraeo@gmail.com">maraeo@gmail.com</a>><br>
Cc: Roland Scheidegger <<a href="mailto:sroland@vmware.com">sroland@vmware.com</a>><br>
Cc: Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>><br>
---<br>
 src/gallium/auxiliary/util/u_<wbr>math.h | 3 ++-<br>
 src/util/macros.h                   | 4 ++--<br>
 2 files changed, 4 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/src/gallium/auxiliary/util/<wbr>u_math.h b/src/gallium/auxiliary/util/<wbr>u_math.h<br>
index 2ab5f03a662..a441b5457b2 100644<br>
--- a/src/gallium/auxiliary/util/<wbr>u_math.h<br>
+++ b/src/gallium/auxiliary/util/<wbr>u_math.h<br>
@@ -605,8 +605,9 @@ util_memcpy_cpu_to_le32(void * restrict dest, const void * restrict src, size_t<br>
 /**<br>
  * Clamp X to [MIN, MAX].<br>
  * This is a macro to allow float, int, uint, etc. types.<br>
+ * We arbitrarily turn NaN into MIN.<br>
  */<br>
-#define CLAMP( X, MIN, MAX )  ( (X)<(MIN) ? (MIN) : ((X)>(MAX) ? (MAX) : (X)) )<br>
+#define CLAMP( X, MIN, MAX )  ( (X)>(MIN) ? ((X)>(MAX) ? (MAX) : (X)) : (MIN) )<br>
<br>
 #define MIN2( A, B )   ( (A)<(B) ? (A) : (B) )<br>
 #define MAX2( A, B )   ( (A)>(B) ? (A) : (B) )<br>
diff --git a/src/util/macros.h b/src/util/macros.h<br>
index a10f1de8145..a66f1bfed07 100644<br>
--- a/src/util/macros.h<br>
+++ b/src/util/macros.h<br>
@@ -244,8 +244,8 @@ do {                       \<br>
 /** Compute ceiling of integer quotient of A divided by B. */<br>
 #define DIV_ROUND_UP( A, B )  ( (A) % (B) == 0 ? (A)/(B) : (A)/(B)+1 )<br>
<br>
-/** Clamp X to [MIN,MAX] */<br>
-#define CLAMP( X, MIN, MAX )  ( (X)<(MIN) ? (MIN) : ((X)>(MAX) ? (MAX) : (X)) )<br>
+/** Clamp X to [MIN,MAX].  Turn NaN into MIN, arbitrarily. */<br>
+#define CLAMP( X, MIN, MAX )  ( (X)>(MIN) ? ((X)>(MAX) ? (MAX) : (X)) : (MIN) )<br>
<br>
 /** Minimum of two values: */<br>
 #define MIN2( A, B )   ( (A)<(B) ? (A) : (B) )<br>
--<br>
2.13.3<br>
<br>
</blockquote></div></div>