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

hash: true cause poor zoom performances #4373

Closed
nicooprat opened this issue Mar 6, 2017 · 10 comments
Closed

hash: true cause poor zoom performances #4373

nicooprat opened this issue Mar 6, 2017 · 10 comments

Comments

@nicooprat
Copy link

mapbox-gl-js version: 0.32.1

Steps to Trigger Behavior

  1. Create a map with options set to {hash: true}
  2. Zoom in and out
  3. FPS drops considerably

Expected Behavior

Smooth

Actual Behavior

Not smooth


I'm actually using a Mac with a trackpad, so the scroll event is triggered a lot when zooming. No problem when dragging the map because the hash is set only after mouse up. So a simple throttle might resolve it...

Can't reproduce on Jsbin & others because the JS seems not be able to update the URL (security I guess). Here are 2 quick screencasts of the result:

Thanks!

@mourner
Copy link
Member

mourner commented Mar 6, 2017

Whoa, that looks quite severe, although I can't reproduce the same effect. Can you share the browser/OS version you're using, and whether this is reproducible for you in other browsers?

@mourner mourner added performance ⚡ Speed, stability, CPU usage, memory usage, or power usage needs information 🙏 labels Mar 6, 2017
@nicooprat
Copy link
Author

I'm using Chrome 56 on Mac Sierra (latest). Even worse on Firefox, a bit better on Safari (all up to date).

@mourner
Copy link
Member

mourner commented Mar 6, 2017

As you see in the Hash handler source, the hash update only happens on moveend. Since the video clearly shows that hash is updated very often, the source of the problem is actually moveend firing during the movement (not afterwards), and it could be specific to your scrolling device.

Are you using an external Apple touchpad, or an internal Macbook one?

@mourner mourner added environment-specific 🖥️ and removed needs information 🙏 performance ⚡ Speed, stability, CPU usage, memory usage, or power usage labels Mar 6, 2017
@nicooprat
Copy link
Author

nicooprat commented Mar 6, 2017 via email

@mourner
Copy link
Member

mourner commented Mar 6, 2017

@nicooprat we would really appreciate you helping us track down this issue. Can you set up the test case with a dev GL JS build and dev tools open, then put a debugger statement on moveend and look through the call trace after starting a scroll-zoom movement to see if there's anything suspicious?

@nicooprat
Copy link
Author

nicooprat commented Mar 7, 2017 via email

@kristfal
Copy link

kristfal commented Mar 7, 2017

@mourner Last time I checked, all multi-touch (touchZoomRotate) interactions fires 'moveend' on iOS, Android and touch enabled desktops. See #3435. The culprit seems to be that touchZoomRotate._onMove uses map.easeTo which triggers Camera._ease and in turn triggers Camera._easeToEnd that fires the moveend event.

We did an internal hotfix where we suppressed Camera events triggered by the _onMove handler by passing along a flag in the params. Not sure if that would be the ideal approach, and we didn't open any PRs due to the planned refactoring mentioned in #3435. Let me know if you want a PR.

@mourner
Copy link
Member

mourner commented Mar 7, 2017

@kristfal thanks for the info! That bug seems pretty severe since it can significantly impact performance on touch devices. @lucaswoj I'm inclined to reopen #3435 because #3357 is a "public interface simplification" ticket about refactoring (without any clear timelines), and #3435 is clearly a bug.

However, the current ticket is about Macbook touchpad which triggers the scrollZoom handler, not the touchZoomRotate one.

@nicooprat
Copy link
Author

Guys, just figured it out. The issue was actually coming from my code. I had something like this:

map.on('zoom', (e) => {
  map.setPitch(xxx)
})

My idea was to pitch the camera at small zoom, like a plane landing :) Looks like the zoomend/moveend event was triggered each time. Removing this part of my code resolved the issue: smooth scrolling and the hash is updated after zoom ends. Don't know if it's the expected behavior, but thanks anyway. As a side note, I couldn't reproduce the issue on iOS when pinching/zooming.

@jfirebaugh
Copy link
Contributor

Thanks for investigating @nicooprat!

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

No branches or pull requests

4 participants