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

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

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 @@ @implementation MGLMultiPoint
  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,41 +55,30 @@ - (void)dealloc
 
 - (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);
 }
 
 - (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 @@ - (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
  }
 }
 
+- (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,15 +135,16 @@ - (BOOL)intersectsOverlayBounds:(MGLCoordinateBounds)overlayBounds
  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>()});
 }
 
 - (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

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))

platform/ios/src/MGLMapView.mm

@@ -1795,6 +1795,29 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
  }
  }
  }
+ 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 @@ - (void)addAnnotations:(NS_ARRAY_OF(id <MGLAnnotation>) *)annotations
  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 @@ - (void)removeAnnotations:(NS_ARRAY_OF(id <MGLAnnotation>) *)annotations
  {
  [(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);
  }

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))

platform/macos/src/MGLMapView.mm

@@ -482,6 +482,25 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(_
  [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 @@ - (void)addAnnotations:(NS_ARRAY_OF(id <MGLAnnotation>) *)annotations {
  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 @@ - (void)removeAnnotations:(NS_ARRAY_OF(id <MGLAnnotation>) *)annotations {
  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);