wayland: Port tablet cursor to {Shape,Surface}CursorSource

Currently the tablet cursor doesn't use SurfaceCursorSource because it
doesn't handle changing the surface size after the set_cursor request.

This change adds missing surface commit handling in the
SurfaceCursorSource so the tablet cursor can use it. As a side effect,
the pointer interface doesn't need to track surface commits anymore.

src/cursorsource.cpp

@@ -135,26 +135,57 @@ KWaylandServer::SurfaceInterface *SurfaceCursorSource::surface() const
     return m_surface;
 }
 
-void SurfaceCursorSource::update(KWaylandServer::SurfaceInterface *surface, const QPointF &hotspot)
+void SurfaceCursorSource::refresh()
 {
-    if (!surface) {
-        m_image = QImage();
-        m_size = QSizeF(0, 0);
-        m_hotspot = QPointF();
-        m_surface = nullptr;
+    auto buffer = qobject_cast<KWaylandServer::ShmClientBuffer *>(m_surface->buffer());
+    if (buffer) {
+        m_image = buffer->data().copy();
+        m_image.setDevicePixelRatio(m_surface->bufferScale());
     } else {
-        auto buffer = qobject_cast<KWaylandServer::ShmClientBuffer *>(surface->buffer());
-        if (buffer) {
-            m_image = buffer->data().copy();
-            m_image.setDevicePixelRatio(surface->bufferScale());
-        } else {
-            m_image = QImage();
-        }
-        m_size = surface->size();
-        m_hotspot = hotspot;
-        m_surface = surface;
+        m_image = QImage();
     }
+    m_size = m_surface->size();
     Q_EMIT changed();
 }
 
+void SurfaceCursorSource::update(KWaylandServer::SurfaceInterface *surface, const QPointF &hotspot)
+{
+    bool dirty = false;
+
+    if (m_hotspot != hotspot) {
+        dirty = true;
+        m_hotspot = hotspot;
+    }
+
+    if (m_surface != surface) {
+        dirty = true;
+
+        if (m_surface) {
+            disconnect(m_surface, &KWaylandServer::SurfaceInterface::committed, this, &SurfaceCursorSource::refresh);
+        }
+
+        m_surface = surface;
+
+        if (m_surface) {
+            auto buffer = qobject_cast<KWaylandServer::ShmClientBuffer *>(surface->buffer());
+            if (buffer) {
+                m_image = buffer->data().copy();
+                m_image.setDevicePixelRatio(surface->bufferScale());
+            } else {
+                m_image = QImage();
+            }
+            m_size = surface->size();
+
+            connect(m_surface, &KWaylandServer::SurfaceInterface::committed, this, &SurfaceCursorSource::refresh);
+        } else {
+            m_image = QImage();
+            m_size = QSizeF(0, 0);
+        }
+    }
+
+    if (dirty) {
+        Q_EMIT changed();
+    }
+}
+
 } // namespace KWin

src/cursorsource.h

@@ -104,6 +104,8 @@ public Q_SLOTS:
     void update(KWaylandServer::SurfaceInterface *surface, const QPointF &hotspot);
 
 private:
+    void refresh();
+
     QPointer<KWaylandServer::SurfaceInterface> m_surface;
 };
 

src/input.cpp

@@ -44,7 +44,6 @@
 #include "wayland/display.h"
 #include "wayland/inputmethod_v1_interface.h"
 #include "wayland/seat_interface.h"
-#include "wayland/shmclientbuffer.h"
 #include "wayland/surface_interface.h"
 #include "wayland/tablet_v2_interface.h"
 #include "wayland_server.h"
@@ -1952,59 +1951,29 @@ class SurfaceCursor : public Cursor
 public:
     explicit SurfaceCursor(KWaylandServer::TabletToolV2Interface *tool)
         : Cursor(tool)
