Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios, macos] Use std::vector for MGLMultiPoint coordinate storage #6890

Merged
merged 5 commits into from
Nov 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions platform/darwin/src/MGLMultiPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,26 @@ NS_ASSUME_NONNULL_BEGIN
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.
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. The array in _`coords`_ must be equal in number to the
length of the range. If you want to append to the existing coordinates
array use `-[MGLMultiPoint appendCoordinates:count:]`.
@param coords The array of coordinates defining the shape. The data in this
array is copied to the object.
array is copied to the object.
*/
- (void)replaceCoordinatesInRange:(NSRange)range withCoordinates:(CLLocationCoordinate2D *)coords;

/**
Appends 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 coords The array of coordinates to add to the shape. The data in this
array is copied to the new object.
@param count The number of items in the `coords` array.
*/
- (void)appendCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count;

@end

NS_ASSUME_NONNULL_END
100 changes: 39 additions & 61 deletions platform/darwin/src/MGLMultiPoint.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

@implementation MGLMultiPoint
{
size_t _count;
MGLCoordinateBounds _bounds;
std::vector<CLLocationCoordinate2D> _coordinates;
}

- (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
Expand All @@ -24,105 +24,83 @@ - (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUI

if (self)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s pretty meaningless to have a multipoint of zero points. In fact, we assert as much below. So we should detect that case early, in here, asserting if count is zero.

{
[self setupWithCoordinates:coords count:count];
NSAssert(count > 0, @"A multipoint must have coordinates");
_coordinates = { coords, coords + count };
[self computeBounds];
}

return self;
}

- (void)setupWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
{
if (_coordinates) free(_coordinates);

_count = count;
_coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));

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

_bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
}

- (void)dealloc
{
free(_coordinates);
}

- (CLLocationCoordinate2D)coordinate
{
assert(_count > 0);

return CLLocationCoordinate2DMake(_coordinates[0].latitude, _coordinates[0].longitude);
NSAssert([self pointCount] > 0, @"A multipoint must have coordinates");
return _coordinates.at(0);
}

- (NSUInteger)pointCount
{
return _count;
return _coordinates.size();
}

+ (NS_SET_OF(NSString *) *)keyPathsForValuesAffectingPointCount
{
return [NSSet setWithObjects:@"coordinates", nil];
}

- (CLLocationCoordinate2D *)coordinates
{
return _coordinates.data();
}

- (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
{
if (range.location + range.length > _count)
if (range.location + range.length > [self pointCount])
{
[NSException raise:NSRangeException
format:@"Invalid coordinate range %@ extends beyond current coordinate count of %zu",
NSStringFromRange(range), _count];
NSStringFromRange(range), [self pointCount]];
}

NSUInteger index = 0;
std::copy(_coordinates.begin() + range.location, _coordinates.begin() + NSMaxRange(range), coords);
}

for (NSUInteger i = range.location; i < range.location + range.length; i++)
{
coords[index] = _coordinates[i];
index++;
}
- (void)appendCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
{
[self willChangeValueForKey:@"coordinates"];
_coordinates.insert(_coordinates.end(), count, *coords);
[self computeBounds];
[self didChangeValueForKey:@"coordinates"];
}

- (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)
if (range.length == 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the developer tries to replace no coordinates, this method should be a no-op instead of raising an exception. That may help to minimize the number of special cases the developer would need to implement.

{
[NSException raise:NSRangeException format:@"Empty coordinate range %@", NSStringFromRange(range)];
return;
}
else if (range.location > _count)

if (NSMaxRange(range) > _coordinates.size())
{
[NSException raise:NSRangeException
format:@"Invalid range %@ for existing coordinate count %zu",
NSStringFromRange(range), _count];
NSStringFromRange(range), [self pointCount]];
}

[self willChangeValueForKey:@"coordinates"];
if (NSMaxRange(range) <= _count)
{
// replacing existing coordinate(s)
memcpy(_coordinates + range.location, coords, range.length * sizeof(CLLocationCoordinate2D));
}
else
std::copy(coords, coords + range.length, _coordinates.begin() + range.location);
[self computeBounds];
[self didChangeValueForKey:@"coordinates"];
}

- (void)computeBounds
{
mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();
for (auto coordinate : _coordinates)
{
// 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);
bounds.extend(mbgl::LatLng(coordinate.latitude, coordinate.longitude));
}
[self didChangeValueForKey:@"coordinates"];
_bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
}

- (MGLCoordinateBounds)overlayBounds
Expand All @@ -144,7 +122,7 @@ - (BOOL)intersectsOverlayBounds:(MGLCoordinateBounds)overlayBounds
- (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)[self pointCount], MGLStringFromCoordinateBounds(_bounds)];
}

@end
41 changes: 17 additions & 24 deletions platform/darwin/src/MGLPointCollection.mm
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

@implementation MGLPointCollection
{
size_t _count;
MGLCoordinateBounds _bounds;
std::vector<CLLocationCoordinate2D> _coordinates;
}

+ (instancetype)pointCollectionWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
Expand All @@ -22,50 +22,43 @@ - (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUI
self = [super init];
if (self)
{
_count = count;
_coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));

_coordinates = { coords, coords + count };
mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();

for (NSUInteger i = 0; i < count; i++)
for (auto coordinate : _coordinates)
{
_coordinates[i] = coords[i];
bounds.extend(mbgl::LatLng(coords[i].latitude, coords[i].longitude));
bounds.extend(mbgl::LatLng(coordinate.latitude, coordinate.longitude));
}

_bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
}
return self;
}

- (NSUInteger)pointCount
{
return _count;
return _coordinates.size();
}

- (CLLocationCoordinate2D *)coordinates
{
return _coordinates.data();
}

- (CLLocationCoordinate2D)coordinate
{
assert(_count > 0);

return CLLocationCoordinate2DMake(_coordinates[0].latitude, _coordinates[0].longitude);
NSAssert([self pointCount] > 0, @"A multipoint must have coordinates");
return _coordinates.at(0);
}

- (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
{
if (range.location + range.length > _count)
if (range.location + range.length > [self pointCount])
{
[NSException raise:NSRangeException
format:@"Invalid coordinate range %@ extends beyond current coordinate count of %zu",
NSStringFromRange(range), _count];
}

NSUInteger index = 0;

for (NSUInteger i = range.location; i < range.location + range.length; i++)
{
coords[index] = _coordinates[i];
index++;
NSStringFromRange(range), [self pointCount]];
}

std::copy(_coordinates.begin() + range.location, _coordinates.begin() + NSMaxRange(range), coords);
}

- (MGLCoordinateBounds)overlayBounds
Expand Down Expand Up @@ -104,7 +97,7 @@ - (NSDictionary *)geoJSONDictionary
- (NSString *)description
{
return [NSString stringWithFormat:@"<%@: %p; count = %lu>",
NSStringFromClass([self class]), (void *)self, (unsigned long)_count];
NSStringFromClass([self class]), (void *)self, (unsigned long)[self pointCount]];
}

@end
Expand Down