805479 - Fling in one direction will fling backwards

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Benoit Girard (:BenWa)]

I've been noticing this recently that doing a fling in one direction will pan in the opposite direction.

STR:
1) Do several flings in one direction
2) fling in the opposite direction
3) Sometimes it will take you in the same direction as (1)

Comment 1

[Chris Lord [:cwiiis]]

Sometimes after step 1, it just goes in the opposite direction, without having changes direction (I think). I don't have solid STR.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I've only ever seen this behaviour when flinging like a madman trying to test checkerboarding. I suspect that the time between flings is so short that the hardware or firmware decides your finger never lifted, and so instead assumes it moved the other way (i.e. the vector that connects the end of one actual fling and the start of the next). That's just a hypothesis though. Have you guys ever seen it happen on an individual fling, or when flinging slowly?

Comment 3

[Chris Lord [:cwiiis]]

(In reply to Kartikaya Gupta (:kats) from comment #2)
> I've only ever seen this behaviour when flinging like a madman trying to
> test checkerboarding. I suspect that the time between flings is so short
> that the hardware or firmware decides your finger never lifted, and so
> instead assumes it moved the other way (i.e. the vector that connects the
> end of one actual fling and the start of the next). That's just a hypothesis
> though. Have you guys ever seen it happen on an individual fling, or when
> flinging slowly?

I've not seen this when flinging slowly, but I've seen this when flinging slowly enough that I wouldn't expect this to happen. I've not observed this in other apps, but like you say, I'm often scrolling like a mad man to test things, so I can't say that it doesn't happen :)

I wonder if our gesture detection code is faulty somehow, or just plain not as good as that built into Android...

Comment 4

[Andreea Pod]

If by fling you mean scroll, I tried this on planet.mozilla.org and I wasn't able to reproduce it. On what version have you seen it and does it happen on some special page or any page? Can you please test again and see if it is still reproducible?

Build: Firefox 19.0a1 (2012-10-29)
Device: Samsung Galaxy Nexus
OS: Android 4.1.2

Comment 5

[Benoit Girard (:BenWa)]

I mean fling i.e. drag in one direction to generate momentum and let go. This is different then scrolling where you can scroll without releasing your finger.

I see this problem when doing aggressive flings on taskjs to test improvements to progressive draw.

Comment 6

[Andreea Pod]

(In reply to Benoit Girard (:BenWa) from comment #5)
> I mean fling i.e. drag in one direction to generate momentum and let go.
> This is different then scrolling where you can scroll without releasing your
> finger.
> 
> I see this problem when doing aggressive flings on taskjs to test
> improvements to progressive draw.

Ok, after fling on taskjs.org in one direction then chance the direction all works as expected for me. Can anyone else reproduce it?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

I have seen it occasionally before but I just spent two minutes trying to reproduce it and couldn't. I tried panning slowly/quickly, with short/long pauses between flings, using my index finger/thumb, and all combinations of the above.

I'm like 99% convinced that this issue, when it happens, is not actually a bug in the code - the hardware is just sending us events that the user does not expect, either because it picks up a really light touch as a real touch, or what I said in comment 2.

Comment 8

[Benoit Girard (:BenWa)]

(In reply to Kartikaya Gupta (:kats) from comment #7)
> I'm like 99% convinced that this issue, when it happens, is not actually a
> bug in the code - the hardware is just sending us events that the user does
> not expect, either because it picks up a really light touch as a real touch,
> or what I said in comment 2.

That's how I feel sometimes but then I think about how I do the same type of aggressive testing in other applications to test their panning performance and I have never seen it in the gmail app or the stock browser.

Comment 9

[Aaron Train [:aaronmt]]

I saw this today testing trunk on the URL http://m.rockstargames.com/newswire

Comment 10

[Cristian Nicolae (:xti)]

I cannot reproduce this issue on the latest Nightly build on taskjs.org, planet.mozilla.org, m.rockstargames.com/newswire.

Aaron, can you still reproduce this issue?

--
Firefox 19.0a1 (2012-11-19)
Device: Galaxy S2
OS: Android 4.0.3

Comment 12

[donrhummy]

This just occured for me in Firefox Nightly on a Nexus 4. I was on the html5test.com page and flicking down when it occurred.

Comment 13

[donrhummy]

It seems to happen pretty frequently on http://html5test.com so you might want to try there

Comment 14

[donrhummy]

this page causes it a lot in beta: http://news.consumerreports.org/cars/2013/01/our-own-tesla-model-s-finally-arrives.html

Comment 15

[Aaron Train [:aaronmt]]

I seem to be hitting this on my Nexus 4 by simply doing quick flings on http://neowin.net with Nightly (02/07)

Comment 16

[donrhummy]

Good video showing the issues with scrolling: https://www.youtube.com/watch?feature=player_detailpage&v=PS4v_BUAq2A#t=166s

Comment 17

[Aaron Train [:aaronmt]]

Yeah Nexus 4 in the video, hitting this more often on my device too. Re-requesting tracking; all channels affected.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

Please grab a log using the try build I at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-894d2db329e6/try-android/ if you can reproduce.

See also https://bugzilla.mozilla.org/show_bug.cgi?id=840880#c2.

Comment 19

[Aaron Train [:aaronmt]]

Attachment: Mobile Wired.com logging (raw log)

Comment 20

[Aaron Train [:aaronmt]]

Attachment: Mobile neowin.net logging (raw log)

Comment 21

[Kartikaya Gupta (email:[e-mail])]

(In reply to Aaron Train [:aaronmt] from comment #19)
> Created attachment 713514 [details]
> Mobile Wired.com logging (raw log)

In this one I see (log cleaned up for clarity):

D/GeckoLayerView(13335): Received event: MotionEvent { action=ACTION_MOVE, id[0]=0, x[0]=535.8153, y[0]=468.5835 }
D/GeckoPanZoomController(13335): y-axis velocity is 18.163681
D/GeckoPanZoomController(13335): y-axis velocity is 19.68943
D/GeckoLayerView(13335): Received event: MotionEvent { action=ACTION_MOVE, id[0]=0, x[0]=559.1388, y[0]=305.89764 }
D/GeckoPanZoomController(13335): y-axis velocity is 20.634523
D/GeckoPanZoomController(13335): y-axis velocity is 23.110666
D/GeckoPanZoomController(13335): y-axis velocity is 23.94265
D/GeckoLayerView(13335): Received event: MotionEvent { action=ACTION_MOVE, id[0]=0, x[0]=555.0, y[0]=332.0 }
D/GeckoPanZoomController(13335): y-axis velocity is -62.148464
D/GeckoLayerView(13335): Received event: MotionEvent { action=ACTION_UP, id[0]=0, x[0]=555.0, y[0]=332.0 }
D/GeckoPanZoomController(13335): During fling, y-axis velocity is -60.28401
D/GeckoPanZoomController(13335): During fling, y-axis velocity is -58.475494
D/GeckoPanZoomController(13335): During fling, y-axis velocity is -56.72123

So here we have a series of decreasing y-coordinates (it decreases down from 803 to the 305 in the snippet above) and then it jumps up to 332 just before the touch-up. So our fling code actually calculates the right fling but for some reason the hardware is sending us a move event to a y-coordinate in the opposite direction. The same thing happens for the third fling in the log file, the y-coordinate decreases from 816 down to 38, and then there's a move event that puts it back at 76 just before the touch-up, so we do a fling in the opposite direction.

Maybe we need to do some more aggressive filtering of these bad events? We do some in Axis.updateWithTouchAt, but we allow direction changes through untouched and maybe we shouldn't.

Comment 22

[Kartikaya Gupta (email:[e-mail])]

(In reply to Aaron Train [:aaronmt] from comment #20)
> Created attachment 713516 [details]
> Mobile neowin.net logging (raw log)

Similar things in this log. I see one fling with y-coordinates going up from 446.5 to 946.279 and then it drops to 926.5 just before the touch-up, sending us in a downward fling instead of an upward one.

Comment 24

[Kartikaya Gupta (email:[e-mail])]

There are more logs/analysis in bug 840880. Has anybody seen this on a device running pre-JellyBean Android? I'm wondering if the weird input events are caused by the "touch prediction" feature supposedly added in Jelly Bean. I don't know exactly how they implemented that but I assume any sort of predictive touch events could backfire in exactly this way.

Comment 25

[James Willcox (:snorp) ([e-mail]) (he/him)]

You can try disabling 'touch resampling' (prediction) by setting the 'debug.inputconsumer.resample' system property to 0.

Comment 26

[Aaron Train [:aaronmt]]

Toss me a build ^ (I can repro on n4)

Comment 27

[Kartikaya Gupta (email:[e-mail])]

I don't think you need a special build, you just need to run "adb shell setprop debug.inputconsumer.resample 0". You'll probably need to restart FF after that.

Comment 28

[Kevin Brosnan [Ex-Mozilla]]

This has greatly improved scrolling on my Nexus 4. No more bouncing so far.

Comment 29

[Aaron Train [:aaronmt]]

Confirming too, looks like that did it. Good job snorp.

Comment 30

[stefan.mueller.83]

nice to hear may it solves [url=https://bugzilla.mozilla.org/show_bug.cgi?id=840880]my bug[/url] as well.
But can you explain how to do that:
"adb shell setprop debug.inputconsumer.resample 0"

thx

stefan

Comment 31

[Kartikaya Gupta (email:[e-mail])]

Well we can't expect end users to be doing that, and it doesn't look like we can do it from within the app and have it take effect, so we'll have to handle the bad input event regardless. I can file an Android bug for it as well so that hopefully whatever hack we put in to work around it can be removed in some future Android version.

Comment 32

[Aaron Train [:aaronmt]]

CC'ing Tyler; re: comment #27, comment #31 - we should get a support article up there as all versions are affected

Comment 33

[Tyler Downer [He/Him]]

Added roland, he'll write any articles we put up on this. Does this affect only Jelly bean?

Comment 34

[Chris Lord [:cwiiis]]

As more phones get ICS and above, is this a good time to reconsider using the built-in gesture detector rather than rolling our own? I'm not convinced the possible responsiveness benefits are worth the unreliability of using raw events.

Comment 35

[Mike Hommey [:glandium]]

Isn't it possible to use the built-in gesture detector when supported and fallback on our own when not?

Comment 36

[Benoit Girard (:BenWa)]

(In reply to Aaron Train [:aaronmt] from comment #32)
> CC'ing Tyler; re: comment #27, comment #31 - we should get a support article
> up there as all versions are affected

I'm not convinced we should ask users to change properties on their phone. I imagine this property can have impact on their other application.

Comment 37

[Kartikaya Gupta (email:[e-mail])]

Yes, we can use the built-in gesture detector. However that is a larger change that I would like to defer and do as part of switching over to using the AsyncPanZoomController (bug 776030 + dependency tree) and ditching the java code entirely. The APZC supports either using their own gesture detector or providing pre-detected gesture events, and we use the android gesture detector to feed gesture events to the APZC.

In the short term though I would like to put in some sort of workaround as a lot of people are hitting this, and possibly uplift that workaround to aurora and beta as well.

Comment 38

[donrhummy]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #31)
> Well we can't expect end users to be doing that, and it doesn't look like we
> can do it from within the app and have it take effect, so we'll have to
> handle the bad input event regardless. I can file an Android bug for it as
> well so that hopefully whatever hack we put in to work around it can be
> removed in some future Android version.

Please link to the bug you file for the issue. 

One caveat: why is firefox the only browser I have this occur for?

Comment 39

[Kartikaya Gupta (email:[e-mail])]

(In reply to donrhummy from comment #38)
> Please link to the bug you file for the issue. 

The link is in the "See also" field of this bug.

> One caveat: why is firefox the only browser I have this occur for?

Presumably because the other browsers use the Android gesture detector code, and we do not.

Comment 40

[Aaron Train [:aaronmt]]

This is tracking 20+, is there anything that can be done on our side? Anything that would make provide a fix for something in the Fx20 time-frame?

Comment 41

[Kevin Brosnan [Ex-Mozilla]]

Suggest relnote.  This affects Jelly Bean (4.2.2) users. Work around in comment 27 though it requires adb and will reset upon restarting the phone.

Comment 42

[Kartikaya Gupta (email:[e-mail])]

I plan on working on this bug this week now that I have a nexus 4 that can reproduce it.

Comment 43

[Aaron Train [:aaronmt]]

(In reply to Kevin Brosnan [:kbrosnan] from comment #41)
> Suggest relnote.  This affects Jelly Bean (4.2.2) users. Work around in
> comment 27 though it requires adb and will reset upon restarting the phone.

Predates 4.2

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #42)
> I plan on working on this bug this week now that I have a nexus 4 that can
> reproduce it.

Sounds good, thanks.

Comment 44

[Kartikaya Gupta (email:[e-mail])]

Attachment: Average last few velocities when calculating fling velocity

This fixes the problem on the N4 and should also improve behaviour on other devices. From logging I noticed that when flinging the last few velocities usually go up and then back down, (e.g. 10, 20, 30, 40, 30, 20, 10) and averaging these provides a better estimate of how hard the user is flinging compared to just taking the last velocity.

Comment 45

[Chris Lord [:cwiiis]]

Comment on attachment 723876 [details] [diff] [review]
Average last few velocities when calculating fling velocity

Review of attachment 723876 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. The only thing that worries me is that time doesn't come into it, so if the system was under pressure or the touch-screen was particularly crap, this could result in slightly odd behaviour... That said, in those situations, behaviour is likely already odd.

Comment 46

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/35bcf4b4af78

Comment 47

[Kartikaya Gupta (email:[e-mail])]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/1deb63f2d936 for possibly making testAxisLocking go perma-orange.

Comment 48

[Kartikaya Gupta (email:[e-mail])]

This patch seems seems to be clean: https://tbpl.mozilla.org/?tree=Try&rev=ace82686d961

Relanded as https://hg.mozilla.org/integration/mozilla-inbound/rev/dd945da3cc5d

Comment 49

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/dd945da3cc5d

Comment 50

[Aaron Train [:aaronmt]]

Is this going to ride?

Comment 51

[Aaron Train [:aaronmt]]

Played around with this this on m-c builds this afternoon and this fix looks fine to me. Verified fixed on the Nexus 4 (Android 4.2.2); also used the Nexus 7 (Android 4.2.2). I have not tried any older Android platform with this fix yet.

Comment 52

[Frederik Niedernolte]

Isn't it possible to release it already with Firefox 21?

Comment 53

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 723876 [details] [diff] [review]
Average last few velocities when calculating fling velocity

[Approval Request Comment]
Bug caused by (feature/regressing bug #): jelly bean
User impact if declined: sometimes flinging on nexus 4 (and possibly other devices) will end up flinging backwards
Testing completed (on m-c, etc.): on m-c, locally
Risk to taking this patch (and alternatives if risky): fairly low risk, i would say. main thing that could go wrong is that if you fling after not moving your finger in a long time the fling velocity might not be quite what you expect. but i think overall this is a huge improvement.
String or UUID changes made by this patch: none

Comment 54

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b37f5b5a7589
https://hg.mozilla.org/releases/mozilla-beta/rev/0ff625326942

Comment 56

[Lukas Blakk [:lsblakk] use ?needinfo]

Since this got fixed in 20 I don't see that we need to relnote.

Comment 57

[Paul Feher]

Verified on Firefox for Android 21.0b6 using: Nexus 4 (4.2.2)