-        , m_source(std::make_unique<ImageCursorSource>())
     {
-        setSource(m_source.get());
         connect(tool, &KWaylandServer::TabletToolV2Interface::cursorChanged, this, [this](KWaylandServer::TabletCursorV2 *tcursor) {
             if (!tcursor || tcursor->enteredSerial() == 0) {
+                if (!m_defaultSource) {
+                    m_defaultSource = std::make_unique<ShapeCursorSource>();
+                }
                 static WaylandCursorImage defaultCursor;
-                defaultCursor.loadThemeCursor(CursorShape(Qt::CrossCursor), m_source.get());
-                return;
+                m_defaultSource->setTheme(defaultCursor.theme());
+                m_defaultSource->setShape(Qt::CrossCursor);
+                setSource(m_defaultSource.get());
+            } else {
+                if (!m_surfaceSource) {
+                    m_surfaceSource = std::make_unique<SurfaceCursorSource>();
+                }
+                m_surfaceSource->update(tcursor->surface(), tcursor->hotspot());
+                setSource(m_surfaceSource.get());
             }
-            auto cursorSurface = tcursor->surface();
-            if (!cursorSurface) {
-                m_source->update(QImage(), QPoint());
-                return;
-            }
-
-            updateCursorSurface(cursorSurface, tcursor->hotspot());
         });
     }
 
-    void updateCursorSurface(KWaylandServer::SurfaceInterface *surface, const QPoint &hotspot)
-    {
-        if (m_surface == surface && hotspot == m_hotspot) {
-            return;
-        }
-
-        if (m_surface) {
-            disconnect(m_surface, nullptr, this, nullptr);
-        }
-        m_surface = surface;
-        m_hotspot = hotspot;
-        connect(m_surface, &KWaylandServer::SurfaceInterface::committed, this, &SurfaceCursor::refresh);
-
-        refresh();
-    }
-
 private:
