After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756729 - No kinetic scrolling on Wayland
No kinetic scrolling on Wayland
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.18.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: WaylandRelated
 
 
Reported: 2015-10-17 01:29 UTC by Lionel Landwerlin
Modified: 2016-03-14 12:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-wayland-add-support-for-wl_pointer-axis_frame-discre.patch (10.05 KB, patch)
2015-10-21 02:38 UTC, Peter Hutterer
reviewed Details | Review
0002-wayland-add-gdk_event_is_scroll_stop_event.patch (5.71 KB, patch)
2015-10-21 02:40 UTC, Peter Hutterer
reviewed Details | Review
wayland: add support for wl_pointer frame/axis_source/axis_discrete/axis_stop (16.30 KB, patch)
2016-01-14 18:33 UTC, Carlos Garnacho
committed Details | Review
wayland: add gdk_event_is_scroll_stop_event() (6.62 KB, patch)
2016-01-14 18:33 UTC, Carlos Garnacho
committed Details | Review
x11: Set event->scroll.is_stop (1.10 KB, patch)
2016-01-14 19:28 UTC, Carlos Garnacho
committed Details | Review

Description Lionel Landwerlin 2015-10-17 01:29:04 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).
Comment 1 Emmanuele Bassi (:ebassi) 2015-10-17 10:39:49 UTC
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.
Comment 2 Lionel Landwerlin 2015-10-17 13:25:42 UTC
Weird, it doesn't seem to work in evolution nor rhythmbox.
Comment 3 Carlos Garnacho 2015-10-17 14:13:48 UTC
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.
Comment 4 Peter Hutterer 2015-10-21 02:38:05 UTC
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.
Comment 5 Peter Hutterer 2015-10-21 02:40:26 UTC
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.
Comment 6 Carlos Garnacho 2015-10-23 16:07:30 UTC
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.
Comment 7 Carlos Garnacho 2015-10-23 16:09:12 UTC
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?
Comment 8 Carlos Garnacho 2015-10-23 16:13:30 UTC
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.
Comment 9 Carlos Garnacho 2016-01-14 18:33:43 UTC
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.
Comment 10 Carlos Garnacho 2016-01-14 18:33:49 UTC
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.
Comment 11 Carlos Garnacho 2016-01-14 18:34:27 UTC
FWIW, filed bug #760637 with the mutter patch.
Comment 12 Matthias Clasen 2016-01-14 18:56:22 UTC
Review of attachment 319038 [details] [review]:

looks good to me
Comment 13 Matthias Clasen 2016-01-14 18:59:54 UTC
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 ?
Comment 14 Carlos Garnacho 2016-01-14 19:28:37 UTC
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.
Comment 15 Matthias Clasen 2016-01-16 00:23:03 UTC
Review of attachment 319045 [details] [review]:

sure
Comment 16 Matthias Clasen 2016-01-16 00:23:12 UTC
Review of attachment 319045 [details] [review]:

sure
Comment 17 Matthias Clasen 2016-01-16 00:23:29 UTC
Review of attachment 319039 [details] [review]:

.
Comment 18 Carlos Garnacho 2016-01-18 20:40:12 UTC
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
Comment 19 Christian Persch 2016-01-19 19:25:04 UTC
+  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?