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

Experiment: don't fade in line labels showing for the first time #10460

Closed

Conversation

ChrisLoer
Copy link
Contributor

Reviewing PR #10436 called our attention to a way masters behavior might be superior. When you cross an integer zoom level, most line labels will move to slightly different positions, so they don't get detected as "duplicates" by the CrossTileSymbolIndex. The old versions will immediately disappear and the new ones will fade in. On master, because there's no fading, and because placement happens before a tile is first loaded, the symbols never disappear, they just instantly shift location. The overall effect is that viewport-collision looks more flickery.

One solution we've discussed is to hold onto the old symbols for a little while so we can do a cross-fade.

This PR experiments with a different approach -- try to replicate masters behavior by disabling fade animations for line labels that are being placed for the first time (e.g. they're part of a higher-res tile that just loaded). There are some kinks I would need to iron out, but after playing with it a little bit I think I'll probably abandon this approach. The problem is that even with the fade animation removed, there's still a gap between when a new tile first loads and when the next placement happens. Depending on how you hit the timing of placements, it can feel more or less flickery.

master behavior
master_instant_switch

viewport-collision behavior
half_fade_switch

This PR behavior
proposed_faster_switch

/cc @ansis @kkaefer @jfirebaugh @nickidlugash

ChrisLoer and others added 21 commits November 9, 2017 14:34
 - Background placement code now just generates static symbol buffers
 - Don't render GeometryTiles until their symbols are loaded. This is necessary for the CrossTileSymbolIndex to successfully prevent flicker.
 - SymbolInstances are transferred to SymbolBucket for use on foreground during collision detection
 - Symbols are sorted on foreground by sorting their index buffer but leaving vertex buffers intact (only works within one segment)
 - Vertical glyphs are generated at same time as horizontal glyphs. `reprojectLineLabels` chooses which one to use at render time and hides the other.
 - Icons are now always represented with a single collision box, even if they're placed along a line (this means their rotation alignment may be wrong, but the approach of representing them with multiple collision boxes wasn't very accurate either).
 - Generate vertices for new debug collision boxes and collision circles
 - Only add symbols within tile boundaries (reduces work, avoids double-draw)
 - Update symbol_projection.cpp to support line label projection calls from CollisionIndex.
Brings gl-native shaping closer to gl-js.
 - CollisionTile
 - FrameHistory
 - PlacementConfig
 - Revert to 16x16 grid for FeatureIndex.
 - Indentation/dead code fixes
 - unordered_set<uint32> -> set<uint32>
`Tile` makes sure the symbols in the resulting tile are tileable while
symbols in `Still` match rendering in `Continuous` mode.
Don't mark items that are outside the collision grid range as placed.
Requires new ignore because GL JS issue #5654 allows insertion of symbols outside the CollisionIndex range, and those symbols can cascade in to affect items within the viewport.
Update ignore links to specific issues.
Bump mapbox-gl-js version to get latest text-pitch-spacing test.
…iously outside the range of the CollisionIndex.

Otherwise there's a weird mix on panning of some labels fading in vs. some popping in.
@ChrisLoer ChrisLoer added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Nov 14, 2017
@kkaefer
Copy link
Member

kkaefer commented Nov 14, 2017

I think the behavior in master/this PR is superior to the current vanish + fade-in. As you mentioned, I think a crossfade behavior would also work.

In the GIF you posted, I'm noticing that there are some frames that don't have any street labels (just neighborhood labels). It looks like there are some situations in which the new labels aren't ready yet, but we no longer show the old labels either. Master continues showing the old labels until it has new labels to display in that position.

@ChrisLoer
Copy link
Contributor Author

It looks like there are some situations in which the new labels aren't ready yet, but we no longer show the old labels either. Master continues showing the old labels until it has new labels to display in that position.

Yeah, on master placement is tied to the tile, so it gets updated at the same time the tile is swapped in. On viewport-collision, placement happens independently of when the tile loads, and that's why the labels disappear for a bit until the next placement happens. In order to keep showing the old labels, we'd have to be rendering symbol buckets from both tiles simultaneously (at which point it would be easy to do a cross-fade).

The current GL JS approach is a little different -- because it requires running a full/synchronous placement every time a tile is added to/removed from the tile pyramid, there's no gap in placement timing (although the fade animation is still an issue: but basically if we did the equivalent to this PR on GL JS, we could replicate masters behavior).

@ChrisLoer
Copy link
Contributor Author

There are two closely-related "flicker" situations for point labels:

  • Zoom-specific label changes (e.g. "CALIF." -> "CALIFORNIA") vanish/fade-in. We sort of assumed from the start that using the label text was a necessary part of identifying duplicates, but I wonder if it would be worth trying location alone?
  • There's a tendency towards point label flicker during fast zoom out operations because we'll remove the high zoom tiles from the render tree before low zoom tiles load. (In contrast, when we're zooming in, we'll overzoom the heck out of a low zoom tile while we wait for high zoom tiles to load).

@ChrisLoer
Copy link
Contributor Author

PR #10468 is looking like a more promising approach.

@ChrisLoer ChrisLoer closed this Nov 15, 2017
@jfirebaugh jfirebaugh deleted the viewport-collision-no-fade-line-labels branch July 27, 2018 22:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants