[osx] Observe annotation coordinates

MGLMapView observes changes to the coordinate property of each MGLAnnotation added to it. Changing the coordinate property in a KVO-compliant way causes the annotation to be relocated and its callout view, if present, to be dismissed. To avoid observing the same annotation twice yet also avoid expensive lookups when adding or removing annotations, MGLMapView indexes added point annotations in an NSMutableSet.

5 files changed, 84 insertions, 35 deletions

diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index 38e0070571..c2a9ac6a82 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -2697,11 +2697,9 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration)
                 return true;
             }
             
-            UIImage *image = annotationImage.image ? annotationImage.image : fallbackImage;
-            
             // Filter out the annotation if the fattened finger didn’t land
             // within the image’s alignment rect.
-            CGRect annotationRect = [self frameOfImage:image centeredAtCoordinate:annotation.coordinate];
+            CGRect annotationRect = [self frameOfImage:annotationImage.image ?: fallbackImage centeredAtCoordinate:annotation.coordinate];
             return !!!CGRectIntersectsRect(annotationRect, hitRect);
         });
         nearbyAnnotations.resize(std::distance(nearbyAnnotations.begin(), end));
diff --git a/platform/osx/osx.xcodeproj/project.pbxproj b/platform/osx/osx.xcodeproj/project.pbxproj
index 6154483c25..9db664b25e 100644
--- a/platform/osx/osx.xcodeproj/project.pbxproj
+++ b/platform/osx/osx.xcodeproj/project.pbxproj
@@ -14,6 +14,7 @@
 		DA839EA01CC2E3400062CAFB /* MapDocument.xib in Resources */ = {isa = PBXBuildFile; fileRef = DA839E9E1CC2E3400062CAFB /* MapDocument.xib */; };
 		DA839EA21CC2E3400062CAFB /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = DA839EA11CC2E3400062CAFB /* Assets.xcassets */; };
 		DA839EA51CC2E3400062CAFB /* MainMenu.xib in Resources */ = {isa = PBXBuildFile; fileRef = DA839EA31CC2E3400062CAFB /* MainMenu.xib */; };
+		DAC2ABC51CC6D343006D18C4 /* MGLAnnotationImage_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = DAC2ABC41CC6D343006D18C4 /* MGLAnnotationImage_Private.h */; };
 		DAE6C2E21CC304F900DB3429 /* Credits.rtf in Resources */ = {isa = PBXBuildFile; fileRef = DAE6C2E11CC304F900DB3429 /* Credits.rtf */; };
 		DAE6C2ED1CC3050F00DB3429 /* DroppedPinAnnotation.m in Sources */ = {isa = PBXBuildFile; fileRef = DAE6C2E41CC3050F00DB3429 /* DroppedPinAnnotation.m */; };
 		DAE6C2EE1CC3050F00DB3429 /* LocationCoordinate2DTransformer.m in Sources */ = {isa = PBXBuildFile; fileRef = DAE6C2E61CC3050F00DB3429 /* LocationCoordinate2DTransformer.m */; };
@@ -143,6 +144,7 @@
 		DA839EA11CC2E3400062CAFB /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = "<group>"; };
 		DA839EA41CC2E3400062CAFB /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.xib; name = Base; path = Base.lproj/MainMenu.xib; sourceTree = "<group>"; };
 		DA839EA61CC2E3400062CAFB /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
+		DAC2ABC41CC6D343006D18C4 /* MGLAnnotationImage_Private.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = MGLAnnotationImage_Private.h; path = src/MGLAnnotationImage_Private.h; sourceTree = SOURCE_ROOT; };
 		DAE6C2E11CC304F900DB3429 /* Credits.rtf */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.rtf; path = Credits.rtf; sourceTree = "<group>"; };
 		DAE6C2E31CC3050F00DB3429 /* DroppedPinAnnotation.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DroppedPinAnnotation.h; sourceTree = "<group>"; };
 		DAE6C2E41CC3050F00DB3429 /* DroppedPinAnnotation.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = DroppedPinAnnotation.m; sourceTree = "<group>"; };
@@ -413,6 +415,7 @@
 			isa = PBXGroup;
 			children = (
 				DAE6C39F1CC31E9400DB3429 /* MGLAnnotationImage.h */,
+				DAC2ABC41CC6D343006D18C4 /* MGLAnnotationImage_Private.h */,
 				DAE6C3A71CC31EF300DB3429 /* MGLAnnotationImage.m */,
 				DAE6C3A81CC31EF300DB3429 /* MGLAttributionButton.h */,
 				DAE6C3A91CC31EF300DB3429 /* MGLAttributionButton.m */,
