Closed Bug 1091049 Opened 5 years ago Closed 5 years ago

Implement fling curving in the APZ code

Categories

(Core :: Panning and Zooming, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(3 files, 2 obsolete files)

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.
See Also: → 1042017
Attached patch Part 2 - Implement fling curving (obsolete) — Splinter Review
Compiles but completely untested. Will try it out after lunch.
Attached image Fling curving graph
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.
Attachment #8513618 - Attachment is obsolete: true
Attachment #8513737 - Flags: review?(botond)
Rebased on top of bug 1091128 and adjusted a pref to make it a little more subtle than the last version.
Attachment #8513619 - Attachment is obsolete: true
Attachment #8513744 - Flags: review?(botond)
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.
Attachment #8513736 - Flags: feedback?(gbrander)
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.
Attachment #8513737 - Flags: review?(botond) → review+
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+
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 [1]. 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

[1] http://cubic-bezier.com/#0,0,.58,1
Blocks: 1348322
You need to log in before you can comment on or make changes to this bug.