From 374d8594931861ef7dfbed8eda5f8552aa4f825c Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Mon, 15 Jul 2024 15:40:29 +0100 Subject: [PATCH] xwayland: Only update keyboard modifers for XWayland's keys KeyboardInterface is a multiplexer, it has a global state to kwin that forwards events the single focussed window. XWayland also forwards events to clients, but uses the keyboard interface. It has some overloads that take a specific client, this was used for key events but not modifiers. The end result was not only that XWayland could miss a modifier update, but also that wayland clients would get modifier updates out of order. Key events must come first. BUG: 490270 --- autotests/integration/x11keyread.cpp | 56 ++++++++++++++++++++++++++++ src/wayland/keyboard.cpp | 9 +++++ src/wayland/keyboard.h | 1 + src/xwayland/xwayland.cpp | 37 ++++++++++++++++-- 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/autotests/integration/x11keyread.cpp b/autotests/integration/x11keyread.cpp index 2b969bc5ee..cfb0b3e089 100644 --- a/autotests/integration/x11keyread.cpp +++ b/autotests/integration/x11keyread.cpp @@ -6,6 +6,8 @@ SPDX-License-Identifier: GPL-2.0-or-later */ +#include "KWayland/Client/keyboard.h" +#include "KWayland/Client/seat.h" #include "kwin_wayland_test.h" #include "input.h" @@ -53,6 +55,7 @@ private Q_SLOTS: void testSimpleLetter(); void onlyModifier(); void letterWithModifier(); + void testWaylandWindowHasFocus(); private: QList recievedX11EventsForInput(const QList &keyEventsIn); @@ -91,10 +94,14 @@ void X11KeyReadTest::init() options->setXwaylandEavesdrops(operatingMode); workspace()->setActiveOutput(QPoint(640, 512)); KWin::input()->pointer()->warp(QPoint(640, 512)); + + QVERIFY(Test::setupWaylandConnection(Test::AdditionalWaylandInterface::Seat)); + QVERIFY(Test::waitForWaylandKeyboard()); } void X11KeyReadTest::cleanup() { + Test::destroyWaylandConnection(); } void X11KeyReadTest::testSimpleLetter() @@ -176,6 +183,55 @@ void X11KeyReadTest::letterWithModifier() QCOMPARE(received, expected); } +void X11KeyReadTest::testWaylandWindowHasFocus() +{ + // A wayland window should be unaffected + int timestamp = 0; + + std::unique_ptr surface(Test::createSurface()); + std::unique_ptr shellSurface(Test::createXdgToplevelSurface(surface.get())); + std::unique_ptr keyboard(Test::waylandSeat()->createKeyboard()); + QSignalSpy modifierSpy(keyboard.get(), &KWayland::Client::Keyboard::modifiersChanged); + QSignalSpy keyChangedSpy(keyboard.get(), &KWayland::Client::Keyboard::keyChanged); + + Window *waylandWindow = Test::renderAndWaitForShown(surface.get(), QSize(10, 10), Qt::blue); + QVERIFY(waylandWindow->isActive()); + modifierSpy.wait(); // initial modifiers + modifierSpy.clear(); + + Test::keyboardKeyPressed(KEY_LEFTALT, timestamp++); + QVERIFY(keyChangedSpy.wait()); + QCOMPARE(keyChangedSpy.last()[0], KEY_LEFTALT); + QCOMPARE(keyChangedSpy.last()[1].value(), KWayland::Client::Keyboard::KeyState::Pressed); + + QCOMPARE(modifierSpy.count(), 1); + QCOMPARE(modifierSpy.last()[0], 8); + QCOMPARE(modifierSpy.last()[1], 0); + QCOMPARE(modifierSpy.last()[2], 0); + QCOMPARE(modifierSpy.last()[3], 0); + + Test::keyboardKeyPressed(KEY_G, timestamp++); + QVERIFY(keyChangedSpy.wait()); + QCOMPARE(keyChangedSpy.last()[0], KEY_G); + QCOMPARE(keyChangedSpy.last()[1].value(), KWayland::Client::Keyboard::KeyState::Pressed); + + Test::keyboardKeyReleased(KEY_G, timestamp++); + QVERIFY(keyChangedSpy.wait()); + QCOMPARE(keyChangedSpy.last()[0], KEY_G); + QCOMPARE(keyChangedSpy.last()[1].value(), KWayland::Client::Keyboard::KeyState::Released); + + Test::keyboardKeyReleased(KEY_LEFTALT, timestamp++); + QVERIFY(keyChangedSpy.wait()); + QCOMPARE(keyChangedSpy.last()[0], KEY_LEFTALT); + QCOMPARE(keyChangedSpy.last()[1].value(), KWayland::Client::Keyboard::KeyState::Released); + + QCOMPARE(modifierSpy.count(), 2); + QCOMPARE(modifierSpy.last()[0], 0); + QCOMPARE(modifierSpy.last()[1], 0); + QCOMPARE(modifierSpy.last()[2], 0); + QCOMPARE(modifierSpy.last()[3], 0); +} + class X11EventRecorder : public QObject diff --git a/src/wayland/keyboard.cpp b/src/wayland/keyboard.cpp index 0d277834e9..a9bbaa639f 100644 --- a/src/wayland/keyboard.cpp +++ b/src/wayland/keyboard.cpp @@ -237,6 +237,15 @@ void KeyboardInterface::sendModifiers(quint32 depressed, quint32 latched, quint3 } } +void KeyboardInterface::sendModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group, ClientConnection *client) +{ + const QList keyboards = d->keyboardsForClient(client); + const quint32 serial = d->seat->display()->nextSerial(); + for (KeyboardInterfacePrivate::Resource *keyboardResource : keyboards) { + d->send_modifiers(keyboardResource->handle, serial, depressed, latched, locked, group); + } +} + void KeyboardInterface::setRepeatInfo(qint32 charactersPerSecond, qint32 delay) { d->keyRepeat.charactersPerSecond = std::max(charactersPerSecond, 0); diff --git a/src/wayland/keyboard.h b/src/wayland/keyboard.h index 3f1bd02e40..20dacb7ebd 100644 --- a/src/wayland/keyboard.h +++ b/src/wayland/keyboard.h @@ -57,6 +57,7 @@ public: void sendKey(quint32 key, KeyboardKeyState state); void sendKey(quint32 key, KeyboardKeyState state, ClientConnection *client); void sendModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group); + void sendModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group, ClientConnection *client); private: void setFocusedSurface(SurfaceInterface *surface, quint32 serial); diff --git a/src/xwayland/xwayland.cpp b/src/xwayland/xwayland.cpp index 6df28fa369..d87ab9460f 100644 --- a/src/xwayland/xwayland.cpp +++ b/src/xwayland/xwayland.cpp @@ -104,6 +104,7 @@ public: keyboard->sendKey(it.key(), KeyboardKeyState::Released, waylandServer()->xWaylandConnection()); } } + m_modifiers = {}; m_states.clear(); } }); @@ -399,12 +400,35 @@ public: } auto xkb = input()->keyboard()->xkb(); + + keyboard->sendKey(event->nativeScanCode(), state, xwaylandClient); + + bool changed = false; + if (m_modifiers.depressed != xkb->modifierState().depressed) { + m_modifiers.depressed = xkb->modifierState().depressed; + changed = true; + } + if (m_modifiers.latched != xkb->modifierState().latched) { + m_modifiers.latched = xkb->modifierState().latched; + changed = true; + } + if (m_modifiers.locked != xkb->modifierState().locked) { + m_modifiers.locked = xkb->modifierState().locked; + changed = true; + } + if (m_modifiers.group != xkb->currentLayout()) { + m_modifiers.group = xkb->currentLayout(); + changed = true; + } + if (!changed) { + return; + } + keyboard->sendModifiers(xkb->modifierState().depressed, xkb->modifierState().latched, xkb->modifierState().locked, - xkb->currentLayout()); - - keyboard->sendKey(event->nativeScanCode(), state, xwaylandClient); + xkb->currentLayout(), + xwaylandClient); } void pointerEvent(KWin::MouseEvent *event) override @@ -447,6 +471,13 @@ public: } QHash m_states; + struct Modifiers + { + quint32 depressed = 0; + quint32 latched = 0; + quint32 locked = 0; + quint32 group = 0; + } m_modifiers; std::function m_filterKey; bool m_filterMouse = false; };