@@ -462,6 +465,7 @@
 				DAE6C35E1CC31E0400DB3429 /* MGLMultiPoint.h in Headers */,
 				DAE6C3971CC31E2A00DB3429 /* NSBundle+MGLAdditions.h in Headers */,
 				DAE6C3631CC31E0400DB3429 /* MGLPointAnnotation.h in Headers */,
+				DAC2ABC51CC6D343006D18C4 /* MGLAnnotationImage_Private.h in Headers */,
 				DAE6C35F1CC31E0400DB3429 /* MGLOfflinePack.h in Headers */,
 				DAE6C39C1CC31E2A00DB3429 /* NSString+MGLAdditions.h in Headers */,
 				DAE6C3861CC31E2A00DB3429 /* MGLGeometry_Private.h in Headers */,
diff --git a/platform/osx/src/MGLAnnotationImage.m b/platform/osx/src/MGLAnnotationImage.m
index 49346b073b..1b545651d2 100644
--- a/platform/osx/src/MGLAnnotationImage.m
+++ b/platform/osx/src/MGLAnnotationImage.m
@@ -1,9 +1,10 @@
-#import "MGLAnnotationImage.h"
+#import "MGLAnnotationImage_Private.h"
 
 @interface MGLAnnotationImage ()
 
 @property (nonatomic) NSImage *image;
 @property (nonatomic) NSString *reuseIdentifier;
+@property (nonatomic, strong, nullable) NSString *styleIconIdentifier;
 
 @end
 
diff --git a/platform/osx/src/MGLAnnotationImage_Private.h b/platform/osx/src/MGLAnnotationImage_Private.h
new file mode 100644
index 0000000000..21963a86a0
--- /dev/null
+++ b/platform/osx/src/MGLAnnotationImage_Private.h
@@ -0,0 +1,8 @@
+#import <Mapbox/Mapbox.h>
+
+@interface MGLAnnotationImage (Private)
+
+/// Unique identifier of the sprite image used by the style to represent the receiver’s `image`.
+@property (nonatomic, strong, nullable) NSString *styleIconIdentifier;
+
+@end
diff --git a/platform/osx/src/MGLMapView.mm b/platform/osx/src/MGLMapView.mm
index 307820334a..db28c14a5e 100644
--- a/platform/osx/src/MGLMapView.mm
+++ b/platform/osx/src/MGLMapView.mm
@@ -1,12 +1,13 @@
 #import "MGLMapView_Private.h"
+#import "MGLAnnotationImage_Private.h"
 #import "MGLAttributionButton.h"
 #import "MGLCompassCell.h"
 #import "MGLOpenGLLayer.h"
 #import "MGLStyle.h"
 
-#import "../../darwin/src/MGLGeometry_Private.h"
-#import "../../darwin/src/MGLMultiPoint_Private.h"
-#import "../../darwin/src/MGLOfflineStorage_Private.h"
+#import "MGLGeometry_Private.h"
+#import "MGLMultiPoint_Private.h"
+#import "MGLOfflineStorage_Private.h"
 
 #import "MGLAccountManager.h"
 #import "MGLMapCamera.h"
@@ -31,10 +32,10 @@
 #import <map>
 #import <unordered_set>
 
-#import "../../darwin/src/NSBundle+MGLAdditions.h"
-#import "../../darwin/src/NSProcessInfo+MGLAdditions.h"
-#import "../../darwin/src/NSException+MGLAdditions.h"
-#import "../../darwin/src/NSString+MGLAdditions.h"
+#import "NSBundle+MGLAdditions.h"
+#import "NSProcessInfo+MGLAdditions.h"
+#import "NSException+MGLAdditions.h"
+#import "NSString+MGLAdditions.h"
 
 #import <QuartzCore/QuartzCore.h>
 
