503362 - kinetic scrolling needs help

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Doug Turner (:dougt)]

our kinetic scrolling algorithm basically does an average over the entire set of mouse movements resulting in an unfair bias of old movements.  For example, 2 or 3 very fast movements followed by a series of very slow movements will result in a kinetic scroll that is not representative of the final movements.

instead of a running average, we should consider using an exponential moving average with a high alpha value (around .8).  We should also make the alpha value, update interval, and the decelerataion rate preference based so that we can configure these on difference devices.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1 (no preferences for alpha, updateInterval, or decel)

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 387710 [details] [diff] [review]
patch v.1 (no preferences for alpha, updateInterval, or decel)

one can ignore the changes to the mobile.js file.

Comment 3

[Doug Turner (:dougt)]

also, lowering the update interval does seem to make a big difference.  it is as if we can't keep up with our painting resulting in a gray screen.

Comment 4

[Stuart Parmenter]

Comment on attachment 387710 [details] [diff] [review]
patch v.1 (no preferences for alpha, updateInterval, or decel)

please don't use tabs:
+	let panned = self._owner._dragBy(dx, dy);
+	if (!panned) {

you don't seem to be using the prefs you're setting..

Comment 5

[Doug Turner (:dougt)]

Comment on attachment 387710 [details] [diff] [review]
patch v.1 (no preferences for alpha, updateInterval, or decel)

stuart, i do not think I added any tabs (what was there was there).  I can untabify this file before I check in.

Comment 6

[Doug Turner (:dougt)]

Attachment: patch v.2

Comment 7

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

Comment on attachment 387923 [details] [diff] [review]
patch v.2

>+    this._updateInterval = gPrefService.getIntPref("browser.ui.kinetic.updateInterval");
>+    // In preferences this value is an int.  We divide so that it can be between 0 and 1;
>+    this._emaAlpha = gPrefService.getIntPref("browser.ui.kinetic.ema.alphaValue") / 10;
>+    // In preferences this value is an int.  We device so that it can be a percent.

"We divide ..."

>       if ((mbLast.sx == sx && mbLast.sy == sy) || mbLast.t == now) {
>-	return;
>+        return;
>       }

nit: Remove braces

Comment 8

[Doug Turner (:dougt)]

3a0ee33e76e7

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

verified with beta3