Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

Wrap coordinates converted from points#4464

Merged
1ec5 merged 1 commit into
release-ios-3.2.0-android-4.0.0from
1ec5-convert-unwrapped-4444
Mar 25, 2016
Merged

Wrap coordinates converted from points#4464
1ec5 merged 1 commit into
release-ios-3.2.0-android-4.0.0from
1ec5-convert-unwrapped-4444

Conversation

@1ec5
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 commented Mar 24, 2016

The release-ios-3.2.0-android-4.0.0 branch includes #4275 but not #4285, meaning that -[MGLMapView convertPoint:toCoordinateFromView:] and dependent methods return latitudes that aren’t wrapped to ±180°. This change restores the wrapping on an interim basis.

@jfirebaugh has made the point that this method shouldn’t wrap anyways, but we don’t want to bump the major version number so close to a release. Although the wrapping isn’t documented anywhere, it’s consistent with MapKit, and it’s very likely that developers have come to rely on this behavior (as has iosapp). We can revisit unwrapping in a future release.

If a developer needs an unwrapped coordinate, they can detect this edge case by comparing the return values of -[MGLMapView convertPoint:toCoordinateFromView:] for two different points on screen (and accounting for rotation and pitch of course). In fact, this is what -convertRect:toRectToView: does (although that may be dead code due to 63820d1 for #3401).

/ref #4444
/cc @brunoabinader @bleege @boundsj

@1ec5 1ec5 self-assigned this Mar 24, 2016
@1ec5 1ec5 added bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS telemetry Integration with Mapbox Telemetry libraries labels Mar 24, 2016
@1ec5 1ec5 added this to the ios-v3.2.0 milestone Mar 24, 2016
@boundsj
Copy link
Copy Markdown
Contributor

boundsj commented Mar 25, 2016

👍

@1ec5 1ec5 force-pushed the 1ec5-convert-unwrapped-4444 branch from fc76689 to 2a56565 Compare March 25, 2016 18:03
@1ec5
Copy link
Copy Markdown
Contributor Author

1ec5 commented Mar 25, 2016

The Android build failure is #4473. Merging.

@1ec5 1ec5 merged commit 2a56565 into release-ios-3.2.0-android-4.0.0 Mar 25, 2016
@1ec5 1ec5 deleted the 1ec5-convert-unwrapped-4444 branch March 25, 2016 18:18
@brunoabinader
Copy link
Copy Markdown
Contributor

Looks good, @1ec5!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS telemetry Integration with Mapbox Telemetry libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants