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

@@ -597,14 +597,6 @@ void PointerInputRedirection::disconnectPointerConstraintsConnection()
     m_constraintsActivatedConnection = QMetaObject::Connection();
 }
 
-template<typename T>
-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;

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<SurfaceInterface *>();
     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<SurfaceInterface *>();
     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);

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

src/wayland/pointerconstraints_v1_interface_p.h

@@ -47,6 +47,7 @@ public:
     LockedPointerV1Interface *q;
     QPointer<SurfaceInterface> surface;
     LockedPointerV1Interface::LifeTime lifeTime;
+    QRegion effectiveRegion;
     QRegion region;
     QRegion pendingRegion;
     QPointF hint = QPointF(-1, -1);
@@ -77,6 +78,7 @@ public:
     ConfinedPointerV1Interface *q;
     QPointer<SurfaceInterface> surface;
     ConfinedPointerV1Interface::LifeTime lifeTime;
+    QRegion effectiveRegion;
     QRegion region;
     QRegion pendingRegion;
     bool hasPendingRegion = false;

src/wayland/surface_interface.cpp

@@ -572,15 +572,6 @@ void SurfaceInterfacePrivate::applyState(SurfaceState *next)
     next->mergeInto(&current);
     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);
     }

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;

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();