From 9644870c89d9a10e656e27331aff54f202c99d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Nogueira?= Date: Sun, 11 Apr 2021 16:11:52 -0300 Subject: [PATCH] ray: simplify nan checking by creating a macro. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoids the #ifdef forest and code duplication resulting from it. There was mismatch in the two code paths, see [1] and [2], and this commit avoids repeating the same mistake. While there, use isnan() instead of fpclassify() == FP_NAN for the case where isnanf() isn't available. We use isnanf() (if available) instead of isnan() due to [3]: I initially used isinf() and isnan(), but those ended up breaking when using GCC because it tried to promote floats to doubles, and the results wouldn't match any more—especially when using GCC vectorisation. It could very well be a bug in GCC. [1] https://github.com/ebassi/graphene/issues/223 [2] https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3976 [3] https://github.com/ebassi/graphene/pull/174#issuecomment-561325167 --- src/graphene-private.h | 8 ++++++++ src/graphene-ray.c | 27 ++++----------------------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/graphene-private.h b/src/graphene-private.h index 32cb9de6..50549659 100644 --- a/src/graphene-private.h +++ b/src/graphene-private.h @@ -105,6 +105,14 @@ #endif /* __GNUC__ */ +#if defined(_M_X64) && ! defined(isnanf) +# define graphene_isnan(x) _isnanf(x) +#elif defined(HAVE_ISNANF) +# define graphene_isnan(x) isnanf(x) +#else +# define graphene_isnan(x) isnan(x) +#endif + static inline bool graphene_approx_val (float a, float b) { diff --git a/src/graphene-ray.c b/src/graphene-ray.c index 66c33931..b5a1e107 100644 --- a/src/graphene-ray.c +++ b/src/graphene-ray.c @@ -52,11 +52,6 @@ #include #include -#if defined(_M_X64) && ! defined(isnanf) -# define isnanf(x) _isnanf(x) -# define HAVE_ISNANF -#endif - /** * graphene_ray_alloc: (constructor) * @@ -555,17 +550,10 @@ graphene_ray_intersect_box (const graphene_ray_t *r, /* These lines also handle the case where tx_min or tx_max is NaN * (result of 0 * INFINITY): NaN != NaN */ -#ifdef HAVE_ISNANF - if (ty_min > tx_min || isnanf (tx_min)) - tx_min = ty_min; - if (ty_max < tx_max || isnanf (tx_max)) - tx_max = ty_max; -#else - if (ty_min > tx_min || fpclassify (tx_min) == FP_NAN) + if (ty_min > tx_min || graphene_isnan (tx_min)) tx_min = ty_min; - if (ty_max > tx_max || fpclassify (tx_max) == FP_NAN) + if (ty_max < tx_max || graphene_isnan (tx_max)) tx_max = ty_max; -#endif float tz_min, tz_max; if (graphene_vec3_get_z (&inv_dir) >= 0.f) @@ -582,17 +570,10 @@ graphene_ray_intersect_box (const graphene_ray_t *r, if ((tx_min > tz_max) || (tz_min > tx_max)) return GRAPHENE_RAY_INTERSECTION_KIND_NONE; -#ifdef HAVE_ISNANF - if (tz_min > tx_min || isnanf (tx_min)) - tx_min = tz_min; - if (tz_max < tx_max || isnanf (tx_max)) - tx_max = tz_max; -#else - if (tz_min > tx_min || fpclassify (tx_min) == FP_NAN) + if (tz_min > tx_min || graphene_isnan (tx_min)) tx_min = tz_min; - if (tz_max < tx_max || fpclassify (tx_max) == FP_NAN) + if (tz_max < tx_max || graphene_isnan (tx_max)) tx_max = tz_max; -#endif /* return the point closest to the ray (positive side) */ if (tx_max < 0)