User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0 Build ID: 20151012030612 Steps to reproduce: Using Firefox nightly with GTK3 on Linux (Ubuntu 15.10), I notice that mousewheel acceleration no longer works. (mousewheel.acceleration.*) I'm guessing this is because of the GTK3 switch.

Probably related to https://bugzilla.mozilla.org/show_bug.cgi?id=1207700

> I doubt it. Bug 1143856 seems more likely. > If toggling layers.async-pan-zoom.enabled resolves the issue, then file a new bug blocking Bug 1143856 > rather than commenting in that bug. That was indeed correct, toggling layers.async-pan-zoom.enabled did resolve it.

Indeed, it looks like the mousewheel.acceleration.* prefs are checked and followed by the main-thread scrolling code [1], but not by the APZ code. [1] https://dxr.mozilla.org/mozilla-central/rev/ccf288f658211b6cfab33c458aaf033baed2375b/dom/events/WheelHandlingHelper.cpp#315

What about other ways to modify the scroll speed, like mousewheel.default.delta_multiplier_x and mousewheel.withnokey.numlines ?

There's a whole page dedicated to this on the wiki: https://wiki.mozilla.org/Gecko:Mouse_Wheel_Scrolling

Modified title to expand scope as per comment 4.

This patch implements mousewheel.acceleration pref support in APZ. The "sScrollSeriesCounter" in WheelHandlingHelper.cpp has been replicated in WheelBlockState, and similarly starts at 0 and resets if 80ms elapse in between two events in the transaction. This counter gets affixed to ScrollWheelInput events, so APZC::GetScrollWheelDelta() can do the acceleration.

This might be all we need. The other multiplier prefs are blacklisted for APZ, so changing them will go through main-thread scrolling, and they should function fine there. kats, do we want to keep doing that, or should we take this bug to add full support for those as well?

Comment on attachment 8691065 [details] [diff] [review] part 1, mousewheel.acceleration support Review of attachment 8691065 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed ::: gfx/layers/apz/src/InputBlockState.cpp @@ +10,5 @@ > #include "mozilla/MouseEvents.h" > #include "mozilla/SizePrintfMacros.h" // for PRIuSIZE > #include "mozilla/layers/APZCTreeManager.h" // for AllowedTouchBehavior > #include "OverscrollHandoffState.h" > +#include "AsyncScrollBase.h" // for kScrollSeriesTimeoutMs Move this up to alphabetical order ::: gfx/layers/apz/src/InputBlockState.h @@ +248,5 @@ > + > + /** > + * Adjust a scroll delta for custom acceleration preferences. > + */ > + double AccelerateScrollAmount(double delta) const; This function signature seems left over, it doesn't have an implementation and isn't called. ::: gfx/thebes/gfxPrefs.h @@ +384,5 @@ > // This and code dependent on it should be removed once containerless scrolling looks stable. > DECL_GFX_PREF(Once, "layout.scroll.root-frame-containers", LayoutUseContainersForRootFrames, bool, true); > > // This affects whether events will be routed through APZ or not. > + DECL_GFX_PREF(Live, "mousewheel.acceleration.factor", MouseWheelAccelerationFactor, int32_t, -1); The comment above this line only belongs on the mousewheel.system_scroll_override_on_root_content.enabled pref. Move the two new prefs up into a separate block separated by new lines ::: layout/generic/AsyncScrollBase.h @@ +90,5 @@ > +// Helper for accelerated wheel deltas. > +static inline double > +ComputeAcceleratedWheelDelta(double aDelta, int32_t aCounter, int32_t aFactor) > +{ > + if (!aDelta) { Add a comment here that says "this may be called from threads other than the main thread" just in case somebody decides to change it in the future to rely on main thread objects

(In reply to David Anderson [:dvander] from comment #8) > This might be all we need. The other multiplier prefs are blacklisted for > APZ, so changing them will go through main-thread scrolling, and they should > function fine there. > > kats, do we want to keep doing that, or should we take this bug to add full > support for those as well? Depends on how complicated the implementation would be, I guess. For now we need to leave the main-thread implementation in place so if it would add a lot of complexity to reimplement it in APZ then I say let's wait. If/when it gets to the point that we can remove the main-thread implementation and move the complexity into APZ we can reconsider it.

Okay. It seems pretty obscure so I'll defer it for now, and add some telemetry to see how prevalent it is.

https://hg.mozilla.org/integration/mozilla-inbound/rev/772ebbbb852446775bb64fda8c5542daa381633a Backed out changeset ebb6fb453cca (bug 1214170) for build bustage on a CLOSED TREE

Comment on attachment 8691807 [details] [diff] [review] telemetry for mousewheel prefs Review of attachment 8691807 [details] [diff] [review]: ----------------------------------------------------------------- This will collect data from all user populations, do we really need data from release users here? Does it need to be in the environment data or could it be submitted in a histogram instead? What questions are you trying to answer with this data & do we really need all the prefs values to answer them? Who will monitor this data and how? The changes look simple enough, but this needs review from a data peer, ideally with info for the above question provided: https://wiki.mozilla.org/Firefox/Data_Collection

Comment on attachment 8691807 [details] [diff] [review] telemetry for mousewheel prefs I just want to know how often these prefs are changed by the user. It doesn't need to ride the train very far, I'd be happy to get data in aurora and back it out after a few weeks.

(In reply to David Anderson [:dvander] from comment #16) > Comment on attachment 8691807 [details] [diff] [review] > telemetry for mousewheel prefs > > I just want to know how often these prefs are changed by the user. It > doesn't need to ride the train very far, I'd be happy to get data in aurora > and back it out after a few weeks. That sounds like it doesn't need to be in the environment - could you just add a single flag histogram instead that tells you whether any of those prefs were changed/non-default by the user? https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe That way you will also get it displayed automatically on the telemetry.mozilla.org dashboards, although it will give you changes "per session", not per individual user.

Comment on attachment 8691807 [details] [diff] [review] telemetry for mousewheel prefs Review of attachment 8691807 [details] [diff] [review]: ----------------------------------------------------------------- The environment block is supposed to consist of configurations & characterstics that significantly affect performance and other behavior. When Telemetry detects that one of these prefs has changed during a session, it immedially starts a new Telemetry subsession. In general, we try keep the list of prefs to a minimum. I think histograms might indeed be better for these measurements. They'll be easier to visualize via dashes too.

Actually I think this is super easy, so we might as well just support it. We just need to communicate the new multiplier to APZ, and we can just compute it before handing the event off on the main thread. This patch also fixes an existing micro-bug where APZ applied the system delta override to pixel delta events (it should only apply to line events).

Don't need this anymore, there are no more prefs that disable APZ.

Comment on attachment 8693413 [details] [diff] [review] part 2, delta multiplier support Review of attachment 8693413 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, did you check if this passes all tests? ::: widget/InputData.h @@ +570,5 @@ > Modifiers aModifiers, > ScrollMode aScrollMode, > ScrollDeltaType aDeltaType, > const ScreenPoint& aOrigin, > + double aDeltaX, double aDeltaY) Undo this change, leave the params as one-per-line

I didn't see any new failures on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8eac8bc7bf2b so attempting to re-land this today.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fffdbaaac8a1 https://hg.mozilla.org/integration/mozilla-inbound/rev/81783ea82024 https://hg.mozilla.org/integration/mozilla-inbound/rev/608d89fa125b

https://hg.mozilla.org/mozilla-central/rev/fffdbaaac8a1 https://hg.mozilla.org/mozilla-central/rev/81783ea82024 https://hg.mozilla.org/mozilla-central/rev/608d89fa125b