GNOME Bugzilla – Bug 756729
No kinetic scrolling on Wayland
Last modified: 2016-03-14 12:07:15 UTC
On X11 if you scroll with 2 fingers, there is some kind of inertia algorithm that keeps the scrolling going even though you've stopped scrolling. This doesn't work on Wayland (either for GTK+ apps directly on the wayland backend or X11 apps like chromium).
The "kinetic scrolling" is done by the Synaptics driver under X11; it's generally considered a mistake, because the driver has no idea as to whether modifiers are in place, or if the interpolated events are crossing window boundaries and ought to be stopped. Under libinput/Wayland, it's up to the toolkits to implement it, using the information on the last scrolling event from libinput. AFAIR, GTK already implements this.
Weird, it doesn't seem to work in evolution nor rhythmbox.
On X11, we do get what results in a GdkEventScroll with dx/dy=0/0 on the devices/scrolling modes that are able to report the end of scrolling. On wayland, the intention is to be quite more detailed here: http://lists.freedesktop.org/archives/wayland-devel/2015-October/024917.html https://git.gnome.org/browse/mutter/log/?h=wip/garnacho/wayland-extra-scroll-events Mutter currently turns these 0/0 events into no-op, we could resort to sending these as well in the mean time, so the same code paths in GTK+ are triggered there, I hope for these protocol additions to land and mutter/gtk+ patches to be ready soon enough though.
Created attachment 313786 [details] [review] 0001-wayland-add-support-for-wl_pointer-axis_frame-discre.patch Implement the new wl_pointer.axis_* events. This is a draft, note that the upstream protocol is not yet stable.
Created attachment 313787 [details] [review] 0002-wayland-add-gdk_event_is_scroll_stop_event.patch Add a new gdk_event_is_scroll_stop_event() to check if a scroll event should be used for kinetic scrolling. This is a draft, the upstream protocol isn't declared stable yet. Note that I'm still ignoring the axis source here - we could make this depending on the 'finger' source, but the 'continuous' source too may trigger kinetic scrolling. An example for continuous scrolling is middle-button scrolling - the scroll stop is triggered when the scroll button is released.
Review of attachment 313786 [details] [review]: Looks quite alright to me, setting to "reviewed" mostly because this is on hold till we get protocol bits merged. Only very minor formatting niggles (which I know the wayland backend code is plagued of, but...). ::: gdk/wayland/gdkdevice-wayland.c @@ +1080,2 @@ static void +flush_scroll_data (GdkWaylandDeviceData *device, GdkWaylandScrollData *scroll) One argument per line, please :). @@ +1173,3 @@ + +static void +pointer_handle_axis_source(void *data, Missing space before '(', variable names should also be aligned to the longest arg.
Review of attachment 313787 [details] [review]: This one looks great to me too. I guess we would need to set the is_scroll_event flag in gdk/x11/gdkdevicemanager-xi2.c as a temporary measure?
FWIW, I filed bug #757026 to Clutter adding similar info/API. The mutter changes still reside in a branch (https://git.gnome.org/browse/mutter/log/?h=wip/garnacho/wayland-extra-scroll-events), and have been updated make use of these Clutter changes.
Created attachment 319038 [details] [review] wayland: add support for wl_pointer frame/axis_source/axis_discrete/axis_stop This adds support for the new wl_pointer events available in v5. The wl_pointer.axis_source events can be ignored for the purposes here, the main reason they exist is so that the combination of axis_source=finger and axis_stop triggers kinetic scrolling. We don't need to care about the source, axis_stop is enough for us to tell us when we're scrolling. The wl_pointer.frame events group events together and is intended as a mechanism to coalesce events together. This for example allows us to now send a single GTK scroll event for a diagonal scroll. Previously, the two wl_pointer.axis events had to be handled separately. The wl_pointer.axis_discrete event sends mouse wheel clicks where appropriate, and is translated into up/down/left/right scroll events.
Created attachment 319039 [details] [review] wayland: add gdk_event_is_scroll_stop_event() And use it to handle kinetic scrolling in the GtkScrolledWindow. However, dropping the delta check causes the X11-based kinetic scroll to break since we don't have the stop event here. Correct handling of xf86-input-libinput-based scroll events is still being discussed.
FWIW, filed bug #760637 with the mutter patch.
Review of attachment 319038 [details] [review]: looks good to me
Review of attachment 319039 [details] [review]: Does this need an X11 implementation ? I mean, shouldn't the x11 backend set is_stop based on dx == dy == 0 (which is what we check in gtkscrolledwindow now ?
Created attachment 319045 [details] [review] x11: Set event->scroll.is_stop We still figure this out from 0/0 scroll events. This method is not intended to last forever, but it's something we can cling to so far.
Review of attachment 319045 [details] [review]: sure
Review of attachment 319039 [details] [review]: .
Attachment 319038 [details] pushed as 3fca361 - wayland: add support for wl_pointer frame/axis_source/axis_discrete/axis_stop Attachment 319039 [details] pushed as 48aa1bb - wayland: add gdk_event_is_scroll_stop_event() Attachment 319045 [details] pushed as f8b8e4e - x11: Set event->scroll.is_stop
+ guint is_stop : 1; Wouldn't it be better to put this bit into the 3-byte hole after .send_event, instead of growing the struct?