Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

First move event can be dropped, leading to perception of poor performance on Android devices. #170

Merged
merged 2 commits into from

2 participants

Jon Klein Matteo Spinelli
Jon Klein

Tested with an HTC desire and Samsung Galaxy II, iscroll performance seems jerky and some quick swipes appear to stutter or get ignored. However, I don't think it's a real performance problem, per se, but rather that the first move event can end up getting dropped on the floor without performing any scrolling.

Watching the sequence of events for a scroll, it looks like a touchstart/touchend with only a single touchmove in between will not cause any scrolling, and contribute to a poor perceived performance on Android:

    if (that.absDistX < 6 && that.absDistY < 6) {
        that.distX += deltaX;
        that.distY += deltaY;
        that.absDistX = m.abs(that.distX);
        that.absDistY = m.abs(that.distY);

        return; // returning here means we miss the first move event
    }

Getting rid of the return statement here (line 436) and allowing the first event to initiate a scroll leads to a much improved user experience. Incidentally, when I say "quick" swipes can provoke this problem, it's not unrealistically fast interaction, it's a fast but natural motion.

As for why this affects Android and not iOS or other devices, I would speculate that the resolution of touch events is simply higher on iOS such that the described scenario does not occur frequently.

Matteo Spinelli
Owner

unfortunately if you remove that threshold becomes very difficult to distinguish unintentional taps

Jon Klein

I suppose there's a subjective question of what "feels" right here, but I did not experience any "unintentional tap" behavior in my testing.

Would you consider incorporating this change if it was enabled with an option?

Matteo Spinelli
Owner

Without the threshold the moved variable is set to true too easily (or: the user wanted to tap and instead scrolling occurred). Also, the threshold is needed by the the lockDirection feature. I understand that responsiveness is top priority over other features, but removing that "return" has a domino effect. If you just need to scroll (no tap) and don't care about 2D scroll, removing the threshold has no impact at all.

Jon Klein

I experienced no issues with this change and catching taps/clicks.

Could the threshold be configurable perhaps? I'm just looking for some way to invoke this behavior if the user desires it. On Android, there are very clear cases of "intentional swipes" that do not initiate scrolling because only a single move event occurs. Being able to enable this behavior with some configuration greatly improves the user experience there.

Matteo Spinelli
Owner

you may try to reduce the threshold from 6 to 3. What device and OS version are you testing on? On mines the tap issue is pretty clear. It doesn't come up on every tap, but sometimes you try to tap and a small (1/2 pixels) scroll occurs instead. It may depends on the touchscreen sensitivity (different for each device).

Jon Klein

Regardless of the actual value of the threshold, when that code path is taken, the first event is unable to initiate a scroll. Again, the scenario I'm seeing is a touchstart/touchend with a single touchmove in between. Shouldn't the return also be conditional on the absolute distance being over the threshold? In other words, if the first move is large enough to meet the criteria of a move, that it get used immediately instead of waiting for the next event.

I'm seeing the actual issue on an HTC Desire with Android 2.2, and also using an iPhone 4S with iOS 5.0 to test my changes.

Matteo Spinelli
Owner

that is a good idea, to make the return conditional just place:

that.distX += deltaX;
that.distY += deltaY;
that.absDistX = m.abs(that.distX);
that.absDistY = m.abs(that.distY);

before the IF.

This way the distance is calculated on first touchmove.

Jon Klein

Okay -- let me test with that setup and see if addresses the original issue I was seeing. If it does (and I expect it will), I'll update the pull request with those changes.

Thanks!

Jon Klein

The previous commit adds the logic we discussed and it works well -- could you have a look at pulling this in?

Matteo Spinelli cubiq merged commit f676436 into from
Matteo Spinelli
Owner

sorry, didn't notice you updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 11, 2012
  1. Jon Klein

    Don't return from first _move event without initiating move, improves…

    jonklein authored
    … poor perceived performance on Android devices
Commits on Jan 12, 2012
  1. Jon Klein

    modify Android optimization fix to use first event, but still respect…

    jonklein authored
    … absolute distance threshold
This page is out of date. Refresh to see the latest.
Showing with 10 additions and 10 deletions.
  1. +5 −5 src/iscroll-lite.js
  2. +5 −5 src/iscroll.js
10 src/iscroll-lite.js
View
@@ -235,12 +235,12 @@ iScroll.prototype = {
newY = that.options.bounce ? that.y + (deltaY / 2) : newY >= 0 || that.maxScrollY >= 0 ? 0 : that.maxScrollY;
}
- if (that.absDistX < 6 && that.absDistY < 6) {
- that.distX += deltaX;
- that.distY += deltaY;
- that.absDistX = m.abs(that.distX);
- that.absDistY = m.abs(that.distY);
+ that.distX += deltaX;
+ that.distY += deltaY;
+ that.absDistX = m.abs(that.distX);
+ that.absDistY = m.abs(that.distY);
+ if (that.absDistX < 6 && that.absDistY < 6) {
return;
}
10 src/iscroll.js
View
@@ -425,12 +425,12 @@ iScroll.prototype = {
newY = that.options.bounce ? that.y + (deltaY / 2) : newY >= that.minScrollY || that.maxScrollY >= 0 ? that.minScrollY : that.maxScrollY;
}
- if (that.absDistX < 6 && that.absDistY < 6) {
- that.distX += deltaX;
- that.distY += deltaY;
- that.absDistX = m.abs(that.distX);
- that.absDistY = m.abs(that.distY);
+ that.distX += deltaX;
+ that.distY += deltaY;
+ that.absDistX = m.abs(that.distX);
+ that.absDistY = m.abs(that.distY);
+ if (that.absDistX < 6 && that.absDistY < 6) {
return;
}
Something went wrong with that request. Please try again.