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

Check for Infinity and NaN values in calculations #3613

Closed
hpinkos opened this issue Feb 19, 2016 · 4 comments
Closed

Check for Infinity and NaN values in calculations #3613

hpinkos opened this issue Feb 19, 2016 · 4 comments

Comments

@hpinkos
Copy link
Contributor

hpinkos commented Feb 19, 2016

Discussed in #3605
"We should start adding validation like this (within debug pragmas) in many more places. Basically, throw as soon as any calculation anywhere produces a NaN. This would help to work to finally track down the dreaded "invalid array length" in the frustum code, which is where all NaNs eventually end up crashing currently."

Few examples I stumbled across:

In barycentricCoordinates:
var q = 1.0 / (dot00 * dot11 - dot01 * dot01);
In GeometryPipeline.computeBinormalAndTangent:
var r = 1.0 / ((st[i12] - wx) * t2 - (st[i22] - wx) * t1);

Because 1 / 0 = Infinity

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 19, 2016

One detail: instead of, for example, checking for infinity afterward, we generally want to add a conditional to detect divide by zero beforehand.

@hpinkos
Copy link
Contributor Author

hpinkos commented Feb 19, 2016

👍 good call

@kring
Copy link
Member

kring commented Feb 21, 2016

Sylvain wrote a nifty tool years ago that would modify .NET code to automatically insert a "check for NaN and infinity and throw" after each floating point arithmetic statement. It always felt a bit like overkill to me, but maybe something to consider.

Also:

One detail: instead of, for example, checking for infinity afterward, we generally want to add a conditional to detect divide by zero beforehand.

I don't agree with this, because it's unreliable. In x / y, a y close but not equal to zero can also result in infinity. And how close it has to be to zero depends on how big x is. Sometimes you can pick a sensible epsilon, but usually epsilons are just pure evil (e.g. does the size of the epsilon need to change if your ellipsoid is Earth's moon versus the sun?). Relative epsilons can help, but they're hard to pick intuitively... at least for me, and I've seen plenty of bugs result from epsilons picked by other folks, too. The most robust thing is to simply check for the bad case after you do the operation.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 21, 2016

The most robust thing is to simply check for the bad case after you do the operation.

OK with me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants