-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
VFPU: Ensure that sin(4*x) returns 0.0 (and cos 1) for all x. Fixes #2921 #6329
Conversation
Was just perusing the comments in #2921, and that was some fine investigative work that led to the solution by @unknownbrackets (even if I didn't understand most of it, was not a math major in college, heh.) Just wanted to point that out. |
Ah yes, sorry :) I did compliment unknown on his work in the thread though :) |
angle -= floorf(angle * 4.0f) * 0.25f; | ||
angle *= (float)M_PI_2; | ||
sine = sinf(angle); | ||
cosine = cosf(angle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android has sincosf() these days (latest ndks) right? Though not sure what our build req is.
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the buildbot upgraded to the latest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last I checked it seemed to have ndk-r9b, from October 2013.
https://code.google.com/p/android/issues/detail?id=38423
Was marked "released" October 31, 2013. Not sure if that means it's in b or c...
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it didn't actually make it until d, but not sure. Maybe c. Pretty sure not b.
} | ||
|
||
inline void vfpu_sincos(float angle, float &sine, float &cosine) { | ||
angle -= floorf(angle * 4.0f) * 0.25f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, not sure this is rounding right?
Angle 1 = 1_pi/2 = sin=1,cos=0
Angle 2 = 2_pi/2 = sin=0,cos=-1
Angle 3 = 3_pi/2 = sin=-1,cos=0
Angle 4 = 4_pi/2 = sin=0,cos=1
However, 3 - floor(3 * 4) * 0.25
= 3 - 12 * 0.25
= 0
. Should be * 0.25 / 4, right?
-[Unknown]
Hmm, oops. Logic shortcircuit in my brain, heh. Will fix. |
Problems have been fixed. |
VFPU: Ensure that sin(4*x) returns 0.0 (and cos 1) for all x. Fixes #2921
This also makes it easier to replace our VFPU sin/cos implementations with some approximation later.
Note that our sinf/cosf function replacements should not call these, they are expected to behave like libc sinf/cosf, I would think.
Fixes #2921