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

Fly across the antimeridan if shorter #1857

Merged
merged 4 commits into from
Dec 22, 2015
Merged

Fly across the antimeridan if shorter #1857

merged 4 commits into from
Dec 22, 2015

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Dec 21, 2015

If flying across the antimeridian yields a shorter flight path than flying across the Prime Meridian, cross the antimeridian. This PR also adds a full complement of unit tests based on the easeTo() unit tests and fixes a couple corner cases that differ between easeTo() and flyTo(): now, if you fly to the current location but with a different zoom level, bearing, or pitch, we ease to the new camera instead of bailing. We also avoid offsetting if the options don’t call for a new center point.

Fixes #1853. Ported from mapbox/mapbox-gl-native@97c143a for mapbox/mapbox-gl-native#3355.

/cc @mourner @lucaswoj

@1ec5 1ec5 added the bug 🐞 label Dec 21, 2015
@1ec5 1ec5 self-assigned this Dec 21, 2015
@jfirebaugh
Copy link
Contributor

Can you add a test please?

@1ec5
Copy link
Contributor Author

1ec5 commented Dec 21, 2015

We don’t have any flyTo tests yet. I’ve been trying to copy the easeTo tests for use as flyTo tests, but I’m getting tripped up by what may be a bug in the existing implementation: the final camera values are always ever-so-slightly off:

        Expected: {"lng":100,"lat":0}
        Actual:   {"lng":99.99999999999994,"lat":0}

In the absence of #1854, I’m making the transition synchronous by setting animate: false in the options object. I don’t know if that has anything to do with the failures.

ref #1773

@jfirebaugh
Copy link
Contributor

Does using fixedLngLat like in the other camera tests help?

@mourner
Copy link
Member

mourner commented Dec 21, 2015

@jfirebaugh I think this is a valid bug, we should set a final position here https://github.com/mapbox/mapbox-gl-js/blob/master/js/ui/camera.js#L632

@jfirebaugh
Copy link
Contributor

@mourner I don't think that should be necessary. The first function passed to this._ease will be called for the final time with a k value corresponding to t=1, and should calculate the correct end point based on that. 99.99999999999994 looks like normal floating point imprecision to me.

1ec5 added 2 commits December 21, 2015 16:18
If the flight path covers no ground, ease to the new camera, so that the requested bearing and pitch take effect. Also, only apply an offset when a new center is requested, for consistency with easeTo().
Based on easeTo() tests, but without the around option and with some additional checks to ensure that the aircraft actually takes off and crosses the prime meridian or antemeridian.
@1ec5
Copy link
Contributor Author

1ec5 commented Dec 22, 2015

Tests added and edge cases fixed. See the original comment, which I’ve edited, for details.

@jfirebaugh
Copy link
Contributor

👍 Awesome

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.

flyTo refuses to cross antimeridian
3 participants