Reimplement MGLAnnotation atop MGLSource and MGLStyleLayer

[1ec5]

The runtime styling API is in many ways more powerful than the MapKit-inspired annotation API, which consists of MGLAnnotation subclasses and styling methods on MGLMapViewDelegate. However, at least at the SDK level, the annotation API needs to stick around in some form to service annotation views, and having a MapKit-like, highly object-oriented API is desirable where feasible, because it’s more direct, focused, and discoverable than the runtime styling API. But over time, we should allow the runtime styling API to subsume the annotation API. Developers should be able to think of the annotation API as nothing more than a shortcut for runtime styling functionality, rather than an entirely separate code path.

As an alternative to or as part of #1734, we could migrate the SDK’s annotation API to MGLSource and MGLStyleLayer. MGLSource is basically a level of indirection around MGLAnnotation and subclasses. MGLStyleLayer serves essentially the same purpose as MKOverlayRenderer.

Per #5626 (comment) et seq., this work is blocked by:

• Add and modify individual features in a GeoJSON source on the fly (Add and modify individual features in a GeoJSON source on the fly #6177) for point annotations
• Optionally persist layers and sources across styles (Optionally persist layers and sources across styles #6180)
• Feature querying should return existing, untiled annotations (Feature querying should return existing annotations #6178) for hit testing
• GeoJSON sources should keep track of original GeoJSON objects (mbgl::style::GeoJSONSource should have GeoJSON getter #7376)
• Add paint property for icon image URL (Add paint property for icon image URL mapbox-gl-style-spec#485) for MGLAnnotationImage (fixed in [ios, macos] possibility to set custom images to style #6637)
• Convert template images to SDF icons (Convert template images to SDF icons #7291)

This work would be complicated by:

• Removing fill-outline-color (Remove "fill-outline-color" mapbox-gl-js#4088)

/cc @incanus @jfirebaugh @frederoni @boundsj

[1ec5]

In addition to supporting annotation views, another reason the annotation API needs to stick around is for interactivity (annotation selection and callouts). GeoJSON is very much a do-it-yourself approach to markers, where the developer has to manage their own gesture recognizer, perform feature querying, and keep track of selection themselves. That can be a boon for very custom implementations, but it’s way too much work for 99% of developers. (This entire example consists of code the SDK should be responsible for.)

[incanus]

the annotation API needs to stick around is for interactivity (annotation selection and callouts).

Though I'm unfamiliar with the current status, we had this initially pre-views by mapping gestures into query regions in core, sprite-based annotations. I agree views are still necessary if we want to allow developers to use existing UIView-derived stuff wholesale, and the higher-level API is also useful and familiar. I support the idea of merging this into core as much as possible, including again exploring the approach of per-frame syncing of view layer contents into core GL textures (something I'd be glad to again deep dive on).

[1ec5]

Though I'm unfamiliar with the current status, we had this initially pre-views by mapping gestures into query regions in core, sprite-based annotations. I agree views are still necessary if we want to allow developers to use existing UIView-derived stuff wholesale, and the higher-level API is also useful and familiar. I support the idea of merging this into core as much as possible, including again exploring the approach of per-frame syncing of view layer contents into core GL textures (something I'd be glad to again deep dive on).

In #6181 (comment), I wasn’t talking about view-backed annotations at all, but rather about annotations in general. The current balance between SDK-specific code and mbgl support code is a good one, because gesture handling, selection state management, and callouts (all handled by the SDK) are extremely platform-specific, while queryRenderedFeatures (part of mbgl) is platform-agnostic.

Baking annotation views into textures is a large effort orthogonal to whether we keep around an object-oriented interface for working with overlay data. My comment is meant to head off suggestions that we eliminate the annotation API in favor of GeoJSON sources, which would throw interactivity out with the bathwater and be incompatible with the goal of encouraging developers to switch from MapKit or the Google Maps SDK.

[incanus]

Ah, got it. Agree!

[1ec5]

I’ve updated the original comment with a list of major blocking issues (currently #6177, #6180, and #6178). I’m sure there are more, but these should be plenty to tide us over for now.

Additionally, it’s very important that any refactoring we do along these lines not regress performance. We should add a performance unit test that measures the time it takes to add 10,000 annotations (with a reasonable but not significant tolerance) and avoid landing the refactoring until that test passes without modification.

[incanus]

To carry over past work & research from #1125, we may be able to get even annotation views down into GL using an approach like this:

http://stackoverflow.com/a/10259503/977220

I would like to look into this again as part of truly accomplishing the following, mentioned above:

Developers should be able to think of the annotation API as nothing more than a shortcut for runtime styling functionality, rather than an entirely separate code path.

[1ec5]

If it’s a shortcut we’re looking to provide, then #1734 (comment) would get us there with much less code.

[1ec5]

Removing the fill-outline-color property, as proposed in mapbox/mapbox-gl-js#4088, would complicate the effort to reimplement polygon annotations atop the runtime styling API. Whereas FillAnnotationImpl could simply add two layers, one for the fill and one for the stroke, moving this feature up to the SDK level would require the SDK to represent polygons as feature collections and prevent the SDK from creating a one-to-one mapping from MGLAnnotation (iOS) or MarkerOptions (Android) objects to style layers.

[1ec5]

The recent “annotation plugin” for the Android map SDK is being considered as a way to bring annotations more in line with the runtime styling API: #12863. That seems like a good path forward for this refactor. #12863 (comment) outlines one possible scenario.

It’s worth noting that some of the blocking issues mentioned above are still outstanding, which means we need to resort to less elegant, possibly less performant workarounds, some of which have already been implemented in the Android plugin. Even after removing RTree (#13232), GL point annotations would still be inherently more performant than any other marker-related API, because any changes after initialization are limited to a single tile rather than the whole world. So any effort at reimplementing annotations atop runtime styling should come with a good understanding of the performance implications.

Add and modify individual features in a GeoJSON source on the fly (#6177) for point annotations

I didn’t call it out specifically, but polyline and polygon annotations are mutable (#6565), meaning you can modify individual vertices on the fly without even so much as recreating the shape. That feature is primarily geared towards two use cases: GPS/fitness tracking and shape drawing. As things stand, I don’t think there’s a way to implement this functionality at the core level, so a runtime styling–based implementation would have to simulate it by clearing and regenerating the entire shape source, affecting other shapes as well as the one being mutated. That’ll inherently be less performant than what we have now, which at most regenerates a single tile, but hopefully the performance hit is slight enough that we can live with it.

/cc @mapbox/maps-ios

[stale]

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

[stale]

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