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

InnerSpace::normalize produces a Vector of NaNs when called on a zeroed Vector #450

Open
mitchmindtree opened this issue Mar 29, 2018 · 3 comments

Comments

@mitchmindtree
Copy link

mitchmindtree commented Mar 29, 2018

This makes sense of course as we divide by the magnitude! However it can be a little tricky for newer users to hunt down the source of these NaNs.

Processing solves this by first checking if the magnitude is zero and if so returns [0, 0]. This is a little more expensive but perhaps also a little more user-friendly?

Perhaps we could leave the normalize method as is and instead add a new method which does this check? Currently it looks like the only solution for users is to 1. check that v.magnitude() > 0.0 before calling normalize() (this means that magnitude ends up gettinng calculated twice) or 2. implement their own normalize function.

cc @JoshuaBatty

@brendanzab
Copy link
Collaborator

Might be interesting to compare to what other maths libraries do with this. Perhaps some documentation on this behaviour could also help. 🤔

@richard-uk1
Copy link

richard-uk1 commented Apr 22, 2018

If you use magnitude2 you avoid the sqrt, otherwise yeah could add a try_normalize

Or alternatively you could add an example to the docs using is_finite.

@thefakeplace
Copy link

this got me while I was coding

I picked the cgmath because it’s marketed as “for games/CG” and this behavior is totally useless in games/CG

adding checked_normalize is one option, but IMO, should just change magnitude to return 0 in this case. it aligns better with the project goals, and I seriously doubt anybody is relying on this behavior for anything— pragmatism is cool in “games/CG”.

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

No branches or pull requests

4 participants