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

Add MGLCircle (with radius expressed in physical units) #14534

Closed
wants to merge 1 commit into from

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Apr 29, 2019

A new MGLCircle class, analogous to MapKit’s MKCircle or the Google Map SDK’s GMSCircle, allows the developer to add spherical caps to the map view as overlays or to a shape source as polygons.

Unlike MGLCircleStyleLayer or the Android annotation plugin’s Circle, spherical caps are measured in meters, don’t necessarily appear circular in the Web Mercator projection, and scale up to the whole world regardless of the current zoom level. Unlike turf-circle or #14429, this implementation is specific to the Web Mercator projection, the number of sides is chosen automatically, and it’s possible for a spherical cap to envelop the north or south pole. Circles still aren’t quite a first-class geometry type: GeoJSON doesn’t have a way to express curves, so under the hood, a circle is approximated as a many-sided regular polygon.

boston-light
Concentric circle annotations with a one-line pulsing animation.

boston-harbor-islands
These circles remain visible even though their centers are out of view. Hit testing works throughout the circles.

daylight
Day and night (and twilight), implemented as a series of concentric circles with radii equal to about a quarter of the Earth’s circumference.

MGLCircle is optimized for accuracy, with some consideration for performance. The number of sides in an MGLCircle’s underlying polygon depends on the circle’s radius and center latitude. This class relies on mbgl to perform Douglas–Peucker simplification on the shape at the tolerance appropriate to the map’s current zoom level. The class tries to choose enough sides that the internal angles are imperceptibly slight, but not so many that the extra detail is lost when mbgl simplifies the shape at the highest possible zoom level. The simplification heuristic determines the number of sides until the radius reaches about 10 kilometers; beyond that, angle heuristic caps the number of sides at 21,600. Despite these optimizations, there is a tradeoff in terms of performance, especially when the application doesn’t need much detail at all.

