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

No annotations at initial zoom level #1675

Closed
1ec5 opened this issue Jun 2, 2015 · 33 comments · Fixed by #2420
Closed

No annotations at initial zoom level #1675

1ec5 opened this issue Jun 2, 2015 · 33 comments · Fixed by #2420
Assignees
Labels
bug iOS Mapbox Maps SDK for iOS

Comments

@1ec5
Copy link
Contributor

1ec5 commented Jun 2, 2015

If you follow the “First steps with Mapbox GL for iOS” guide, you wind up initializing an MGLMapView, centering it on the U.S. President’s front yard, and placing an annotation there, all within -[ViewController viewDidLoad]. But if you do all that in -viewDidLoad, the annotations fail to display at the initial zoom level. They appear when you zoom in or out to a different zoom level and disappear when you reenter the initial zoom level.

At launch, the map view happens to show z15 tiles:

initial

Then zoom in, right to the cusp of z16:

in

And zoom in just a little farther to z17:

inner

Or zoom out, right to the cusp of z14:

out

And a little further out to 14:

outer

The upshot is that our guide does not work as designed. We didn’t catch this issue in development because the iOS demo app only adds annotations in response to an action sheet button, not within -viewDidLoad.

Reproduces in ios-v0.3.1 as well as master, regardless of style.

/cc @incanus

@1ec5 1ec5 added bug iOS Mapbox Maps SDK for iOS labels Jun 2, 2015
@incanus
Copy link
Contributor

incanus commented Jun 2, 2015

Likely the same issue as #1279.

@1ec5
Copy link
Contributor Author

1ec5 commented Jun 3, 2015

Because of this issue, bsudekum/react-native-mapbox-gl’s -[RCTMapboxGL setAnnotations:] has to defer the call to -addAnnotations: or -removeAnnotation:.

/cc @bsudekum

@incanus incanus added this to the iOS Beta 3 milestone Jun 15, 2015
@incanus
Copy link
Contributor

incanus commented Jun 15, 2015

Let's hit this as part of our focus on annotation solidification in b3.

@friedbunny
Copy link
Contributor

Trying to draw a shape annotation in viewDidLoad immediately after map initialization fails to render the shape on the map, too.

Here's a GeoJSON polyline demo for iOS using the same delay technique as react-native-mapbox-gl.

@ansis ansis self-assigned this Jun 29, 2015
@ansis
Copy link
Contributor

ansis commented Jul 1, 2015

@1ec5 I think this should be fixed by the fix-1675 branch. Can you check?

@tmpsantos do these partial parsing changes look right? 5f90c60

@1ec5
Copy link
Contributor Author

1ec5 commented Jul 1, 2015

@ansis, yes, that fixes it. Thanks!

ansis added a commit that referenced this issue Jul 1, 2015
@ansis
Copy link
Contributor

ansis commented Jul 2, 2015

This was also fixed by @jfirebaugh's work that merged yesterday

@ansis ansis closed this as completed Jul 2, 2015
@tmpsantos
Copy link
Contributor

@tmpsantos do these partial parsing changes look right? 5f90c60

Perfect.

@friedbunny friedbunny reopened this Jul 17, 2015
@friedbunny
Copy link
Contributor

Still seeing this issue.

Annotations don't load at the initial zoom, but load (and stay) after zooming in/out to the next level.

@1ec5
Copy link
Contributor Author

1ec5 commented Jul 18, 2015

So was @ansis' patch needed then?

@tmpsantos
Copy link
Contributor

Annotations don't load at the initial zoom, but load (and stay) after zooming in/out to the next level.

I have theory why this is happening: could be that we add the layer so fast that we emit the update() request while the tiles are still being parsed for the first time and after that we think that parsing is done, not aware of a new layer that was added. I can have a look at this monday morning.

@incanus incanus modified the milestones: v0.6.0, iOS Beta 3 Jul 21, 2015
@incanus
Copy link
Contributor

incanus commented Jul 27, 2015

@tmpsantos Any more movement on this? Is this something you can look into?

@tmpsantos
Copy link
Contributor

@tmpsantos Any more movement on this? Is this something you can look into?

Yes, I managed to reproduce this early today. But I'll have to get back to this one later. :-/

@tmpsantos
Copy link
Contributor

Working on this one today.

