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

Override trig functions for infinite precision in set cases #81

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eric-wieser
Copy link

This fixes #80

The underlying issue here is that lcMatrix44FromEulerAngles(LC_PI/2) was not returning an exact 90 degree rotation matrix.

Overriding sin/cos might be overkill, but it did solve the problem on my machine, and seems pretty harmless anyway

@leozide
Copy link
Owner

leozide commented Jul 19, 2017

I've looked at this for a little bit today, I think using doubles instead of floats is a more generic solution and will work for more cases.

Changing:

RotationMatrix = lcMul(lcMatrix33RotationY(Angles[1] * M_PI / 180), RotationMatrix);

and:

inline lcMatrix33 lcMatrix33RotationY(const double Radians)
{
	float s, c;

	s = sin(Radians);
	c = cos(Radians);

	lcMatrix33 m;

	m.r[0] = lcVector3(   c, 0.0f,   -s);
	m.r[1] = lcVector3(0.0f, 1.0f, 0.0f);
	m.r[2] = lcVector3(   s, 0.0f,    c);

	return m;
}

Fixes the problem you described. I'll need to experiment some more to see where the loss of precision is causing problems (sinf/cosf or LC_DTOR).

@eric-wieser
Copy link
Author

eric-wieser commented Jul 20, 2017

Thanks for taking a look. Moving to doubles would definitely help, but might just hide the problem.

The problem is that in C, sin(2*M_PI) != 0, because 2*M_PI is not actually 2pi, just a floating point number close to 2pi. Even casting to float doesn't discard enough precision for you to get away with it.

Of course, the approach in this patch is sketchy, as it uses equality comparisons of floats. Passing angles in degrees in these cases would work better.

@leozide
Copy link
Owner

leozide commented Jul 21, 2017

I've updated the angle/pi defines and it seems to have solved the problem in a lot of cases.

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.

Precision of orientation is lost after 90 degree rotations
2 participants