wayland: Fix cross cursor in Xwayland apps

Startup code in plasmashell was changed so xsetroot is not called
anymore, which is sort of fine.

Unfortunately (or not?), it exposed a bug in kwin. Cursor::x11Cursor()
only works in the standalone X11 session.

On Wayland, Cursor::x11Cursor() will return XCB_NONE which results in
seeing cross cursor when there should be arrow cursor.

This change moves xcb_cursor_t look up code from X11Cursor to the base
Cursor class. In hindsight, I would like to introduce a window manager
class where the xcb cursor and other x11 specific code can be moved in
the future for better encapsulation of platform-specific code.

CCBUG: 442539

autotests/CMakeLists.txt

@@ -190,6 +190,8 @@ target_link_libraries(testScriptedEffectLoader
     KF5::Notifications
     KF5::Package
 
+    XCB::CURSOR
+
     kwineffects
     kwin4_effect_builtins
 )

src/CMakeLists.txt

@@ -212,6 +212,7 @@ target_link_libraries(kwin
     Plasma::KWaylandServer
 
     XCB::COMPOSITE
+    XCB::CURSOR
     XCB::DAMAGE
     XCB::GLX
     XCB::ICCCM

src/cursor.cpp

@@ -24,6 +24,8 @@
 #include <QScreen>
 #include <QTimer>
 
+#include <xcb/xcb_cursor.h>
+
 namespace KWin
 {
 Cursors *Cursors::s_self = nullptr;
@@ -125,6 +127,7 @@ void Cursor::updateTheme(const QString &name, int size)
     if (m_themeName != name || m_themeSize != size) {
         m_themeName = name;
         m_themeSize = size;
+        m_cursors.clear();
         Q_EMIT themeChanged();
     }
 }
@@ -180,26 +183,44 @@ void Cursor::updateCursor(const QImage &image, const QPoint &hotspot)
     Q_EMIT cursorChanged();
 }
 
-xcb_cursor_t Cursor::getX11Cursor(CursorShape shape)
-{
-    Q_UNUSED(shape)
-    return XCB_CURSOR_NONE;
-}
-
-xcb_cursor_t Cursor::getX11Cursor(const QByteArray &name)
-{
-    Q_UNUSED(name)
-    return XCB_CURSOR_NONE;
-}
-
 xcb_cursor_t Cursor::x11Cursor(CursorShape shape)
 {
-    return getX11Cursor(shape);
+    return x11Cursor(shape.name());
 }
 
 xcb_cursor_t Cursor::x11Cursor(const QByteArray &name)
 {
-    return getX11Cursor(name);
+    Q_ASSERT(kwinApp()->x11Connection());
+    auto it = m_cursors.constFind(name);
+    if (it != m_cursors.constEnd()) {
+        return it.value();
+    }
+
+    if (name.isEmpty()) {
+        return XCB_CURSOR_NONE;
+    }
+
+    xcb_cursor_context_t *ctx;
+    if (xcb_cursor_context_new(kwinApp()->x11Connection(), kwinApp()->x11DefaultScreen(), &ctx) < 0) {
+        return XCB_CURSOR_NONE;
+    }
+
+    xcb_cursor_t cursor = xcb_cursor_load_cursor(ctx, name.constData());
+    if (cursor == XCB_CURSOR_NONE) {
+        const auto &names = Cursor::cursorAlternativeNames(name);
+        for (const QByteArray &cursorName : names) {
+            cursor = xcb_cursor_load_cursor(ctx, cursorName.constData());
+            if (cursor != XCB_CURSOR_NONE) {
+                break;
+            }
+        }
+    }
+    if (cursor != XCB_CURSOR_NONE) {
+        m_cursors.insert(name, cursor);
+    }
+
+    xcb_cursor_context_free(ctx);
+    return cursor;
 }
 
 void Cursor::doSetPos()

src/cursor.h

@@ -189,18 +189,6 @@ Q_SIGNALS:
     void rendered(const QRect &geometry);
 
 protected:
-    /**
-     * Called from x11Cursor to actually retrieve the X11 cursor. Base implementation returns
-     * a null cursor, an implementing subclass should implement this method if it can provide X11
-     * mouse cursors.
-     */
-    virtual xcb_cursor_t getX11Cursor(CursorShape shape);
-    /**
-     * Called from x11Cursor to actually retrieve the X11 cursor. Base implementation returns
-     * a null cursor, an implementing subclass should implement this method if it can provide X11
-     * mouse cursors.
-     */
-    virtual xcb_cursor_t getX11Cursor(const QByteArray &name);
     /**
      * Performs the actual warping of the cursor.
      */
