815862 - Increase max allowed acceleration for high-res devices (e.g. Nexus 10)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Johnny Stenback (:jst)]

When viewing a longish page (i.e. bug 588909) and panning (scrolling) down the page the panning speed tracks my finger well but once I let go it seems it slows down. This makes it take really really long to scroll to the bottom of a long page. This seems like it's not a problem on a phone (tested on Galaxy Nexus), but this seems pretty apparent on the Nexus 10. This might be related to the high resolution of the screen on the Nexus 10, or it could of course be something else.

Comment 1

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

I don't have a Nexus 10 so I can't reproduce this, but could you try bumping up the ui.scrolling.max_event_acceleration to see if it helps? The pref will likely be set to -1 which means a default of 12 gets used; try setting to something like 50 and see if that helps. It should allow for larger changes in velocity.

You can also try increasing ui.scrolling.friction_fast and ui.scrolling.friction_slow to 990 (they should be in the range 0-1000, and currently default to 970 and 850 respectively). Preferably try these one at a time so we know which one makes the difference; I would expect ui.scrolling.friction_fast to help but not ui.scrolling.friction_slow.

Unfortunately you'll need to restart the browser after each pref change.

Comment 2

[Johnny Stenback (:jst)]

Thanks for the suggestions! I set ui.scrolling.max_event_acceleration to 50 and restarted, and that made things much better! Then I set it to 200, and I think that's even better yet...

Comment 3

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

Thanks for the info! We'll probably need to do some more tuning here to figure out how to automatically adjust that value for different screen resolutions.

Comment 4

[enrico]

Change ui.scrolling.max_event_acceleration to 50 or 200 doesn't help.. still lags!

Comment 5

[enrico]

 i tried to change ui.scrolling.friction_fast and no results.. firefox on nexus 10 is unusable :/

Comment 6

[Johnny Stenback (:jst)]

enrico, did you make sure that you really did restart Firefox? I.e. you removed it from the task switcher etc? If in doubt, shut down the tablet completely and power it back on and wait for it to boot again, then test again. I use Firefox on a nexus 10 all the time, and it works very well after I made the tweaks mentioned here.

Comment 7

[enrico]

yes i restarted.. i tryed again and no results.. do you have android 4.2.1 or 4.2.2? the problems started with 4.2.2.

Comment 8

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

Attachment: Patch

This will need testing on the Flyer since that's the device that made us put this code in in the first place.

Comment 9

[Chris Lord [:cwiiis]]

Comment on attachment 723878 [details] [diff] [review]
Patch

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

I think the HTC fixed the touch events on the Flyer in Honeycomb anyway, so I'd say this is just good to go. Would be nice to have this value be a function of the DPI, but let's deal with that if it comes to it.

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/2decb8b91c3b

Comment 11

[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 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Just to note, The HTC Flyer device that I am using has been updated to Honeycomb.  Will be waiting to test once I get a build that has a patch for this.

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Just to note as well, testing on the nightly with the bug that was mentioned : bug  588909 seems to happen more so on the flyer when in landscape.

Please need info on me when there is a build available with a patch.

Comment 15

[Kevin Brosnan [Ex-Mozilla]]

Setting to 20 based on duped bug. It would be good to get a fix asap for this.

Comment 16

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

The patch that was landed is a "correct" fix, I think. I just need to modify the test to deal with the new code so that it doesn't fail. New try push in progress with a possible test fix + some logging at https://tbpl.mozilla.org/?tree=Try&rev=ca4aaa32e6f9

Comment 17

[enrico]

the patch is in nightly?

Comment 18

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

No, not yet. It did land but then was backed out for test failures.

Comment 19

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

Attachment: Patch (v2)

Kind of an ugly fix for the test, but it does the job and I can't really think of anything better. It's kind of simulating what a user would do to pan and ensure no fling happened.

Comment 20

[Chris Lord [:cwiiis]]

Comment on attachment 725073 [details] [diff] [review]
Patch (v2)

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

r+ but with the proviso that the comment is addressed somehow, or another bug is filed to make sure our fling calculations take into account event time.

::: mobile/android/base/tests/MotionEventHelper.java.in
@@ +71,5 @@
>                  }
> +                // sleep a bit before sending the last move so that the calculated
> +                // fling velocity is low and we don't end up doing a fling afterwards.
> +                try {
> +                    Thread.sleep(1000L);

Shouldn't this sleep go after the last move() and before the up()? If that doesn't cause this to be a drag instead of a fling, I say we have a more serious problem in our code :/

Comment 21

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

(In reply to Chris Lord [:cwiiis] from comment #20)
> Shouldn't this sleep go after the last move() and before the up()? If that
> doesn't cause this to be a drag instead of a fling, I say we have a more
> serious problem in our code :/

Putting the delay between the last move and the up won't have the desired effect, because that won't get factored into the velocity calculations. This is intentional because most devices will send the last move and the up event with the same coordinates, so if we calculated a velocity based on those two events then it would always be zero and we would never fling (except because of the recently-added averaging code).

I'll land this now as-is but if you think there's something we need to change let me know and I can do that in a separate bug.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3bcfa552c4c

Comment 22

[Scott]

Hi, thanks for your efforts in resolving this issue.


Apologies but I'm trying to work out when this would hit the nightly. I notice you say above that your going to land it, which I assume means that its going to hit nightly soon, but I can't find a nightly change log or an indication as to when this will hit nightly.

Comment 23

[Chris Lord [:cwiiis]]

(In reply to Scott from comment #22)
> Hi, thanks for your efforts in resolving this issue.
> 
> 
> Apologies but I'm trying to work out when this would hit the nightly. I
> notice you say above that your going to land it, which I assume means that
> its going to hit nightly soon, but I can't find a nightly change log or an
> indication as to when this will hit nightly.

Hi Scott - inbound is merged to central (from which we build Nightly) every 24 hours (possibly more frequently, I'm not sure), but sometimes there are issues that prevent this. It looks like we're having major infrastructure problems at the moment, so all the trees are closed. You can see the status of each tree here: https://tbpl.mozilla.org/

Under normal circumstances, you can expect that when a change hits inbound, it'll be merged to central within 24 hours and appear in Nightly within another 24 hours. If you have questions, or want a less vague answer (sorry :)), you can ask people in #developers or #ateam on irc.mozilla.org, or check out the wiki at https://wiki.mozilla.org/

Hope that helps!

Comment 24

[Phil Ringnalda (:philor)]

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

Comment 25

[Scott]

Hi, using the latest nightly 22.01a from last night (16th) this is still an issue although I can't tell if the fix has made it to the nighty yet other than it being married as fixed above. What's worse is that you also seem to be no longer able to use the tab selector on the left hand side to change between open windows.

I'm not sure if this is related or coincidence.

Comment 26

[Scoobidiver (away)]

(In reply to Scott from comment #25)
> Hi, using the latest nightly 22.01a from last night (16th)
It's fixed in Nightly from March 17th not yet released.

Comment 27

[Scott]

(In reply to Scoobidiver from comment #26)
> (In reply to Scott from comment #25)
> > Hi, using the latest nightly 22.01a from last night (16th)
> It's fixed in Nightly from March 17th not yet released.


Thank you for the feedback. Are you aware of the issue with the tab selector I mention above in the latest nightly though?

Comment 28

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

(In reply to Scott from comment #27)
> Thank you for the feedback. Are you aware of the issue with the tab selector
> I mention above in the latest nightly though?

I'm not aware of this; please file a separate bug for that issue.

Comment 29

[enrico]

the nightly of 17th doesn't correct the lagging performance on Nexus 10. maybe it's not a general problem but it's related only to this tablet? who closed the "Bug 845944 - Large 'janky' slowdown in panning on the Samsung Nexus 10" tryed this patch before? like i said changing the values of friction and max accelaration don't help on this device. i can't understand why are (plural) you putting so fast the "Resolved fixed" status and mark as duplicate the other bug. i hope it's been fixed (maybe the last nightly doesn't include the patch?) but seems it's not fixed at all :( if you don't trust me i can make a video.

Comment 30

[Scott]

(In reply to enrico from comment #29)
> the nightly of 17th doesn't correct the lagging performance on Nexus 10.
> maybe it's not a general problem but it's related only to this tablet? who
> closed the "Bug 845944 - Large 'janky' slowdown in panning on the Samsung
> Nexus 10" tryed this patch before? like i said changing the values of
> friction and max accelaration don't help on this device. i can't understand
> why are (plural) you putting so fast the "Resolved fixed" status and mark as
> duplicate the other bug. i hope it's been fixed (maybe the last nightly
> doesn't include the patch?) but seems it's not fixed at all :( if you don't
> trust me i can make a video.


Hi Enrico, it was mentioned above that the fix does not hit the nightly build until tonight's build. This is the 17th, but the build for the 17th has not been built yet.

So wait for the next build.

Also, would be interested to know if you can switch between tabs on the build released today (16th).

Comment 31

[Scoobidiver (away)]

(In reply to Scott from comment #30)
> This is the 17th, but the build for the 17th has not been built yet.
That's not true: ftp://ftp.mozilla.org/pub/mobile/nightly/2013-03-17-03-09-23-mozilla-central-android/

Comment 32

[Scott]

(In reply to Scoobidiver from comment #31)
> (In reply to Scott from comment #30)
> > This is the 17th, but the build for the 17th has not been built yet.
> That's not true:
> ftp://ftp.mozilla.org/pub/mobile/nightly/2013-03-17-03-09-23-mozilla-central-
> android/

Sorry,
You are correct, not 5 minutes after posting did I get the update notification.

Unfortunately as has been reported above this is still an issue, and scrolling is far from smooth and very jittery.

Comment 33

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

Ok, I will reopen bug 845944 to track the jittery scrolling problems. I'll leave this one as fixed since the acceleration bump did fix the original issue reported in comment #0 (at least based on comment #2).

Comment 34

[enrico]

Good decision, thanks :)

(Scott - same problem, i can't switch between tabs on nightly)

Comment 35

[Aaron Train [:aaronmt]]

Channel uplift?

Comment 36

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

Can you confirm that the benefit from this patch on a Nexus 10 is worth uplifting? I've tried it on a nexus 4 but I didn't feel like the difference there was really enough to warrant uplifting.