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

[DNM] Further experiments with fading adjustments during zoom-out (#8627) #8764

Closed
wants to merge 5 commits into from

Conversation

vakila
Copy link

@vakila vakila commented Sep 17, 2019

This PR extends #8628 with some debugging & experimentation that @asheemmamoowala and I did today to try to resolve #8627 while still not introducing jank.

Changes from #8628:

  • [dnm] Adds a debug page for easier testing of rapid zoom in/out
  • [dnm] Adds some debug logging in style.js and placement.js
  • Attempts to prevent lingering label clusters by forcing a full placement during a zoom instead of waiting for the previous placement to complete

Notes:

  • In manual testing on Ubuntu Firefox & Chrome, this arguably results in a better label experience when zooming out
  • However, it may introduce jank - this was more noticeable in Chrome - and hasn't been tested in all situations (e.g. how does it look in a long flyTo crossing a large area?)
  • This does not solve the label clustering/not fading problem when the zoom is instantaneous, e.g. jumpTo/setCenter

Launch Checklist

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • manually test the debug page

Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vakila I did a round of testing on this PR. Discovered a CPU throttle option in chrome debugger tools and was able to use that a bit to get more consistent reproductions of the original issue

Some findings:

  1. On desktop Chrome (with CPU Throttling) placement happens during the zoom and prevents bunching of symbols during fast zoom out. This does make some (1 in 10) zoom outs a little stuttery. I also noticed when zooming in and out from the same spot that the stutter sometimes shows up on zoom in as well. Could the fix be applied only for zoom out, instead of all zooms?
  2. On a OnePlus 5t, on Chrome I see a different issue. Some symbols disappear during the zoom and reappear during the next placement or at the end of the zoom. The effect makes the map feel gittery. (todo: screen recording) It feels like placement is happening more frequently than it did before the fix, and in some cases it results in a few symbols disappearing on each consecutive frame making it feel like symbols are fading out immediately instead of with an animation.

I would like to do some testing with Edge/IE11 as well.

Another option to consider here is whether a different easing curve can be applied to the fade duration during zoom out. This would mean accelerating the opacity changes while zooming, instead of reducing the fade duration in placement#stillRecent.

@asheemmamoowala
Copy link
Contributor

Closing now since the findings from this PR were rolled up into #8628 and merged

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

Successfully merging this pull request may close these issues.

3 participants