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 765939 - [Wayland] very slow scrolling in GtkMenu using the touchpad
[Wayland] very slow scrolling in GtkMenu using the touchpad
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkMenu
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-05-03 12:25 UTC by Olivier Fourdan
Modified: 2016-05-10 14:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Simple reproducer for testing purpose. (1.49 KB, text/plain)
2016-05-03 12:25 UTC, Olivier Fourdan
  Details
[PATCH 1/2] gdkevent: make _gdk_event_get_pointer_emulated() private (1.45 KB, patch)
2016-05-03 14:49 UTC, Olivier Fourdan
committed Details | Review
[PATCH 2/2] gtkmenu: ignore emulated scroll events (1.98 KB, patch)
2016-05-03 14:49 UTC, Olivier Fourdan
none Details | Review
[PATCH 2/2] gtkmenu: ignore emulated scroll events (1.45 KB, patch)
2016-05-03 14:51 UTC, Olivier Fourdan
none Details | Review
[PATCH 2/2] gtkmenu: ignore emulated scroll events (1.98 KB, patch)
2016-05-03 14:52 UTC, Olivier Fourdan
none Details | Review
[PATCH 2/2] gtkmenu: ignore emulated scroll events (2.03 KB, patch)
2016-05-03 14:55 UTC, Olivier Fourdan
none Details | Review
[PATCH 2/3] gtkmenu: ignore emulated scroll events (1.63 KB, patch)
2016-05-09 07:19 UTC, Olivier Fourdan
committed Details | Review
[PATCH 3/3] gtkmenu: ignore left/right scroll events (956 bytes, patch)
2016-05-09 07:22 UTC, Olivier Fourdan
none Details | Review
[PATCH 3/3] gtkmenu: ignore left/right scroll events (1.43 KB, patch)
2016-05-09 08:02 UTC, Olivier Fourdan
committed Details | Review

Description Olivier Fourdan 2016-05-03 12:25:57 UTC
Created attachment 327221 [details]
Simple reproducer for testing purpose.

+++ This bug was initially created as a clone of Bug #765907 +++

This is a follow up on bug 765907 which was a follow up on bug 765907 as well

Using the touchpad to scroll through the list of entries is very slow under Wayland. 

No such issue under X11.

Attaching a simple reproducer.

Steps to reproduce:

1. Save and build the source

   $ gcc menuitems.c -o menuitems -Wall `pkg-config gtk+-3.0 --cflags --libs`

2. Run under Wayland
3. Open the menu and try to scroll through the list of entries using the scrolling on the touchpad.

Expected results:

Using the touchpad, the entries scroll fast enough.

Actual results:

Scrolling with the touchpad is very slow on Wayland.

Additional data:

GtkMenu uses both SCROLL_UP/DOWN events and SCROLL_SMOOTH events, but gets (and uses) only smooth events on Wayland, so it's much slower on Wayland than on X11.

The solution would be to use only non-emulated events smooth events and adjust the scroll velocity in GtkMenu.
Comment 1 Olivier Fourdan 2016-05-03 14:49:03 UTC
Created attachment 327231 [details] [review]
[PATCH 1/2] gdkevent: make _gdk_event_get_pointer_emulated() private

And not just internal to gdk, so we can use it in gtk as well, to
differentiate emulated scroll events from others.
Comment 2 Olivier Fourdan 2016-05-03 14:49:49 UTC
Created attachment 327232 [details] [review]
[PATCH 2/2] gtkmenu: ignore emulated scroll events

On X11, we get both smooth and emulated scroll events, whereas other
backends such as wayland will give smooth events only with touchpad
scrolling.

Discard emulated scroll events so that we get consistent behaviours
between backends.

Allow for both horizontal and vertical smooth events for scrolling so
that horizontal scrolling still works without emulated scroll events as
well, again for consistency between gdk backends.
Comment 3 Olivier Fourdan 2016-05-03 14:51:51 UTC
Created attachment 327233 [details] [review]
[PATCH 2/2] gtkmenu: ignore emulated scroll events

