From 751aff2ddd1770d9842c273f65e8aad768112151 Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Fri, 4 Nov 2016 13:50:53 -0700 Subject: [PATCH] [ios, macos] Work around annotation race condition (#6924) When annotations are removed concurrently with being updated because of a map redraw, it is possible that mbgl will still consider an annotation to exist depending on what part of the map lifecycle the call update annotations happens. However, the annotation context by tag map in MGLMapView (on the main thread) will always be up to date. This adds a guard that uses that map to avoid considering annotations that have actually been removed as visible. Another issue was that adding the same annotation object twice has always been allowed and has worked, visually. However, a recent refactor to make the reuse queue work correctly is not tolerant of the a single view representing two annotation instances that are the same. The effect is that a new UIView will be generated for each (clone) annotation that is added and only the last view will move with the map. The other views become detached and float around in the view as the map is panned. This commit adds a guard to make adding an annotation instance a no-op if it has already been added before. --- platform/ios/src/MGLMapView.mm | 12 +++++++++++- platform/macos/src/MGLMapView.mm | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 25a9b8518..00d2425a4 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -2836,7 +2836,11 @@ public: for (auto const& annotationTag: annotationTags) { - MGLAnnotationContext annotationContext = _annotationContextsByAnnotationTag[annotationTag]; + if (!_annotationContextsByAnnotationTag.count(annotationTag)) + { + continue; + } + MGLAnnotationContext annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); [annotations addObject:annotationContext.annotation]; } @@ -2903,6 +2907,12 @@ public: { NSAssert([annotation conformsToProtocol:@protocol(MGLAnnotation)], @"annotation should conform to MGLAnnotation"); + // adding the same annotation object twice is a no-op + if ([self.annotations containsObject:annotation]) + { + continue; + } + if ([annotation isKindOfClass:[MGLMultiPoint class]]) { // The polyline or polygon knows how to style itself (with the map view’s help). diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index 4df365378..6679aaaa6 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -1635,7 +1635,11 @@ public: for (auto const& annotationTag: annotationTags) { - MGLAnnotationContext annotationContext = _annotationContextsByAnnotationTag[annotationTag]; + if (!_annotationContextsByAnnotationTag.count(annotationTag)) + { + continue; + } + MGLAnnotationContext annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); [annotations addObject:annotationContext.annotation]; } @@ -1687,6 +1691,12 @@ public: for (id annotation in annotations) { NSAssert([annotation conformsToProtocol:@protocol(MGLAnnotation)], @"Annotation does not conform to MGLAnnotation"); + // adding the same annotation object twice is a no-op + if ([self.annotations containsObject:annotation]) + { + continue; + } + if ([annotation isKindOfClass:[MGLMultiPoint class]]) { // The multipoint knows how to style itself (with the map view’s help). MGLMultiPoint *multiPoint = (MGLMultiPoint *)annotation;