mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.38k stars 1.32k forks source link

Uncaught exception setting a style’s sources #7707

Closed 1ec5 closed 5 years ago

1ec5 commented 7 years ago

MGLStyle.sources’ setter always raises an exception because it first tries to remove all the sources to make room for the new ones, but one of the sources is mbgl::AnnotationManager’s source, which is wrapped by MGLSource itself and therefore cannot be removed.

caught "NSInvalidArgumentException", "The source <MGLSource: 0x10055b0c0; identifier = com.mapbox.annotations> cannot be removed from the style. Make sure the source was created as a member of a concrete subclass of MGLSource."
    0   CoreFoundation                      0x00007fff8b1c4452 __exceptionPreprocess + 178
    1   libobjc.A.dylib                     0x00007fff8f222f7e objc_exception_throw + 48
    2   CoreFoundation                      0x00007fff8b22b41d +[NSException raise:format:] + 205
    3   Mapbox                              0x0000000108ef5b23 -[MGLSource removeFromMapView:] + 115
    4   Mapbox                              0x0000000108eb56c9 -[MGLStyle removeSource:] + 281
    5   Mapbox                              0x0000000108eb47fc -[MGLStyle setSources:] + 476

We need a special case to avoid removing the annotation source and another special case to avoid adding it to the style redundantly.

/cc @boundsj

boundsj commented 7 years ago

Noting that we discussed and since this particular API is unlikely to be used frequently, we are not going to block the 3.4.0 release on this issue. I've moved to 3.4.1 for now.

jfirebaugh commented 7 years ago

Should we special-case the annotation-related source and layers in the core runtime styling API, so that they are not visible as an implementation detail to SDKs?

1ec5 commented 7 years ago

That’s an intriguing idea. That could’ve addressed some of the timing-related issues that have tended to crop up in KVO-intensive applications like macosapp in the past.

When you add a layer to the style using the public runtime styling API, should it go above or below the annotation layers? (If I’m not mistaken, it goes above them, at least if you wait until the map finishes loading.) If you query rendered features, do the results still include features from annotation layers (more important once #7162 is fixed)?

Note that, if at some point we’re unblocked to pursue #6181, we’d end up special-casing annotation layers at the SDK level anyways.

I just hope pushing this special case down to mbgl doesn’t make it harder to implement https://github.com/mapbox/mapbox-gl-native/issues/1734#issuecomment-242802281. 😏

stale[bot] commented 5 years ago

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