On X11, we get both smooth and emulated scroll events, whereas other
backends such as wayland will give smooth events only with touchpad
scrolling.

Discard emulated scroll events so that we get consistent behaviours
between backends.

Allow for both horizontal and vertical smooth events for scrolling so
that horizontal scrolling still works without emulated scroll events as
well, again for consistency between gdk backends.

--
v2: fix the usual space before parenthesis mistake
Comment 4 Olivier Fourdan 2016-05-03 14:52:49 UTC
Created attachment 327234 [details] [review]
[PATCH 2/2] gtkmenu: ignore emulated scroll events

On X11, we get both smooth and emulated scroll events, whereas other
backends such as wayland will give smooth events only with touchpad
scrolling.

Discard emulated scroll events so that we get consistent behaviours
between backends.

Allow for both horizontal and vertical smooth events for scrolling so
that horizontal scrolling still works without emulated scroll events as
well, again for consistency between gdk backends.
Comment 5 Olivier Fourdan 2016-05-03 14:55:48 UTC
Created attachment 327235 [details] [review]
[PATCH 2/2] gtkmenu: ignore emulated scroll events

Sorry for the noise, seems I can't get something as simple as attaching a patch right today...
Comment 6 Carlos Garnacho 2016-05-05 12:33:38 UTC
Comment on attachment 327231 [details] [review]
[PATCH 1/2] gdkevent: make _gdk_event_get_pointer_emulated() private

LGTM
Comment 7 Carlos Garnacho 2016-05-05 12:50:04 UTC
Comment on attachment 327235 [details] [review]
[PATCH 2/2] gtkmenu: ignore emulated scroll events

I think you could do with the if (!_gdk_event_get_pointer_emulated(...)) check on the top of the function, we also want to avoid the case of x11 drivers sending both 4-7 button and smooth scroll events with mouse wheels (in that case the latter has the "emulated" flag)

