Closed Bug 1091049 Opened 5 years ago Closed 5 years ago
Implement fling curving in the APZ code
We discussed fine-tuning the scrolling behavior for 2.2 with Gordon (UX) today. One of the outcomes was that we want to implement some "fling curving" thing, where when the velocity exceeds a base threshold, it gets amplified further. This is intended to reflect the user's intent where hitting this high velocity indicates they want to go fast and don't care so much about precision and the content staying under their finger. The intent is that this will co-exist with flywheel scrolling, and will apply to any pan/fling gesture where the physical finger velocity goes above the base threshold.
Compiles but completely untested. Will try it out after lunch.
Depends on: 1091128
This is basically what I did for the fling curving: for each physical velocity we compute based on input events, I map it to a visual velocity using this graph (there are prefs to control the shape of the graph), and then it goes through our usual processing code.
Rebased on top of bug 1091128 and adjusted a pref to make it a little more subtle than the last version.
Comment on attachment 8513736 [details] Fling curving graph Also f? to gordon to make sure this graph looks like what we discussed and all the right knobs are exposed.
So if I understand correctly, this only applies when the user's finger goes faster than some threshold, then it curves the acceleration of the fling, not necessarily the slowdown?
Yes, this is not directly related to bug 1042017.
Comment on attachment 8513744 [details] [diff] [review] Part 2 - Implement fling curving Review of attachment 8513744 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/apz/src/AsyncPanZoomController.cpp @@ +896,5 @@ > + gVelocityCurveFunction->Init( > + nsTimingFunction(gfxPrefs::APZCurveFunctionX1(), > + gfxPrefs::APZCurveFunctionX2(), > + gfxPrefs::APZCurveFunctionY1(), > + gfxPrefs::APZCurveFunctionY2())); I screwed up the order of arguments here; X2 <-> Y1. Fixed locally.
Comment on attachment 8513744 [details] [diff] [review] Part 2 - Implement fling curving Review of attachment 8513744 [details] [diff] [review]: ----------------------------------------------------------------- Looks reasonable to me. I'm curious to see how this ends up feeling once we tune the prefs. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp @@ +187,5 @@ > + * "apz.curve_function_x1" > + * "apz.curve_function_y1" > + * "apz.curve_function_x2" > + * "apz.curve_function_y2" > + * "apz.curve_threshold_inches_per_ms" I think we can make these pref names slightly more descriptive by s/curve/velocity_curve or s/curve/fling_curve. @@ +205,5 @@ > + * bound of the velocity curve. > + * The points (x1, y1) and (x2, y2) used as the two intermediate control points > + * in the cubic bezier curve; the first and last points are (0,0) and (1,1). > + * Some example values for these prefs can be found at > + * http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.cpp?rev=21282be9ad95#2462 So that's where the 0.58 comes from :) ::: gfx/layers/apz/src/Axis.cpp @@ +74,5 @@ > + float funcInput = (newVelocity - curveThreshold) / scale; > + float funcOutput = gVelocityCurveFunction->GetValue(funcInput); > + AXIS_LOG("Curving up velocity from %f to %f\n", newVelocity, > + (funcOutput * scale) + curveThreshold); > + newVelocity = (funcOutput * scale) + curveThreshold; Let's not repeat "(funcOutput * scale) + curveThreshold"; it makes it easy for the code to get out of sync with the logging.
Attachment #8513744 - Flags: review?(botond) → review+
Made the requested changes and landed: remote: https://hg.mozilla.org/integration/b2g-inbound/rev/ea9a3cbcdd0b remote: https://hg.mozilla.org/integration/b2g-inbound/rev/13c0917e919b
https://hg.mozilla.org/mozilla-central/rev/ea9a3cbcdd0b https://hg.mozilla.org/mozilla-central/rev/13c0917e919b https://hg.mozilla.org/mozilla-central/rev/30917fc0c535
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
FYI Gordon, the relevant prefs for adjusting the fling curving behaviour are: apz.fling_curve_threshold_inches_per_ms - This controls the threshold velocity at which fling curving starts taking effect (the left edge of the box in the graph). It's a float value (so you need to put it in quotes in the prefs.js file), and the default on B2G is "0.03" apz.max_velocity_inches_per_ms - This controls the upper bound of the velocity (the right edge of the box in the graph). It's also a float value, current default is "0.07" apz.fling_curve_function_x1 apz.fling_curve_function_y1 apz.fling_curve_function_x2 apz.fling_curve_function_y2 These are the parameters for the cubic-bezier curve. The current defaults are 0,0,0.58,1 which gives the CSS ease-out graph that looks like . You can fiddle with the graph's look on cubic-bezier.com and then just copy the four numbers to the prefs. These are also all floats and need to be entered as strings in prefs.js  http://cubic-bezier.com/#0,0,.58,1
Depends on: 1095727
See Also: → 1100315
You need to log in before you can comment on or make changes to this bug.