From 0bb587dcb9de0a3bc688019fd66b224e713b3501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Sun, 26 Mar 2017 15:53:09 +0200 Subject: [PATCH] Ensure PointerInputRedirection::processMotion finishes prior to warping Summary: Consider the following situation: we have three InputEventFilter linked in the sequence A - B - C. The input filters are processing pointer motion events. The expected behavior is that the new motion is processed in the sequence A -> B -> C So far this did not work correctly if the pointer gets warped during the processing. If e.g. filter B warps the pointer we get a motion sequence: A (1) -> B (1) -> A (2) -> B (2) -> C (2) -> C (1) The filters following the one warping the pointer get first the newer than the older position. This is obviously wrong. Unfortunately it is not just a theoretical condition, but a condition happening when interacting with the screenedges, which warp the pointer. This change introduces a PositionUpdateBlocker in PointerInputRedirection::processMotion to ensure that a processMotion call finishes prior to the next update. If the PositionUpdateBlocker is blocked the new position gets scheduled and processed once the PositionUpdateBlocker gets destroyed. With this we get the expected sequence for B warping pointer: A (1) -> B (1) -> C (1) -> A (2) -> B (2) -> C (2) This should hopefully improve the interaction with screen edges on Wayland. CCBUG: 374867 Test Plan: Added an auto test demonstrating the issue of incorrect ordering caused by screenedges. Prior to the change the test is failing. Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D5182 --- autotests/integration/pointer_input.cpp | 46 +++++++++++++++++++++++ pointer_input.cpp | 49 +++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/autotests/integration/pointer_input.cpp b/autotests/integration/pointer_input.cpp index 0026c1edc4..6fbb5fc1c0 100644 --- a/autotests/integration/pointer_input.cpp +++ b/autotests/integration/pointer_input.cpp @@ -22,6 +22,7 @@ along with this program. If not, see . #include "abstract_client.h" #include "cursor.h" #include "deleted.h" +#include "effects.h" #include "pointer_input.h" #include "options.h" #include "screenedge.h" @@ -57,6 +58,7 @@ private Q_SLOTS: void cleanup(); void testWarpingUpdatesFocus(); void testWarpingGeneratesPointerMotion(); + void testWarpingDuringFilter(); void testUpdateFocusAfterScreenChange(); void testModifierClickUnrestrictedMove_data(); void testModifierClickUnrestrictedMove(); @@ -211,6 +213,50 @@ void PointerInputTest::testWarpingGeneratesPointerMotion() QCOMPARE(movedSpy.last().first().toPointF(), QPointF(26, 26)); } +void PointerInputTest::testWarpingDuringFilter() +{ + // this test verifies that pointer motion is handled correctly if + // the pointer gets warped during processing of input events + using namespace KWayland::Client; + + // create pointer + auto pointer = m_seat->createPointer(m_seat); + QVERIFY(pointer); + QVERIFY(pointer->isValid()); + QSignalSpy movedSpy(pointer, &Pointer::motion); + QVERIFY(movedSpy.isValid()); + + // warp cursor into expected geometry + Cursor::setPos(10, 10); + + // create a window + QSignalSpy clientAddedSpy(waylandServer(), &WaylandServer::shellClientAdded); + QVERIFY(clientAddedSpy.isValid()); + Surface *surface = Test::createSurface(m_compositor); + QVERIFY(surface); + ShellSurface *shellSurface = Test::createShellSurface(surface, surface); + QVERIFY(shellSurface); + render(surface); + QVERIFY(clientAddedSpy.wait()); + AbstractClient *window = workspace()->activeClient(); + QVERIFY(window); + + QCOMPARE(window->pos(), QPoint(0, 0)); + QVERIFY(window->geometry().contains(Cursor::pos())); + + // is PresentWindows effect for top left screen edge loaded + QVERIFY(static_cast(effects)->isEffectLoaded("presentwindows")); + QVERIFY(movedSpy.isEmpty()); + quint32 timestamp = 0; + kwinApp()->platform()->pointerMotion(QPoint(0, 0), timestamp++); + // screen edges push back + QCOMPARE(Cursor::pos(), QPoint(1, 1)); + QVERIFY(movedSpy.wait()); + QCOMPARE(movedSpy.count(), 2); + QCOMPARE(movedSpy.at(0).first().toPoint(), QPoint(0, 0)); + QCOMPARE(movedSpy.at(1).first().toPoint(), QPoint(1, 1)); +} + void PointerInputTest::testUpdateFocusAfterScreenChange() { // this test verifies that a pointer enter event is generated when the cursor changes to another diff --git a/pointer_input.cpp b/pointer_input.cpp index 1ebff9a6bb..95e128d080 100644 --- a/pointer_input.cpp +++ b/pointer_input.cpp @@ -220,11 +220,60 @@ void PointerInputRedirection::processMotion(const QPointF &pos, uint32_t time, L processMotion(pos, QSizeF(), QSizeF(), time, 0, device); } +class PositionUpdateBlocker +{ +public: + PositionUpdateBlocker(PointerInputRedirection *pointer) + : m_pointer(pointer) + { + s_counter++; + } + ~PositionUpdateBlocker() { + s_counter--; + if (s_counter == 0) { + if (!s_scheduledPositions.isEmpty()) { + const auto pos = s_scheduledPositions.takeFirst(); + m_pointer->processMotion(pos.pos, pos.delta, pos.deltaNonAccelerated, pos.time, pos.timeUsec, nullptr); + } + } + } + + static bool isPositionBlocked() { + return s_counter > 0; + } + + static void schedulePosition(const QPointF &pos, const QSizeF &delta, const QSizeF &deltaNonAccelerated, uint32_t time, quint64 timeUsec) { + s_scheduledPositions.append({pos, delta, deltaNonAccelerated, time, timeUsec}); + } + +private: + static int s_counter; + struct ScheduledPosition { + QPointF pos; + QSizeF delta; + QSizeF deltaNonAccelerated; + quint32 time; + quint64 timeUsec; + }; + static QVector s_scheduledPositions; + + PointerInputRedirection *m_pointer; +}; + +int PositionUpdateBlocker::s_counter = 0; +QVector PositionUpdateBlocker::s_scheduledPositions; + void PointerInputRedirection::processMotion(const QPointF &pos, const QSizeF &delta, const QSizeF &deltaNonAccelerated, uint32_t time, quint64 timeUsec, LibInput::Device *device) { if (!m_inited) { return; } + if (PositionUpdateBlocker::isPositionBlocked()) { + PositionUpdateBlocker::schedulePosition(pos, delta, deltaNonAccelerated, time, timeUsec); + return; + } + + PositionUpdateBlocker blocker(this); updatePosition(pos); MouseEvent event(QEvent::MouseMove, m_pos, Qt::NoButton, m_qtButtons, m_input->keyboardModifiers(), time,