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

proposal: shape layer grouping API #1708

Closed
incanus opened this issue Jun 10, 2015 · 9 comments
Closed

proposal: shape layer grouping API #1708

incanus opened this issue Jun 10, 2015 · 9 comments
Labels
iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@incanus
Copy link
Contributor

incanus commented Jun 10, 2015

If we proceed with this, it will also happen on Android.

Per voice with @kkaefer, our shape annotation implementation in #1655 could be improved by creating a layer grouping API identically-styled shapes.

Currently, each shape annotation gets its own style layer for reasons of:

  1. Eventual z-ordering by the client, much like MapKit's insertOverlay:aboveOverlay:.
  2. Styling, as shapes in the same layer get the same styling.

However, performance is a concern here. Adding 10,000 similarly-styled shapes means 10,000 more style layers, which means 10,000 more GL draw calls. 🚨 NOT TENABLE 🚨

If we introduced the concept of a shape group for identically-styled shapes, a consumer could add 10,000 blue polylines to the map and incur 1 style layer and 1 draw call.

Instead of:

mapView.addAnnotation(polyline1)
mapView.addAnnotation(polyline2)
// ...
mapView.addAnnotation(polyline10000)

It would look something like:

let polylineGroup = MGLMultiPointGroup
polylineGroup.addObject(polyline1)
polylineGroup.addObject(polyline2)
// ...
polylineGroup.addObject(polyline10000)
mapView.addAnnotationGroup(polylineGroup)

This would also allow functionality like:

  • Easy restyling of shape groups
  • Easy toggling of hide/show for shape groups

We would still support mapView.addAnnotation(polyline) like MapKit, with the understanding that it's more efficient to add many shapes with the groups API instead.

Thoughts @1ec5 @friedbunny?

@incanus incanus added iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage labels Jun 10, 2015
@incanus
Copy link
Contributor Author

incanus commented Jun 10, 2015

If we are liking the looks of this, we should mock out what this Cocoa API looks like exactly, including styling configuration, how it interacts (if at all) with eventual layer reordering, and removal/other management.

@1ec5
Copy link
Contributor

1ec5 commented Jun 10, 2015

Sure, why not?

@incanus
Copy link
Contributor Author

incanus commented Jun 10, 2015

Only because it's unlike Apple's API and may raise questions as to when or when not to use it. The answer to that is:

  • When adding many shapes with identical style properties.

@friedbunny
Copy link
Contributor

Sounds like the performance gains would be worth the divergence/complexity. Having a built-in construct for managing groups of annotations is also a friendly addition in general.

Would this also carry over into marker annotations? If so, we should be careful not to make this layer grouping too easily confused with our eventual support for clustering.

@mb12
Copy link

mb12 commented Jun 10, 2015

This optimization is not needed for lines. LineBucket in gl-native is actually a MultiLine.

@ansis
Copy link
Contributor

ansis commented Jun 11, 2015

I think it might be worth waiting until data driven styling is implemented before worrying about this. Data driven styling will change the way this problem looks. I don't think the first implementation of shapes needs to be able to support thousands or even hundreds of shapes.

@jfirebaugh
Copy link
Contributor

+1 to what @ansis said. With data driven styles we will have more optimization options for transparently combining multiple layers into one draw call.

@incanus
Copy link
Contributor Author

incanus commented Jun 11, 2015

I think it might be worth waiting

This was my first inclination in talking with @kkaefer as well since adding this API can be classified as "works now, gets faster / more performant later", much like the R-tree stuff in #1694.

Let's put this on the b3 milestone then.

@jfirebaugh
Copy link
Contributor

Working towards #837 instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

6 participants