[Mesa-dev] [PATCH 2/2] util: Make CLAMP turn NaN into MIN.

[Roland Scheidegger]

Reviewed-by: Roland Scheidegger <sroland at vmware.com>

Interesting side-effect there with the results being different if max >
min. But hopefully not an issue anywhere else...

Roland

Am 14.07.2017 um 06:38 schrieb Kenneth Graunke:
> 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.
> 
> Cc: Marek Olšák <maraeo at gmail.com>
> Cc: Roland Scheidegger <sroland at vmware.com>
> Cc: Ian Romanick <idr at freedesktop.org>
> ---
>  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 2ab5f03a662..a441b5457b2 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 a10f1de8145..a66f1bfed07 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) )
>