Mapbox-gl-js: symbol layer fades in upon setData

[andrewharvey]

mapbox-gl-js version: git bisect reveals this is a regression in https://github.com/mapbox/mapbox-gl-js/commit/135561825addae07b253bab2167cee72f00f3ac9 (#5150)

When updating a source with setData any symbol layers using that source will fadein again after the setData. This causes the symbol to flicker when setData is fired frequently such as from a cursor event.

See this in action at https://jsbin.com/kezucog/edit?html,output (just move the cursor around the screen). This wasn't a problem in 0.41.0, but is a problem with 0.42.0 and I pinpointed it to the commit mentioned.

I can see that GL JS now does cross source symbol placement so previously the "Mapbox DC" (from the example) would overlap existing labels, now it will actually fade out the other labels to make room for it. To get the 0.41 behaviour back it's just a matter of using "text-ignore-placement": true, "icon-ignore-placement": true in that layer's layout properties. However even with this, the label still fades in again every time it is updated.

/cc @ChrisLoer

[ryanbaumann]

I can replicate this issue as described by @andrewharvey on Chrome, Firefox, and Edge. Examples:

1. 0.41.0 example - setData() on a symbol layer animates symbols on setData() without fade.
2. 0.42.0 example - setData() on a symbol layer causes significant icon-image fade.

[ChrisLoer]

@ansis This sounds like it's probably another issue with how CrossTileSymbolIndex updates when a tile reload happens (like #5491, except this time it's from setData instead of setFilter)? I'm not sure why the fix for #5491 isn't also working here, but this seems like something that the back-port from native would address. Do you see a quick fix we can do now for this, or is the back-port coming so soon it doesn't matter?

@ryanbaumann I shouldn't make light of broken functionality, but that fleet of ghost-planes crossing the South Pacific is... _kind of awesome_.

[lbud]

Also reported as a bug with the "Animate a point along a route" example in https://github.com/mapbox/mapbox-gl-js/issues/5490#issuecomment-345669666.

[pathmapper]

I think it is the same issue, so posting it here:

There is also an effect on a fill-pattern:

Here are two examples, just zoom in to see the issue:

v.0.41.0 example -> the blurry lines are only a short moment visible when zooming

v.0.42.2 example -> the blurry lines are staying visible at certain zoom levels

[ansis]

@pathmapper I think the underlying cause for that is different so I split your comment into a separate issue: https://github.com/mapbox/mapbox-gl-js/issues/5740#issue-276117185

[pathmapper]

@ansis sure and thanks!

[livemixlove]

I've upgraded to v0.43.0-beta.1, and while the symbol flickering seems improved, I'm still seeing it happen:

[ChrisLoer]

