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

[Marek Olšák]

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Jul 14, 2017 6:38 AM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:

> 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) )
> --
> 2.13.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170714/979c830e/attachment-0001.html>