MGLCircle primarily targets the annotation API because only the annotation API is capable of persisting the circle as a circle (#6178, #7376), which makes it possible for the developer to modify the center coordinate and radius on the fly (#6177) without explicitly rebuilding the entire polygon from scratch. Most of the functionality introduced in this PR will remain relevant even after the annotation API is reimplemented atop the runtime styling API (#6181, #12863), though some operations will likely become less performant or more unwieldy internally. For now, I’m labeling this PR as DO NOT MERGE in case @mapbox/maps-ios has concerns about the new annotation API’s ability to maintain compatibility with this new annotation type, but it is already functional.

The Debug ‣ Add Polyline and Polygon Annotations menu item in macosapp has been replaced by a Simulate Nightfall menu item that turns the map into a daylight chart (including the different degrees of twilight) and adds a pulsing light that radiates out from Boston Light, one of the oldest lighthouses in the United States.

Geodesic polylines (#1726) could be implemented using a similar approach as this PR.

Fixes #2167.

@1ec5 1ec5 added feature iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold annotations Annotations on iOS and macOS or markers on Android MapKit parity For feature parity with MapKit on iOS or macOS runtime styling Google Maps parity For feature parity with the Google Maps SDK for Android or iOS labels Apr 29, 2019
@1ec5 1ec5 requested a review from a team April 29, 2019 01:40
@1ec5 1ec5 self-assigned this Apr 29, 2019
@1ec5 1ec5 changed the title Add MGLCircle (with radius expressed in physical units) [POC] Add MGLCircle (with radius expressed in physical units) Apr 29, 2019
@1ec5 1ec5 force-pushed the 1ec5-circle-2167 branch 4 times, most recently from 922e142 to 60c4918 Compare April 29, 2019 08:42
@1ec5 1ec5 changed the title [POC] Add MGLCircle (with radius expressed in physical units) Add MGLCircle (with radius expressed in physical units) Apr 29, 2019
@1ec5 1ec5 force-pushed the 1ec5-circle-2167 branch 2 times, most recently from 9594de6 to 4e4faa9 Compare May 3, 2019 17:46
Added an MGLCircle class that generates a many-sided polygon under the hood.
@1ec5
Copy link
Contributor Author

1ec5 commented May 8, 2019

allows the developer to add spherical caps … to a shape source as polygons

To clarify, it is possible to use MGLCircle with the runtime styling API, by putting one in an MGLShapeSource. However, it isn’t possible to preserve any metadata associated with the circle when storing it that way. Until #7454 is fixed, we’d either need to define an MGLCircleFeature class or an MGLPolygonFeature initializer that accepts an MGLCircle.

MGLCircle primarily targets the annotation API because only the annotation API is capable of persisting the circle as a circle (#6178, #7376), which makes it possible for the developer to modify the center coordinate and radius on the fly (#6177) without explicitly rebuilding the entire polygon from scratch.

To expand on this point: in theory, MGLCircleFeature or MGLPolygonFeature could stash MGLCircle’s coordinate and radius properties in attributes (namespaced to avoid collisions with user-defined attributes), and any conversion from GeoJSON to MGLShape objects could use these attributes to reconstruct an MGLCircle instead of an MGLPolygon. Unfortunately, even shape sources get tiled up (#6178), potentially splicing the circle into a bunch of irregular polygons that vary by zoom level. It would be difficult, though possible, to reconstruct a single circle based on the attributes saved from the original MGLCircle, then manually cut it back up into tiles.

@1ec5 1ec5 removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label May 23, 2019
@julianrex
Copy link
Contributor

@asheemmamoowala a while back you had suggested using computed shape sources for this - can you comment on why that might be preferable?

@1ec5
Copy link
Contributor Author

1ec5 commented May 23, 2019

I did consider a computed shape source as a solution to this problem but decided against it. Conceptually, a circle is a geometry; any higher-level abstraction such as a source or layer would unnecessarily limit the feature’s interoperability. For example, only a geometry class can be used with both the runtime styling API and the annotation API – or even standalone without a map.

In theory, for large circles, such as a nighttime spherical cap that spans a whole hemisphere, a computed shape source would only have to generate visible portions of the circle, and only as many points as needed for the current zoom level. That could potentially cut down on memory and CPU usage. But generating only the visible portion of the circle would require nontrivial tile cover logic that I’m uncomfortable implementing at the SDK level.

It turns out that mbgl is already pretty efficient at both simplifying and tiling shapes, so I’d contend that it’s better to generate the entire circle upfront and let mbgl tile and simplify it dynamically as the zoom level changes. This keeps the SDK implementation lightweight, leveraging existing functionality in mbgl.

In effect, we’re optimizing for the user panning and zooming rather than statically viewing only a small portion of the circle. If this isn’t the right tradeoff for a particular developer’s use case, they’re free to implement a computed shape source on their own.

@friedbunny friedbunny removed the request for review from a team May 24, 2019 20:56
@asheemmamoowala
Copy link
Contributor

generating only the visible portion of the circle would require nontrivial tile cover logic that I’m uncomfortable implementing at the SDK level.

This is a good reason not to use a custom source. If we wanted to support this on all platforms - it might still be worth investigating - but the cost of tiling will still be non-trivial.

@@ -542,9 +542,9 @@
<action selector="dropManyPins:" target="-1" id="Rtv-8N-3Z8"/>
</connections>
</menuItem>
<menuItem title="Add Polygon and Polyline" keyEquivalent="l" id="DVr-vT-lpe">
<menuItem title="Simulate Nightfall" keyEquivalent="l" id="DVr-vT-lpe">
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you add a subtitle/tooltip that this item uses Spherical caps or MGLCircle

GPS location update, the regulated airspace around an airport, or the
addressable consumer market around a city.

You can add a circle overlay directly to a map view using the
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it is necessary to call out that this type does not support all the same paint/layout options as a shape added to an MGLFillStyleLayer? I'm thinking specifically of fill patterns, but there are probably others as well.

@stale stale bot added the archived Archived because of inactivity label Aug 3, 2019
@stale
Copy link

stale bot commented Aug 3, 2019

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Aug 3, 2019
@chloekraw chloekraw reopened this Aug 3, 2019
@stale stale bot removed the archived Archived because of inactivity label Aug 3, 2019
@jlubeck
Copy link

jlubeck commented Nov 23, 2019

What happened to this PR? Was so looking forward to it!

@1ec5
Copy link
Contributor Author

1ec5 commented Dec 7, 2019

This PR will need to be moved over to the mapbox-gl-native-ios repository, now that the iOS and macOS map SDKs are maintained there. I’d be happy to do so, once there’s a decision on whether this feature will be accepted: #2167 (comment).

If -[MGLMapView addAnnotations:] and platform-specific annotation functionality is removed at any time before this feature lands, then MGLCircle itself will continue to exist, but it would need to be used through MGLShapeSource, which converts the shape into a many-sided polygon with a fixed radius.

Separately, mapbox/turf-swift#12 tracks a standalone method for generating a great circle or geodesic polyline, which could be used in conjunction with the runtime styling API or in other use cases.

@stale stale bot added the archived Archived because of inactivity label Mar 17, 2020
@stale
Copy link

stale bot commented Mar 17, 2020

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Mar 17, 2020
@julianrex julianrex reopened this Mar 17, 2020
@stale stale bot removed the archived Archived because of inactivity label Mar 17, 2020
@stale stale bot added the archived Archived because of inactivity label May 16, 2020
@stale
Copy link

stale bot commented May 16, 2020

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this May 16, 2020
@1ec5 1ec5 reopened this Jun 4, 2020
@stale stale bot removed the archived Archived because of inactivity label Jun 4, 2020
@bsudekum
Copy link

The people need MGLCircle.

@stale stale bot added the archived Archived because of inactivity label Aug 9, 2020
@stale
Copy link

stale bot commented Aug 9, 2020

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Aug 9, 2020
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 archived Archived because of inactivity feature Google Maps parity For feature parity with the Google Maps SDK for Android or iOS iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS MapKit parity For feature parity with MapKit on iOS or macOS runtime styling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add MGLCircle (with radius expressed in physical units)
6 participants