Mesa (master): util: Make CLAMP turn NaN into MIN.

Kenneth Graunke kwg at kemper.freedesktop.org
Wed Jul 19 06:48:58 UTC 2017


Module: Mesa
Branch: master
Commit: 2412c4c81ea0488df865817a0de91ec46e359b72
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=2412c4c81ea0488df865817a0de91ec46e359b72

Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Thu Jul 13 16:22:12 2017 -0700

util: Make CLAMP turn NaN into MIN.

The previous implementation of CLAMP() allowed NaN to pass through
unscathed, by failing both comparisons.  NaN isn't exactly a value
between MIN and MAX, which can break the assumptions of many callers.

This patch changes CLAMP to convert NaN to MIN, arbitrarily.  Callers
that need NaN to be handled in a specific manner should probably open
code something, or use a macro specifically designed to do that.

Section 2.3.4.1 of the OpenGL 4.5 spec says:

   "Any representable floating-point value is legal as input to a GL
    command that requires floating-point data. The result of providing a
    value that is not a floating-point number to such a command is
    unspecified, but must not lead to GL interruption or termination.
    In IEEE arithmetic, for example, providing a negative zero or a
    denormalized number to a GL command yields predictable results,
    while providing a NaN or an infinity yields unspecified results."

While CLAMP may apply to more than just GL inputs, it seems reasonable
to follow those rules, and allow MIN as an "unspecified result".

This prevents assertion failures in i965 when running the games
"XCOM: Enemy Unknown" and "XCOM: Enemy Within", which call

   glTexEnv(GL_TEXTURE_FILTER_CONTROL_EXT, GL_TEXTURE_LOD_BIAS_EXT,
            -nan(0x7ffff3));

presumably unintentionally.  i965 clamps the LOD bias to be in range,
and asserts that it's in the proper range when converting to fixed
point.  NaN is not, so it crashed.  We'd like to at least avoid that.

Reviewed-by: Roland Scheidegger <sroland at vmware.com>
Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

---

 src/gallium/auxiliary/util/u_math.h | 3 ++-
 src/util/macros.h                   | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h
index 2ab5f03a66..a441b5457b 100644
--- a/src/gallium/auxiliary/util/u_math.h
+++ b/src/gallium/auxiliary/util/u_math.h
@@ -605,8 +605,9 @@ util_memcpy_cpu_to_le32(void * restrict dest, const void * restrict src, size_t
 /**
  * Clamp X to [MIN, MAX].
  * This is a macro to allow float, int, uint, etc. types.
+ * We arbitrarily turn NaN into MIN.
  */
-#define CLAMP( X, MIN, MAX )  ( (X)<(MIN) ? (MIN) : ((X)>(MAX) ? (MAX) : (X)) )
+#define CLAMP( X, MIN, MAX )  ( (X)>(MIN) ? ((X)>(MAX) ? (MAX) : (X)) : (MIN) )
 
 #define MIN2( A, B )   ( (A)<(B) ? (A) : (B) )
 #define MAX2( A, B )   ( (A)>(B) ? (A) : (B) )
diff --git a/src/util/macros.h b/src/util/macros.h
index a10f1de814..a66f1bfed0 100644
--- a/src/util/macros.h
+++ b/src/util/macros.h
@@ -244,8 +244,8 @@ do {                       \
 /** Compute ceiling of integer quotient of A divided by B. */
 #define DIV_ROUND_UP( A, B )  ( (A) % (B) == 0 ? (A)/(B) : (A)/(B)+1 )
 
-/** Clamp X to [MIN,MAX] */
-#define CLAMP( X, MIN, MAX )  ( (X)<(MIN) ? (MIN) : ((X)>(MAX) ? (MAX) : (X)) )
+/** Clamp X to [MIN,MAX].  Turn NaN into MIN, arbitrarily. */
+#define CLAMP( X, MIN, MAX )  ( (X)>(MIN) ? ((X)>(MAX) ? (MAX) : (X)) : (MIN) )
 
 /** Minimum of two values: */
 #define MIN2( A, B )   ( (A)<(B) ? (A) : (B) )




More information about the mesa-commit mailing list