From ca7298a3254b53a895d83fe9f488235399e2cee2 Mon Sep 17 00:00:00 2001 From: Weng Xuetian Date: Wed, 29 Dec 2021 09:19:43 -0800 Subject: [PATCH] Ensure modifier change is forwarded after the key sending to input method. Same as real hardware wl_keyboard, key should be sent before modifier change. For example, Left Ctrl press and release should produce key events in the order of Control_L and Control+Control_L. --- autotests/integration/inputmethod_test.cpp | 76 +++++++++++++++++++++- src/inputmethod.cpp | 13 +++- src/inputmethod.h | 7 +- src/keyboard_input.cpp | 6 +- 4 files changed, 95 insertions(+), 7 deletions(-) diff --git a/autotests/integration/inputmethod_test.cpp b/autotests/integration/inputmethod_test.cpp index 5fed417f5d..b367cfb9c1 100644 --- a/autotests/integration/inputmethod_test.cpp +++ b/autotests/integration/inputmethod_test.cpp @@ -32,11 +32,13 @@ #include #include +#include #include #include +#include #include #include -#include +#include using namespace KWin; using namespace KWayland::Client; @@ -59,6 +61,7 @@ private Q_SLOTS: void testSwitchFocusedSurfaces(); void testV3Styling(); void testDisableShowInputPanel(); + void testModifierForwarding(); private: void touchNow() { @@ -462,6 +465,77 @@ void InputMethodTest::testDisableShowInputPanel() QVERIFY(!InputMethod::self()->isActive()); } +void InputMethodTest::testModifierForwarding() +{ + // Create an xdg_toplevel surface and wait for the compositor to catch up. + QScopedPointer surface(Test::createSurface()); + QScopedPointer shellSurface(Test::createXdgToplevelSurface(surface.data())); + AbstractClient *client = Test::renderAndWaitForShown(surface.data(), QSize(1280, 1024), Qt::red); + QVERIFY(client); + QVERIFY(client->isActive()); + QCOMPARE(client->frameGeometry().size(), QSize(1280, 1024)); + + Test::TextInputV3 *textInputV3 = new Test::TextInputV3(); + textInputV3->init(Test::waylandTextInputManagerV3()->get_text_input(*(Test::waylandSeat()))); + textInputV3->enable(); + + QSignalSpy inputMethodActiveSpy(InputMethod::self(), &InputMethod::activeChanged); + QSignalSpy inputMethodActivateSpy(Test::inputMethod(), &Test::MockInputMethod::activate); + // just enabling the text-input should not show it but rather on commit + QVERIFY(!InputMethod::self()->isActive()); + textInputV3->commit(); + QVERIFY(inputMethodActiveSpy.count() || inputMethodActiveSpy.wait()); + QVERIFY(InputMethod::self()->isActive()); + QVERIFY(inputMethodActivateSpy.wait()); + auto context = Test::inputMethod()->context(); + QScopedPointer keyboardGrab(new KWayland::Client::Keyboard); + keyboardGrab->setup(zwp_input_method_context_v1_grab_keyboard(context)); + QSignalSpy modifierSpy(keyboardGrab.get(), &Keyboard::modifiersChanged); + // Wait for initial modifiers update + QVERIFY(modifierSpy.wait()); + + quint32 timestamp = 1; + + QSignalSpy keySpy(keyboardGrab.get(), &Keyboard::keyChanged); + bool keyChanged = false; + bool modifiersChanged = false; + // We want to verify the order of two signals, so SignalSpy is not very useful here. + auto keyChangedConnection = connect(keyboardGrab.get(), &Keyboard::keyChanged, [&keyChanged, &modifiersChanged]() { + QVERIFY(!modifiersChanged); + keyChanged = true; + }); + auto modifiersChangedConnection = connect(keyboardGrab.get(), &Keyboard::modifiersChanged, [&keyChanged, &modifiersChanged]() { + QVERIFY(keyChanged); + modifiersChanged = true; + }); + kwinApp()->platform()->keyboardKeyPressed(KEY_LEFTCTRL, timestamp++); + QVERIFY(keySpy.count() == 1 || keySpy.wait()); + QVERIFY(modifierSpy.count() == 2 || modifierSpy.wait()); + disconnect(keyChangedConnection); + disconnect(modifiersChangedConnection); + + kwinApp()->platform()->keyboardKeyPressed(KEY_A, timestamp++); + QVERIFY(keySpy.count() == 2 || keySpy.wait()); + QVERIFY(modifierSpy.count() == 2 || modifierSpy.wait()); + + // verify the order of key and modifiers again. Key first, then modifiers. + keyChanged = false; + modifiersChanged = false; + keyChangedConnection = connect(keyboardGrab.get(), &Keyboard::keyChanged, [&keyChanged, &modifiersChanged]() { + QVERIFY(!modifiersChanged); + keyChanged = true; + }); + modifiersChangedConnection = connect(keyboardGrab.get(), &Keyboard::modifiersChanged, [&keyChanged, &modifiersChanged]() { + QVERIFY(keyChanged); + modifiersChanged = true; + }); + kwinApp()->platform()->keyboardKeyReleased(KEY_LEFTCTRL, timestamp++); + QVERIFY(keySpy.count() == 3 || keySpy.wait()); + QVERIFY(modifierSpy.count() == 3 || modifierSpy.wait()); + disconnect(keyChangedConnection); + disconnect(modifiersChangedConnection); +} + WAYLANDTEST_MAIN(InputMethodTest) #include "inputmethod_test.moc" diff --git a/src/inputmethod.cpp b/src/inputmethod.cpp index bd73934484..4ae5ee95f5 100644 --- a/src/inputmethod.cpp +++ b/src/inputmethod.cpp @@ -105,7 +105,9 @@ void InputMethod::init() connect(textInputV3, &TextInputV3Interface::enabledChanged, this, &InputMethod::textInputInterfaceV3EnabledChanged); } - connect(input()->keyboard()->xkb(), &Xkb::modifierStateChanged, this, &InputMethod::forwardModifiers); + connect(input()->keyboard()->xkb(), &Xkb::modifierStateChanged, this, [this]() { + m_hasPendingModifiers = true; + }); } } @@ -550,8 +552,13 @@ void InputMethod::modifiers(quint32 serial, quint32 mods_depressed, quint32 mods xkb->updateModifiers(mods_depressed, mods_latched, mods_locked, group); } -void InputMethod::forwardModifiers() +void InputMethod::forwardModifiers(ForwardModifiersForce force) { + const bool sendModifiers = m_hasPendingModifiers || force == Force; + m_hasPendingModifiers = false; + if (!sendModifiers) { + return; + } auto xkb = input()->keyboard()->xkb(); if (m_keyboardGrab) { m_keyboardGrab->sendModifiers(waylandServer()->display()->nextSerial(), @@ -719,7 +726,7 @@ void InputMethod::installKeyboardGrab(KWaylandServer::InputMethodGrabV1 *keyboar auto xkb = input()->keyboard()->xkb(); m_keyboardGrab = keyboardGrab; keyboardGrab->sendKeymap(xkb->keymapContents()); - forwardModifiers(); + forwardModifiers(Force); } void InputMethod::updateModifiersMap(const QByteArray &modifiers) diff --git a/src/inputmethod.h b/src/inputmethod.h index a5d5d32f0e..0b7cdb2eed 100644 --- a/src/inputmethod.h +++ b/src/inputmethod.h @@ -43,6 +43,8 @@ class KWIN_EXPORT InputMethod : public QObject { Q_OBJECT public: + enum ForwardModifiersForce { NoForce = 0, Force = 1 }; + ~InputMethod() override; void init(); @@ -63,6 +65,8 @@ public: KWaylandServer::InputMethodGrabV1 *keyboardGrab(); bool shouldShowOnActive() const; + void forwardModifiers(ForwardModifiersForce force); + Q_SIGNALS: void activeChanged(bool active); void enabledChanged(bool enabled); @@ -102,7 +106,6 @@ private: void updateModifiersMap(const QByteArray &modifiers); bool touchEventTriggered() const; - void forwardModifiers(); void resetPendingPreedit(); struct { @@ -122,6 +125,8 @@ private: uint m_inputMethodCrashes = 0; QString m_inputMethodCommand; + bool m_hasPendingModifiers = false; + KWIN_SINGLETON(InputMethod) }; diff --git a/src/keyboard_input.cpp b/src/keyboard_input.cpp index bfc78b2cad..3cf5c84b9d 100644 --- a/src/keyboard_input.cpp +++ b/src/keyboard_input.cpp @@ -7,15 +7,16 @@ SPDX-License-Identifier: GPL-2.0-or-later */ #include "keyboard_input.h" +#include "abstract_client.h" #include "input_event.h" #include "input_event_spy.h" +#include "inputmethod.h" #include "keyboard_layout.h" #include "keyboard_repeat.h" -#include "abstract_client.h" #include "modifier_only_shortcuts.h" -#include "utils.h" #include "screenlockerwatcher.h" #include "toplevel.h" +#include "utils.h" #include "wayland_server.h" #include "workspace.h" // KWayland @@ -247,6 +248,7 @@ void KeyboardInputRedirection::processKey(uint32_t key, InputRedirection::Keyboa m_input->processFilters(std::bind(&InputEventFilter::keyEvent, std::placeholders::_1, &event)); m_xkb->forwardModifiers(); + InputMethod::self()->forwardModifiers(InputMethod::NoForce); if (event.modifiersRelevantForGlobalShortcuts() == Qt::KeyboardModifier::NoModifier && type != QEvent::KeyRelease) { m_keyboardLayout->checkLayoutChange(previousLayout);