@@ -130,9 +131,8 @@ mbgl::Color MGLColorObjectFromNSColor(NSColor *color) {
 class MGLAnnotationContext {
 public:
     id <MGLAnnotation> annotation;
-    /// mbgl-given identifier for the annotation image used by this annotation.
-    /// Based on the annotation image’s reusable identifier.
-    NSString *symbolIdentifier;
+    /// The annotation’s image’s reuse identifier.
+    NSString *imageReuseIdentifier;
 };
 
 @interface MGLMapView () <NSPopoverDelegate, MGLMultiPointDelegate>
@@ -165,6 +165,7 @@ public:
     BOOL _didHideCursorDuringGesture;
     
     MGLAnnotationContextMap _annotationContextsByAnnotationTag;
+    NS_MUTABLE_SET_OF(id <MGLAnnotation>) *_pointAnnotations;
     MGLAnnotationTag _selectedAnnotationTag;
     MGLAnnotationTag _lastSelectedAnnotationTag;
     /// Size of the rectangle formed by unioning the maximum slop area around every annotation image.
@@ -279,6 +280,7 @@ public:
     // Set up annotation management and selection state.
     _annotationImagesByIdentifier = [NSMutableDictionary dictionary];
     _annotationContextsByAnnotationTag = {};
+    _pointAnnotations = [NSMutableSet set];
     _selectedAnnotationTag = MGLAnnotationTagNotFound;
     _lastSelectedAnnotationTag = MGLAnnotationTagNotFound;
     _annotationsNearbyLastClick = {};
@@ -464,6 +466,18 @@ public:
     if ([keyPath isEqualToString:@"contentLayoutRect"] ||
         [keyPath isEqualToString:@"titlebarAppearsTransparent"]) {
         [self adjustContentInsets];
+    } else if ([keyPath isEqualToString:@"coordinate"] &&
+               [object conformsToProtocol:@protocol(MGLAnnotation)]) {
+        id <MGLAnnotation> annotation = object;
+        MGLAnnotationTag annotationTag = [self annotationTagForAnnotation:annotation];
+        if (annotationTag != MGLAnnotationTagNotFound) {
+            const mbgl::LatLng latLng = MGLLatLngFromLocationCoordinate2D(annotation.coordinate);
+            MGLAnnotationImage *annotationImage = [self imageOfAnnotationWithTag:annotationTag];
+            _mbglMap->updatePointAnnotation(annotationTag, { latLng, annotationImage.styleIconIdentifier.UTF8String ?: "" });
+            if (annotationTag == _selectedAnnotationTag) {
+                [self deselectAnnotation:annotation];
+            }
+        }
     }
 }
 
@@ -1539,6 +1553,7 @@ public:
     
     std::vector<mbgl::PointAnnotation> points;
     std::vector<mbgl::ShapeAnnotation> shapes;
+    NSMutableArray *annotationImages = [NSMutableArray arrayWithCapacity:annotations.count];
     
     for (id <MGLAnnotation> annotation in annotations) {
         NSAssert([annotation conformsToProtocol:@protocol(MGLAnnotation)], @"Annotation does not conform to MGLAnnotation");
@@ -1555,25 +1570,27 @@ public:
                 annotationImage = [self dequeueReusableAnnotationImageWithIdentifier:MGLDefaultStyleMarkerSymbolName];
             }
             if (!annotationImage) {
-                // Create a default annotation image that depicts a round pin
-                // rising from the center, with a shadow slightly below center.
-                // The alignment rect therefore excludes the bottom half.
-                NSImage *image = MGLDefaultMarkerImage();
-                NSRect alignmentRect = image.alignmentRect;
-                alignmentRect.origin.y = NSMidY(alignmentRect);
-                alignmentRect.size.height /= 2;
-                image.alignmentRect = alignmentRect;
-                annotationImage = [MGLAnnotationImage annotationImageWithImage:image
-                                                               reuseIdentifier:MGLDefaultStyleMarkerSymbolName];
+                annotationImage = self.defaultAnnotationImage;
+            }
+            
+            NSString *symbolName = annotationImage.styleIconIdentifier;
+            if (!symbolName) {
+                symbolName = [MGLAnnotationSpritePrefix stringByAppendingString:annotationImage.reuseIdentifier];
+                annotationImage.styleIconIdentifier = symbolName;
             }
             
             if (!self.annotationImagesByIdentifier[annotationImage.reuseIdentifier]) {
                 self.annotationImagesByIdentifier[annotationImage.reuseIdentifier] = annotationImage;
                 [self installAnnotationImage:annotationImage];
             }
+            [annotationImages addObject:annotationImage];
+            
+            points.emplace_back(MGLLatLngFromLocationCoordinate2D(annotation.coordinate), symbolName.UTF8String ?: "");
             
-            NSString *symbolName = [MGLAnnotationSpritePrefix stringByAppendingString:annotationImage.reuseIdentifier];
-            points.emplace_back(MGLLatLngFromLocationCoordinate2D(annotation.coordinate), symbolName ? [symbolName UTF8String] : "");
+            if ( ! [_pointAnnotations containsObject:annotation] && [annotation isKindOfClass:[NSObject class]]) {
+                [(NSObject *)annotation addObserver:self forKeyPath:@"coordinate" options:0 context:NULL];
+                [_pointAnnotations addObject:annotation];
+            }
             
             // Opt into potentially expensive tooltip tracking areas.
             if (annotation.toolTip.length) {
@@ -1587,9 +1604,12 @@ public:
         std::vector<MGLAnnotationTag> pointAnnotationTags = _mbglMap->addPointAnnotations(points);
         
         for (size_t i = 0; i < pointAnnotationTags.size(); ++i) {
+            MGLAnnotationImage *annotationImage = annotationImages[i];
+            annotationImage.styleIconIdentifier = @(points[i].icon.c_str());
+            
             MGLAnnotationContext context;
             context.annotation = annotations[i];
-            context.symbolIdentifier = @(points[i].icon.c_str());
+            context.imageReuseIdentifier = annotationImage.reuseIdentifier;
             _annotationContextsByAnnotationTag[pointAnnotationTags[i]] = context;
         }
     }
