jquery-archive / jquery-mobile

jQuery Mobile Framework
https://jquerymobile.com
Other
9.7k stars 2.41k forks source link

scrollSupressionThreshold - scrolling smoothness impact #2107

Closed adriancd closed 11 years ago

adriancd commented 12 years ago

Is there any point in scrollSupressionThreshold?

I have removed the function which used this variable and scroll touch performance has greatly improved. On iPhone i find performance to be excellent and like native app. Android was also much better but not perfect, some touch actions are still missed but acceptable performance now

If this value has a use somewhere then an option to disable this parameter & function is needed

toddparker commented 12 years ago

We'll take a look at this. Seems like we've heard a few reports that the swipe events may need another look for performance reasons.

toddparker commented 12 years ago

Kin - this is a good one for you to look at.

jblas commented 12 years ago

@adriancd

Are you talking about $.event.special.swipe.scrollSupressionThreshold that is used within the swipe event code?

adriancd commented 12 years ago

Yes

i delete the function below which uses it and touch response/performance in phonegap is much better. Dont see what it is used for

toddparker commented 12 years ago

@jblas - did you get a chance to try this? Would be nice to know if this was a bottleneck.

jblas commented 12 years ago

@adriancd

What did you mean by "i delete the function below which uses it ..."?

Are you saying that you completely deleted this piece of code in the moveHandler()?

            // prevent scrolling
            if ( Math.abs( start.coords[ 0 ] - stop.coords[ 0 ] ) > $.event.special.swipe.scrollSupressionThreshold ) {
                event.preventDefault();
            }

Or did you delete the entire moveHandler() function?

Looking at the code the $.event.special.swipe.scrollSupression really isn't necessary. We can simply remove the whole "if" expression and just call event.preventDefault() since most browsers won't scroll anyways until after the 10px threshold is hit. The user won't see any difference between us checking or not checking ... but I'm not sure if the calculations in the "if" expression are what you are referring to as the thing killing your performance?

toddparker commented 12 years ago

Worth trying to remove that @jblas...did you give it a try to see if you saw a difference? If it's functionally the same, might as well remove it.

adriancd commented 12 years ago

I remove the block you highlighted and performance is greatly improved and havent seen any issues

jblas commented 12 years ago

@adriancd

Hmmm, ok ... that block, specifically event.preventDefault() is supposed to STOP scrolling from happening.

adriancd commented 12 years ago

yes, that was very annoying and much better with it removed!

jblas commented 12 years ago

@toddparker

It just occurred to me when I was thinking about this the other day ... that on some platforms, if you allow the browser to scroll the viewport, they stop dispatching touch events, including the touchend ... this would be bad because it wouldn't remove the touchmove or touchend listeners it added during touchstart off the element.

toddparker commented 12 years ago

So there might be something to this? I disabled this line and didn't see a difference on iOS 5, but it might be worth testing a bit more if you have time.

@adriancd - can you point us to a specific test page and platform/version where the difference is most clear with this removed?

jblas commented 12 years ago

@toddparker

What I was trying to say was that removing that line completely, will allow the viewport to scroll, which may have bad consequences on the platforms that stop dispatching touchmove/touchend if the viewport scrolls.

adriancd commented 12 years ago

In Phonegap, removing this line makes a big difference on ios and android. I'm moving to web app so have been testing in iPhone safari where i don't think it makes much difference. In Android browser it does help to remove.

toddparker commented 12 years ago

We're going to re-visit all the event in 1.1 so hopefully, we can figure out what's going on here at the same time.

francescortiz commented 12 years ago

Hi,

I am using 1.1.0 rc1.

I went crazy until I found that block of code jblas pasted in the source code. I didn't erase it, though. What I did was:

$.event.special.swipe.scrollSupressionThreshold = -1;

It doesn't seem to make sense to me, because that is enforcing the event.preventDefault() call, but it makes my scrolling work.

What happened to me is that vmousemove events only got triggered after starting a swipe, so I decided to make the system think that I am always doing a swipe.

jblas commented 12 years ago

@toddparker

So I'm a bit confused here. As I mentioned above, we can remove the checks around the preventDefault() and always execute it, but it sounded as if @adriancd wanted it removed completely to make performance better? Also @francescortiz comment is a bit confusing because calling preventDefault() should stop scrolling ... unless he is testing on Android, where calling preventDefault does nothing, you actually have to call preventDefault on the touchstart.

What is the desired behavior here? To not scroll when swiping? Or to allow scrolling and still generate a swipe?

