Client: Refactor cursors and fix various bugs

This patch is mostly a cleanup to prepare for implementations of
xcursor-configuration, but also fixes a couple of issues.

Most of the logic has now been moved out of QWaylandDisplay and QWaylandCursor
and into QWaylandInputDevice and QWaylandInputDevice::Pointer. QWaylandDisplay
now only contains mechanisms for avoiding loading the same theme multiple
times.

There is now only one setCursor method on QWaylandInputDevice, accepting a
QCursor and storing its values so changing scale factor doesn't require calling
setCursor again. QWaylandInputDevice::Pointer::updateCursor() is called
instead.

Cursor buffer scale is now set according to enter/leave events of the cursor
surface itself instead of the current window, this fixes incorrect buffer
scales for cursors on windows that span multiple outputs. The window buffer
scale can still be passed into the seat as a fallback until the first enter
event is received.

This also fixes a bug where the QWaylandBuffer of a bitmap cursor could be
deleted while it was being used as a cursor.

[ChangeLog][QPA plugin] Fixed a bug where the DPI of bitmap cursors were not
sent to the compositor, leading to the compositor incorrectly scaling the
cursor up or down.

[ChangeLog][QPA plugin] Fixed a bug where bitmap cursor hotspots were off when
the screen scale factor was different from the bitmap cursor device pixel
ratio.

Task-number: QTBUG-68571
Change-Id: I747a47ffff01b7b5f6a0ede3552ab37884c4fa60
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>

4 files changed, 192 insertions, 24 deletions

diff --git a/tests/auto/client/seatv4/seatv4.pro b/tests/auto/client/seatv4/seatv4.pro
index c02db5855..7a86cbf03 100644
--- a/tests/auto/client/seatv4/seatv4.pro
+++ b/tests/auto/client/seatv4/seatv4.pro
@@ -1,4 +1,9 @@
 include (../shared/shared.pri)
 
+qtConfig(cursor) {
+    QMAKE_USE += wayland-cursor
+    QT += gui-private
+}
+
 TARGET = tst_seatv4
 SOURCES += tst_seatv4.cpp
diff --git a/tests/auto/client/seatv4/tst_seatv4.cpp b/tests/auto/client/seatv4/tst_seatv4.cpp
index f1e948ee2..bc768fa61 100644
--- a/tests/auto/client/seatv4/tst_seatv4.cpp
+++ b/tests/auto/client/seatv4/tst_seatv4.cpp
@@ -30,6 +30,12 @@
 
 #include <QtGui/QRasterWindow>
 #include <QtGui/QOpenGLWindow>
+#if QT_CONFIG(cursor)
+#include <wayland-cursor.h>
+#include <QtGui/private/qguiapplication_p.h>
+#include <QtWaylandClient/private/qwaylanddisplay_p.h>
+#include <QtWaylandClient/private/qwaylandintegration_p.h>
+#endif
 
 using namespace MockCompositor;
 
@@ -59,6 +65,8 @@ class tst_seatv4 : public QObject, private SeatV4Compositor
 private slots:
     void cleanup();
     void bindsToSeat();
+    void keyboardKeyPress();
+#if QT_CONFIG(cursor)
     void createsPointer();
     void setsCursorOnEnter();
     void usesEnterSerial();
@@ -67,8 +75,10 @@ private slots:
     void simpleAxis();
     void invalidPointerEvents();
     void scaledCursor();
-
-    void keyboardKeyPress();
+    void bitmapCursor();
+    void hidpiBitmapCursor();
+    void hidpiBitmapCursorNonInt();
+#endif
 };
 
 void tst_seatv4::cleanup()
@@ -83,6 +93,31 @@ void tst_seatv4::bindsToSeat()
     QCOMPOSITOR_COMPARE(get<Seat>()->resourceMap().first()->version(), 4);
 }
 
+void tst_seatv4::keyboardKeyPress()
+{
+    class Window : public QRasterWindow {
+    public:
+        void keyPressEvent(QKeyEvent *) override { m_pressed = true; }
+        bool m_pressed = false;
+    };
+
+    Window window;
+    window.resize(64, 64);
+    window.show();
+    QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
+
+    uint keyCode = 80; // arbitrarily chosen
+    exec([&] {
+        auto *surface = xdgSurface()->m_surface;
+        keyboard()->sendEnter(surface);
+        keyboard()->sendKey(client(), keyCode, Keyboard::key_state_pressed);
+        keyboard()->sendKey(client(), keyCode, Keyboard::key_state_released);
+    });
+    QTRY_VERIFY(window.m_pressed);
+}
+
+#if QT_CONFIG(cursor)
+
 void tst_seatv4::createsPointer()
 {
     QCOMPOSITOR_TRY_COMPARE(pointer()->resourceMap().size(), 1);
@@ -253,9 +288,36 @@ void tst_seatv4::invalidPointerEvents()
     xdgPingAndWaitForPong();
 }
 
