From cba414a26b58b867d0c46ac214606e29e4bbdd94 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Thu, 15 Mar 2018 18:07:48 +0100 Subject: xcb: cleanup the code for detecting when to create/destroy SHM segment - removed the check for "m_segmentSize > 0" as according to the code it will never be <= 0. - wrap the entire logic in connection()->hasShm() { .. } as that is when the logic becomes relevant. This makes the code more readable. Change-Id: I572420df8e29cc46593f8a13c250f8c05c6a9108 Reviewed-by: Laszlo Agocs --- src/plugins/platforms/xcb/qxcbbackingstore.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'src/plugins/platforms/xcb') diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp index 659d1c53cb..8cfcc49f9a 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -219,12 +219,14 @@ void QXcbBackingStoreImage::create(const QSize &size, const xcb_format_t *fmt, Q if (!segmentSize) return; - if (hasShm() && m_segmentSize > 0 && (m_segmentSize < segmentSize || m_segmentSize / 2 >= segmentSize)) - destroyShmSegment(m_segmentSize); - if (!hasShm() && connection()->hasShm()) - { - qCDebug(lcQpaXcb) << "creating shared memory" << segmentSize << "for" << size << "depth" << fmt->depth << "bits" << fmt->bits_per_pixel; - createShmSegment(segmentSize); + if (connection()->hasShm()) { + if (m_shm_info.shmaddr && (m_segmentSize < segmentSize || m_segmentSize / 2 >= segmentSize)) + destroyShmSegment(m_segmentSize); + if (!m_shm_info.shmaddr) { + qCDebug(lcQpaXcb) << "creating shared memory" << segmentSize << "for" + << size << "depth" << fmt->depth << "bits" << fmt->bits_per_pixel; + createShmSegment(segmentSize); + } } m_xcb_image->data = m_shm_info.shmaddr ? m_shm_info.shmaddr : (uint8_t *)malloc(segmentSize); -- cgit v1.2.3 From 7286d9d8dd1f8543007218a91d5c6767a7a7b152 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Fri, 23 Mar 2018 14:24:23 +0100 Subject: xcb: fix bitmap cursor loading performance regression ... introduced by 422838685c31d9b57133a8711bfd5db92095d96d. 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 422838685c31d9b57133a8711bfd5db92095d96d. Task-number: QTBUG-66897 Change-Id: I14f80b46f97fd0e2c920e17a31ffbc0441cd9d22 Reviewed-by: Uli Schlachter Reviewed-by: Robin Burchell Reviewed-by: Laszlo Agocs --- src/plugins/platforms/xcb/qxcbcursor.cpp | 42 +++++++++++++++++--------------- src/plugins/platforms/xcb/qxcbcursor.h | 16 +++++++++++- src/plugins/platforms/xcb/qxcbwindow.cpp | 22 ----------------- src/plugins/platforms/xcb/qxcbwindow.h | 3 --- 4 files changed, 38 insertions(+), 45 deletions(-) (limited to 'src/plugins/platforms/xcb') diff --git a/src/plugins/platforms/xcb/qxcbcursor.cpp b/src/plugins/platforms/xcb/qxcbcursor.cpp index 34b7d0d236..70138abede 100644 --- a/src/plugins/platforms/xcb/qxcbcursor.cpp +++ b/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(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 QXcbCursorCacheKey key(*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)); + if (shape == Qt::BitmapCursor) { + auto *bitmap = m_bitmapCache.object(key); + if (bitmap) { + c = bitmap->cursor; + } else { + c = createBitmapCursor(cursor); + m_bitmapCache.insert(key, new CachedCursor(xcb_connection(), c)); } - c = it.value(); } else { - // Do not cache bitmap cursors, as otherwise they have unclear - // lifetime (we effectively leak xcb_cursor_t). - c = createBitmapCursor(cursor); + 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(window->handle()); + xcb_change_window_attributes(xcb_connection(), w->xcb_window(), XCB_CW_CURSOR, &c); + xcb_flush(xcb_connection()); } static int cursorIdForShape(int cshape) diff --git a/src/plugins/platforms/xcb/qxcbcursor.h b/src/plugins/platforms/xcb/qxcbcursor.h index e3f88518fe..5bc806381c 100644 --- a/src/plugins/platforms/xcb/qxcbcursor.h +++ b/src/plugins/platforms/xcb/qxcbcursor.h @@ -43,6 +43,8 @@ #include #include "qxcbscreen.h" +#include + 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 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 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, diff --git a/src/plugins/platforms/xcb/qxcbwindow.cpp b/src/plugins/platforms/xcb/qxcbwindow.cpp index fed096f311..f87da28ee0 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.cpp +++ b/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()) { diff --git a/src/plugins/platforms/xcb/qxcbwindow.h b/src/plugins/platforms/xcb/qxcbwindow.h index 957c4e9cbd..cb8af9e666 100644 --- a/src/plugins/platforms/xcb/qxcbwindow.h +++ b/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 -- cgit v1.2.3 From d5ac11891d8237ca2f02390ffd0ff103578b520e Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Tue, 10 Apr 2018 11:50:31 +0200 Subject: xcb: prevent crash with pixmap cursors on XRender-less X servers We were using xcb_render_* APIs without checking if the server even supports this extension. Attempting to use an extension which is not present will always result in a crash. This patch adds the required guards and refactors how we detect presence of XRender extension. Also instead of falling back to some odd-looking bitmapped version just leave the current cursor unchanged. That is how we did it in Qt4 AFAICT. Task-number: QTBUG-66935 Change-Id: I4f27f1d65a77563ec34f3e0e94492c9236d7f9a6 Reviewed-by: Laszlo Agocs --- src/plugins/platforms/xcb/qxcbconnection.cpp | 11 ++++++++--- src/plugins/platforms/xcb/qxcbconnection.h | 10 +++++++++- src/plugins/platforms/xcb/qxcbcursor.cpp | 15 +++++++++++---- 3 files changed, 28 insertions(+), 8 deletions(-) (limited to 'src/plugins/platforms/xcb') diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp index 2431f99ab8..444e3a7669 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.cpp +++ b/src/plugins/platforms/xcb/qxcbconnection.cpp @@ -2136,17 +2136,22 @@ void QXcbConnection::initializeXRender() { #if QT_CONFIG(xcb_render) const xcb_query_extension_reply_t *reply = xcb_get_extension_data(m_connection, &xcb_render_id); - if (!reply || !reply->present) + if (!reply || !reply->present) { + qCDebug(lcQpaXcb, "XRender extension not present on the X server"); return; + } auto xrender_query = Q_XCB_REPLY(xcb_render_query_version, m_connection, XCB_RENDER_MAJOR_VERSION, XCB_RENDER_MINOR_VERSION); - if (!xrender_query || (xrender_query->major_version == 0 && xrender_query->minor_version < 5)) { - qWarning("QXcbConnection: Failed to initialize XRender"); + if (!xrender_query) { + qCWarning(lcQpaXcb, "xcb_render_query_version failed"); return; } + has_render_extension = true; + m_xrenderVersion.first = xrender_query->major_version; + m_xrenderVersion.second = xrender_query->minor_version; #endif } diff --git a/src/plugins/platforms/xcb/qxcbconnection.h b/src/plugins/platforms/xcb/qxcbconnection.h index ced5208c81..d9321d94d0 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.h +++ b/src/plugins/platforms/xcb/qxcbconnection.h @@ -478,7 +478,13 @@ public: bool hasXRandr() const { return has_randr_extension; } bool hasInputShape() const { return has_input_shape; } bool hasXKB() const { return has_xkb; } - bool hasXRender() const { return has_render_extension; } + bool hasXRender(int major = -1, int minor = -1) const + { + if (has_render_extension && major != -1 && minor != -1) + return m_xrenderVersion >= qMakePair(major, minor); + + return has_render_extension; + } bool hasXInput2() const { return m_xi2Enabled; } bool hasShm() const { return has_shm; } bool hasShmFd() const { return has_shm_fd; } @@ -707,6 +713,8 @@ private: bool has_shm = false; bool has_shm_fd = false; + QPair m_xrenderVersion; + Qt::MouseButtons m_buttonState = 0; Qt::MouseButton m_button = Qt::NoButton; diff --git a/src/plugins/platforms/xcb/qxcbcursor.cpp b/src/plugins/platforms/xcb/qxcbcursor.cpp index 70138abede..8d151b760b 100644 --- a/src/plugins/platforms/xcb/qxcbcursor.cpp +++ b/src/plugins/platforms/xcb/qxcbcursor.cpp @@ -601,12 +601,19 @@ xcb_cursor_t QXcbCursor::createFontCursor(int cshape) xcb_cursor_t QXcbCursor::createBitmapCursor(QCursor *cursor) { - xcb_connection_t *conn = xcb_connection(); QPoint spot = cursor->hotSpot(); xcb_cursor_t c = XCB_NONE; - if (cursor->pixmap().depth() > 1) - c = qt_xcb_createCursorXRender(m_screen, cursor->pixmap().toImage(), spot); - if (!c) { + if (cursor->pixmap().depth() > 1) { +#if QT_CONFIG(xcb_render) + if (connection()->hasXRender(0, 5)) + c = qt_xcb_createCursorXRender(m_screen, cursor->pixmap().toImage(), spot); + else + qCWarning(lcQpaXcb, "xrender >= 0.5 required to create pixmap cursors"); +#else + qCWarning(lcQpaXcb, "This build of Qt does not support pixmap cursors"); +#endif + } else { + xcb_connection_t *conn = xcb_connection(); xcb_pixmap_t cp = qt_xcb_XPixmapFromBitmap(m_screen, cursor->bitmap()->toImage()); xcb_pixmap_t mp = qt_xcb_XPixmapFromBitmap(m_screen, cursor->mask()->toImage()); c = xcb_generate_id(conn); -- cgit v1.2.3