[tabbox] Intercept QWheelEvents on QQuickWindow for scrolling
ClosedPublic

Authored by graesslin on Oct 6 2016, 6:18 AM.

Details

Summary

The TabBox performs the scrolling of the items by itself in order to
support wheel events even if the mouse is not on the TabBox. For that
KWin grabs pointer events on X11 (on Wayland an input filter is used)
and forwards them to the TabBox.

Qt uses Xinput2 for scrolling on the QQuickWindow. Due to that KWin
does not get any xcb core button press/release events when scrolling
inside the QQuickWindow and thus scrolling doesn't work.

There are three possible approaches to fix this:

  1. Implement scrolling support in each of the QML switchers
  2. Add an xinput2 filter to TabBox
  3. Intercept the QWheelEvents on the QQuickWindow

The first approach has the disadvantage that all themes need
adjustment and that there might be behaviorial difference whether one
scrolls on the TabBox window or outside the window.

The second approach would be most in line with the other filters, but
is difficult due to the nature of xinput2 (no xcb bindings, etc).

Thus the third approach might be the best solution. Wheel events are
only delivered to the QQuickWindow if the native events were not already
intercepted, thus we know it won't have side effects for the case that
Wayland is used or xinput2 is not supported.

The implementation installs an event filter on the QQuickWindow which
gets created when showing the TabBox and inside the filter waits till
there is an angleDelta of +/-120 and scrolls by one per every 120 angle
delta as described in the QWheelEvent documentation.

BUG: 369661
FIXED-IN: 5.8.1

Test Plan

Scrolled with touchpad and mouse wheel.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin updated this revision to Diff 7129.Oct 6 2016, 6:18 AM
graesslin retitled this revision from to [tabbox] Intercept QWheelEvents on QQuickWindow for scrolling.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added reviewers: KWin, Plasma, broulik.
Restricted Application added a project: KWin. · View Herald TranscriptOct 6 2016, 6:18 AM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
lukedashjr accepted this revision.Oct 6 2016, 6:43 AM
lukedashjr added a reviewer: lukedashjr.
lukedashjr added a subscriber: lukedashjr.

I'm not familiar with the codebase, but the changes look reasonable, and I can confirm it fixes the issue (applied on top of 5.6.5).

This revision is now accepted and ready to land.Oct 6 2016, 6:43 AM
luebking added inline comments.
tabbox/tabboxhandler.cpp
618

test type first - you'll get many paint events etc. but the filter is installed to only one object, ie. first check will currently always hit and second very few times only

621

x + y?

626

draw the indev var out, find the last one and if it's in the end valid, set it only once?

graesslin added inline comments.Oct 6 2016, 7:22 AM
tabbox/tabboxhandler.cpp
621

dangerous. If there is a scrolling in both horizontal and vertical (could happen on touchpad) it could result in no scrolling if one is negative and the other is positive.

luebking added inline comments.Oct 6 2016, 7:27 AM
tabbox/tabboxhandler.cpp
621

One might call that unsoecific user behavior :-P
But on that touchpad the present implementation will ignore large vertical scrolls for the slightest horizontal movement.

-> pick the only with the bigger absolute?

graesslin marked 2 inline comments as done.Oct 12 2016, 9:10 AM
graesslin added inline comments.
tabbox/tabboxhandler.cpp
626

just checked - doesn't work as nextPrev doesn't update the reference, but just returns the index for the next one. Without setting the index, the next call would return the same index.

graesslin updated this revision to Diff 7337.Oct 12 2016, 9:21 AM
graesslin edited edge metadata.

Updated to Thomas's suggestions as far as the code allowed to.

Changed fixed-in to 5.8.2

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptOct 12 2016, 9:21 AM
This revision was automatically updated to reflect the committed changes.