Reviewed-by: Roland Scheidegger <srol...@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 <mar...@gmail.com>
> Cc: Roland Scheidegger <srol...@vmware.com>
> Cc: Ian Romanick <i...@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) )
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
