xcb: fix bitmap cursor loading performance regression

... introduced by 422838685c.
Instead of completely droping caching for bitmap cursors we can do
the same what is done in libXcursor - have a fixed size cache, with
oldest entries eventually being replaced with new bitmaps. This
fixes the original issue, where the hash was growing indefinitely
until running out of file descriptors and won't have the performance
penalty as in 422838685c.

Task-number: QTBUG-66897
Change-Id: I14f80b46f97fd0e2c920e17a31ffbc0441cd9d22
Reviewed-by: Uli Schlachter <psychon@znc.in>
Reviewed-by: Robin Burchell <robin.burchell@crimson.no>
Reviewed-by: Laszlo Agocs <laszlo.agocs@qt.io>

src/plugins/platforms/xcb/qxcbcursor.cpp

@@ -301,6 +301,9 @@ QXcbCursorCacheKey::QXcbCursorCacheKey(const QCursor &c)
 QXcbCursor::QXcbCursor(QXcbConnection *conn, QXcbScreen *screen)
     : QXcbObject(conn), m_screen(screen), m_gtkCursorThemeInitialized(false)
 {
+    // see NUM_BITMAPS in libXcursor/src/xcursorint.h
+    m_bitmapCache.setMaxCost(8);
+
     if (cursorCount++)
         return;
 
@@ -351,37 +354,38 @@ QXcbCursor::~QXcbCursor()
 }
 
 #ifndef QT_NO_CURSOR
-void QXcbCursor::changeCursor(QCursor *cursor, QWindow *widget)
+void QXcbCursor::changeCursor(QCursor *cursor, QWindow *window)
 {
-    QXcbWindow *w = 0;
-    if (widget && widget->handle())
-        w = static_cast<QXcbWindow *>(widget->handle());
-    else
-        // No X11 cursor control when there is no widget under the cursor
+    if (!window || !window->handle())
         return;
 
     xcb_cursor_t c = XCB_CURSOR_NONE;
-    bool isBitmapCursor = false;
-
     if (cursor) {
-        const Qt::CursorShape shape = cursor->shape();
-        isBitmapCursor = shape == Qt::BitmapCursor;
-
-        if (!isBitmapCursor) {
         const QXcbCursorCacheKey key(*cursor);
-            CursorHash::iterator it = m_cursorHash.find(key);
-            if (it == m_cursorHash.end()) {
-                it = m_cursorHash.insert(key, createFontCursor(shape));
-            }
-            c = it.value();
+        const Qt::CursorShape shape = cursor->shape();
+
+        if (shape == Qt::BitmapCursor) {
+            auto *bitmap = m_bitmapCache.object(key);
+            if (bitmap) {
+                c = bitmap->cursor;
             } else {
-            // Do not cache bitmap cursors, as otherwise they have unclear
-            // lifetime (we effectively leak xcb_cursor_t).
                 c = createBitmapCursor(cursor);
+                m_bitmapCache.insert(key, new CachedCursor(xcb_connection(), c));
+            }
+        } else {
+            auto it = m_cursorHash.find(key);
+            if (it == m_cursorHash.end()) {
+                c = createFontCursor(shape);
+                m_cursorHash.insert(key, c);
+            } else {
+                c = it.value();
+            }
         }
     }
 
-    w->setCursor(c, isBitmapCursor);
+    auto *w = static_cast<QXcbWindow *>(window->handle());
+    xcb_change_window_attributes(xcb_connection(), w->xcb_window(), XCB_CW_CURSOR, &c);
+    xcb_flush(xcb_connection());
 }
 
 static int cursorIdForShape(int cshape)

src/plugins/platforms/xcb/qxcbcursor.h

@@ -43,6 +43,8 @@
 #include <qpa/qplatformcursor.h>
 #include "qxcbscreen.h"
 
+#include <QtCore/QCache>
+
 QT_BEGIN_NAMESPACE
 
 #ifndef QT_NO_CURSOR
@@ -76,7 +78,7 @@ public:
     QXcbCursor(QXcbConnection *conn, QXcbScreen *screen);
     ~QXcbCursor();
 #ifndef QT_NO_CURSOR
-    void changeCursor(QCursor *cursor, QWindow *widget) override;
+    void changeCursor(QCursor *cursor, QWindow *window) override;
 #endif
     QPoint pos() const override;
     void setPos(const QPoint &pos) override;
@@ -89,9 +91,20 @@ public:
 #endif
 
 private:
+
 #ifndef QT_NO_CURSOR
     typedef QHash<QXcbCursorCacheKey, xcb_cursor_t> CursorHash;
 
+    struct CachedCursor
+    {
+        explicit CachedCursor(xcb_connection_t *conn, xcb_cursor_t c)
+            : cursor(c), connection(conn) {}
+        ~CachedCursor() { xcb_free_cursor(connection, cursor); }
+        xcb_cursor_t cursor;
+        xcb_connection_t *connection;
+    };
+    typedef QCache<QXcbCursorCacheKey, CachedCursor> BitmapCursorCache;
+
     xcb_cursor_t createFontCursor(int cshape);
     xcb_cursor_t createBitmapCursor(QCursor *cursor);
     xcb_cursor_t createNonStandardCursor(int cshape);
@@ -100,6 +113,7 @@ private:
     QXcbScreen *m_screen;
 #ifndef QT_NO_CURSOR
     CursorHash m_cursorHash;
+    BitmapCursorCache m_bitmapCache;
 #endif
 #if QT_CONFIG(xcb_xlib) && QT_CONFIG(library)
     static void cursorThemePropertyChanged(QXcbVirtualDesktop *screen,

src/plugins/platforms/xcb/qxcbwindow.cpp

@@ -562,10 +562,6 @@ void QXcbWindow::create()
 
 QXcbWindow::~QXcbWindow()
 {
-    if (m_currentBitmapCursor != XCB_CURSOR_NONE) {
-        xcb_free_cursor(xcb_connection(), m_currentBitmapCursor);
-    }
-
     destroy();
 }
 
@@ -2591,24 +2587,6 @@ bool QXcbWindow::setMouseGrabEnabled(bool grab)
     return result;
 }
 
-void QXcbWindow::setCursor(xcb_cursor_t cursor, bool isBitmapCursor)
-{
-    xcb_connection_t *conn = xcb_connection();
-
-    xcb_change_window_attributes(conn, m_window, XCB_CW_CURSOR, &cursor);
-    xcb_flush(conn);
-
-    if (m_currentBitmapCursor != XCB_CURSOR_NONE) {
-        xcb_free_cursor(conn, m_currentBitmapCursor);
-    }
-
-    if (isBitmapCursor) {
-        m_currentBitmapCursor = cursor;
-    } else {
-        m_currentBitmapCursor = XCB_CURSOR_NONE;
-    }
-}
-
 void QXcbWindow::windowEvent(QEvent *event)
 {
     switch (event->type()) {

src/plugins/platforms/xcb/qxcbwindow.h

@@ -103,8 +103,6 @@ public:
     bool setKeyboardGrabEnabled(bool grab) override;
     bool setMouseGrabEnabled(bool grab) override;
 
-    void setCursor(xcb_cursor_t cursor, bool isBitmapCursor);
-
     QSurfaceFormat format() const override;
 
     void windowEvent(QEvent *event) override;
@@ -285,7 +283,6 @@ protected:
     SyncState m_syncState = NoSyncNeeded;
 
     QXcbSyncWindowRequest *m_pendingSyncRequest = nullptr;
-    xcb_cursor_t m_currentBitmapCursor = XCB_CURSOR_NONE;
 };
 
 class QXcbForeignWindow : public QXcbWindow