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

More precision for fastCos and fastSin #264

Merged
merged 4 commits into from
Nov 14, 2014

Conversation

Nojan
Copy link

@Nojan Nojan commented Nov 5, 2014

I tested the precision of glm::fastCos and glm::fastSin against glm::cos and glm::sin on the range [-pi pi]. The max error is ~0.2 on my computer.
I've made some changes and now it's ~0.000007. It's also slightly faster and work on a wider range.

To give some context, I had a problem of cross platform float determinism whith glm::cos and glm::sin, because theses methods rely on .
Using the fastCos and fastSin solve my determinism problem, but the accuracy was not good enough anymore.

If you are ok with the code, I can improve the rest of the fast_trigonometry.
For reference, I used the same method as the one found in "A Guide to Approximations" by Jack G. Ganssle

fastCos and fastSin had a max error of ~0.2 on [-pi pi].
The updated version is ~0.000007.
@@ -67,6 +73,12 @@ namespace glm
}

template <typename genType>
GLM_FUNC_QUALIFIER genType three_half_pi()
Copy link
Member

Choose a reason for hiding this comment

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

This name is confusing. I had to make calculation to understand what it meant.

"three_over_two_pi" follows better GLM precedent.

@Groovounet
Copy link
Member

It looks good. It could be valuable to had some performance tests in a similar fashion than https://github.com/g-truc/glm/blob/vectorize/test/gtc/gtc_bitfield.cpp.

But really, the fast trigonometry is pretty broken right now so regardless, it's a very valuable addition.

Thanks,
Christophe

Groovounet added a commit that referenced this pull request Nov 14, 2014
More precision for fastCos and fastSin #264
@Groovounet Groovounet merged commit 5c85bcc into g-truc:0.9.5 Nov 14, 2014
@Groovounet Groovounet added the bug label Nov 14, 2014
@Groovounet Groovounet added this to the GLM 0.9.6 milestone Nov 14, 2014
@Groovounet
Copy link
Member

I didn't realized that your pull request was against 0.9.5. I'll manually pull it for 0.9.6.

Thanks,
Christophe

@Groovounet Groovounet self-assigned this Nov 14, 2014
Groovounet pushed a commit that referenced this pull request Nov 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants