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

More ways to reshape an MGLMultiPoint #7251

Merged
merged 3 commits into from
Dec 7, 2016

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Nov 30, 2016

Added the complete set of methods for mutating the vertices of an MGLMultiPoint. Invalidate overlayBounds whenever the coordinates change, but don’t recompute the bounds until they’re requested. Previously, -[MGLMultiPoint replaceCoordinatesInRange:withCoordinates:] failed to recompute the bounds.

Rewrote MGLMultiPoint documentation to refer to vertices instead of points. Removed a paragraph in MGLOverlay documentation that was full of references to features that exist in MKOverlay but not MGLOverlay.

Added a utility function for testing whether two MGLCoordinateBoundses intersect, based on mbgl::LatLngBounds::intersects(). Removed unused color conversion code.

Fixes #6583.

/cc @boundsj @incanus

@1ec5 1ec5 added annotations Annotations on iOS and macOS or markers on Android iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Nov 30, 2016
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Nov 30, 2016
@1ec5 1ec5 self-assigned this Nov 30, 2016
@mention-bot
Copy link

@1ec5, thanks for your PR! By analyzing this pull request, we identified @boundsj, @incanus and @jfirebaugh to be potential reviewers.

@@ -68,7 +68,7 @@ - (MGLCoordinateBounds)overlayBounds

- (BOOL)intersectsOverlayBounds:(MGLCoordinateBounds)overlayBounds
{
return MGLLatLngBoundsFromCoordinateBounds(_bounds).intersects(MGLLatLngBoundsFromCoordinateBounds(overlayBounds));
return MGLCoordinateBoundsIntersectsCoordinateBounds(_bounds, overlayBounds);
Copy link
Contributor

Choose a reason for hiding this comment

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

This approach could also be used in MGLMultiPolyline(gon) intersectsOverlayBounds:. In general, I wonder if it'd ever be useful to add an intersectsOverlay: <MGLOverlay> API for all of these objects that conform to MGLOverlay?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, I wonder if it'd ever be useful to add an intersectsOverlay: <MGLOverlay> API for all of these objects that conform to MGLOverlay?

This SDK isn’t a general-purpose geometry library, so I think we should only add geometric methods as we need them or if we need them for parity with MapKit. In the future, we may want to spin out these geometry classes as part of a separate library that would serve as a more natural home for such geometry functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve updated MGLMultiPolygon and MGLMultiPolyline to use the simplified expression too.

Added the complete set of methods for mutating the vertices of an MGLMultiPoint. Also rewrote MGLMultiPoint documentation to refer to vertices instead of points.
@1ec5 1ec5 force-pushed the 1ec5-multipoint-remove-6890 branch from a1444e9 to 441aeb2 Compare December 7, 2016 05:32
Invalidate the bounds whenever the coordinates change, but don’t recompute the bounds until they’re requested. Simplified -intersectsOverlayBounds: for immutable overlay classes.

Added a utility function for testing whether two MGLCoordinateBounds intersect, based on mbgl::LatLngBounds::intersects().

Removed unused color conversion code.
This paragraph is full of references to features that exist in MKOverlay but not MGLOverlay.
@1ec5 1ec5 force-pushed the 1ec5-multipoint-remove-6890 branch from 441aeb2 to d1dd30a Compare December 7, 2016 06:58
@1ec5 1ec5 merged commit 4c2aec2 into release-ios-v3.4.0 Dec 7, 2016
@1ec5 1ec5 deleted the 1ec5-multipoint-remove-6890 branch December 7, 2016 07:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
annotations Annotations on iOS and macOS or markers on Android iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants