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

Improve touch handler perceived performance #1844

Closed
kristfal opened this issue Dec 17, 2015 · 11 comments
Closed

Improve touch handler perceived performance #1844

kristfal opened this issue Dec 17, 2015 · 11 comments
Labels
feature 🍏 performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@kristfal
Copy link

Hey,

the perceived performance / UX of mapbox-gl-js is poorer than it could be on mobile because touch handlers are a bit wonky.

I've noted some issues for myself that I'll look into when time allows, feel free to discuss and add:

  1. Zoom and rotate touch handlers doesn't have an easing function.

  2. Fling calculation during _onUp only uses first and last value of the inertia array, which works well in a 60fps desktop environment but is prone to random huge flings on mobile due to GC-ticks or other main thread lock-ups. I believe a potential improvement could be to average the fling over the length of the entire inertia array. This is mostly a non-issue on faster mobile devices but very significant in a sub-30-fps situation.

  3. Zoom and rotate doesn't have a 'threshold' for transitioning between a non-rotating zoom and a rotating zoom.

@bhousel
Copy link
Contributor

bhousel commented Dec 17, 2015

Hi @kristfal, this is definitely something we want to get right.. Can you let me know which devices you've tested on?

also 3) above may be a dup of #1839

@lucaswoj
Copy link
Contributor

I've definitely experienced 2) and don't know of any other ticket tracking the issue. I don't think I've noticed 1), it might be a platform-specific problem.

@kristfal
Copy link
Author

  1. The issue seems present on all touch devices across our testing environments (iOS, Android, Cordova and non-Cordova). I can't see any easing methods in the handlers either. Haven't updated gl-is in a few weeks, so unless it's added lately, it shouldn't ease on any zoom or rotate events.

  2. Is a dupe of the issue, but reducing sensitivity is probably not the way to go. I suggest mirroring gl-native with a threshold before rotating if the primary intent of the user is to zoom.

Regd testing:

  1. Is easy to notice on a high end device (iPhone6+)
  2. Is easiest to reproduce on a mid range Android device, for instance a Moto G2 or a Nexus 7

@mourner
Copy link
Member

mourner commented Dec 17, 2015

Yeah, the touch handler for mobile definitely needs some improvement. I got it to a state of "kinda working" but didn't spend enough time to get it to a really good experience. Also would be stoked to see some PRs on this if you want to help out.

@mourner mourner added feature 🍏 performance ⚡ Speed, stability, CPU usage, memory usage, or power usage labels Dec 17, 2015
@liebrand
Copy link
Contributor

FYI not sure if this is related, but even normal panning / dragging the map around seems slower than on native. At first I thought this might be down to webGL, but looking more closely I think this is the perceived performance. Basically, it appears the chart starts panning about 50ms after I drag my finger... Making it feel really sluggish.

I'm comparing the "share link" of my map in Chrome on Android, to a quick test I made by updating the tutorial "My First GL App" for android to point to that same map. The native GL pans fine, but the in-browser is slow. The same thing shows if I wrap it up in a cordova app, as you would expect I guess, as they use the same mapbox-gl-js.

Another thing I noticed while panning around, is that the tile loading appears slower on mapbox-gl-js than it does for the native app I created. I'm guessing the processing of the vector tiles in JS are just slower than native (and this is thus probably a separate bug?).

Ps. I've been testing on a nexus 5x

@liebrand
Copy link
Contributor

liebrand commented Jan 3, 2016

Some more testing does hint at more than just perceived performance. The more complicated the map (more layers), the lower the fps in drag/panning the map around. Whereas the native android sdk does not show that same slow down (eg the device itself, and the openGL/webGL should be able to handle it).

Looking at the profiler, it seems the map is "updated"/"rerendered" on every mouse move event, which feels expensive/excessive. Even though the updating of the sources is debounced for 50ms, even just updating the map itself can take anywhere from 25ms to 40ms, which is well over the 16ms guidelines to achieve 60fps.

Perhaps there are more efficient ways to handle the panning of the map than simply rerendering it completely on every move event?

@mourner
Copy link
Member

mourner commented Jan 3, 2016

@liebrand also see #1879 for discussion on rendering performance.

We discussed doing some kind of rendering into a tile framebuffer and then tiling when panning to optimize it, but this would have a big performance hit on non-panning movements. Also, I'd expect Android to be much less efficient with WebGL than iOS. Normally rendering should happen sub-16ms even on mobile devices unless you use a very complicated style with a huge amount of layers.

@liebrand
Copy link
Contributor

liebrand commented Jan 4, 2016

Ok but the 40ms was actually on my macbook, not on mobile... But on a rather complex style. I'll use a simpler style for now.

As for the touch handlers, I've just forked the repo and will see if I can get a PR up soon to improve the handling (and also add long-press event to the mix)

@liebrand
Copy link
Contributor

FYI: I've pushed a PR to fix the accidental rotation when pinch zooming: #1917

I plan to push a separate PR to add support for long-press.

@kristfal I'm not sure averaging the fling over the entire inertia array would be what we want? A fling gesture (as opposed to the more linear mouse move) tends to accelerate when you flick. By averging the entire array, you lose that acceleration dont you? I guess the 'best' solution would be to do some quadratic regression on the array, but that might be overkill? Perhaps taking the average of the first few points in the array, vs the average of the last few points is best?

@kristfal
Copy link
Author

@liebrand

The array is shifted to only include events from the last 50ms of the gesture. From a bit of testing yesterday, it seems like 2) mainly happens when the inertia array only has two entries.

On slow devices, or right after a GC tick, a 50 ms cutoff often produces 2 entries.

I've increased the cutoff and reduced the max inertia speed in the #1920 PR. Could possibly do an average, but this is probably an ok first step. Feel free to tweak / add as you'd like.

Nice work on the rotation block btw, this makes the map a lot more enjoyable on mobile.

@bhousel
Copy link
Contributor

bhousel commented Jan 14, 2016

committed as
76497fb
f17246e
4166ec3

@bhousel bhousel closed this as completed Jan 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🍏 performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

5 participants