@livemixlove I'm not sure how you're adding that marker to the map, but this looks like expected collision detection behavior to me. Starting with v.42, we detect label collisions between symbols from different sources (previously, it was a limitation of GL JS that we couldn't detect them).

If you want the symbols to be allowed to overlap, you have to explicitly use the *-ignore-placement and/or *-allow-overlap options in your style.

[livemixlove]

The label collision feature is awesome. But, notice how the marker disappears (quickly fades in and out ) in a place where there should be no collision (below "Northern BLVD"). You might have to watch the gif a few times. This looks like a bug to me, unless I need to be setting some new parameters for layers.

[livemixlove]

Also, when I say "marker", I'm only referring to the green marker being dragged.

[livemixlove]

I added

"icon-ignore-placement": true,
"icon-allow-overlap": true,

to the green icon's layout properties, and I still see the same behavior.

[ChrisLoer]

Ah OK thanks for the clarification, I was paying attention to the wrong thing -- looking at the labels underneath instead of that little flicker in the marker itself.

A guess: this might be happening when the marker crosses a tile boundary (you can see tile boundaries by setting map.showTileBoundaries = true)? Crossing from one tile to the next would skip the justReloaded check we put in to try to support "setData animation without fading". This might be an argument for adding per-layer collision fade duration configurations, so that we could just disable the fading for layers animated with setData....

[livemixlove]

More info:

• I turned on showTileBoundaries and was able to reproduce the effect without crossing a boundary.

• I just realized that there's actually a second marker beneath the green marker. So I'm actually dragging two markers (one larger visible one and one smaller obscured) in the gif. Both markers have "icon-allow-overlap": true.

[ChrisLoer]

Shucks, I thought it was such a good guess.

🤔 Do you think you could put together a minimal JSFiddle demonstrating the behavior?

[livemixlove]

Ok, I made a minimal example. I modified the https://www.mapbox.com/mapbox-gl-js/example/drag-a-point/ example:

https://codepen.io/anon/pen/xpwqpe (make sure to add your accessToken)

If you increase the number of geojson features, the effect becomes more pronounced. On line 65 I add 2000 additional features that aren't being dragged.

I think the bug happens when you use "type": "symbol" for the layer. The glitch isn't visible to me when you use circle

I'm on OSX 10.12.3 , Chrome Version 62.0.3202.94 (Official Build) (64-bit)

[andrewharvey]

I can replicate the flashing, to make it easier to see I forked your example to https://codepen.io/anon/pen/JMYWBg adding:

• larger icon-size
• icon-ignore-placement: true to allow other symbols to be shown even if they overlap the icon (eg labels)

[livemixlove]

@ChrisLoer , should we reopen this issue?

[ChrisLoer]

Yeah, let's re-open for now. I think we're probably looking at a different underlying problem, but might as well track it here until we figure it out.

Thanks for the codepen!

[livemixlove]

Related note: I'm getting buggy mouseover/queryRenderedFeatures behavior. I think it's the same issue where passing large objects via setData is making the layer disappear briefly.

v0.37.0

v0.43.0-beta.1

[ChrisLoer]

That's a good hint @livemixlove, and there's definitely a problem in queryRenderedFeatures. I filed it as https://github.com/mapbox/mapbox-gl-js/issues/5887.

I'm not certain it explains what you're seeing (depends on a lot of timing), but it definitely could.

[ChrisLoer]

@andrewharvey @livemixlove I'm not actually sure what I'm looking for in your codepens. I set the map's fadeDuration to 2000ms so I could definitely catch any spurious fading-in, and I don't see any when I move the circle around. The performance is generally bad, though, as long as the point with 2000 features is within the set of loaded tiles: calling setData this frequently just really stresses out our tile loading infrastructure. I have ideas about how to improve that, but first I want to track down whatever was happening at https://github.com/mapbox/mapbox-gl-js/issues/5716#issuecomment-352127101.

[andrewharvey]

@ChrisLoer In https://codepen.io/anon/pen/JMYWBg when you drag that circle around (you need to hover over the centre to get the grab mouse handler, and then you can drag it), and move it quickly it will flash in and out, rather than just moving from one location to the next as you'd expect. I'm not sure if this is the intended behaviour or a bug, ie. should it really fade in then out when using set data? When animating like this you probably want to skip the fade in/out. I just made a few changes to @livemixlove example to try to demonstrate the flashing easier.

[livemixlove]

This is what it looks like to me.

Does fadeDuration force the full duration to happen or does it just slow the effect? I can't find any documentation. Might setting fadeDuration to longer make the fade glitch less obvious?

[livemixlove]

I still see the effect with new mapboxgl.Map({...,fadeDuration: 2000,...}). It just the page/flashing is a little different.

[ChrisLoer]

Ah! I definitely do not see that on my machine. Not sure why.

You're right, fadeDuration is undocumented. It defaults to 300ms. In this case, setting it to a longer value will make the glitch more obvious because the circle will disappear instantly and then fade in over the longer period. But it probably doesn't matter -- 300ms is enough to see the effect, I'm just apparently getting different results from you two...

[livemixlove]

My machine: OSX 10.12.3 , Chrome Version 62.0.3202.94
MacBook Pro (Retina, 13-inch, Early 2015)

[ChrisLoer]

I'm on macOS 10.13.2, but I doubt it makes a difference.

In the example you showed, it's crossing over tile boundaries (or at least I think it is because it's at null island, and tiles always come together there) -- can you turn tile boundaries on and reproduce in an area without them? Because this can definitely happen crossing tile boundaries.

The tile boundary issue is just a limitation of how we do fading (we don't have any reliable way to tell that two icons in two different tiles are conceptually "the same" and thus shouldn't be cross-faded): I think that's an argument for allowing individual layers to opt-out of fade animations.

[livemixlove]

Yep, same as before without crossing tiles:

(the lack of cursor motion is just because the example lags so much behind the mouse movement)

[ChrisLoer]

Here's the closest I can get: one missing circle on frame 20. But this is not the same as what you're seeing -- note that there's no fading.

@ansis is almost ready to land a PR that will change this code pretty significantly: https://github.com/mapbox/mapbox-gl-js/pull/5890. Might be worth checking out that branch to see if you can reproduce with that code as well.

[livemixlove]

@ChrisLoer Maybe your computer is just much faster? I'm not sure if Chrome's dev tools CPU throttling would help recreate.

[ansis]

I can reproduce both issues on https://github.com/mapbox/mapbox-gl-js/pull/5890. I'll try to find the cause of the flickering within the tile bounds.

[ansis]

@andrewharvey @livemixlove thanks for the test cases. They should be fixed by https://github.com/mapbox/mapbox-gl-js/pull/5890 which was just merged into master.