Skip to content

ray: simplify nan checking by creating a macro.#225

Merged
ebassi merged 1 commit intoebassi:masterfrom
ericonr:isnan
Jun 6, 2021
Merged

ray: simplify nan checking by creating a macro.#225
ebassi merged 1 commit intoebassi:masterfrom
ericonr:isnan

Conversation

@ericonr
Copy link
Contributor

@ericonr ericonr commented Apr 11, 2021

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.

[1] #223
[2] https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3976

Fixes #223

@Gottox
Copy link
Contributor

Gottox commented Apr 11, 2021

Be aware of this comment: #174 (comment)

@ericonr
Copy link
Contributor Author

ericonr commented Apr 11, 2021

@Gottox I am still using isnanf when available, just avoiding the code duplication resulting from how it was originally implemented.

@ericonr
Copy link
Contributor Author

ericonr commented May 17, 2021

Ping?

Copy link
Owner

@ebassi ebassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

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] #223
[2] https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3976
[3] #174 (comment)
@ebassi ebassi merged commit cd4a490 into ebassi:master Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong results for ray/box intersection on systems without isnanf

4 participants

Comments