@@ -1610,9 +1630,25 @@ public:
     [self updateAnnotationTrackingAreas];
 }
 
+/// Initializes and returns a default annotation image that depicts a round pin
+/// rising from the center, with a shadow slightly below center. The alignment
+/// rect therefore excludes the bottom half.
+- (MGLAnnotationImage *)defaultAnnotationImage {
+    NSImage *image = MGLDefaultMarkerImage();
+    NSRect alignmentRect = image.alignmentRect;
+    alignmentRect.origin.y = NSMidY(alignmentRect);
+    alignmentRect.size.height /= 2;
+    image.alignmentRect = alignmentRect;
+    return [MGLAnnotationImage annotationImageWithImage:image
+                                        reuseIdentifier:MGLDefaultStyleMarkerSymbolName];
+}
+
 /// Sends the raw pixel data of the annotation image’s image to mbgl and
 /// calculates state needed for hit testing later.
 - (void)installAnnotationImage:(MGLAnnotationImage *)annotationImage {
+    NSString *iconIdentifier = annotationImage.styleIconIdentifier;
+    self.annotationImagesByIdentifier[annotationImage.reuseIdentifier] = annotationImage;
+    
     NSImage *image = annotationImage.image;
     NSSize size = image.size;
     if (size.width == 0 || size.height == 0 || !image.valid) {
@@ -1634,8 +1670,7 @@ public:
     std::copy(rep.bitmapData, rep.bitmapData + cPremultipliedImage.size(), cPremultipliedImage.data.get());
     auto cSpriteImage = std::make_shared<mbgl::SpriteImage>(std::move(cPremultipliedImage),
                                                             (float)(rep.pixelsWide / size.width));
-    NSString *symbolName = [MGLAnnotationSpritePrefix stringByAppendingString:annotationImage.reuseIdentifier];
-    _mbglMap->addAnnotationIcon(symbolName.UTF8String, cSpriteImage);
+    _mbglMap->addAnnotationIcon(iconIdentifier.UTF8String, cSpriteImage);
     
     // Create a slop area with a “radius” equal to the annotation image’s entire
     // size, allowing the eventual click to be on any point within this image.
@@ -1678,6 +1713,11 @@ public:
         }
         
         _annotationContextsByAnnotationTag.erase(annotationTag);
+        
+        if ([annotation isKindOfClass:[NSObject class]]) {
+            [(NSObject *)annotation removeObserver:self forKeyPath:@"coordinate"];
+        }
+        [_pointAnnotations removeObject:annotation];
     }
     
     [self willChangeValueForKey:@"annotations"];
@@ -1688,11 +1728,6 @@ public:
 }
 
 - (nullable MGLAnnotationImage *)dequeueReusableAnnotationImageWithIdentifier:(NSString *)identifier {
-    // This prefix is used to avoid collisions with style-defined sprites in
-    // mbgl, but reusable identifiers are never prefixed.
-    if ([identifier hasPrefix:MGLAnnotationSpritePrefix]) {
-        identifier = [identifier substringFromIndex:MGLAnnotationSpritePrefix.length];
-    }
     return self.annotationImagesByIdentifier[identifier];
 }
 
@@ -1951,6 +1986,9 @@ public:
     }
     NSImage *image = [self imageOfAnnotationWithTag:annotationTag].image;
     if (!image) {
+        image = [self dequeueReusableAnnotationImageWithIdentifier:MGLDefaultStyleMarkerSymbolName].image;
+    }
+    if (!image) {
         return NSZeroRect;
     }
     
@@ -1974,7 +2012,7 @@ public:
         return nil;
     }
     
-    NSString *customSymbol = _annotationContextsByAnnotationTag.at(annotationTag).symbolIdentifier;
+    NSString *customSymbol = _annotationContextsByAnnotationTag.at(annotationTag).imageReuseIdentifier;
     NSString *symbolName = customSymbol.length ? customSymbol : MGLDefaultStyleMarkerSymbolName;
     
     return [self dequeueReusableAnnotationImageWithIdentifier:symbolName];