From 6c71b8720038e108ce4e494b0ef99e15e8e5b67d Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 11 May 2023 23:22:02 +0300 Subject: [PATCH] wayland: Fix null pointer constraint region handling If the pointer constraint region is null, the input region must be used instead. If the pointer constraint region is valid, it should be intersected with the input region. BUG: 457021 --- src/pointer_input.cpp | 38 +++++++--------- .../client/test_pointer_constraints.cpp | 28 ++++++++++-- .../pointerconstraints_v1_interface.cpp | 45 ++++++++++++++----- .../pointerconstraints_v1_interface_p.h | 2 + src/wayland/surface_interface.cpp | 19 ++++---- src/window.cpp | 10 ----- src/window.h | 1 - 7 files changed, 86 insertions(+), 57 deletions(-) diff --git a/src/pointer_input.cpp b/src/pointer_input.cpp index f69f5dbe64..9f2b3db1c5 100644 --- a/src/pointer_input.cpp +++ b/src/pointer_input.cpp @@ -597,14 +597,6 @@ void PointerInputRedirection::disconnectPointerConstraintsConnection() m_constraintsActivatedConnection = QMetaObject::Connection(); } -template -static QRegion getConstraintRegion(Window *window, T *constraint) -{ - const QRegion windowShape = window->inputShape(); - const QRegion intersected = constraint->region().isEmpty() ? windowShape : windowShape.intersected(constraint->region()); - return intersected.translated(window->mapFromLocal(QPointF(0, 0)).toPoint()); -} - void PointerInputRedirection::setEnableConstraints(bool set) { if (m_enableConstraints == set) { @@ -640,8 +632,7 @@ void PointerInputRedirection::updatePointerConstraints() } return; } - const QRegion r = getConstraintRegion(focus(), cf); - if (canConstrain && r.contains(m_pos.toPoint())) { + if (canConstrain && cf->region().contains(focus()->mapToLocal(m_pos).toPoint())) { cf->setConfined(true); m_confined = true; m_confinedPointerRegionConnection = connect(cf, &KWaylandServer::ConfinedPointerV1Interface::regionChanged, this, [this]() { @@ -653,7 +644,7 @@ void PointerInputRedirection::updatePointerConstraints() return; } const auto cf = s->confinedPointer(); - if (!getConstraintRegion(focus(), cf).contains(m_pos.toPoint())) { + if (!cf->region().contains(focus()->mapToLocal(m_pos).toPoint())) { // pointer no longer in confined region, break the confinement cf->setConfined(false); m_confined = false; @@ -684,8 +675,7 @@ void PointerInputRedirection::updatePointerConstraints() } return; } - const QRegion r = getConstraintRegion(focus(), lock); - if (canConstrain && r.contains(m_pos.toPoint())) { + if (canConstrain && lock->region().contains(focus()->mapToLocal(m_pos).toPoint())) { lock->setLocked(true); m_locked = true; @@ -728,20 +718,22 @@ QPointF PointerInputRedirection::applyPointerConfinement(const QPointF &pos) con return pos; } - const QRegion confinementRegion = getConstraintRegion(focus(), cf); - if (confinementRegion.contains(flooredPoint(pos))) { + const QPointF localPos = focus()->mapToLocal(pos); + if (cf->region().contains(flooredPoint(localPos))) { return pos; } - QPointF p = pos; - // allow either x or y to pass - p = QPointF(m_pos.x(), pos.y()); - if (confinementRegion.contains(flooredPoint(p))) { - return p; + const QPointF currentPos = focus()->mapToLocal(m_pos); + + // allow either x or y to pass + QPointF p(currentPos.x(), localPos.y()); + if (cf->region().contains(flooredPoint(p))) { + return focus()->mapFromLocal(p); } - p = QPointF(pos.x(), m_pos.y()); - if (confinementRegion.contains(flooredPoint(p))) { - return p; + + p = QPointF(localPos.x(), currentPos.y()); + if (cf->region().contains(flooredPoint(p))) { + return focus()->mapFromLocal(p); } return m_pos; diff --git a/src/wayland/autotests/client/test_pointer_constraints.cpp b/src/wayland/autotests/client/test_pointer_constraints.cpp index 65a1ddc97c..0683574a68 100644 --- a/src/wayland/autotests/client/test_pointer_constraints.cpp +++ b/src/wayland/autotests/client/test_pointer_constraints.cpp @@ -13,6 +13,7 @@ #include "KWayland/Client/pointerconstraints.h" #include "KWayland/Client/registry.h" #include "KWayland/Client/seat.h" +#include "KWayland/Client/shm_pool.h" #include "KWayland/Client/surface.h" // server #include "wayland/compositor_interface.h" @@ -52,6 +53,7 @@ private: KWayland::Client::EventQueue *m_queue = nullptr; KWayland::Client::Compositor *m_compositor = nullptr; KWayland::Client::Seat *m_seat = nullptr; + KWayland::Client::ShmPool *m_shm = nullptr; KWayland::Client::Pointer *m_pointer = nullptr; KWayland::Client::PointerConstraints *m_pointerConstraints = nullptr; }; @@ -95,6 +97,11 @@ void TestPointerConstraints::init() registry.setup(); QVERIFY(interfacesAnnouncedSpy.wait()); + m_shm = new KWayland::Client::ShmPool(this); + m_shm->setup(registry.bindShm(registry.interface(KWayland::Client::Registry::Interface::Shm).name, + registry.interface(KWayland::Client::Registry::Interface::Shm).version)); + QVERIFY(m_shm->isValid()); + m_compositor = registry.createCompositor(registry.interface(KWayland::Client::Registry::Interface::Compositor).name, registry.interface(KWayland::Client::Registry::Interface::Compositor).version, this); QVERIFY(m_compositor); @@ -125,6 +132,7 @@ void TestPointerConstraints::cleanup() CLEANUP(m_compositor) CLEANUP(m_pointerConstraints) CLEANUP(m_pointer) + CLEANUP(m_shm) CLEANUP(m_seat) CLEANUP(m_queue) if (m_connection) { @@ -167,6 +175,12 @@ void TestPointerConstraints::testLockPointer() QVERIFY(surface->isValid()); QVERIFY(surfaceCreatedSpy.wait()); + QImage image(QSize(100, 100), QImage::Format_ARGB32_Premultiplied); + image.fill(Qt::black); + surface->attachBuffer(m_shm->createBuffer(image)); + surface->damage(image.rect()); + surface->commit(KWayland::Client::Surface::CommitFlag::None); + auto serverSurface = surfaceCreatedSpy.first().first().value(); QVERIFY(serverSurface); QVERIFY(!serverSurface->lockedPointer()); @@ -186,7 +200,7 @@ void TestPointerConstraints::testLockPointer() QVERIFY(!serverSurface->confinedPointer()); QCOMPARE(serverLockedPointer->isLocked(), false); - QCOMPARE(serverLockedPointer->region(), QRegion()); + QCOMPARE(serverLockedPointer->region(), QRegion(0, 0, 100, 100)); QFETCH(LockedPointerV1Interface::LifeTime, serverLifeTime); QCOMPARE(serverLockedPointer->lifeTime(), serverLifeTime); // setting to unlocked now should not trigger an unlocked spy @@ -206,7 +220,7 @@ void TestPointerConstraints::testLockPointer() lockedPointer->setRegion(nullptr); surface->commit(KWayland::Client::Surface::CommitFlag::None); QVERIFY(regionChangedSpy.wait()); - QCOMPARE(serverLockedPointer->region(), QRegion()); + QCOMPARE(serverLockedPointer->region(), QRegion(0, 0, 100, 100)); // let's lock the surface QSignalSpy lockedChangedSpy(serverLockedPointer, &LockedPointerV1Interface::lockedChanged); @@ -277,6 +291,12 @@ void TestPointerConstraints::testConfinePointer() QVERIFY(surface->isValid()); QVERIFY(surfaceCreatedSpy.wait()); + QImage image(QSize(100, 100), QImage::Format_ARGB32_Premultiplied); + image.fill(Qt::black); + surface->attachBuffer(m_shm->createBuffer(image)); + surface->damage(image.rect()); + surface->commit(KWayland::Client::Surface::CommitFlag::None); + auto serverSurface = surfaceCreatedSpy.first().first().value(); QVERIFY(serverSurface); QVERIFY(!serverSurface->lockedPointer()); @@ -296,7 +316,7 @@ void TestPointerConstraints::testConfinePointer() QVERIFY(!serverSurface->lockedPointer()); QCOMPARE(serverConfinedPointer->isConfined(), false); - QCOMPARE(serverConfinedPointer->region(), QRegion()); + QCOMPARE(serverConfinedPointer->region(), QRegion(0, 0, 100, 100)); QFETCH(ConfinedPointerV1Interface::LifeTime, serverLifeTime); QCOMPARE(serverConfinedPointer->lifeTime(), serverLifeTime); // setting to unconfined now should not trigger an unconfined spy @@ -316,7 +336,7 @@ void TestPointerConstraints::testConfinePointer() confinedPointer->setRegion(nullptr); surface->commit(KWayland::Client::Surface::CommitFlag::None); QVERIFY(regionChangedSpy.wait()); - QCOMPARE(serverConfinedPointer->region(), QRegion()); + QCOMPARE(serverConfinedPointer->region(), QRegion(0, 0, 100, 100)); // let's confine the surface QSignalSpy confinedChangedSpy(serverConfinedPointer, &ConfinedPointerV1Interface::confinedChanged); diff --git a/src/wayland/pointerconstraints_v1_interface.cpp b/src/wayland/pointerconstraints_v1_interface.cpp index 35aef57da2..4898a74ca6 100644 --- a/src/wayland/pointerconstraints_v1_interface.cpp +++ b/src/wayland/pointerconstraints_v1_interface.cpp @@ -141,21 +141,35 @@ LockedPointerV1InterfacePrivate::LockedPointerV1InterfacePrivate(LockedPointerV1 , q(q) , surface(surface) , lifeTime(lifeTime) - , region(region) + , pendingRegion(region) + , hasPendingRegion(true) { + commit(); } void LockedPointerV1InterfacePrivate::commit() { - qreal scaleOverride = surface->scaleOverride(); + const QRegion oldRegion = effectiveRegion; + const QPointF oldHint = hint; + if (hasPendingRegion) { - region = mapScaleOverride(pendingRegion, scaleOverride); + region = mapScaleOverride(pendingRegion, surface->scaleOverride()); hasPendingRegion = false; - Q_EMIT q->regionChanged(); } if (hasPendingHint) { - hint = pendingHint / scaleOverride; + hint = pendingHint / surface->scaleOverride(); hasPendingHint = false; + } + + effectiveRegion = surface->input(); + if (!region.isEmpty()) { + effectiveRegion &= region; + } + + if (oldRegion != effectiveRegion) { + Q_EMIT q->regionChanged(); + } + if (oldHint != hint) { Q_EMIT q->cursorPositionHintChanged(); } } @@ -200,7 +214,7 @@ LockedPointerV1Interface::LifeTime LockedPointerV1Interface::lifeTime() const QRegion LockedPointerV1Interface::region() const { - return d->region; + return d->effectiveRegion; } QPointF LockedPointerV1Interface::cursorPositionHint() const @@ -244,16 +258,27 @@ ConfinedPointerV1InterfacePrivate::ConfinedPointerV1InterfacePrivate(ConfinedPoi , q(q) , surface(surface) , lifeTime(lifeTime) - , region(region) + , pendingRegion(region) + , hasPendingRegion(true) { + commit(); } void ConfinedPointerV1InterfacePrivate::commit() { - qreal scaleOverride = surface->scaleOverride(); + const QRegion oldRegion = effectiveRegion; + if (hasPendingRegion) { - region = mapScaleOverride(pendingRegion, scaleOverride); + region = mapScaleOverride(pendingRegion, surface->scaleOverride()); hasPendingRegion = false; + } + + effectiveRegion = surface->input(); + if (!region.isEmpty()) { + effectiveRegion &= region; + } + + if (oldRegion != effectiveRegion) { Q_EMIT q->regionChanged(); } } @@ -291,7 +316,7 @@ ConfinedPointerV1Interface::LifeTime ConfinedPointerV1Interface::lifeTime() cons QRegion ConfinedPointerV1Interface::region() const { - return d->region; + return d->effectiveRegion; } bool ConfinedPointerV1Interface::isConfined() const diff --git a/src/wayland/pointerconstraints_v1_interface_p.h b/src/wayland/pointerconstraints_v1_interface_p.h index 7f24af6d26..67e9394fc2 100644 --- a/src/wayland/pointerconstraints_v1_interface_p.h +++ b/src/wayland/pointerconstraints_v1_interface_p.h @@ -47,6 +47,7 @@ public: LockedPointerV1Interface *q; QPointer surface; LockedPointerV1Interface::LifeTime lifeTime; + QRegion effectiveRegion; QRegion region; QRegion pendingRegion; QPointF hint = QPointF(-1, -1); @@ -77,6 +78,7 @@ public: ConfinedPointerV1Interface *q; QPointer surface; ConfinedPointerV1Interface::LifeTime lifeTime; + QRegion effectiveRegion; QRegion region; QRegion pendingRegion; bool hasPendingRegion = false; diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp index 586257e11b..5ca7ae9a79 100644 --- a/src/wayland/surface_interface.cpp +++ b/src/wayland/surface_interface.cpp @@ -572,15 +572,6 @@ void SurfaceInterfacePrivate::applyState(SurfaceState *next) next->mergeInto(¤t); scaleOverride = pendingScaleOverride; - if (lockedPointer) { - auto lockedPointerPrivate = LockedPointerV1InterfacePrivate::get(lockedPointer); - lockedPointerPrivate->commit(); - } - if (confinedPointer) { - auto confinedPointerPrivate = ConfinedPointerV1InterfacePrivate::get(confinedPointer); - confinedPointerPrivate->commit(); - } - if (bufferRef != current.buffer) { if (bufferRef) { bufferRef->unref(); @@ -646,6 +637,16 @@ void SurfaceInterfacePrivate::applyState(SurfaceState *next) surfaceToBufferMatrix = buildSurfaceToBufferMatrix(); bufferToSurfaceMatrix = surfaceToBufferMatrix.inverted(); + + if (lockedPointer) { + auto lockedPointerPrivate = LockedPointerV1InterfacePrivate::get(lockedPointer); + lockedPointerPrivate->commit(); + } + if (confinedPointer) { + auto confinedPointerPrivate = ConfinedPointerV1InterfacePrivate::get(confinedPointer); + confinedPointerPrivate->commit(); + } + if (opaqueRegionChanged) { Q_EMIT q->opaqueChanged(opaqueRegion); } diff --git a/src/window.cpp b/src/window.cpp index abad641f07..20ef92bb70 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -369,16 +369,6 @@ QString Window::windowRole() const return QString(); } -QRegion Window::inputShape() const -{ - if (m_surface) { - return m_surface->input(); - } else { - // TODO: maybe also for X11? - return QRegion(); - } -} - QMatrix4x4 Window::inputTransformation() const { QMatrix4x4 m; diff --git a/src/window.h b/src/window.h index 1d0f4ec63b..04f6b8b0cd 100644 --- a/src/window.h +++ b/src/window.h @@ -754,7 +754,6 @@ public: virtual pid_t pid() const; bool readyForPainting() const; // true if the window has been already painted its contents - QRegion inputShape() const; void setOpacity(qreal opacity); qreal opacity() const; virtual bool setupCompositing();