francescortiz commented 12 years ago

i usar an androod device

dcarrith commented 11 years ago

@jblas @toddparker - I was doing some testing last night on my shiny new Nexus 7 and noticed that listview scrolling performance was about as bad as I've ever experienced in the project I've been working on for the past 8 months or so. I've read all the posts that suggest to disable drop shadows and other css to make the listview style as simple as possible. I've also read all the posts about how bad scrolling performance is when the listview items are links (<li><a>whatever</a></li>). So, I removed the <a> tags and made my listviews just listviews that don't link to anything. Scrolling performance was still the same...horrible. But, then I noticed that if I scrolled on the left or right side of the listview very carefully without touching the listview items, then scrolling was vastly improved (close to native). But, I just didn't understand why scrolling seemed to stop anytime my scroll ventured just a wee bit over to where it hovered over any listview item while scrolling. Here's the answer I came up with (and it seems so did @adriancd a year ago :\ )

Line 125 of js/events/touch.js

Or, line 1898 of the latest release jquery.mobile-1.1.1.js

scrollSupressionThreshold: 10, // More than this horizontal displacement, and we will suppress scrolling.

So, it wasn't the fact that my finger was venturing over a listview item as I scrolled with a slight arc at the finish of my scroll (or during the scroll) ... it was that the "horizontal displacement" of my scroll was at some point exceeding the threshold of 10px!!? and suppressing the scroll! As a test, I changed it to 100 and it drastically improved the scrolling performance (because it was no longer suppressing it). I put the <a> tags back in just to make sure everything was still breezing along and sure enough...listview scrolling was still vastly improved.

I haven't taken the time to investigate the side-effects of setting it to 100, but I can't imagine any side effect would be worse than suppressing my scrolling.

I was going to create a new issue to report this, but first I searched through the issues and came across this one. So, I'm bumping it up.

dcarrith commented 11 years ago

By the way...I don't think this is only an issue with Phonegap / Cordova. I just tested in Chrome on my Nexus 7 and scrolling performance is vastly improved as much as it is in my Cordova app.

toddparker commented 11 years ago

Thanks @dcarrith - So we just upgraded our Nexus 7 to Jellybean this morning so I can't test it with ICS. I tested the list perf page running 4.1 and didn't see much of a difference. Same goes for a Galaxy Nexus S running 4.0.4 with the default browser and Chrome.

Is there a test page and steps to reproduce for me to reproduce this?

scottjehl commented 11 years ago

Hmm. Good find.

So this is just changing the default vertical movement tolerance in the swipe event, huh? 10px seems a bit low, I agree. However, 100px seems too high to make sense. What about something closer to 35px or so?

dcarrith commented 11 years ago

My Nexus 7 is at 4.1.1. I also confirmed the list scrolling perf improvements on GNex with ICS 4.0.4.

dcarrith commented 11 years ago

@scottjehl - unless I'm misunderstanding something, I think the 10px "horizontal displacement" threshold is left/right movement threshold. So, I think 100px still makes sense. A swipe left or right would be more like 200-400 right?

dcarrith commented 11 years ago

A natural scroll with my thumb is more like a letter r with a hook right at the end (if right-handed). That 10px threshold was triggering and it was suppressing my scroll. However, at 100px, the right hook of the top part of the r is not triggering the scroll suppression.

scottjehl commented 11 years ago

I don't have any hard science on it, but I think a native swipe is triggered on iOS at around 30px, depending on the speed.

dcarrith commented 11 years ago

ah, ic what you're saying. I'll see if I still see the improvements when the scrollSuppression is set to 30 then. Or, maybe 50 and then horizontalDistanceThreshold could be increased from 30 to 50. ??

toddparker commented 11 years ago

@dcarrith - we're open to changing this default if it improves things. I'm still struggling to see a difference in scrolling between this being set to 2, 10, 100 or even 1,000 on my nexus 7. Does this have to be on a very long listview to see the problem? I've been testing this page locally and tinkering with numbers. http://jquerymobile.com/test/docs/lists/lists-performance.html

I've been scrolling with my thumb while holding the device to swipe in an arc too.

dcarrith commented 11 years ago

The page I'm testing on has about 20 list items and lazyloads up to 187. To make sure it's not my lazyloader that's causing the issues, I made sure to let them all load before really testing to see if that scroll suppression setting made a difference. I'll try the lists-performance page too. It's strange you're not able to see a difference unless you are a perfect straight up and down scroller. ;)

dcarrith commented 11 years ago