+static bool supportsCursorSize(uint size, wl_shm *shm)
+{
+    auto *theme = wl_cursor_theme_load(nullptr, size, shm);
+    if (!theme)
+        return false;
+
+    constexpr std::array<const char *, 4> names{"left_ptr", "default", "left_arrow", "top_left_arrow"};
+    for (const char *name : names) {
+        if (auto *cursor = wl_cursor_theme_get_cursor(theme, name)) {
+            auto *image = cursor->images[0];
+            return image->width == image->height && image->width == size;
+        }
+    }
+    return false;
+}
+
+static bool supportsCursorSizes(const QVector<uint> &sizes)
+{
+    auto *waylandIntegration = static_cast<QtWaylandClient::QWaylandIntegration *>(QGuiApplicationPrivate::platformIntegration());
+    wl_shm *shm = waylandIntegration->display()->shm()->object();
+    return std::all_of(sizes.begin(), sizes.end(), [=](uint size) {
+        return supportsCursorSize(size, shm);
+    });
+}
+
 void tst_seatv4::scaledCursor()
 {
-    QSKIP("Currently broken and should be fixed");
+    if (!supportsCursorSizes({32, 64}))
+        QSKIP("Cursor themes with sizes 32 and 64 not found.");
+
     // Add a highdpi output
     exec([&] {
         OutputData d;
@@ -289,28 +351,122 @@ void tst_seatv4::scaledCursor()
     exec([&] { remove(output(1)); });
 }
 
-void tst_seatv4::keyboardKeyPress()
+void tst_seatv4::bitmapCursor()
 {
-    class Window : public QRasterWindow {
-    public:
-        void keyPressEvent(QKeyEvent *) override { m_pressed = true; }
-        bool m_pressed = false;
-    };
+    // Add a highdpi output
+    exec([&] {
+        OutputData d;
+        d.scale = 2;
+        d.position = {1920, 0};
+        add<Output>(d);
+    });
 
-    Window window;
+    QRasterWindow window;
     window.resize(64, 64);
+
+    QPixmap pixmap(24, 24);
+    pixmap.setDevicePixelRatio(1);
+    QPoint hotspot(12, 12); // In device pixel coordinates
+    QCursor cursor(pixmap, hotspot.x(), hotspot.y());
+    window.setCursor(cursor);
+
     window.show();
     QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
 
-    uint keyCode = 80; // arbitrarily chosen
+    exec([=] { pointer()->sendEnter(xdgSurface()->m_surface, {32, 32}); });
+    QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface());
+    QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface()->m_committed.buffer);
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size(), QSize(24, 24));
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 1);
+    QCOMPOSITOR_COMPARE(pointer()->m_hotspot, QPoint(12, 12));
+
+    exec([=] {
+        auto *surface = pointer()->cursorSurface();
+        surface->sendEnter(getAll<Output>()[1]);
+        surface->sendLeave(getAll<Output>()[0]);
+    });
+
+    xdgPingAndWaitForPong();
+
+    // Everything should remain the same, the cursor still has dpr 1
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 1);
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size(), QSize(24, 24));
+    QCOMPOSITOR_COMPARE(pointer()->m_hotspot, QPoint(12, 12));
+
+    // Remove the extra output to clean up for the next test
+    exec([&] { remove(getAll<Output>()[1]); });
+}
+
+void tst_seatv4::hidpiBitmapCursor()
+{
+    // Add a highdpi output
     exec([&] {
-        auto *surface = xdgSurface()->m_surface;
-        keyboard()->sendEnter(surface);
-        keyboard()->sendKey(client(), keyCode, Keyboard::key_state_pressed);
-        keyboard()->sendKey(client(), keyCode, Keyboard::key_state_released);
+        OutputData d;
+        d.scale = 2;
+        d.position = {1920, 0};
+        add<Output>(d);
     });
-    QTRY_VERIFY(window.m_pressed);
+
+    QRasterWindow window;
+    window.resize(64, 64);
+
+    QPixmap pixmap(48, 48);
+    pixmap.setDevicePixelRatio(2);
+    QPoint hotspot(12, 12); // In device pixel coordinates
+    QCursor cursor(pixmap, hotspot.x(), hotspot.y());
+    window.setCursor(cursor);
+
+    window.show();
+    QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
+
+    exec([=] { pointer()->sendEnter(xdgSurface()->m_surface, {32, 32}); });
+    QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface());
+    QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface()->m_committed.buffer);
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size(), QSize(48, 48));
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 2);
+    QCOMPOSITOR_COMPARE(pointer()->m_hotspot, QPoint(12, 12));
+
+    exec([=] {
+        auto *surface = pointer()->cursorSurface();
+        surface->sendEnter(getAll<Output>()[1]);
+        surface->sendLeave(getAll<Output>()[0]);
+    });
+
+    xdgPingAndWaitForPong();
+
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 2);
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size(), QSize(48, 48));
+    QCOMPOSITOR_COMPARE(pointer()->m_hotspot, QPoint(12, 12));
+
+    // Remove the extra output to clean up for the next test
+    exec([&] { remove(getAll<Output>()[1]); });
 }
 
