[ios, macos] fixes #6160: allow updating multipoint coordinates (#6565)

6 files changed, 150 insertions, 39 deletions

diff --git a/platform/darwin/src/MGLMultiPoint.h b/platform/darwin/src/MGLMultiPoint.h
index 59d9cd9429..69c7295842 100644
--- a/platform/darwin/src/MGLMultiPoint.h
+++ b/platform/darwin/src/MGLMultiPoint.h
@@ -15,7 +15,15 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @interface MGLMultiPoint : MGLShape
 
-/** The array of coordinates associated with the shape. */
+/**
+ The array of coordinates associated with the shape.
+ 
+ This C array is a pointer to a structure inside the multipoint object, 
+ which may have a lifetime shorter than the multipoint object and will 
+ certainly not have a longer lifetime. Therefore, you should copy the C 
+ array if it needs to be stored outside of the memory context in which you 
+ use this property.
+ */
 @property (nonatomic, readonly) CLLocationCoordinate2D *coordinates NS_RETURNS_INNER_POINTER;
 
 /** The number of coordinates associated with the shape. (read-only) */
@@ -35,6 +43,22 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range;
 
+/**
+ Updates one or more coordinates for the shape, which will instantaneously 
+ cause the shape to be redrawn if it is currently visible on the map.
+ 
+ @param range The range of points to update. The `location` field indicates the
+ first point you are replacing, with `0` being the first point, `1` being
+ the second point, and so on. The `length` field indicates the number of
+ points to update. You can append to an existing array of coordinates
+ by specifying a range with a `location` at the end of the existing array 
+ and/or a `length` which extends past the end of the existing array. The array 
+ in _`coords`_ must be equal in number to the length of the range.
+ @param coords The array of coordinates defining the shape. The data in this
+ array is copied to the object.
+ */
+- (void)replaceCoordinatesInRange:(NSRange)range withCoordinates:(CLLocationCoordinate2D *)coords;
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/platform/darwin/src/MGLMultiPoint.mm b/platform/darwin/src/MGLMultiPoint.mm
index a5b9eb8efc..17a61ed081 100644
--- a/platform/darwin/src/MGLMultiPoint.mm
+++ b/platform/darwin/src/MGLMultiPoint.mm
@@ -1,12 +1,12 @@
 #import "MGLMultiPoint_Private.h"
 #import "MGLGeometry_Private.h"
+#import "MGLTypes.h"
 
 #import <mbgl/util/geo.hpp>
 
-mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor) {
-    if (!cgColor) {
-        return { 0, 0, 0, 0 };
-    }
+mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor)
+{
+    if (!cgColor) return { 0, 0, 0, 0 };
     NSCAssert(CGColorGetNumberOfComponents(cgColor) >= 4, @"Color must have at least 4 components");
     const CGFloat *components = CGColorGetComponents(cgColor);
     return { (float)components[0], (float)components[1], (float)components[2], (float)components[3] };
@@ -18,28 +18,34 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor) {
     MGLCoordinateBounds _bounds;
 }
 
-- (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords
-                              count:(NSUInteger)count
+- (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
 {
     self = [super init];
 
     if (self)
     {
-        _count = count;
-        _coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));
+        [self setupWithCoordinates:coords count:count];
+    }
 
-        mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();
+    return self;
+}
 
-        for (NSUInteger i = 0; i < _count; i++)
-        {
-            _coordinates[i] = coords[i];
-            bounds.extend(mbgl::LatLng(coords[i].latitude, coords[i].longitude));
-        }
+- (void)setupWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
+{
+    if (_coordinates) free(_coordinates);
+
+    _count = count;
+    _coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));
 
-        _bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
+    mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();
+
+    for (NSUInteger i = 0; i < _count; i++)
+    {
+        _coordinates[i] = coords[i];
+        bounds.extend(mbgl::LatLng(coords[i].latitude, coords[i].longitude));
     }
 
-    return self;
+    _bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
 }
 
 - (void)dealloc