@@ -251,6 +239,7 @@ private Q_SLOTS:
 private:
     void updateTheme(const QString &name, int size);
     void loadThemeFromKConfig();
+    QHash<QByteArray, xcb_cursor_t > m_cursors;
     QPoint m_pos;
     QPoint m_hotspot;
     QImage m_image;

src/plugins/platforms/x11/standalone/CMakeLists.txt

@@ -17,7 +17,7 @@ set(X11PLATFORM_SOURCES
 
 add_library(KWinX11Platform MODULE ${X11PLATFORM_SOURCES})
 set_target_properties(KWinX11Platform PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin/org.kde.kwin.platforms/")
-target_link_libraries(KWinX11Platform eglx11common kwin kwinxrenderutils SceneOpenGLBackend VsyncSupport Qt::X11Extras XCB::CURSOR KF5::Crash X11::X11)
+target_link_libraries(KWinX11Platform eglx11common kwin kwinxrenderutils SceneOpenGLBackend VsyncSupport Qt::X11Extras KF5::Crash X11::X11)
 if (X11_Xi_FOUND)
     target_sources(KWinX11Platform PRIVATE xinputintegration.cpp)
     target_link_libraries(KWinX11Platform X11::Xi)

src/plugins/platforms/x11/standalone/x11cursor.cpp

@@ -16,8 +16,6 @@
 #include <QAbstractEventDispatcher>
 #include <QTimer>
 
-#include <xcb/xcb_cursor.h>
-
 namespace KWin
 {
 
@@ -37,8 +35,6 @@ X11Cursor::X11Cursor(QObject *parent, bool xInputSupport)
     m_mousePollingTimer->setInterval(50);
     connect(m_mousePollingTimer, &QTimer::timeout, this, &X11Cursor::mousePolled);
 
-    connect(this, &Cursor::themeChanged, this, [this] { m_cursors.clear(); });
-
     if (m_hasXInput) {
         connect(qApp->eventDispatcher(), &QAbstractEventDispatcher::aboutToBlock, this, &X11Cursor::aboutToBlock);
     }
@@ -134,46 +130,6 @@ void X11Cursor::mousePolled()
     }
 }
 
-xcb_cursor_t X11Cursor::getX11Cursor(CursorShape shape)
-{
-    return getX11Cursor(shape.name());
-}
-
-xcb_cursor_t X11Cursor::getX11Cursor(const QByteArray &name)
-{
-    auto it = m_cursors.constFind(name);
-    if (it != m_cursors.constEnd()) {
-        return it.value();
-    }
-    return createCursor(name);
-}
-
-xcb_cursor_t X11Cursor::createCursor(const QByteArray &name)
-{
-    if (name.isEmpty()) {
-        return XCB_CURSOR_NONE;
-    }
-    xcb_cursor_context_t *ctx;
-    if (xcb_cursor_context_new(kwinApp()->x11Connection(), kwinApp()->x11DefaultScreen(), &ctx) < 0) {
-        return XCB_CURSOR_NONE;
-    }
-    xcb_cursor_t cursor = xcb_cursor_load_cursor(ctx, name.constData());
-    if (cursor == XCB_CURSOR_NONE) {
-        const auto &names = cursorAlternativeNames(name);
-        for (auto cit = names.begin(); cit != names.end(); ++cit) {
-            cursor = xcb_cursor_load_cursor(ctx, (*cit).constData());
-            if (cursor != XCB_CURSOR_NONE) {
-                break;
-            }
-        }
-    }
-    if (cursor != XCB_CURSOR_NONE) {
-        m_cursors.insert(name, cursor);
-    }
-    xcb_cursor_context_free(ctx);
-    return cursor;
-}
-
 void X11Cursor::notifyCursorChanged()
 {
     if (!isCursorTracking()) {

src/plugins/platforms/x11/standalone/x11cursor.h

@@ -35,8 +35,6 @@ public:
     void notifyCursorChanged();
 
 protected:
-    xcb_cursor_t getX11Cursor(CursorShape shape) override;
-    xcb_cursor_t getX11Cursor(const QByteArray &name) override;
     void doSetPos() override;
     void doGetPos() override;
     void doStartMousePolling() override;
@@ -54,8 +52,6 @@ private Q_SLOTS:
     void mousePolled();
     void aboutToBlock();
 private:
-    xcb_cursor_t createCursor(const QByteArray &name);
-    QHash<QByteArray, xcb_cursor_t > m_cursors;
     xcb_timestamp_t m_timeStamp;
     uint16_t m_buttonMask;
     QTimer *m_resetTimeStampTimer;