It is indeed related to adding the annotation before the style gets parsed. I can reproduce adding the annotation after calling map.setStyleURL but not after map.setStyleJSON. The former will issue a network request that sometimes can be answered before we add the annotations (so it works) but most of the time will be late (so we don't see the marker) and the latter is guaranteed that the style gets parsed before we add the marker because there is no network request involved.

@mikemorris
Copy link
Contributor

Possibly relevant @tmpsantos: cutting-room-floor/tilelive-gl#11 (comment)

@picciano
Copy link

I put together a small app that reproduces this issue.
https://github.com/tallygo/MissingMapMarkers

Tapping the "Redraw" button re-creates the MapView and sometimes gives a different manifestation of the problem. This project uses the latest published pod, but this is also seen from latest source in Master.

ios simulator screen shot sep 15 2015 5 27 24 pm
ios simulator screen shot sep 15 2015 5 27 36 pm

@incanus
Copy link
Contributor

incanus commented Sep 16, 2015

Thanks @picciano — reopening.

@jfirebaugh
Copy link
Contributor

It looks like the cases where shape annotations render at all is entirely by accident. There is no guarantee that MapContext::updateAnnotationTiles invalidates any of the necessary tiles for shape annotations after it creates the style layers and style buckets for shape annotations:

  • MapContext::updateAnnotationTiles invalidates only the passed in ids, not any of the "stale tiles" accumulated by annotationManager->markStaleTiles().
  • But even if it did invalidate the union of ids and staleTiles, it still wouldn't guarantee to invalidate all or even any of the necessary tiles because for shape annotations, MapContext::updateAnnotationTiles is passed an empty unordered_set -- AnnotationManager::addShapeFeature does not calculate the affected tiles.

I'm not sure how this was intended to work. @incanus?

@incanus
Copy link
Contributor

incanus commented Sep 21, 2015

Keep in mind that this was perhaps significantly changed recently in 2141d95 and I'm unfamiliar with that work. Do things look the same on the iOS release branch or master before that date @jfirebaugh?

@jfirebaugh
Copy link
Contributor

That commit didn't change this aspect of annotation behavior (or to my knowledge any aspects: it was a pure refactor).

@incanus
Copy link
Contributor

incanus commented Sep 21, 2015

OTOH, I think shape tiles behave differently from point tiles, since shape tiles are actually generated by geojson-vt so that it can simplify shapes as well as trivially create tiles which are wholly inside filled polygons.

I can get back into this shortly and try to provide more details after reviewing the code a bit.

@incanus incanus reopened this Sep 24, 2015
jfirebaugh added a commit that referenced this issue Sep 25, 2015
First, move style mutation code out of StyleParser and into AnnotationManager,
coalescing it with the mutation code for shape layers.

Second, allow AnnotationManager to keep track of stale tiles entirely
internally. There's no reason to pass sets of TileIDs around.

Third, correct the logic for invalidating the shape source. Since
AnnotationManager does not track shape invalidations on a tile-by-tile basis,
don't try to invalidate the shape source tile-by-tile.

Fixes #1675
Fixes #2322
Fixes #2095
jfirebaugh added a commit that referenced this issue Sep 28, 2015
First, move style mutation code out of StyleParser and into AnnotationManager,
coalescing it with the mutation code for shape layers.

Second, allow AnnotationManager to keep track of stale tiles entirely
internally. There's no reason to pass sets of TileIDs around.

Third, correct the logic for invalidating the shape source. Since
AnnotationManager does not track shape invalidations on a tile-by-tile basis,
don't try to invalidate the shape source tile-by-tile.

Fixes #1675
Fixes #2322
Fixes #2095
AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this issue Nov 6, 2015
AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this issue Nov 6, 2015
AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this issue Nov 6, 2015
AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this issue Nov 6, 2015
First, move style mutation code out of StyleParser and into AnnotationManager,
coalescing it with the mutation code for shape layers.

Second, allow AnnotationManager to keep track of stale tiles entirely
internally. There's no reason to pass sets of TileIDs around.

Third, correct the logic for invalidating the shape source. Since
AnnotationManager does not track shape invalidations on a tile-by-tile basis,
don't try to invalidate the shape source tile-by-tile.

Fixes mapbox#1675
Fixes mapbox#2322
Fixes mapbox#2095
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants