Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 6, 2021
Merged

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

merged 1 commit into from
Jun 6, 2021

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.

src/graphene-ray.c Outdated Show resolved Hide resolved
@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!

src/graphene-ray.c Outdated Show resolved Hide resolved
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