Multiplying the smooth scroll value by MENU_SCROLL_STEP2 sounds alright, I'm not sure I like much obeying delta_x though :). IMO it's better to obey only the axis that matches visually when we know the device can do both, otherwise scrolling along the right diagonal would/could make scrolling jump between upwards and downwards.
Comment 8 Olivier Fourdan 2016-05-05 13:17:18 UTC
(In reply to Carlos Garnacho from comment #7)
> I'm not sure I like much obeying delta_x though :). IMO it's better to obey only
> the axis that matches visually when we know the device can do both,
> otherwise scrolling along the right diagonal would/could make scrolling jump
> between upwards and downwards.

I would tend to agree, but the problem is that we do check for GDK_SCROLL_LEFT/RIGHT so if we filter out the emulated events, we'd lose that ability using only one axis with smooth scrolls.
Comment 9 Carlos Garnacho 2016-05-05 22:09:17 UTC
(In reply to Olivier Fourdan from comment #8)
> I would tend to agree, but the problem is that we do check for
> GDK_SCROLL_LEFT/RIGHT so if we filter out the emulated events, we'd lose
> that ability using only one axis with smooth scrolls.

I fear I don't completely follow :).

We have 3 cases to support (provided we only listen to non-emulated events):

- Mice capable of discrete vertical scrolling, the typical scroll wheel
- Mice capable of discrete v/h scrolling, usually scroll wheels that can "click" towards the sides, or just additional buttons.
- Devices capable of continuous v/h scrolling, this gathers touchpads, trackballs, trackpads and mice, which all have at least one way available to smooth scroll (because the device itself allows it, or because there's an assigned scroll button)

Out of those cases, only in the first one there's an axis we can't scroll across, so it used to make sense to allow scrolling horizontally with vertical events if the widget needed this so. But IMO that's about the only situation where it makes sense to mix scrolling axes (and I'd wish we could discern devices from the first and second case so we don't needlessly do this), for the other cases we have real means to scroll in that direction.

So, I'd even argue to remove GDK_SCROLL_LEFT/RIGHT management from GtkMenu :).
Comment 10 Olivier Fourdan 2016-05-09 06:56:25 UTC
(In reply to Carlos Garnacho from comment #9)
> [...]
> 
> We have 3 cases to support (provided we only listen to non-emulated events):
> 
> - Mice capable of discrete vertical scrolling, the typical scroll wheel
> - Mice capable of discrete v/h scrolling, usually scroll wheels that can
> "click" towards the sides, or just additional buttons.
> - Devices capable of continuous v/h scrolling, this gathers touchpads,
> trackballs, trackpads and mice, which all have at least one way available to
> smooth scroll (because the device itself allows it, or because there's an
> assigned scroll button)
> 
> Out of those cases, only in the first one there's an axis we can't scroll
> across, so it used to make sense to allow scrolling horizontally with
> vertical events if the widget needed this so. But IMO that's about the only
> situation where it makes sense to mix scrolling axes (and I'd wish we could
> discern devices from the first and second case so we don't needlessly do
> this), for the other cases we have real means to scroll in that direction.

Yes, but we had support for left/right so I thought we would want to retain that functionality.
 
> So, I'd even argue to remove GDK_SCROLL_LEFT/RIGHT management from GtkMenu
> :).

What about making this a separate patch, so that if someone complain, we can always revert that one alone?
Comment 11 Olivier Fourdan 2016-05-09 07:19:59 UTC
Created attachment 327490 [details] [review]
[PATCH 2/3] gtkmenu: ignore emulated scroll events

Update patch
Comment 12 Olivier Fourdan 2016-05-09 07:22:03 UTC
Created attachment 327491 [details] [review]
[PATCH 3/3] gtkmenu: ignore left/right scroll events

As per comment 9:

Menus are placed vertically by definition, it does not make much sense
to support horizontal axis for scrolling.
Comment 13 Carlos Garnacho 2016-05-09 07:38:57 UTC
Comment on attachment 327490 [details] [review]
[PATCH 2/3] gtkmenu: ignore emulated scroll events

Looks good. And sorry for the ramblings :)
Comment 14 Carlos Garnacho 2016-05-09 07:45:16 UTC
Comment on attachment 327491 [details] [review]
[PATCH 3/3] gtkmenu: ignore left/right scroll events

You've removed left+up :). *hands a coffee, sips own*

Looks fine to push with the obvious fix anyway, I guess I'd also add a "default:" branch for those so the handler returns GDK_EVENT_PROPAGATE.
Comment 15 Olivier Fourdan 2016-05-09 08:02:22 UTC
Created attachment 327492 [details] [review]
[PATCH 3/3] gtkmenu: ignore left/right scroll events

(In reply to Carlos Garnacho from comment #14)
> Comment on attachment 327491 [details] [review] [review]
> [PATCH 3/3] gtkmenu: ignore left/right scroll events
> 
> You've removed left+up :). *hands a coffee, sips own*

lol, <facepalm> - And obviously I didn't notice as I test in Wayland...

> Looks fine to push with the obvious fix anyway, I guess I'd also add a
> "default:" branch for those so the handler returns GDK_EVENT_PROPAGATE.

Good idea, and also replace TRUE/FALSE with GDK_EVENT_STOP/GDK_EVENT_PROPAGATE.
Comment 16 Olivier Fourdan 2016-05-10 14:12:26 UTC
attachment 327231 [details] [review] pushed as commit e405c27 - gdkevent: make _gdk_event_get_pointer_emulated() private
attachment 327490 [details] [review] pushed as commit c134d52 - gtkmenu: ignore emulated scroll events
attachment 327492 [details] [review] pushed as commit 126156e - gtkmenu: ignore left/right scroll events