From 6c8f46f4c7c44bfc58ed8c9a2c5436c1ca4f23d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Wed, 1 Apr 2020 00:24:59 -0700 Subject: [PATCH 1/2] [ios, macos] Wrap map in unique pointer Also removed unused method declarations. --- platform/ios/src/MGLMapView.mm | 23 ++++++++++------------- platform/macos/src/MGLMapView.mm | 13 +++---------- platform/macos/src/MGLMapView_Private.h | 2 -- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 56ede0bb9b..ab0eba2591 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -280,13 +280,11 @@ @interface MGLMapView () _mbglMap; std::unique_ptr _mbglView; std::unique_ptr _rendererFrontend; @@ -499,7 +497,7 @@ - (void)commonInit .withAssetPath([NSBundle mainBundle].resourceURL.path.UTF8String); NSAssert(!_mbglMap, @"_mbglMap should be NULL"); - _mbglMap = new mbgl::Map(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions); + _mbglMap = std::make_unique(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions); // start paused if in IB if (background) { @@ -716,8 +714,7 @@ - (void)destroyCoreObjects { // Tear down C++ objects, insuring worker threads correctly terminate. // Because of how _mbglMap is constructed, we need to destroy it first. - delete _mbglMap; - _mbglMap = nullptr; + _mbglMap.reset(); _mbglView.reset(); @@ -2807,12 +2804,12 @@ - (void)setShowsScale:(BOOL)showsScale - (void)setPrefetchesTiles:(BOOL)prefetchesTiles { - _mbglMap->setPrefetchZoomDelta(prefetchesTiles ? mbgl::util::DEFAULT_PREFETCH_ZOOM_DELTA : 0); + self.mbglMap.setPrefetchZoomDelta(prefetchesTiles ? mbgl::util::DEFAULT_PREFETCH_ZOOM_DELTA : 0); } - (BOOL)prefetchesTiles { - return _mbglMap->getPrefetchZoomDelta() > 0 ? YES : NO; + return self.mbglMap.getPrefetchZoomDelta() > 0 ? YES : NO; } #pragma mark - Accessibility - @@ -3527,24 +3524,24 @@ - (double)maximumZoomLevel - (CGFloat)minimumPitch { - return *_mbglMap->getBounds().minPitch; + return *self.mbglMap.getBounds().minPitch; } - (void)setMinimumPitch:(CGFloat)minimumPitch { MGLLogDebug(@"Setting minimumPitch: %f", minimumPitch); - _mbglMap->setBounds(mbgl::BoundOptions().withMinPitch(minimumPitch)); + self.mbglMap.setBounds(mbgl::BoundOptions().withMinPitch(minimumPitch)); } - (CGFloat)maximumPitch { - return *_mbglMap->getBounds().maxPitch; + return *self.mbglMap.getBounds().maxPitch; } - (void)setMaximumPitch:(CGFloat)maximumPitch { MGLLogDebug(@"Setting maximumPitch: %f", maximumPitch); - _mbglMap->setBounds(mbgl::BoundOptions().withMaxPitch(maximumPitch)); + self.mbglMap.setBounds(mbgl::BoundOptions().withMaxPitch(maximumPitch)); } - (MGLCoordinateBounds)visibleCoordinateBounds @@ -6476,7 +6473,7 @@ - (void)didFailToLoadImage:(NSString *)imageName { MGLImage *imageToLoad = [self.delegate mapView:self didFailToLoadImage:imageName]; if (imageToLoad) { auto image = [imageToLoad mgl_styleImageWithIdentifier:imageName]; - _mbglMap->getStyle().addImage(std::move(image)); + self.mbglMap.getStyle().addImage(std::move(image)); } } } diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index a234199d4e..61f49c08f6 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -154,7 +154,7 @@ @interface MGLMapView () _mbglMap; std::unique_ptr _mbglView; std::unique_ptr _rendererFrontend; @@ -295,7 +295,7 @@ - (void)commonInit { resourceOptions.withCachePath([[MGLOfflineStorage sharedOfflineStorage] mbglCachePath]) .withAssetPath([NSBundle mainBundle].resourceURL.path.UTF8String); - _mbglMap = new mbgl::Map(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions); + _mbglMap = std::make_unique(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions); // Notify map object when network reachability status changes. _reachability = [MGLReachability reachabilityForInternetConnection]; @@ -539,10 +539,7 @@ - (void)dealloc { // Removing the annotations unregisters any outstanding KVO observers. [self removeAnnotations:self.annotations]; - if (_mbglMap) { - delete _mbglMap; - _mbglMap = nullptr; - } + _mbglMap.reset(); _mbglView.reset(); } @@ -662,10 +659,6 @@ - (void)setPrefetchesTiles:(BOOL)prefetchesTiles _mbglMap->setPrefetchZoomDelta(prefetchesTiles ? mbgl::util::DEFAULT_PREFETCH_ZOOM_DELTA : 0); } -- (mbgl::Map *)mbglMap { - return _mbglMap; -} - - (mbgl::Renderer *)renderer { return _rendererFrontend->getRenderer(); } diff --git a/platform/macos/src/MGLMapView_Private.h b/platform/macos/src/MGLMapView_Private.h index 3d9b36c30a..05e932d84e 100644 --- a/platform/macos/src/MGLMapView_Private.h +++ b/platform/macos/src/MGLMapView_Private.h @@ -55,8 +55,6 @@ namespace mbgl { - (BOOL)isTargetingInterfaceBuilder; -- (nonnull mbgl::Map *)mbglMap; - - (nonnull mbgl::Renderer *)renderer; @end From 3a1ede2edb0a18a8227ed4e08fbf73742ba28651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Wed, 1 Apr 2020 00:59:14 -0700 Subject: [PATCH 2/2] [ios] Avoid removing annotations from torn-down map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the underlying mbgl::Map has been torn down, so has the mbgl annotation, so don’t worry about the mbgl::Map but keep unregistering KVO and other SDK-side annotation resources. --- platform/ios/CHANGELOG.md | 1 + platform/ios/src/MGLMapView.mm | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 9835d0b899..ff34a959be 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -30,6 +30,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT ### Other changes * Added the `MGLMapView.minimumPitch` and `MGLMapView.maximumPitch` properties to further limit how much the user or your code can tilt the map. ([#208](https://github.com/mapbox/mapbox-gl-native-ios/pull/208)) +* Fixed a crash while quitting the application after adding an annotation to `MGLMapView`. ([#252](https://github.com/mapbox/mapbox-gl-native-ios/pull/252)) * Improved performance when continuously animating a tilted map. ([#16287](https://github.com/mapbox/mapbox-gl-native/pull/16287)) * Fixed a memory leak when zooming with any options enabled in the `MGLMapView.debugMask` property. ([#15179](https://github.com/mapbox/mapbox-gl-native/issues/15179)) diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index ab0eba2591..f12354f4db 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -4612,7 +4612,12 @@ - (void)removeAnnotations:(NSArray> *)annotations } _isChangingAnnotationLayers = YES; - self.mbglMap.removeAnnotation(annotationTag); + // If the underlying map is gone, there’s nothing to remove, but still + // continue to unregister KVO and other annotation resources. + if (_mbglMap) + { + self.mbglMap.removeAnnotation(annotationTag); + } } [self updatePresentsWithTransaction];