Introduce symbol cross fading when crossing integer zoom levels.

[ChrisLoer]

Fixes issue #934: when tiles containing symbols are removed from the render tree, hold onto the tiles until fadeDuration ms have elapsed, and continue rendering just the symbol layers so that the fade animation can finish.

Before

After

Rough port of logic in mapbox/mapbox-gl-native#10468: although the idea is the same, the logic ends up looking pretty different because (1) painter/SourceCache are so different from renderer/TilePyramid, and (2) gl-js supports variable fade durations.

My testing so far has been entirely manual, and I never figured out an automated test for this on the native side. I think I might be able to do something similar to regressions/mapbox-gl-js#5740 that exercises some of this logic, but most of the things I'd worry about getting wrong (like holding on to too many tiles) wouldn't really show up. To do my manual testing, I entered log statements to track tile rentention, and watched that tiles were being added and removed when expected as I crossed integer zoom boundaries. I also tracked the total number of tiles in the source cache before and after the change during a few animations. As expected: the count of tiles was the same at rest, and peaked a little bit higher during animation.

The paint benchmark doesn't show a noticeable change, but I'd expect the cost here to show up during animations, so I may have to come up with some sort of before on after. On native, the cost never seemed that bad, but it's hard to quantify -- and there's definitely some added cost from drawing extra symbol layers (and some extra memory used holding onto tiles). As in native, we hold onto tiles regardless of the reason they were removed from the render tree, just because the logic is simpler. We could try to exclude tiles that we know fall outside the viewport, but they should also be pretty cheap to render because their fragment shaders won't run?

• briefly describe the changes in this PR
• write tests for all new functionality
• document any changes to public APIs
• post benchmark scores
• manually test the debug page
• tagged @mapbox/studio and/or @mapbox/maps-design if this PR includes style spec changes

/cc @mourner @ansis @brunoabinader @kkaefer

[ChrisLoer]

I also used log statements to watch how long it took to go from "last map movement" to "last frame rendered" -- my concern was that these changes could cause us to keep rendering longer because we're holding onto (and doing placement) on extra tiles. It looks like in most cases we're still looking at ~300ms of rendering after movement stops (excluding renders triggered by tiles loading from the network). The basic logic of why we don't incur extra cost is something like:

placementChanged only gets triggered when a symbol (identified by crossTileID):

• Goes from placed to not-placed or vice versa
  • There's no change happening for the "hold for fade" tiles, since they're hard-wired to always be not-placed
• Is missing from current placement and was placed in previous placement
  • When the held for fade tile gets dropped, it'll be missing from the next placement, but it will have been not-placed in the previous placement, so no change.
• Was not in the previous placement and is placed now
  • The held-for-fade tiles don't add anything new

[ChrisLoer]

To get an idea for the performance impact on zoom, I set up an 1103x929px viewport centered on 16/34/-118.4 (LA streets), and then counted the frames rendered over the course of a 10 second zoom animation out to 7, another 10 seconds back to 16, and then repeated again. With the changes, the animation averaged 59.05 FPS. Without the changes, the animation averaged 59.45 FPS. Although it sounds small written that way, it is roughly a doubling of dropped frames... There's enough noise in FPS counting that it's hard to be too precise about what this means, but the pattern of a slight decrease in frame rate has so far been consistent. My interpretation is:

• There's probably a non-negligible performance hit
• There's probably not a huge/immediately noticeable performance hit
• From a UX point of view, I think the benefit of smoother label transitions will outweigh the potential cost in extra dropped frames

[ansis]

Looks good

From a UX point of view, I think the benefit of smoother label transitions will outweigh the potential cost in extra dropped frames

I agree on this. And if it turns out that it is too slow having this merged will put pressure on us to optimize further.

One minor semi-related thing: layers with minzoom aren't faded. They are instantly hidden once you cross the zoom threshold. For example, some of the pois here disappear instant when you zoom out: http://localhost:9966/debug/#15.05/38.90034/-77.02687 Is this something we want to support fading for?

[ChrisLoer]

One minor semi-related thing: layers with minzoom aren't faded. They are instantly hidden once you cross the zoom threshold.

Yeah, that's filed as #5726, I noticed that in the native cross-fading but forgot about it. I don't think it's urgent to fix, but there is maybe a little UX regression if you're crossing back and forth over one of the minzoom limits because showing/hiding a symbol that's in the middle of a fade animation looks worse than just plain showing/hiding.

I think I might be able to do something similar to regressions/mapbox-gl-js#5740 that exercises some of this logic

I've been trying and failing to get a render test working using the ["wait", <ms>] syntax. I don't think it's critical for this PR, but I'm actually kind of confused about the way the test harness is failing and want to spend a little time trying to figure it out.

[ChrisLoer]

I got the cross-fade render test working and pushed it. The interaction between the test harness's clock-shifting and the placement logic is kind of hard to reason about and I worry the test might end up being a bit brittle, but I think it's also good to have at least one render test that exercises symbol fading.

[ChrisLoer]

@nickidlugash just a heads-up this is coming down-stream to a gl-js renderer near you. I don't think it should be very disruptive since we already have this behavior on gl-native and I think it's the behavior people would usually expect in the first place.