-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Camera Fix #3810
Camera Fix #3810
Conversation
Update CHANGES.md. Is it possible to add a reasonable test to verify that the vectors are orthonormal? @bagnell merge when you are happy. |
I believe that's already happening here: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Scene/Camera.js#L490-L506 but maybe the logic requires the vectors to be normalized before hand. |
Oh.. do you mean a spec test? |
Yes, for the unit tests. |
@@ -507,7 +507,6 @@ defineSuite([ | |||
moveMouse(MouseButtons.LEFT, startPosition, endPosition, true); | |||
updateController(); | |||
expect(camera.position).toEqual(position); | |||
expect(camera.direction).not.toEqual(Cartesian3.normalize(Cartesian3.negate(camera.position, new Cartesian3()), new Cartesian3())); |
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 removed this line because it wasn't actually doing anything. It only passed before because (0,0,-0.999999) did not equal (0,0,-1.0). Or maybe it should be checking .toEqual
instead of .not.toEqual
?
Fixes #3178
Only the last change was required to fix the problem, but the others looked similar enough that I changed them too. Another approach is normalizing the direction, up, and right vectors in
Camera.updateMembers
to catch all cases of this happening, like: