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
This commit is contained in:
Martin Gräßlin 2017-03-26 15:53:09 +02:00
parent 36c22ed0cb
commit 0bb587dcb9
2 changed files with 95 additions and 0 deletions

View file

@ -22,6 +22,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#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<EffectsHandlerImpl*>(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

View file

@ -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<ScheduledPosition> s_scheduledPositions;
PointerInputRedirection *m_pointer;
};
int PositionUpdateBlocker::s_counter = 0;
QVector<PositionUpdateBlocker::ScheduledPosition> 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,