-    void refresh()
-    {
-        auto buffer = qobject_cast<KWaylandServer::ShmClientBuffer *>(m_surface->buffer());
-        if (!buffer) {
-            m_source->update(QImage(), QPoint());
-            return;
-        }
-
-        QImage cursorImage;
-        cursorImage = buffer->data().copy();
-        cursorImage.setDevicePixelRatio(m_surface->bufferScale());
-        m_source->update(cursorImage, m_hotspot);
-    }
-
-    QPointer<KWaylandServer::SurfaceInterface> m_surface;
-    std::unique_ptr<ImageCursorSource> m_source;
-    QPoint m_hotspot;
+    std::unique_ptr<ShapeCursorSource> m_defaultSource;
+    std::unique_ptr<SurfaceCursorSource> m_surfaceSource;
 };
 
 /**

src/pointer_input.cpp

@@ -1072,37 +1072,6 @@ void WaylandCursorImage::updateCursorTheme()
     Q_EMIT themeChanged();
 }
 
-void WaylandCursorImage::loadThemeCursor(const CursorShape &shape, ImageCursorSource *source)
-{
-    loadThemeCursor(shape.name(), source);
-}
-
-void WaylandCursorImage::loadThemeCursor(const QByteArray &name, ImageCursorSource *source)
-{
-    if (loadThemeCursor_helper(name, source)) {
-        return;
-    }
-
-    const auto alternativeNames = Cursor::cursorAlternativeNames(name);
-    for (const QByteArray &alternativeName : alternativeNames) {
-        if (loadThemeCursor_helper(alternativeName, source)) {
-            return;
-        }
-    }
-
-    qCWarning(KWIN_CORE) << "Failed to load theme cursor for shape" << name;
-}
-
-bool WaylandCursorImage::loadThemeCursor_helper(const QByteArray &name, ImageCursorSource *source)
-{
-    const QVector<KXcursorSprite> sprites = m_cursorTheme.shape(name);
-    if (sprites.isEmpty()) {
-        return false;
-    }
-    source->update(sprites.first().data(), sprites.first().hotspot());
-    return true;
-}
-
 void CursorImage::reevaluteSource()
 {
     if (waylandServer()->isScreenLocked()) {

src/pointer_input.h

@@ -190,14 +190,10 @@ public:
 
     KXcursorTheme theme() const;
 
-    void loadThemeCursor(const CursorShape &shape, ImageCursorSource *source);
-    void loadThemeCursor(const QByteArray &name, ImageCursorSource *source);
-
 Q_SIGNALS:
     void themeChanged();
 
 private:
-    bool loadThemeCursor_helper(const QByteArray &name, ImageCursorSource *source);
     void updateCursorTheme();
 
     KXcursorTheme m_cursorTheme;

src/wayland/autotests/client/test_wayland_seat.cpp

@@ -105,7 +105,6 @@ private Q_SLOTS:
     void testPointerHoldGesture();
     void testPointerAxis();
     void testCursor();
-    void testCursorDamage();
     void testKeyboard();
     void testSelection();
     void testDataDeviceForKeyboardSurface();
@@ -1333,125 +1332,34 @@ void TestWaylandSeat::testCursor()
     QVERIFY(cursor);
     QVERIFY(!cursor->surface());
     QCOMPARE(cursor->hotspot(), QPoint());
-    QCOMPARE_GT(cursor->enteredSerial(), serial);
-    QCOMPARE(cursor->pointer(), m_seatInterface->pointer());
-
-    QSignalSpy hotspotChangedSpy(cursor, &KWaylandServer::Cursor::hotspotChanged);
-    QSignalSpy surfaceChangedSpy(cursor, &KWaylandServer::Cursor::surfaceChanged);
-    QSignalSpy enteredSerialChangedSpy(cursor, &KWaylandServer::Cursor::enteredSerialChanged);
-    QSignalSpy changedSpy(cursor, &KWaylandServer::Cursor::changed);
 
     // test changing hotspot
     p->setCursor(nullptr, QPoint(1, 2));
-    QVERIFY(hotspotChangedSpy.wait());
-    QCOMPARE(hotspotChangedSpy.count(), 1);
-    QCOMPARE(changedSpy.count(), 1);
+    QVERIFY(cursorChangedSpy.wait());
     QCOMPARE(cursorChangedSpy.count(), 2);
+    QCOMPARE(cursor->surface(), nullptr);
     QCOMPARE(cursor->hotspot(), QPoint(1, 2));
-    QVERIFY(enteredSerialChangedSpy.isEmpty());
-    QVERIFY(surfaceChangedSpy.isEmpty());
 
     // set surface
-    auto cursorSurface = m_compositor->createSurface(m_compositor);
-    QVERIFY(cursorSurface->isValid());
-    p->setCursor(cursorSurface, QPoint(1, 2));
-    QVERIFY(surfaceChangedSpy.wait());
-    QCOMPARE(surfaceChangedSpy.count(), 1);
-    QCOMPARE(changedSpy.count(), 2);
-    QCOMPARE(cursorChangedSpy.count(), 3);
-    QVERIFY(enteredSerialChangedSpy.isEmpty());
-    QCOMPARE(cursor->hotspot(), QPoint(1, 2));
-    QVERIFY(cursor->surface());
-
-    // and add an image to the surface
     QImage img(QSize(10, 20), QImage::Format_RGB32);
     img.fill(Qt::red);
+    auto cursorSurface = m_compositor->createSurface(m_compositor);
     cursorSurface->attachBuffer(m_shm->createBuffer(img));
     cursorSurface->damage(QRect(0, 0, 10, 20));
     cursorSurface->commit(KWayland::Client::Surface::CommitFlag::None);
-    QVERIFY(changedSpy.wait());
-    QCOMPARE(changedSpy.count(), 3);
-    QCOMPARE(cursorChangedSpy.count(), 4);
-    QCOMPARE(surfaceChangedSpy.count(), 1);
+    p->setCursor(cursorSurface, QPoint(1, 2));
+    QVERIFY(cursorChangedSpy.wait());
+    QCOMPARE(cursorChangedSpy.count(), 3);
+    QCOMPARE(cursor->hotspot(), QPoint(1, 2));
+    QVERIFY(cursor->surface());
     QCOMPARE(qobject_cast<ShmClientBuffer *>(cursor->surface()->buffer())->data(), img);
 
-    // and add another image to the surface
-    QImage blue(QSize(10, 20), QImage::Format_ARGB32_Premultiplied);
-    blue.fill(Qt::blue);
-    cursorSurface->attachBuffer(m_shm->createBuffer(blue));
-    cursorSurface->damage(QRect(0, 0, 10, 20));
-    cursorSurface->commit(KWayland::Client::Surface::CommitFlag::None);
-    QVERIFY(changedSpy.wait());
-    QCOMPARE(changedSpy.count(), 4);
-    QCOMPARE(cursorChangedSpy.count(), 5);
-    QCOMPARE(qobject_cast<ShmClientBuffer *>(cursor->surface()->buffer())->data(), blue);
-
     p->hideCursor();
-    QVERIFY(surfaceChangedSpy.wait());
-    QCOMPARE(changedSpy.count(), 5);
-    QCOMPARE(cursorChangedSpy.count(), 6);
-    QCOMPARE(surfaceChangedSpy.count(), 2);
+    QVERIFY(cursorChangedSpy.wait());
+    QCOMPARE(cursorChangedSpy.count(), 4);
     QVERIFY(!cursor->surface());
 }
 
-void TestWaylandSeat::testCursorDamage()
-{
-    // this test verifies that damaging a cursor surface triggers a cursor changed on the server
-    using namespace KWaylandServer;
-
-    QSignalSpy pointerSpy(m_seat, &KWayland::Client::Seat::hasPointerChanged);
-    m_seatInterface->setHasPointer(true);
-    QVERIFY(pointerSpy.wait());
-
-    // create pointer
-    std::unique_ptr<KWayland::Client::Pointer> p(m_seat->createPointer());
-    QVERIFY(p->isValid());
-    QSignalSpy enteredSpy(p.get(), &KWayland::Client::Pointer::entered);
-    // create surface
-    QSignalSpy surfaceCreatedSpy(m_compositorInterface, &CompositorInterface::surfaceCreated);
-    KWayland::Client::Surface *surface = m_compositor->createSurface(m_compositor);
-    QVERIFY(surfaceCreatedSpy.wait());
-    SurfaceInterface *serverSurface = surfaceCreatedSpy.first().first().value<KWaylandServer::SurfaceInterface *>();
-    QVERIFY(serverSurface);
-
-    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);
-    QSignalSpy committedSpy(serverSurface, &KWaylandServer::SurfaceInterface::committed);
-    QVERIFY(committedSpy.wait());
-
-    // send enter to the surface
-    m_seatInterface->notifyPointerEnter(serverSurface, QPointF(0, 0));
-    QVERIFY(enteredSpy.wait());
-
-    // create a signal spy for the cursor changed signal
-    auto pointer = m_seatInterface->pointer();
-    QSignalSpy cursorChangedSpy(pointer, &PointerInterface::cursorChanged);
-
-    // now let's set the cursor
-    KWayland::Client::Surface *cursorSurface = m_compositor->createSurface(m_compositor);
-    QVERIFY(cursorSurface);
-    QImage red(QSize(10, 10), QImage::Format_ARGB32_Premultiplied);
-    red.fill(Qt::red);
-    cursorSurface->attachBuffer(m_shm->createBuffer(red));
-    cursorSurface->damage(QRect(0, 0, 10, 10));
-    cursorSurface->commit(KWayland::Client::Surface::CommitFlag::None);
-    p->setCursor(cursorSurface, QPoint(0, 0));
-    QVERIFY(cursorChangedSpy.wait());
-    QCOMPARE(qobject_cast<ShmClientBuffer *>(pointer->cursor()->surface()->buffer())->data(), red);
-
-    // and damage the surface
-    QImage blue(QSize(10, 10), QImage::Format_ARGB32_Premultiplied);
-    blue.fill(Qt::blue);
-    cursorSurface->attachBuffer(m_shm->createBuffer(blue));
-    cursorSurface->damage(QRect(0, 0, 10, 10));
-    cursorSurface->commit(KWayland::Client::Surface::CommitFlag::None);
-    QVERIFY(cursorChangedSpy.wait());
-    QCOMPARE(qobject_cast<ShmClientBuffer *>(pointer->cursor()->surface()->buffer())->data(), blue);
-}
-
 void TestWaylandSeat::testKeyboard()
 {
     using namespace KWaylandServer;

src/wayland/pointer_interface.cpp

@@ -23,15 +23,8 @@ namespace KWaylandServer
 class CursorPrivate
 {
 public:
-    CursorPrivate(Cursor *q, PointerInterface *pointer);
-
-    Cursor *q;
-    PointerInterface *pointer;
-    quint32 enteredSerial = 0;
     QPointF hotspot;
     QPointer<SurfaceInterface> surface;
-
-    void update(SurfaceInterface *surface, quint32 serial, const QPointF &hotspot);
 };
 
 PointerInterfacePrivate *PointerInterfacePrivate::get(PointerInterface *pointer)
@@ -85,13 +78,12 @@ void PointerInterfacePrivate::pointer_set_cursor(Resource *resource, uint32_t se
     }
 
     if (!cursor) { // TODO: Assign the cursor surface role.
-        cursor = new Cursor(q);
-        cursor->d->update(surface, serial, QPointF(hotspot_x, hotspot_y) / focusedSurface->client()->scaleOverride());
-        QObject::connect(cursor, &Cursor::changed, q, &PointerInterface::cursorChanged);
-        Q_EMIT q->cursorChanged();
-    } else {
-        cursor->d->update(surface, serial, QPointF(hotspot_x, hotspot_y) / focusedSurface->client()->scaleOverride());
+        cursor = std::make_unique<Cursor>();
     }
+    cursor->d->hotspot = QPointF(hotspot_x, hotspot_y) / focusedSurface->client()->scaleOverride();
+    cursor->d->surface = surface;
+
+    Q_EMIT q->cursorChanged();
 }
 
 void PointerInterfacePrivate::pointer_release(Resource *resource)
@@ -350,7 +342,7 @@ void PointerInterface::sendFrame()
 
 Cursor *PointerInterface::cursor() const
 {
-    return d->cursor;
+    return d->cursor.get();
 }
 
 SeatInterface *PointerInterface::seat() const
@@ -366,44 +358,8 @@ PointerInterface *PointerInterface::get(wl_resource *native)
     return nullptr;
 }
 
-CursorPrivate::CursorPrivate(Cursor *q, PointerInterface *pointer)
-    : q(q)
-    , pointer(pointer)
-{
-}
-
-void CursorPrivate::update(SurfaceInterface *s, quint32 serial, const QPointF &p)
-{
-    bool emitChanged = false;
-    if (enteredSerial != serial) {
-        enteredSerial = serial;
-        emitChanged = true;
-        Q_EMIT q->enteredSerialChanged();
-    }
-    if (hotspot != p) {
-        hotspot = p;
-        emitChanged = true;
-        Q_EMIT q->hotspotChanged();
-    }
-    if (surface != s) {
-        if (!surface.isNull()) {
-            QObject::disconnect(surface.data(), &SurfaceInterface::committed, q, &Cursor::changed);
-        }
-        surface = s;
-        if (!surface.isNull()) {
-            QObject::connect(surface.data(), &SurfaceInterface::committed, q, &Cursor::changed);
-        }
-        emitChanged = true;
-        Q_EMIT q->surfaceChanged();
-    }
-    if (emitChanged) {
-        Q_EMIT q->changed();
-    }
-}
-
-Cursor::Cursor(PointerInterface *parent)
-    : QObject(parent)
-    , d(new CursorPrivate(this, parent))
+Cursor::Cursor()
+    : d(new CursorPrivate())
 {
 }
 
@@ -411,21 +367,11 @@ Cursor::~Cursor()
 {
 }
 
-quint32 Cursor::enteredSerial() const
-{
-    return d->enteredSerial;
-}
-
 QPointF Cursor::hotspot() const
 {
     return d->hotspot;
 }
 
-PointerInterface *Cursor::pointer() const
-{
-    return d->pointer;
-}
-
 SurfaceInterface *Cursor::surface() const
 {
     return d->surface;

src/wayland/pointer_interface.h

@@ -84,39 +84,24 @@ private:
 /**
  * @brief Class encapsulating a Cursor image.
  */
-class KWIN_EXPORT Cursor : public QObject
+class KWIN_EXPORT Cursor
 {
-    Q_OBJECT
-
 public:
-    virtual ~Cursor();
+    Cursor();
+    ~Cursor();
+
     /**
      * The hotspot of the cursor image in surface-relative coordinates.
      */
     QPointF hotspot() const;
-    /**
-     * The entered serial when the Cursor got set.
-     */
-    quint32 enteredSerial() const;
-    /**
-     * The PointerInterface this Cursor belongs to.
-     */
-    PointerInterface *pointer() const;
     /**
      * The SurfaceInterface for the image content of the Cursor.
      */
     SurfaceInterface *surface() const;
 
-Q_SIGNALS:
-    void hotspotChanged();
-    void enteredSerialChanged();
-    void surfaceChanged();
-    void changed();
-
 private:
     std::unique_ptr<CursorPrivate> d;
     friend class PointerInterfacePrivate;
-    explicit Cursor(PointerInterface *parent);
 };
 
 } // namespace KWaylandServer

src/wayland/pointer_interface_p.h

@@ -37,7 +37,7 @@ public:
     SeatInterface *seat;
     SurfaceInterface *focusedSurface = nullptr;
     QMetaObject::Connection destroyConnection;
-    Cursor *cursor = nullptr;
+    std::unique_ptr<Cursor> cursor;
     std::unique_ptr<RelativePointerV1Interface> relativePointersV1;
     std::unique_ptr<PointerSwipeGestureV1Interface> swipeGesturesV1;
     std::unique_ptr<PointerPinchGestureV1Interface> pinchGesturesV1;