@scottjehl - 30px seems to be a much better default. I'll still probably override it and make it 50 or so because my thumb swipes veer pretty far left and right. I'll have to do a lot of testing to make sure it doesn't interfere with my swipe functionality (swipe to change tracks).

toddparker commented 11 years ago

Oh, I'm not for sure. I have the dev tools on to tell me the offsets when I scroll and it can easily be 100px. The issue is that screen is 800px wide but pretty small so moving just a quarter of the screen width is 200px and it reports it as such. On iOS, they report things differently. Using my thumb, it's easy to hit 40, 70 and even over 100 with a swipe on the Nexus 7.

dcarrith commented 11 years ago

It's worth noting that I'm clearing my cache (in browser and cordova app) each time I make a change to the scrollSupressionThreshold. I just wanted to confirm that you guys are doing the same to make sure the new value is taking effect? I'm seeing a very drastic improvement.

dcarrith commented 11 years ago

@toddparker - good point. Nexus 7 does have a big beautiful screen doesn't it. I'll also turn on the dev tools so it shows me the offsets when I scroll.

dcarrith commented 11 years ago

well...both the lists-performance.html pages

here: http://jquerymobile.com/test/docs/lists/lists-performance.html

and

here: http://jquerymobile.com/demos/1.1.1/docs/lists/lists-performance.html

seem equally snappy.

What data attributes are on those lists?

Sorry, an important piece of info I didn't mention. My listview has search filters. Hmm...wait...so do the lists-performance.html pages. I have counts which is an extra <span> but I don't know why that would matter as far as this scroll suppression variable is concerned. It's just a slightly more complex document.

Here's the markup for my <ul> with one <li> as an example:

<ul id="artistsList" data-role="listview" data-filter="true" data-inset="true" data-theme="a" data-divider-theme="a"> <li data-role="list-divider">Artists</li> <li> <a href='http://www.musotic.com/artist/Gentleman/albums' data-transition='slide'>Gentleman <span class='ui-li-count ui-btn-up-a ui-btn-corner-all'>12</span> </a> </li> <li data-role="list-divider"></li> </ul>

toddparker commented 11 years ago

Yeah, seems like those pages are very similar to yours then. We've had a few people say this is troublesome, I'm just struggling to find a good test page to help us dial this in. Could be it's only a problem in a Cordova app?

dcarrith commented 11 years ago

Nah...I see the same differences in Chrome. I can test other browsers too.

dcarrith commented 11 years ago

Well, stock browser on ICS doesn't seem to have any scrolling issues no matter what that setting is set to.

jblas commented 11 years ago

@dcarrith What is listening for swipe left/right on your page? We don't have many pages in the tests that make use of those events so the code in action may notbeinplay, hence the difference.

dcarrith commented 11 years ago

@jblas - Good f'n question! I didn't think to comment out the swipeleft and swiperight event handlers to see if that scrollSupressionThreshold would still be used for differentiating between a scroll with a swipe. After commenting them out, scrolling is very much improved since that moveHandler code isn't trying to grab stop and start position and then compare all the thresholds, which most of the time seems to evaluate to true because of some wacky values ... thereby calling preventDefault.

So, the next test I did was to comment out all the code inside of the swipeleft and swiperight which was actually an old hack that dates back to when double and triple events would be fired for swipeleft and swiperight. So, now I just have two empty swipeleft and swiperight handlers:

$('body').on('swipeleft', function(evt, ui) { //empty });

$('body').on('swiperight', function(evt, ui) { // empty });

I added in an alert right above the event.preventDefault just to see what it's comparing to scrollSupressionThreshold each time and am seeing some strange values (On Galaxy Nexus ICS 4.0.4 in a Cordova App as well as Chrome and on a Nexus 7 in a cordova app and in Chrome). I'm not really sure what to make of it. But, in the simplest test, if you scroll on one side of the listview and then scroll on the other side of the listview, the if statement evaluates to true and event.preventDefault is called. Even with the smallest scroll, it triggers and the alerts I have in place are all over the map. It's really hard to describe. But, if you have a chance, throw this alert in there and take a look.

// prevent scrolling if ( Math.abs( start.coords[ 0 ] - stop.coords[ 0 ] ) > $.event.special.swipe.scrollSupressionThreshold ) { alert("Math.abs("+start.coords[0]+" - "+stop.coords[0]+") > "+$.event.special.swipe.scrollSupressionThreshold+"\n so, would normally have called event.preventDefault to prevent scrolling"); event.preventDefault(); }