+void tst_seatv4::hidpiBitmapCursorNonInt()
+{
+    QRasterWindow window;
+    window.resize(64, 64);
+
+    QPixmap pixmap(100, 100);
+    pixmap.setDevicePixelRatio(2.5); // dpr width is now 100 / 2.5 = 40
+    QPoint hotspot(20, 20); // In device pixel coordinates (middle of buffer)
+    QCursor cursor(pixmap, hotspot.x(), hotspot.y());
+    window.setCursor(cursor);
+
+    window.show();
+    QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
+
+    exec([=] { pointer()->sendEnter(xdgSurface()->m_surface, {32, 32}); });
+    QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface());
+    QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface()->m_committed.buffer);
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size(), QSize(100, 100));
+    QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 2);
+    // Verify that the hotspot was scaled correctly
+    // Surface size is now 100 / 2 = 50, so the middle should be at 25 in surface coordinates
+    QCOMPOSITOR_COMPARE(pointer()->m_hotspot, QPoint(25, 25));
+}
+
+#endif // QT_CONFIG(cursor)
+
 QCOMPOSITOR_TEST_MAIN(tst_seatv4)
 #include "tst_seatv4.moc"
diff --git a/tests/auto/client/shared/coreprotocol.cpp b/tests/auto/client/shared/coreprotocol.cpp
index 6f51a9793..c4dc3f341 100644
--- a/tests/auto/client/shared/coreprotocol.cpp
+++ b/tests/auto/client/shared/coreprotocol.cpp
@@ -258,18 +258,19 @@ uint Pointer::sendEnter(Surface *surface, const QPointF &position)
 {
     wl_fixed_t x = wl_fixed_from_double(position.x());
     wl_fixed_t y = wl_fixed_from_double(position.y());
-    m_enterSerial = m_seat->m_compositor->nextSerial();
+
+    uint serial = m_seat->m_compositor->nextSerial();
+    m_enterSerials << serial;
 
     wl_client *client = surface->resource()->client();
     const auto pointerResources = resourceMap().values(client);
     for (auto *r : pointerResources)
-        wl_pointer::send_enter(r->handle, m_enterSerial, surface->resource()->handle, x ,y);
-    return m_enterSerial;
+        wl_pointer::send_enter(r->handle, serial, surface->resource()->handle, x ,y);
+    return serial;
 }
 
 uint Pointer::sendLeave(Surface *surface)
 {
-    m_enterSerial = 0;
     uint serial = m_seat->m_compositor->nextSerial();
 
     wl_client *client = surface->resource()->client();
@@ -315,8 +316,6 @@ void Pointer::sendAxis(wl_client *client, axis axis, qreal value)
 void Pointer::pointer_set_cursor(Resource *resource, uint32_t serial, wl_resource *surface, int32_t hotspot_x, int32_t hotspot_y)
 {
     Q_UNUSED(resource);
-    Q_UNUSED(hotspot_x);
-    Q_UNUSED(hotspot_y);
     auto *s = fromResource<Surface>(surface);
     QVERIFY(s);
 
@@ -328,7 +327,14 @@ void Pointer::pointer_set_cursor(Resource *resource, uint32_t serial, wl_resourc
         m_cursorRole = new CursorRole(s); //TODO: make sure we don't leak CursorRole
         s->m_role = m_cursorRole;
     }
-//    QCOMPARE(serial, m_enterSerial); //TODO: uncomment when this bug is fixed
+
+    // Directly checking the last serial would be racy, we may just have sent leaves/enters which
+    // the client hasn't yet seen. Instead just check if the serial matches an enter serial since
+    // the last time the client sent a set_cursor request.
+    QVERIFY(m_enterSerials.contains(serial));
+    while (m_enterSerials.first() < serial) { m_enterSerials.removeFirst(); }
+
+    m_hotspot = QPoint(hotspot_x, hotspot_y);
     emit setCursor(serial);
 }
 
diff --git a/tests/auto/client/shared/coreprotocol.h b/tests/auto/client/shared/coreprotocol.h
index 249c16f42..699dcbded 100644
--- a/tests/auto/client/shared/coreprotocol.h
+++ b/tests/auto/client/shared/coreprotocol.h
@@ -284,7 +284,8 @@ public:
     void sendAxis(wl_client *client, axis axis, qreal value);
 
     Seat *m_seat = nullptr;
-    uint m_enterSerial = 0;
+    QVector<uint> m_enterSerials;
+    QPoint m_hotspot;
 
 signals:
     void setCursor(uint serial); //TODO: add arguments?