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

[core] Remove StyleSourcedAnnotation support #8908

Merged
merged 1 commit into from
May 10, 2017

Conversation

jfirebaugh
Copy link
Contributor

The functionality this provided has been subsumed by the runtime styling API.

@jfirebaugh
Copy link
Contributor Author

Putting this out for discussion. We probably don't have to remove this if it's a key API in the Qt SDK, but it's duplicative of the runtime styling API and keeping it around is a slight drag on velocity whenever we go to make changes to the core style code, as we're doing now for async rendering.

@mb12
Copy link

mb12 commented May 6, 2017

The advantage of annotations over runtime styling is that they persist across style changes without any code changes required by the sdk user. Do layers added via runtime styling persist across style changes? If not, this would require additional complexity to any application already using it.

@ivovandongen
Copy link
Contributor

Do layers added via runtime styling persist across style changes

It does on Qt specifically, not on other SDKs

@brunoabinader
Copy link
Member

it's duplicative of the runtime styling API and keeping it around is a slight drag on velocity whenever we go to make changes to the core style code

Agreed - we had this in mind when implementing the Mapbox GL QML plugin in the Qt SDK - and thus opted for avoiding annotations at all in favor of runtime styles.

Besides client needs in which we can adapt the style source annotation with runtime style behavior, I'm fine with removing this 👍

Do layers added via runtime styling persist across style changes?

No - but for the Mapbox GL QML plugin we have persistent runtime style by caching all the style changes - e.g. see here - and re-applying them whenever needed e.g. when a style URL changes.

@1ec5
Copy link
Contributor

1ec5 commented May 8, 2017

For what it’s worth, StyleSourcedAnnotation appeared to be relevant to #1734 (comment), at least in terms of getting around the issues blocking #6181, although for #1734 (comment) I had been prototyping something more direct, in which an annotation could hang onto a layer ID.

The functionality this provided has been subsumed by the runtime styling API.
@jfirebaugh jfirebaugh force-pushed the rm-style-sourced-annotation branch from 69d9529 to 659f661 Compare May 9, 2017 20:16
@kkaefer kkaefer added the Core The cross-platform C++ core, aka mbgl label May 10, 2017
@jfirebaugh jfirebaugh merged commit c130d47 into master May 10, 2017
@jfirebaugh jfirebaugh deleted the rm-style-sourced-annotation branch May 10, 2017 21:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants