Switching styles turns annotations invisible

[1ec5]

Changing the style causes any annotations in view to become invisible. Any popover remains visible, and invisible annotations continue to respond to taps.

[1ec5]

Zooming in and out causes the annotations to appear and disappear – with the old style's sprites.

@bsudekum, this is likely the issue you were seeing with your React Native project.

[incanus]

This will go away when we lift annotation imagery out of the sprite sheet. This might be an acceptable part of the b1 stopgap.

[incanus]

I'm ok with this being a stopgap again in b2, meaning you need to set style and leave it before adding/managing annotations for now. b3's focus thus far is better annotations.

[ejdslayer]

Hi, so is there a stable beta version available that will address this issue for now? I don't mind re-adding the annotations etc, I just need this ready for a project due to launch in 3 weeks. Thanks

[kristfal]

Ref #2326, is there any known workarounds for this?

This issue now persists across app launches in ios-v2.1.2 (even when closing the app from the task manager), making annotations unusable in any context where a style may be swapped.

@incanus Any ideas for how to bridge this bug? We've tried to looked into the c++ core, but can't really find the root problem.

[robipresotto]

@incanus map caches the icon annotation anyway if we set it after or before the map style. Any suggestion?

[maciekish]

cc @Zakay

[maciekish]

FWIW 3ec610c did not fix the issue.

[Zakay]

+1 I'm really hoping this gets fixed asap, we are so close to launch our app with a reach of 1million users, and this is a major blocking issue.

[ljbade]

@jfirebaugh I vote associating it with annotation object, makes more sense. Like setSpriteForAnnotation

[incanus]

@jfirebaugh Ok to pass this to @adam-mapbox? He's gonna dig a bit on possible solutions, and I think what you did in annotations recently lays the foundation / this doesn't intersect too much with the style API work?

[jfirebaugh]

👍

To expand on my previous comment: the issue is that sprite icons are owned by Style (via SpriteStore), so when you change the style, all annotation sprites added via Map::setSprite are gone.

Associating the icon directly with the annotation object seems like a decent solution (should be done by modifying PointAnnotation class, not by adding setSpriteForAnnotation though). However, this will require that SpriteStore deduplicates sprites based on their contents, so as not to blow up the store with multiple copies of the same sprite, when used by multiple annotations.

[ljbade]

@jfirebaugh While someone is in here, fixing removeSprite will also be useful for when annotations are deleted. Currently if you add and delete enough unique sprite annotations, the sprite store will fill up.

Ref #2560

[adam-mapbox]

#3049

[mb12]

1.) SpriteAtlas is expensive (memory wise). Is it possible to delay creation of annotationSpriteAtlas until it is actually needed (first time a marker point is added to the map)?

2.) Also is it possible to expose and test a remove method in the SpriteAtlas/SpriteStore that can used to free up the memory allocated for a marker image when its no longer needed?

[adam-mapbox]

#1 is complete

For #2: What is the right way to expose a method by which sprites can be removed? Should there be a sister method to -MGLAnnotationImage annotationImageWithImage:reuseIdentifier: ? Maybe some kind of deleteAnnotationImageWithReuseIdentifier:? There already is a way to remove sprites from a sprite store, I just have to expose it somehow.

[1ec5]

What is the right way to expose a method by which sprites can be removed?

-[MGLMapView didReceiveMemoryWarning] would be a good place to perform this kind of work.

[incanus]

-[MGLMapView didReceiveMemoryWarning] would be a good place to perform this kind of work.

Not sure about that; if memory gets low, we could legitimately jettison cached tiles and the like, but getting rid of sprites would cause the map to not render annotations?

[incanus]

Also, let's keep the discussion about @adam-mapbox's code in #3049 over in that ticket, instead of here in the problem side of things.

[adam-mapbox]

I think @1ec5 's idea was that we only jettison those that aren't being used anymore? But that's actually pretty tough to do if we're not informed of them. It might also break the contract of the API.

[incanus]

🎉

This should land in iOS 3.1.0 (next feature release) so no need to cherry-pick the fix anywhere just yet, as that will be based off of master.