Crash rotating padded map — latitude must not be NaN #15165
Comments
1ec5
changed the title
As of #14813, the content inset is added to this padding. The code snippet above sets the top inset to 80% of the view’s height and the bottom inset to 20% of the view’s height. Together, the insets add up to the full height of the view. While it may seem rather contrived to pad the entire view, this is in fact how various implementations affix the user puck at just the right spot on the screen. |
@1ec5 , |
Suggest removing release blocker: this is reproducible (the same exception is thrown |
In fact, on a tilted map, setting the edge padding (not the content inset) like this is the only way to ensure that the map’s center coordinate is at the desired screen coordinate – important for positioning the user puck at the correct location. We implemented this approach in mapbox/mapbox-navigation-ios#402 because of the need to synchronize the user puck view with a specific geographic coordinate while keeping the view’s screen coordinate fixed: The map SDK takes a somewhat different approach for its built-in puck: mapbox-gl-native/platform/ios/src/MGLMapView.mm Lines 5655 to 5671 in 4a93f39 which is easier to see if we simplify the formula: CGRect bounds = self.bounds;
return UIEdgeInsetsMake(correctPoint.y - CGRectGetMidY(bounds),
correctPoint.x - CGRectGetMidX(bounds),
CGRectGetMidY(bounds) - correctPoint.y,
CGRectGetMidX(bounds) - correctPoint.x);The map SDK’s edge padding results in a screen rectangle as large as the original content frame, shifted according to the desired anchor point. But this approach is inaccurate because, on a tilted map, the scale varies as you go from the bottom of the screen to the top of the screen. It’s even more inaccurate if there’s an additional top content inset. The navigation SDK’s approach would be unnecessary if there were a way to set the map’s camera with respect to an |
|
I think it would be OK to disallow covering the entire map with content insets, as long as it remains valid to cover the entire map with non-persistent edge padding, as required for user location tracking. If we disallow full-view content insets, I think the exception should happen when setting the content insets, not when the user rotates the map and the map view asks its delegate whether rotating should be allowed. |
A map that has a top and bottom content inset crashes when the user rotates it with two fingers.
Steps to reproduce
Add the following code to any view controller with a full-screen map:
For example, iosapp:
Wait for the map to fully load, then perform a two-finger rotation gesture in either direction.
Logs
The crash occurs here:
mapbox-gl-native/include/mbgl/util/geo.hpp
Lines 34 to 36 in 4a93f39
Here’s some of the console spew leading up to the crash:
Diagnosis
The passed-in padding is zero on all sides, despite the warning about vertical or horizontal padding. However, it does seem incorrect that
-[MGLMapView cameraByZoomingToZoomLevel:aroundAnchorPoint:]calls-cameraThatFitsCoordinateBounds:rather than-cameraThatFitsCoordinateBounds:edgePadding:.mapbox-gl-native/platform/ios/src/MGLMapView.mm
Line 2081 in 4a93f39
Configuration
Mapbox SDK versions: 4a93f39
iOS/macOS versions: iOS 12.2
Device/simulator models: iPhone X
Xcode version: 10.2.1
/cc @mapbox/maps-ios @astojilj
The text was updated successfully, but these errors were encountered: