-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Fix flyTo rectangle in 2D #3695
Conversation
Thanks @novacto2! Can you please send in a Contributor License Agreement so we can review this? |
We now have a CLA from @novacto2. Thanks! |
I'm not sure if deleting that code is the correct solution here. |
I've made some changes @hpinkos It seems that the transformation of the camera position from cartesian to cartographic gives a negative height (in this specific case - rectangle in 2D). The computed height is stored in the 'z' component of cartesian, so the zoom limits should be tested on this number. |
Thanks for the update! @bagnell, does this code look good to you? |
if (destinationCartographic.height === sscc.minimumZoomDistance || destinationCartographic.height === sscc.maximumZoomDistance) { | ||
destination = ellipsoid.cartographicToCartesian(destinationCartographic); | ||
} | ||
if (mode === SceneMode.SCENE2D && isRectangle) { |
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.
This is true for Columbus view as well. I would change this to mode !== SceneMode.SCENE3D
.
Looks good besides the one comment and formatting. @hpinkos Can you merge when you're ready? |
Alright, looks great @novacto2! Thanks! |
Is it ok @hpinkos ? |
Thanks @novacto2! Just a few tiny things: And it looks like your formatting is still off. Please see the Coding Guide |
I'm sorry about the mistakes. Hope it's ok now. Thanks for going easy on me @hpinkos :) |
Looks great! Thanks @novacto2! |
This is for #3688
Zoom limits check leads to a change of coordinates.