Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zooming out and quickly using setData after that causes labels to vanish #5837

Closed
ttsirkia opened this issue Dec 10, 2017 · 13 comments
Closed

Comments

@ttsirkia
Copy link

mapbox-gl-js version: 0.42.2

Steps to Trigger Behavior

  1. Static background for the map
  2. Dynamic content (circles with labels) which is constantly set by using setData
  3. Zoom out and after a while (200 ms - 1500 ms) the circle labels become invisible

Expected Behavior

Labels stay visible.

Actual Behavior

Labels may disappear.

In my case, I have setInterval with interval of 2 seconds. It seems that if the updating function which eventually calls setData is called when the visible labels is still being calculated after zooming out, most of the labels will become invisible.

There is a workaround which seems to work. Prevent setData being called before, for example, two seconds has passed after zooming.

I'm trying to generate a JSFiddle which is the smallest working example.

@ttsirkia
Copy link
Author

JSFiddle: https://jsfiddle.net/c0tg4h5o/

First zoom in, and then zoom out. You are likely to see the red labels to disappear. Repeat for a few times if it does not happen or change the interval to be shorter.

@jfirebaugh
Copy link
Contributor

cc @ChrisLoer @ansis

@ChrisLoer
Copy link
Contributor

This sounds like #5741, which has merged to master but just barely missed making it into the .42.2 release.

@ttsirkia
Copy link
Author

#5471 removes the unnecessary transition effect but this is also in master.

@ChrisLoer
Copy link
Contributor

Oops, sorry for jumping to conclusions @ttsirkia! I can at least sometimes reproduce this locally, and it looks like there's some sort of starvation condition when setData is continually triggering reloadTile that prevents the initial load from completing. I'm digging in to figure out the root cause...

@ChrisLoer
Copy link
Contributor

@ttsirkia Sorry I'm a little unclear from the report, but is the problem you're seeing that labels disappear and then never reappear, or is it just that they disappear and then after some time start showing?

The behavior I think I'm seeing locally is that when setData is called very frequently, the worker threads end up with a backlog of reloadTile requests, because the requests are getting generated faster than they can be processed and there's no coalescing behavior for the requests (also, each one of the requests requires a round-trip getGlyphs call to the main thread, which means several reloadTile calls for the same tile can all be in progress at the same time in the background). When you change zoom levels so that a new GeoJSON tile has to be generated, there's a delay, but eventually the tile loads and the labels re-appear.

If it's just a matter of tiles from the 'data' source you created taking longer to load than tiles from the 'towns' and 'smalltowns' source, it's not too hard to explain the disappearance. Say you're at zoom level 10 showing tiles from all three data sources, then you zoom out to zoom level 6. As you zoom out, 'towns' and 'smalltowns' will be busy replacing their tiles with tiles at different zooms, but 'data' won't be updated in time. I believe we have a max underzooming of 3 set in SourceCache, so by the time you get to zoom level 6, we won't keep displaying the old z10 tile for 'data', and the circles will disappear for a while.

@ttsirkia
Copy link
Author

The problem description was indeed a bit unclear. The problem is that the red labels will vanish and they won't appear again before zooming back in. I made an animation so that we can be sure that the situation is the same because I noticed that the parameters may have to be changed a little depending on the computer.

mapbox-label-bug

In that example the density of town labels is quite high but in my application there is plenty of space to place the labels so the reason cannot be that there are overlapping labels. In my app, the update period is 2 seconds but because the map in the example is so trivial, I had to generate quite a lot the labels and circles with short update period to get the issue be visible.

@ChrisLoer
Copy link
Contributor

Let's see if I'm paying attention to the right things... In your screen grab, I see:

  • Frame 14: circles and their labels disappear (while zooming out)
  • Frame 19: zooming out finished
  • Frame 25: circles re-appear, but without their labels
  • Frame 71: after zooming in, "Circle #N" labels re-appear under circles on the top half of the screen (I suspect we're seeing a tile boundary there)

So the unexpected behavior is not so much that the entire data source stops showing between frames 14 and 25, but that once it starts showing again, only the "circle" layer (and not the "circleLabels" layer) is visible until frame 71? I'll dig deeper into that now...

@ttsirkia
Copy link
Author

Yes, that is exactly the case!

There is also one variation. With some computers the circles do not disappear at all (frames 14 - 25), only the labels. Otherwise the result is the same. Or at least the time for the hidden circles is so short that it cannot be seen.

@ChrisLoer
Copy link
Contributor

Ah thanks, I get it now. I totally got distracted looking at the wrong problem, although maybe we should do something to reduce the amount of unnecessary work when setData is called frequently.

The reason the labels aren't showing up is that they're marked "duplicate" in the CrossTileSymbolIndex. They're being marked duplicate against tiles that have been removed from the render tree, which isn't supposed to happen. The sequence of events that allows it to happen is:

  • Tile X starts loading (or reloading, in the case of the continual setData calls), sends request to worker thread
  • Tile X removed from render tree with _removeTile, and also removed from CrossTileSymbolIndex
  • Worker thread finishes, _tileLoaded gets called on the foreground for tile X
  • Tile X doesn't get re-added to the SourceCache tile set, but we call tile.added(), which incorrectly adds tile X back into the CrossTileSymbolIndex

This race condition exists for vector tiles loaded from the network too, but you're less likely to see it because _removeTile will abort background network requests which will usually prevent _tileLoaded from getting called on a tile that's been removed.

When this does happen for a given tile X at say zoom 12, it will cause duplicate symbols from tile X to be hidden in parent tiles until tile X is reloaded. So for instance if "New York" was present in tile X, it could be hidden all the way out to much lower zooms. 😬

I believe @ansis's work back-porting the CrossTileSymbolIndex changes from gl-native will fix this (and hopefully make it easier to reason about correctness of the index).

@ttsirkia
Copy link
Author

Nice to hear that you were able to track the root cause of the problem! The workaround seems to work perfectly so not very urgent issue and kind of rare to occur if there are only static elements.

ChrisLoer added a commit that referenced this issue Dec 14, 2017
…der tree.

Fixes issue #5837, which could cause symbols to be incorrectly hidden.
ChrisLoer added a commit that referenced this issue Dec 14, 2017
…der tree.

Fixes issue #5837, which could cause symbols to be incorrectly hidden.
@ChrisLoer
Copy link
Contributor

Fixed with a patch in #5867 while we wait for the back-port from gl-native. I tested the changes locally using the example map from this issue. @ttsirkia if you get a chance could you verify that this fixes the problem on your side?

Thanks for your help finding this issue!

@ttsirkia
Copy link
Author

Thank you for very quick fix! Yes, I can confirm that I cannot reproduce the issue anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants