522504 - Panning speed is sometimes erratic

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Benjamin Stover (:stechz)]

I think it is caused by how we determine our kinetic speed:

Fast mouse movements have a way of being drowned out by later mousemoves, even if they all occurred in quick succession.  If we get less resolution in our mousemove events, these big values aren't dampened out and speed is much faster.  Instead of basing final speed on position in queue, we should weight them by how close they occurred to mouseup for more predictable panning.

Comment 1

[Benjamin Stover (:stechz)]

Attachment: Patch

Although it isn't quite perfect, I don't have near as many crazy pans as I used to.  Please try on the N900 and see what you think.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Not for nothing, this might be the sixth "adjust" to kinetic scrolling. I think everyone on the team has taken a crack at it. Since Doug was the last person to have a turn, I think he should take a look too.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

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

* Is event.timeStamp sane on WinMo?

* Comment for doDragStop mentions a secret third parameter, which is now the fourth parameter.

Comment 4

[Ben Combee (:bcombee)]

So far, code looks fine, but I can't R+ until I finish my build work to move things to /opt and can put something on my N900 again to test it out.

Comment 5

[Doug Turner (:dougt)]

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

please make prefs for the new tweakables that you moved to the top of the file.

with that and mfinkle's nits, r=me.

Comment 6

[Ben Combee (:bcombee)]

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

Sidebars aren't snapping correctly with this applied on desktop... at end of drag, they were staying partially on screen.

Comment 7

[Ben Combee (:bcombee)]

Problem on Win32 desktop is that multiple events have same timestamp, so we get a timeDiff value of zero which means divide by zero in figuring out initial speed.

Comment 8

[Ben Combee (:bcombee)]

Problem is that code in addData that eliminates events with same timestamp wasn't preventing the push at end due to no early return.  Adding an early return helps.

Comment 9

[Benjamin Stover (:stechz)]

Attachment: Code review changes

Addresses preferences and bcombee's bug.

Comment 10

[Ben Combee (:bcombee)]

Comment on attachment 407098 [details] [diff] [review]
Code review changes

Looks good on desktop browser, my issue was addressed.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
https://hg.mozilla.org/mobile-browser/rev/6c3a8e6aeda4

Comment 12

[Aakash Desai [:aakashd]]

I think there's still some kinks with the zoom that I'd like to bring up in separate bugs, but this definitely helps with jitter on trying to click a link, having multiple pan events in quick successive actions among other panning actions in build:

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091020
Fennec/1.0b5pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20091020
Fennec/1.0b5pre

and 

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2b1pre) Gecko/20091020 Fennec/1.0a4pre