@@ -49,13 +55,6 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor) {
 
 - (CLLocationCoordinate2D)coordinate
 {
-    if ([self isMemberOfClass:[MGLMultiPoint class]])
-    {
-        [[NSException exceptionWithName:@"MGLAbstractClassException"
-                                 reason:@"MGLMultiPoint is an abstract class"
-                               userInfo:nil] raise];
-    }
-
     assert(_count > 0);
 
     return CLLocationCoordinate2DMake(_coordinates[0].latitude, _coordinates[0].longitude);
@@ -63,27 +62,23 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor) {
 
 - (NSUInteger)pointCount
 {
-    if ([self isMemberOfClass:[MGLMultiPoint class]])
-    {
-        [[NSException exceptionWithName:@"MGLAbstractClassException"
-                                 reason:@"MGLMultiPoint is an abstract class"
-                               userInfo:nil] raise];
-    }
-
     return _count;
 }
 
++ (NS_SET_OF(NSString *) *)keyPathsForValuesAffectingPointCount
+{
+    return [NSSet setWithObjects:@"coordinates", nil];
+}
+
 - (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
 {
-    if ([self isMemberOfClass:[MGLMultiPoint class]])
+    if (range.location + range.length > _count)
     {
-        [[NSException exceptionWithName:@"MGLAbstractClassException"
-                                 reason:@"MGLMultiPoint is an abstract class"
-                               userInfo:nil] raise];
+        [NSException raise:NSRangeException
+                    format:@"Invalid coordinate range %@ extends beyond current coordinate count of %zu",
+                        NSStringFromRange(range), _count];
     }
 
-    assert(range.location + range.length <= _count);
-
     NSUInteger index = 0;
 
     for (NSUInteger i = range.location; i < range.location + range.length; i++)
@@ -93,6 +88,43 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor) {
     }
 }
 
+- (void)replaceCoordinatesInRange:(NSRange)range withCoordinates:(CLLocationCoordinate2D *)coords
+{
+    if ((coords >= _coordinates && coords < _coordinates + _count) ||
+        (coords + range.length >= _coordinates && coords + range.length < _coordinates + _count))
+    {
+        [NSException raise:NSRangeException format:@"Reuse of existing coordinates array %p not supported", coords];
+    }
+    else if (range.length == 0)
+    {
+        [NSException raise:NSRangeException format:@"Empty coordinate range %@", NSStringFromRange(range)];
+    }
+    else if (range.location > _count)
+    {
+        [NSException raise:NSRangeException
+                    format:@"Invalid range %@ for existing coordinate count %zu",
+                        NSStringFromRange(range), _count];
+    }
+
+    [self willChangeValueForKey:@"coordinates"];
+    if (NSMaxRange(range) <= _count)
+    {
+        // replacing existing coordinate(s)
+        memcpy(_coordinates + range.location, coords, range.length * sizeof(CLLocationCoordinate2D));
+    }
+    else
+    {
+        // appending new coordinate(s)
+        NSUInteger newCount = NSMaxRange(range);
+        CLLocationCoordinate2D *newCoordinates = (CLLocationCoordinate2D *)malloc(newCount * sizeof(CLLocationCoordinate2D));
+        memcpy(newCoordinates, _coordinates, fmin(_count, range.location) * sizeof(CLLocationCoordinate2D));
+        memcpy(newCoordinates + range.location, coords, range.length * sizeof(CLLocationCoordinate2D));
+        [self setupWithCoordinates:newCoordinates count:newCount];
+        free(newCoordinates);
+    }
+    [self didChangeValueForKey:@"coordinates"];
+}
+
 - (MGLCoordinateBounds)overlayBounds
 {
     return _bounds;
@@ -103,7 +135,8 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor) {
     return MGLLatLngBoundsFromCoordinateBounds(_bounds).intersects(MGLLatLngBoundsFromCoordinateBounds(overlayBounds));
 }
 
-- (mbgl::Annotation)annotationObjectWithDelegate:(__unused id <MGLMultiPointDelegate>)delegate {
+- (mbgl::Annotation)annotationObjectWithDelegate:(__unused id <MGLMultiPointDelegate>)delegate
+{
     NSAssert(NO, @"Cannot add an annotation from an instance of %@", NSStringFromClass([self class]));
     return mbgl::SymbolAnnotation({mbgl::Point<double>()});
 }
@@ -111,7 +144,7 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor) {
 - (NSString *)description
 {
     return [NSString stringWithFormat:@"<%@: %p; count = %lu; bounds = %@>",
-            NSStringFromClass([self class]), (void *)self, (unsigned long)_count, MGLStringFromCoordinateBounds(_bounds)];
+               NSStringFromClass([self class]), (void *)self, (unsigned long)_count, MGLStringFromCoordinateBounds(_bounds)];
 }
 
 @end
diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
index 4a445a075a..07f0a3c89d 100644
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -40,6 +40,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 
 ### Annotations
 
+* MGLPolyline annotations and the exterior coordinates of MGLPolygon annotations are now able to be mutated, part or all, and changes are displayed immediately. ([#6565](https://github.com/mapbox/mapbox-gl-native/pull/6565))
 * Fixed an issue preventing MGLAnnotationView from animating when its coordinate changes. ([#6215](https://github.com/mapbox/mapbox-gl-native/pull/6215))
 * Fixed an issue causing the wrong annotation view to be selected when tapping an annotation view with a center offset applied. ([#5931](https://github.com/mapbox/mapbox-gl-native/pull/5931))
 * Fixed an issue that assigned annotation views to polyline and polygon annotations. ([#5770](https://github.com/mapbox/mapbox-gl-native/pull/5770))
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index bc11303ee8..9067efc033 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -1795,6 +1795,29 @@ public:
             }
         }
     }
+    else if ([keyPath isEqualToString:@"coordinates"] && [object isKindOfClass:[MGLMultiPoint class]])
+    {
+        MGLMultiPoint *annotation = object;
+        MGLAnnotationTag annotationTag = (MGLAnnotationTag)(NSUInteger)context;
+        // We can get here because a subclass registered itself as an observer
+        // of the coordinates key path of a multipoint annotation but failed
+        // to handle the change. This check deters us from treating the
+        // subclass’s context as an annotation tag. If the context happens to
+        // match a valid annotation tag, the annotation will be unnecessarily
+        // but safely updated.
+        if (annotation == [self annotationWithTag:annotationTag])
+        {
+            // Update the annotation’s backing geometry to match the annotation model object.
+            _mbglMap->updateAnnotation(annotationTag, [annotation annotationObjectWithDelegate:self]);
+
+            // We don't current support shape multipoint annotation selection, but let's make sure
+            // deselection is handled just to avoid problems in the future.
+            if (annotationTag == _selectedAnnotationTag)
+            {
+                [self deselectAnnotation:annotation animated:YES];
+            }
+        }
+    }
 }
 
 + (NS_SET_OF(NSString *) *)keyPathsForValuesAffectingZoomEnabled
@@ -2840,6 +2863,8 @@ public:
             MGLAnnotationContext context;
             context.annotation = annotation;
             _annotationContextsByAnnotationTag[annotationTag] = context;
+
+            [(NSObject *)annotation addObserver:self forKeyPath:@"coordinates" options:0 context:(void *)(NSUInteger)annotationTag];
         }
         else if ( ! [annotation isKindOfClass:[MGLMultiPolyline class]]
                  || ![annotation isKindOfClass:[MGLMultiPolygon class]]
@@ -3132,6 +3157,10 @@ public:
         {
             [(NSObject *)annotation removeObserver:self forKeyPath:@"coordinate" context:(void *)(NSUInteger)annotationTag];
         }
+        else if ([annotation isKindOfClass:[MGLMultiPoint class]])
+        {
+            [(NSObject *)annotation removeObserver:self forKeyPath:@"coordinates" context:(void *)(NSUInteger)annotationTag];
+        }
 
         _mbglMap->removeAnnotation(annotationTag);
     }
diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
index c182367bf6..e23d0199e9 100644
--- a/platform/macos/CHANGELOG.md
+++ b/platform/macos/CHANGELOG.md
@@ -8,6 +8,7 @@
 * Fixed an issue with symbols not being properly stripped from the dynamic framework when built with `make xpackage SYMBOLS=NO`. ([#6531](https://github.com/mapbox/mapbox-gl-native/pull/6531))
 * Added `showAnnotations:animated:` and `showAnnotations:edgePadding:animated:`, which moves the map viewport to show the specified annotations. ([#5749](https://github.com/mapbox/mapbox-gl-native/pull/5749))
 * Fixed an issue where the map view’s center would always be calculated as if the view occupied the entire window. ([#6102](https://github.com/mapbox/mapbox-gl-native/pull/6102))
+* MGLPolyline annotations and the exterior coordinates of MGLPolygon annotations are now able to be mutated, part or all, and changes are displayed immediately. ([#6565](https://github.com/mapbox/mapbox-gl-native/pull/6565))
 * To make an MGLPolyline or MGLPolygon span the antimeridian, specify coordinates with longitudes greater than 180° or less than −180°. ([#6088](https://github.com/mapbox/mapbox-gl-native/pull/6088))
 * MGLMapView’s `styleURL` property can now be set to an absolute file URL. ([#6026](https://github.com/mapbox/mapbox-gl-native/pull/6026))
 * GeoJSON sources specified by the stylesheet at design time now support `cluster`, `clusterMaxZoom`, and `clusterRadius` attributes for clustering point features on the base map. ([#5724](https://github.com/mapbox/mapbox-gl-native/pull/5724))
diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm
index b40f6ebc62..8fca7b86ff 100644
--- a/platform/macos/src/MGLMapView.mm
+++ b/platform/macos/src/MGLMapView.mm
@@ -482,6 +482,25 @@ public:
                 [self deselectAnnotation:annotation];
             }
         }
+    } else if ([keyPath isEqualToString:@"coordinates"] &&
+               [object isKindOfClass:[MGLMultiPoint class]]) {
+        MGLMultiPoint *annotation = object;
+        MGLAnnotationTag annotationTag = (MGLAnnotationTag)(NSUInteger)context;
+        // We can get here because a subclass registered itself as an observer
+        // of the coordinates key path of a multipoint annotation but failed
+        // to handle the change. This check deters us from treating the
+        // subclass’s context as an annotation tag. If the context happens to
+        // match a valid annotation tag, the annotation will be unnecessarily
+        // but safely updated.
+        if (annotation == [self annotationWithTag:annotationTag]) {
+            _mbglMap->updateAnnotation(annotationTag, [annotation annotationObjectWithDelegate:self]);
+            // We don't current support shape multipoint annotation selection, but let's make sure
+            // deselection is handled just to avoid problems in the future.
+            if (annotationTag == _selectedAnnotationTag)
+            {
+                [self deselectAnnotation:annotation];
+            }
+        }
     }
 }
 
@@ -1625,6 +1644,8 @@ public:
             MGLAnnotationContext context;
             context.annotation = annotation;
             _annotationContextsByAnnotationTag[annotationTag] = context;
+
+            [(NSObject *)annotation addObserver:self forKeyPath:@"coordinates" options:0 context:(void *)(NSUInteger)annotationTag];
         } else if (![annotation isKindOfClass:[MGLMultiPolyline class]]
                    || ![annotation isKindOfClass:[MGLMultiPolygon class]]
                    || ![annotation isKindOfClass:[MGLShapeCollection class]]) {
@@ -1762,6 +1783,8 @@ public:
         if ([annotation isKindOfClass:[NSObject class]] &&
             ![annotation isKindOfClass:[MGLMultiPoint class]]) {
             [(NSObject *)annotation removeObserver:self forKeyPath:@"coordinate" context:(void *)(NSUInteger)annotationTag];
+        } else if ([annotation isKindOfClass:[MGLMultiPoint class]]) {
+            [(NSObject *)annotation removeObserver:self forKeyPath:@"coordinates" context:(void *)(NSUInteger)annotationTag];
         }
 
         _mbglMap->removeAnnotation(annotationTag);