From 843c98d2d2488eeafc08412b6cc2992ad8b4e536 Mon Sep 17 00:00:00 2001 From: Paul Olav Tvete Date: Fri, 8 Feb 2019 13:16:53 +0100 Subject: [PATCH 01/11] Fix race condition in autotests The QVector m_surface is modified in both threads. Change-Id: I1818a5e1307d191f1613513b86703eaa7bda3c6e Reviewed-by: Johan Helsing --- .../wayland/shared_old/mockcompositor.cpp | 24 ++++++++++++------- .../auto/wayland/shared_old/mockcompositor.h | 5 +++- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/tests/auto/wayland/shared_old/mockcompositor.cpp b/tests/auto/wayland/shared_old/mockcompositor.cpp index f6a8757472c..0dfaef5eaa6 100644 --- a/tests/auto/wayland/shared_old/mockcompositor.cpp +++ b/tests/auto/wayland/shared_old/mockcompositor.cpp @@ -220,12 +220,14 @@ QSharedPointer MockCompositor::surface() { QSharedPointer result; lock(); - QVector surfaces = m_compositor->surfaces(); - foreach (Impl::Surface *surface, surfaces) { - // we don't want to mistake the cursor surface for a window surface - if (surface->isMapped()) { - result = surface->mockSurface(); - break; + { + QVector surfaces = m_compositor->surfaces(); + foreach (Impl::Surface *surface, surfaces) { + // we don't want to mistake the cursor surface for a window surface + if (surface->isMapped()) { + result = surface->mockSurface(); + break; + } } } unlock(); @@ -307,7 +309,7 @@ void *MockCompositor::run(void *data) { MockCompositor *controller = static_cast(data); - Impl::Compositor compositor; + Impl::Compositor compositor(controller); controller->m_compositor = &compositor; controller->m_waitCondition.wakeOne(); @@ -332,8 +334,8 @@ void *MockCompositor::run(void *data) namespace Impl { -Compositor::Compositor() - : m_display(wl_display_create()) +Compositor::Compositor(MockCompositor *mockCompositor) + : m_mockCompositor(mockCompositor), m_display(wl_display_create()) { if (wl_display_add_socket(m_display, 0)) { fprintf(stderr, "Fatal: Failed to open server socket\n"); @@ -445,15 +447,19 @@ uint32_t Compositor::nextSerial() void Compositor::addSurface(Surface *surface) { + m_mockCompositor->lock(); m_surfaces << surface; + m_mockCompositor->unlock(); } void Compositor::removeSurface(Surface *surface) { + m_mockCompositor->lock(); m_surfaces.removeOne(surface); m_keyboard->handleSurfaceDestroyed(surface); m_pointer->handleSurfaceDestroyed(surface); m_fullScreenShellV1->removeSurface(surface); + m_mockCompositor->unlock(); } Surface *Compositor::resolveSurface(const QVariant &v) diff --git a/tests/auto/wayland/shared_old/mockcompositor.h b/tests/auto/wayland/shared_old/mockcompositor.h index 404a18e7550..2433ac005da 100644 --- a/tests/auto/wayland/shared_old/mockcompositor.h +++ b/tests/auto/wayland/shared_old/mockcompositor.h @@ -45,6 +45,8 @@ #include #include +class MockCompositor; + namespace Impl { typedef void (**Implementation)(void); @@ -63,7 +65,7 @@ class XdgShellV6; class Compositor { public: - Compositor(); + Compositor(MockCompositor *mockCompositor); ~Compositor(); int fileDescriptor() const { return m_fd; } @@ -114,6 +116,7 @@ private: void initShm(); + MockCompositor *m_mockCompositor = nullptr; QRect m_outputGeometry; wl_display *m_display = nullptr; From e1eec6874231c4405dcd786d56318d9df2780ed3 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 4 Feb 2019 16:34:52 +0100 Subject: [PATCH 02/11] Client xdg-shell: Fix crash when switching popups The call to flushWindowSystemEvents() sometimes caused new popups to be shown in the middle of hiding another. I.e. if multiple events show and hide surfaces, they would be shown/hidden in opposite order of their corresponding events. QMenus sometimes suffered from this (can be seen with the qopenglwidget example from qtbase). When the flush was added 5 years ago in 70c578cb5, it was to "reduce the chances of seeing a bad frame". I don't see any rendering artifacts, though, and I can't find any bug report on it. So let's hope it's safe to remove the hack. [ChangeLog][QPA plugin] Fixed a crash that sometimes happened when switching popups. Also adds more info to the workaround warning message that appears when a popup grab is attempted and there already is another grabbing popup that is not the parent. Fixes: QTBUG-73524 Change-Id: Ibfcbb48c4bbe295c2be1a30add2d7e05cad398c5 Reviewed-by: Paul Olav Tvete --- .../shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp | 9 ++++++++- .../shellintegration/xdg-shell/qwaylandxdgshell.cpp | 9 ++++++++- src/plugins/platforms/wayland/qwaylandwindow.cpp | 9 ++------- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp index 9e55e3e1680..2f729d9717b 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp @@ -335,7 +335,14 @@ void QWaylandXdgSurfaceV6::setGrabPopup(QWaylandWindow *parent, QWaylandInputDev auto *top = m_shell->m_topmostGrabbingPopup; if (top && top->m_xdgSurface != parentXdgSurface) { - qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; + qCWarning(lcQpaWayland) << "setGrabPopup called with a parent," << parentXdgSurface + << "which does not match the current topmost grabbing popup," + << top->m_xdgSurface << "According to the xdg-shell-v6 protocol, this" + << "is not allowed. The wayland QPA plugin is currently handling" + << "it by setting the parent to the topmost grabbing popup." + << "Note, however, that this may cause positioning errors and" + << "popups closing unxpectedly because xdg-shell-v6 mandate that child" + << "popups close before parents"; parent = top->m_xdgSurface->m_window; } setPopup(parent); diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index 1310e340da2..d2452aa483d 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -369,7 +369,14 @@ void QWaylandXdgSurface::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevic auto *top = m_shell->m_topmostGrabbingPopup; if (top && top->m_xdgSurface != parentXdgSurface) { - qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; + qCWarning(lcQpaWayland) << "setGrabPopup called with a parent," << parentXdgSurface + << "which does not match the current topmost grabbing popup," + << top->m_xdgSurface << "According to the xdg-shell protocol, this" + << "is not allowed. The wayland QPA plugin is currently handling" + << "it by setting the parent to the topmost grabbing popup." + << "Note, however, that this may cause positioning errors and" + << "popups closing unxpectedly because xdg-shell mandate that child" + << "popups close before parents"; parent = top->m_xdgSurface->m_window; } setPopup(parent); diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 282179efb53..c46c49813fb 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -395,14 +395,9 @@ void QWaylandWindow::setVisible(bool visible) // QWaylandShmBackingStore::beginPaint(). } else { sendExposeEvent(QRect()); - // when flushing the event queue, it could contain a close event, in which - // case 'this' will be deleted. When that happens, we must abort right away. - QPointer deleteGuard(this); - QWindowSystemInterface::flushWindowSystemEvents(); - if (!deleteGuard.isNull() && window()->type() == Qt::Popup) + if (window()->type() == Qt::Popup) closePopups(this); - if (!deleteGuard.isNull()) - reset(); + reset(); } } From a5bcdd0f8679504aa434bf3260e40c11e7b7c5aa Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Thu, 6 Dec 2018 12:00:55 +0100 Subject: [PATCH 03/11] 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 --- .../platforms/wayland/qwaylandcursor.cpp | 65 ++--- .../platforms/wayland/qwaylandcursor_p.h | 6 +- .../platforms/wayland/qwaylanddisplay.cpp | 40 +-- .../platforms/wayland/qwaylanddisplay_p.h | 9 +- .../platforms/wayland/qwaylandinputdevice.cpp | 257 ++++++++++++------ .../platforms/wayland/qwaylandinputdevice_p.h | 45 ++- .../platforms/wayland/qwaylandscreen.cpp | 11 +- .../platforms/wayland/qwaylandscreen_p.h | 1 - .../platforms/wayland/qwaylandwindow.cpp | 3 +- tests/auto/wayland/seatv4/seatv4.pro | 5 + tests/auto/wayland/seatv4/tst_seatv4.cpp | 190 +++++++++++-- tests/auto/wayland/shared/coreprotocol.cpp | 20 +- tests/auto/wayland/shared/coreprotocol.h | 3 +- 13 files changed, 437 insertions(+), 218 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandcursor.cpp b/src/plugins/platforms/wayland/qwaylandcursor.cpp index 6947e97f1d7..8b2ed036def 100644 --- a/src/plugins/platforms/wayland/qwaylandcursor.cpp +++ b/src/plugins/platforms/wayland/qwaylandcursor.cpp @@ -41,7 +41,6 @@ #include "qwaylanddisplay_p.h" #include "qwaylandinputdevice_p.h" -#include "qwaylandscreen_p.h" #include "qwaylandshmbackingstore_p.h" #include @@ -53,12 +52,6 @@ QT_BEGIN_NAMESPACE namespace QtWaylandClient { -QWaylandCursorTheme *QWaylandCursorTheme::create(QWaylandShm *shm, int size) -{ - static QString themeName = qEnvironmentVariable("XCURSOR_THEME", QStringLiteral("default")); - return create(shm, size, themeName); -} - QWaylandCursorTheme *QWaylandCursorTheme::create(QWaylandShm *shm, int size, const QString &themeName) { QByteArray nameBytes = themeName.toLocal8Bit(); @@ -244,56 +237,32 @@ struct wl_cursor_image *QWaylandCursorTheme::cursorImage(Qt::CursorShape shape) return image; } -QWaylandCursor::QWaylandCursor(QWaylandScreen *screen) - : mDisplay(screen->display()) - , mCursorTheme(mDisplay->loadCursorTheme(screen->devicePixelRatio())) +QWaylandCursor::QWaylandCursor(QWaylandDisplay *display) + : mDisplay(display) { } -QSharedPointer QWaylandCursor::cursorBitmapImage(const QCursor *cursor) +QSharedPointer QWaylandCursor::cursorBitmapBuffer(QWaylandDisplay *display, const QCursor *cursor) { - if (cursor->shape() != Qt::BitmapCursor) - return QSharedPointer(); - + Q_ASSERT(cursor->shape() == Qt::BitmapCursor); const QImage &img = cursor->pixmap().toImage(); - QSharedPointer buffer(new QWaylandShmBuffer(mDisplay, img.size(), img.format())); - memcpy(buffer->image()->bits(), img.bits(), img.sizeInBytes()); - return buffer; -} - -struct wl_cursor_image *QWaylandCursor::cursorImage(Qt::CursorShape shape) -{ - if (!mCursorTheme) - return nullptr; - return mCursorTheme->cursorImage(shape); + QSharedPointer buffer(new QWaylandShmBuffer(display, img.size(), img.format())); + memcpy(buffer->image()->bits(), img.bits(), size_t(img.sizeInBytes())); + return std::move(buffer); } void QWaylandCursor::changeCursor(QCursor *cursor, QWindow *window) { - const Qt::CursorShape newShape = cursor ? cursor->shape() : Qt::ArrowCursor; + Q_UNUSED(window); + // Create the buffer here so we don't have to create one per input device + QSharedPointer bitmapBuffer; + if (cursor && cursor->shape() == Qt::BitmapCursor) + bitmapBuffer = cursorBitmapBuffer(mDisplay, cursor); - if (newShape == Qt::BlankCursor) { - mDisplay->setCursor(nullptr, nullptr, 1); - return; - } - - if (newShape == Qt::BitmapCursor) { - mDisplay->setCursor(cursorBitmapImage(cursor), cursor->hotSpot(), window->screen()->devicePixelRatio()); - return; - } - - if (!mCursorTheme) { - qCWarning(lcQpaWayland) << "Can't set cursor from shape with no cursor theme"; - return; - } - - if (struct ::wl_cursor_image *image = mCursorTheme->cursorImage(newShape)) { - struct wl_buffer *buffer = wl_cursor_image_get_buffer(image); - mDisplay->setCursor(buffer, image, window->screen()->devicePixelRatio()); - return; - } - - qCWarning(lcQpaWayland) << "Unable to change to cursor" << cursor; + int fallbackOutputScale = int(window->devicePixelRatio()); + const auto seats = mDisplay->inputDevices(); + for (auto *seat : seats) + seat->setCursor(cursor, bitmapBuffer, fallbackOutputScale); } void QWaylandCursor::pointerEvent(const QMouseEvent &event) @@ -312,6 +281,6 @@ void QWaylandCursor::setPos(const QPoint &pos) qCWarning(lcQpaWayland) << "Setting cursor position is not possible on wayland"; } -} +} // namespace QtWaylandClient QT_END_NAMESPACE diff --git a/src/plugins/platforms/wayland/qwaylandcursor_p.h b/src/plugins/platforms/wayland/qwaylandcursor_p.h index 71f9cd1b8b7..6c48fb628b8 100644 --- a/src/plugins/platforms/wayland/qwaylandcursor_p.h +++ b/src/plugins/platforms/wayland/qwaylandcursor_p.h @@ -73,7 +73,6 @@ class QWaylandShm; class Q_WAYLAND_CLIENT_EXPORT QWaylandCursorTheme { public: - static QWaylandCursorTheme *create(QWaylandShm *shm, int size); static QWaylandCursorTheme *create(QWaylandShm *shm, int size, const QString &themeName); ~QWaylandCursorTheme(); struct wl_cursor_image *cursorImage(Qt::CursorShape shape); @@ -122,19 +121,18 @@ private: class Q_WAYLAND_CLIENT_EXPORT QWaylandCursor : public QPlatformCursor { public: - QWaylandCursor(QWaylandScreen *screen); + explicit QWaylandCursor(QWaylandDisplay *display); void changeCursor(QCursor *cursor, QWindow *window) override; void pointerEvent(const QMouseEvent &event) override; QPoint pos() const override; void setPos(const QPoint &pos) override; - QSharedPointer cursorBitmapImage(const QCursor *cursor); + static QSharedPointer cursorBitmapBuffer(QWaylandDisplay *display, const QCursor *cursor); struct wl_cursor_image *cursorImage(Qt::CursorShape shape); private: QWaylandDisplay *mDisplay = nullptr; - QWaylandCursorTheme *mCursorTheme = nullptr; QPoint mLastPos; }; diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index fc4241f82da..3675806b2d8 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -160,7 +160,7 @@ QWaylandDisplay::~QWaylandDisplay(void) delete mDndSelectionHandler.take(); #endif #if QT_CONFIG(cursor) - qDeleteAll(mCursorThemesBySize); + qDeleteAll(mCursorThemes); #endif if (mDisplay) wl_display_disconnect(mDisplay); @@ -505,40 +505,20 @@ QWaylandInputDevice *QWaylandDisplay::defaultInputDevice() const #if QT_CONFIG(cursor) -void QWaylandDisplay::setCursor(struct wl_buffer *buffer, struct wl_cursor_image *image, qreal dpr) +QWaylandCursor *QWaylandDisplay::waylandCursor() { - /* Qt doesn't tell us which input device we should set the cursor - * for, so set it for all devices. */ - for (int i = 0; i < mInputDevices.count(); i++) { - QWaylandInputDevice *inputDevice = mInputDevices.at(i); - inputDevice->setCursor(buffer, image, dpr); - } + if (!mCursor) + mCursor.reset(new QWaylandCursor(this)); + return mCursor.data(); } -void QWaylandDisplay::setCursor(const QSharedPointer &buffer, const QPoint &hotSpot, qreal dpr) +QWaylandCursorTheme *QWaylandDisplay::loadCursorTheme(const QString &name, int pixelSize) { - /* Qt doesn't tell us which input device we should set the cursor - * for, so set it for all devices. */ - for (int i = 0; i < mInputDevices.count(); i++) { - QWaylandInputDevice *inputDevice = mInputDevices.at(i); - inputDevice->setCursor(buffer, hotSpot, dpr); - } -} - -QWaylandCursorTheme *QWaylandDisplay::loadCursorTheme(qreal devicePixelRatio) -{ - constexpr int defaultCursorSize = 32; - static const int xCursorSize = qEnvironmentVariableIntValue("XCURSOR_SIZE"); - int cursorSize = xCursorSize > 0 ? xCursorSize : defaultCursorSize; - - if (compositorVersion() >= 3) // set_buffer_scale is not supported on earlier versions - cursorSize *= devicePixelRatio; - - if (auto *theme = mCursorThemesBySize.value(cursorSize, nullptr)) + if (auto *theme = mCursorThemes.value({name, pixelSize}, nullptr)) return theme; - if (auto *theme = QWaylandCursorTheme::create(shm(), cursorSize)) { - mCursorThemesBySize[cursorSize] = theme; + if (auto *theme = QWaylandCursorTheme::create(shm(), pixelSize, name)) { + mCursorThemes[{name, pixelSize}] = theme; return theme; } @@ -547,6 +527,6 @@ QWaylandCursorTheme *QWaylandDisplay::loadCursorTheme(qreal devicePixelRatio) #endif // QT_CONFIG(cursor) -} +} // namespace QtWaylandClient QT_END_NAMESPACE diff --git a/src/plugins/platforms/wayland/qwaylanddisplay_p.h b/src/plugins/platforms/wayland/qwaylanddisplay_p.h index 41ad7025fb1..4a98b935bb6 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay_p.h +++ b/src/plugins/platforms/wayland/qwaylanddisplay_p.h @@ -93,6 +93,7 @@ class QWaylandWindow; class QWaylandIntegration; class QWaylandHardwareIntegration; class QWaylandShellSurface; +class QWaylandCursor; class QWaylandCursorTheme; typedef void (*RegistryListener)(void *data, @@ -121,9 +122,8 @@ public: QWaylandWindowManagerIntegration *windowManagerIntegration() const; #if QT_CONFIG(cursor) - void setCursor(struct wl_buffer *buffer, struct wl_cursor_image *image, qreal dpr); - void setCursor(const QSharedPointer &buffer, const QPoint &hotSpot, qreal dpr); - QWaylandCursorTheme *loadCursorTheme(qreal devicePixelRatio); + QWaylandCursor *waylandCursor(); + QWaylandCursorTheme *loadCursorTheme(const QString &name, int pixelSize); #endif struct wl_display *wl_display() const { return mDisplay; } struct ::wl_registry *wl_registry() { return object(); } @@ -210,7 +210,8 @@ private: QList mRegistryListeners; QWaylandIntegration *mWaylandIntegration = nullptr; #if QT_CONFIG(cursor) - QMap mCursorThemesBySize; + QMap, QWaylandCursorTheme *> mCursorThemes; // theme name and size + QScopedPointer mCursor; #endif #if QT_CONFIG(wayland_datadevice) QScopedPointer mDndSelectionHandler; diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp index 31495a45324..cb26def687a 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp @@ -173,8 +173,8 @@ void QWaylandInputDevice::Keyboard::stopRepeat() mRepeatTimer.stop(); } -QWaylandInputDevice::Pointer::Pointer(QWaylandInputDevice *p) - : mParent(p) +QWaylandInputDevice::Pointer::Pointer(QWaylandInputDevice *seat) + : mParent(seat) { } @@ -186,6 +186,153 @@ QWaylandInputDevice::Pointer::~Pointer() wl_pointer_destroy(object()); } +#if QT_CONFIG(cursor) + +class CursorSurface : public QtWayland::wl_surface { +public: + explicit CursorSurface(QWaylandInputDevice::Pointer *pointer, QWaylandDisplay *display) + : m_pointer(pointer) + { + init(display->createSurface(this)); + //TODO: When we upgrade to libwayland 1.10, use wl_surface_get_version instead. + m_version = display->compositorVersion(); + } + + void hide() + { + m_pointer->set_cursor(m_pointer->mEnterSerial, nullptr, 0, 0); + m_setSerial = 0; + } + + // Size and hotspot are in surface coordinates + void update(wl_buffer *buffer, const QPoint &hotspot, const QSize &size, int bufferScale) + { + // Calling code needs to ensure buffer scale is supported if != 1 + Q_ASSERT(bufferScale == 1 || m_version >= 3); + + auto enterSerial = m_pointer->mEnterSerial; + if (m_setSerial < enterSerial || m_hotspot != hotspot) { + m_pointer->set_cursor(m_pointer->mEnterSerial, object(), hotspot.x(), hotspot.y()); + m_setSerial = enterSerial; + m_hotspot = hotspot; + } + + if (m_version >= 3) + set_buffer_scale(bufferScale); + + attach(buffer, 0, 0); + damage(0, 0, size.width(), size.height()); + commit(); + } + + int outputScale() const { return m_outputScale; } + +protected: + void surface_enter(struct ::wl_output *output) override + { + //TODO: Can be improved to keep track of all entered screens + int scale = QWaylandScreen::fromWlOutput(output)->scale(); + if (scale == m_outputScale) + return; + + m_outputScale = scale; + m_pointer->updateCursor(); + } + +private: + QWaylandInputDevice::Pointer *m_pointer = nullptr; + uint m_version = 0; + uint m_setSerial = 0; + QPoint m_hotspot; + int m_outputScale = 0; +}; + +QString QWaylandInputDevice::Pointer::cursorThemeName() const +{ + static QString themeName = qEnvironmentVariable("XCURSOR_THEME", QStringLiteral("default")); + return themeName; +} + +int QWaylandInputDevice::Pointer::cursorSize() const +{ + constexpr int defaultCursorSize = 32; + static const int xCursorSize = qEnvironmentVariableIntValue("XCURSOR_SIZE"); + return xCursorSize > 0 ? xCursorSize : defaultCursorSize; +} + +int QWaylandInputDevice::Pointer::idealCursorScale() const +{ + // set_buffer_scale is not supported on earlier versions + if (seat()->mQDisplay->compositorVersion() < 3) + return 1; + + if (auto *s = mCursor.surface.data()) { + if (s->outputScale() > 0) + return s->outputScale(); + } + + return seat()->mCursor.fallbackOutputScale; +} + +void QWaylandInputDevice::Pointer::updateCursorTheme() +{ + int scale = idealCursorScale(); + int pixelSize = cursorSize() * scale; + auto *display = seat()->mQDisplay; + mCursor.theme = display->loadCursorTheme(cursorThemeName(), pixelSize); + mCursor.themeBufferScale = scale; +} + +void QWaylandInputDevice::Pointer::updateCursor() +{ + if (mEnterSerial == 0) + return; + + auto shape = seat()->mCursor.shape; + + if (shape == Qt::BlankCursor) { + if (mCursor.surface) + mCursor.surface->hide(); + return; + } + + if (shape == Qt::BitmapCursor) { + auto buffer = seat()->mCursor.bitmapBuffer; + if (!buffer) { + qCWarning(lcQpaWayland) << "No buffer for bitmap cursor, can't set cursor"; + return; + } + auto hotspot = seat()->mCursor.hotspot; + int bufferScale = seat()->mCursor.bitmapScale; + getOrCreateCursorSurface()->update(buffer->buffer(), hotspot, buffer->size(), bufferScale); + return; + } + + if (!mCursor.theme || idealCursorScale() != mCursor.themeBufferScale) + updateCursorTheme(); + + // Set from shape using theme + if (struct ::wl_cursor_image *image = mCursor.theme->cursorImage(shape)) { + struct wl_buffer *buffer = wl_cursor_image_get_buffer(image); + int bufferScale = mCursor.themeBufferScale; + QPoint hotspot = QPoint(image->hotspot_x, image->hotspot_y) / bufferScale; + QSize size = QSize(image->width, image->height) / bufferScale; + getOrCreateCursorSurface()->update(buffer, hotspot, size, bufferScale); + return; + } + + qCWarning(lcQpaWayland) << "Unable to change to cursor" << shape; +} + +CursorSurface *QWaylandInputDevice::Pointer::getOrCreateCursorSurface() +{ + if (!mCursor.surface) + mCursor.surface.reset(new CursorSurface(this, seat()->mQDisplay)); + return mCursor.surface.get(); +} + +#endif // QT_CONFIG(cursor) + QWaylandInputDevice::Touch::Touch(QWaylandInputDevice *p) : mParent(p) { @@ -359,87 +506,33 @@ Qt::KeyboardModifiers QWaylandInputDevice::Keyboard::modifiers() const } #if QT_CONFIG(cursor) -uint32_t QWaylandInputDevice::cursorSerial() const +void QWaylandInputDevice::setCursor(const QCursor *cursor, const QSharedPointer &cachedBuffer, int fallbackOutputScale) { + CursorState oldCursor = mCursor; + mCursor = CursorState(); // Clear any previous state + mCursor.shape = cursor ? cursor->shape() : Qt::ArrowCursor; + mCursor.hotspot = cursor ? cursor->hotSpot() : QPoint(); + mCursor.fallbackOutputScale = fallbackOutputScale; + + if (mCursor.shape == Qt::BitmapCursor) { + mCursor.bitmapBuffer = cachedBuffer ? cachedBuffer : QWaylandCursor::cursorBitmapBuffer(mQDisplay, cursor); + qreal dpr = cursor->pixmap().devicePixelRatio(); + mCursor.bitmapScale = int(dpr); // Wayland doesn't support fractional buffer scale + // If there was a fractional part of the dpr, we need to scale the hotspot accordingly + if (mCursor.bitmapScale < dpr) + mCursor.hotspot *= dpr / mCursor.bitmapScale; + } + + // Return early if setCursor was called redundantly (mostly happens from decorations) + if (mCursor.shape != Qt::BitmapCursor + && mCursor.shape == oldCursor.shape + && mCursor.hotspot == oldCursor.hotspot + && mCursor.fallbackOutputScale == oldCursor.fallbackOutputScale) { + return; + } + if (mPointer) - return mPointer->mCursorSerial; - return 0; -} - -void QWaylandInputDevice::setCursor(Qt::CursorShape newShape, QWaylandScreen *screen) -{ - struct wl_cursor_image *image = screen->waylandCursor()->cursorImage(newShape); - if (!image) { - return; - } - - struct wl_buffer *buffer = wl_cursor_image_get_buffer(image); - setCursor(buffer, image, screen->devicePixelRatio()); -} - -void QWaylandInputDevice::setCursor(const QCursor &cursor, QWaylandScreen *screen) -{ - if (mPointer->mCursorSerial >= mPointer->mEnterSerial && (cursor.shape() != Qt::BitmapCursor && cursor.shape() == mPointer->mCursorShape)) - return; - - mPointer->mCursorShape = cursor.shape(); - if (cursor.shape() == Qt::BitmapCursor) { - setCursor(screen->waylandCursor()->cursorBitmapImage(&cursor), cursor.hotSpot(), screen->devicePixelRatio()); - return; - } - setCursor(cursor.shape(), screen); -} - -void QWaylandInputDevice::setCursor(struct wl_buffer *buffer, struct wl_cursor_image *image, int bufferScale) -{ - if (image) { - // Convert from pixel coordinates to surface coordinates - QPoint hotspot = QPoint(image->hotspot_x, image->hotspot_y) / bufferScale; - QSize size = QSize(image->width, image->height) / bufferScale; - setCursor(buffer, hotspot, size, bufferScale); - } else { - setCursor(buffer, QPoint(), QSize(), bufferScale); - } -} - -// size and hotspot are in surface coordinates -void QWaylandInputDevice::setCursor(struct wl_buffer *buffer, const QPoint &hotSpot, const QSize &size, int bufferScale) -{ - if (mCaps & WL_SEAT_CAPABILITY_POINTER) { - bool force = mPointer->mEnterSerial > mPointer->mCursorSerial; - - if (!force && mPointer->mCursorBuffer == buffer) - return; - - mPixmapCursor.clear(); - mPointer->mCursorSerial = mPointer->mEnterSerial; - - mPointer->mCursorBuffer = buffer; - - /* Hide cursor */ - if (!buffer) - { - mPointer->set_cursor(mPointer->mEnterSerial, nullptr, 0, 0); - return; - } - - if (!pointerSurface) - pointerSurface = mQDisplay->createSurface(this); - - mPointer->set_cursor(mPointer->mEnterSerial, pointerSurface, - hotSpot.x(), hotSpot.y()); - wl_surface_attach(pointerSurface, buffer, 0, 0); - if (mQDisplay->compositorVersion() >= 3) - wl_surface_set_buffer_scale(pointerSurface, bufferScale); - wl_surface_damage(pointerSurface, 0, 0, size.width(), size.height()); - wl_surface_commit(pointerSurface); - } -} - -void QWaylandInputDevice::setCursor(const QSharedPointer &buffer, const QPoint &hotSpot, int bufferScale) -{ - setCursor(buffer->buffer(), hotSpot, buffer->size(), bufferScale); - mPixmapCursor = buffer; + mPointer->updateCursor(); } #endif @@ -467,7 +560,7 @@ void QWaylandInputDevice::Pointer::pointer_enter(uint32_t serial, struct wl_surf #if QT_CONFIG(cursor) // Depends on mEnterSerial being updated - window->window()->setCursor(window->window()->cursor()); + updateCursor(); #endif QWaylandWindow *grab = QWaylandWindow::mouseGrab(); diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h index c2fd57bb0df..e57d7f4fd53 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h +++ b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h @@ -88,6 +88,10 @@ class QWaylandWindow; class QWaylandDisplay; class QWaylandDataDevice; class QWaylandTextInput; +#if QT_CONFIG(cursor) +class QWaylandCursorTheme; +class CursorSurface; +#endif class Q_WAYLAND_CLIENT_EXPORT QWaylandInputDevice : public QObject @@ -107,10 +111,7 @@ public: struct ::wl_seat *wl_seat() { return QtWayland::wl_seat::object(); } #if QT_CONFIG(cursor) - void setCursor(const QCursor &cursor, QWaylandScreen *screen); - void setCursor(struct wl_buffer *buffer, struct ::wl_cursor_image *image, int bufferScale); - void setCursor(struct wl_buffer *buffer, const QPoint &hotSpot, const QSize &size, int bufferScale); - void setCursor(const QSharedPointer &buffer, const QPoint &hotSpot, int bufferScale); + void setCursor(const QCursor *cursor, const QSharedPointer &cachedBuffer = {}, int fallbackOutputScale = 1); #endif void handleWindowDestroyed(QWaylandWindow *window); void handleEndDrag(); @@ -134,22 +135,27 @@ public: Qt::KeyboardModifiers modifiers() const; uint32_t serial() const; - uint32_t cursorSerial() const; virtual Keyboard *createKeyboard(QWaylandInputDevice *device); virtual Pointer *createPointer(QWaylandInputDevice *device); virtual Touch *createTouch(QWaylandInputDevice *device); private: - void setCursor(Qt::CursorShape cursor, QWaylandScreen *screen); - QWaylandDisplay *mQDisplay = nullptr; struct wl_display *mDisplay = nullptr; int mVersion; uint32_t mCaps = 0; - struct wl_surface *pointerSurface = nullptr; +#if QT_CONFIG(cursor) + struct CursorState { + QSharedPointer bitmapBuffer; // not used with shape cursors + int bitmapScale = 1; + Qt::CursorShape shape = Qt::ArrowCursor; + int fallbackOutputScale = 1; + QPoint hotspot; + } mCursor; +#endif #if QT_CONFIG(wayland_datadevice) QWaylandDataDevice *mDataDevice = nullptr; @@ -169,8 +175,6 @@ private: QTouchDevice *mTouchDevice = nullptr; - QSharedPointer mPixmapCursor; - friend class QWaylandTouchExtension; friend class QWaylandQtKeyExtension; }; @@ -247,11 +251,20 @@ private: class Q_WAYLAND_CLIENT_EXPORT QWaylandInputDevice::Pointer : public QtWayland::wl_pointer { - public: - Pointer(QWaylandInputDevice *p); + explicit Pointer(QWaylandInputDevice *seat); ~Pointer() override; +#if QT_CONFIG(cursor) + QString cursorThemeName() const; + int cursorSize() const; // in surface coordinates + int idealCursorScale() const; + void updateCursorTheme(); + void updateCursor(); + CursorSurface *getOrCreateCursorSurface(); +#endif + QWaylandInputDevice *seat() const { return mParent; } +protected: void pointer_enter(uint32_t serial, struct wl_surface *surface, wl_fixed_t sx, wl_fixed_t sy) override; void pointer_leave(uint32_t time, struct wl_surface *surface) override; @@ -263,13 +276,19 @@ public: uint32_t axis, wl_fixed_t value) override; +public: void releaseButtons(); QWaylandInputDevice *mParent = nullptr; QPointer mFocus; uint32_t mEnterSerial = 0; #if QT_CONFIG(cursor) - uint32_t mCursorSerial = 0; + struct { + uint32_t serial = 0; + QWaylandCursorTheme *theme = nullptr; + int themeBufferScale = 0; + QScopedPointer surface; + } mCursor; #endif QPointF mSurfacePos; QPointF mGlobalPos; diff --git a/src/plugins/platforms/wayland/qwaylandscreen.cpp b/src/plugins/platforms/wayland/qwaylandscreen.cpp index a6caae0da4b..b2e3ce819b9 100644 --- a/src/plugins/platforms/wayland/qwaylandscreen.cpp +++ b/src/plugins/platforms/wayland/qwaylandscreen.cpp @@ -176,19 +176,10 @@ qreal QWaylandScreen::refreshRate() const } #if QT_CONFIG(cursor) - QPlatformCursor *QWaylandScreen::cursor() const { - return const_cast(this)->waylandCursor(); + return mWaylandDisplay->waylandCursor(); } - -QWaylandCursor *QWaylandScreen::waylandCursor() -{ - if (!mWaylandCursor) - mWaylandCursor.reset(new QWaylandCursor(this)); - return mWaylandCursor.data(); -} - #endif // QT_CONFIG(cursor) QWaylandScreen * QWaylandScreen::waylandScreenFromWindow(QWindow *window) diff --git a/src/plugins/platforms/wayland/qwaylandscreen_p.h b/src/plugins/platforms/wayland/qwaylandscreen_p.h index 36009cce866..4ef58c0c1d7 100644 --- a/src/plugins/platforms/wayland/qwaylandscreen_p.h +++ b/src/plugins/platforms/wayland/qwaylandscreen_p.h @@ -98,7 +98,6 @@ public: #if QT_CONFIG(cursor) QPlatformCursor *cursor() const override; - QWaylandCursor *waylandCursor(); #endif uint32_t outputId() const { return m_outputId; } diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 9e95b306867..ad7fe680fb3 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -962,7 +962,8 @@ void QWaylandWindow::handleScreenChanged() #if QT_CONFIG(cursor) void QWaylandWindow::setMouseCursor(QWaylandInputDevice *device, const QCursor &cursor) { - device->setCursor(cursor, waylandScreen()); + int fallbackBufferScale = int(devicePixelRatio()); + device->setCursor(&cursor, {}, fallbackBufferScale); } void QWaylandWindow::restoreMouseCursor(QWaylandInputDevice *device) diff --git a/tests/auto/wayland/seatv4/seatv4.pro b/tests/auto/wayland/seatv4/seatv4.pro index c02db58559c..7a86cbf0390 100644 --- a/tests/auto/wayland/seatv4/seatv4.pro +++ b/tests/auto/wayland/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/wayland/seatv4/tst_seatv4.cpp b/tests/auto/wayland/seatv4/tst_seatv4.cpp index f1e948ee25c..bc768fa61d1 100644 --- a/tests/auto/wayland/seatv4/tst_seatv4.cpp +++ b/tests/auto/wayland/seatv4/tst_seatv4.cpp @@ -30,6 +30,12 @@ #include #include +#if QT_CONFIG(cursor) +#include +#include +#include +#include +#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()->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 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 &sizes) +{ + auto *waylandIntegration = static_cast(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(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([&] { - auto *surface = xdgSurface()->m_surface; - keyboard()->sendEnter(surface); - keyboard()->sendKey(client(), keyCode, Keyboard::key_state_pressed); - keyboard()->sendKey(client(), keyCode, Keyboard::key_state_released); + 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()[1]); + surface->sendLeave(getAll()[0]); }); - QTRY_VERIFY(window.m_pressed); + + 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()[1]); }); } +void tst_seatv4::hidpiBitmapCursor() +{ + // Add a highdpi output + exec([&] { + OutputData d; + d.scale = 2; + d.position = {1920, 0}; + add(d); + }); + + 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()[1]); + surface->sendLeave(getAll()[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()[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/wayland/shared/coreprotocol.cpp b/tests/auto/wayland/shared/coreprotocol.cpp index 6f51a9793c9..c4dc3f34156 100644 --- a/tests/auto/wayland/shared/coreprotocol.cpp +++ b/tests/auto/wayland/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); 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/wayland/shared/coreprotocol.h b/tests/auto/wayland/shared/coreprotocol.h index 249c16f428b..699dcbdeda7 100644 --- a/tests/auto/wayland/shared/coreprotocol.h +++ b/tests/auto/wayland/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 m_enterSerials; + QPoint m_hotspot; signals: void setCursor(uint serial); //TODO: add arguments? From 04be9cf380dee87f2f6797a4383d119589c313e0 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Thu, 14 Feb 2019 13:41:30 +0100 Subject: [PATCH 04/11] Client: Don't leak wl_data_offers [ChangeLog][QPA plugin] Fixed a leak of wl_data_offers. Data offers would previously leak when a surface with keyboard focus was destroyed... or if it was hidden... or if it changed type, and so on. Make keyboard focus follow a wl_surface instead of a QWaylandWindow. This also fixes a couple of other minor issues, such as the mRepeatTimer not stopping when a wl_surface was destroyed. Ideally, we would have a QWaylandSurface class separate from the QWaylandWindow, but that's out of scope for this fix. Fixes: QTBUG-73825 Change-Id: I56e502512c3959e3fcdb63744adc4a7698e3d53d Reviewed-by: Paul Olav Tvete --- .../platforms/wayland/qwaylanddisplay.cpp | 1 + .../platforms/wayland/qwaylandinputdevice.cpp | 77 ++++++++++++------- .../platforms/wayland/qwaylandinputdevice_p.h | 7 +- .../platforms/wayland/qwaylandwindow.cpp | 23 +----- .../platforms/wayland/qwaylandwindow_p.h | 4 +- 5 files changed, 62 insertions(+), 50 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index 3675806b2d8..491b66e153a 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -50,6 +50,7 @@ #endif #if QT_CONFIG(wayland_datadevice) #include "qwaylanddatadevicemanager_p.h" +#include "qwaylanddatadevice_p.h" #endif #if QT_CONFIG(cursor) #include diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp index cb26def687a..43ca6dbd018 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp @@ -168,9 +168,9 @@ QWaylandInputDevice::Keyboard::~Keyboard() wl_keyboard_destroy(object()); } -void QWaylandInputDevice::Keyboard::stopRepeat() +QWaylandWindow *QWaylandInputDevice::Keyboard::focusWindow() const { - mRepeatTimer.stop(); + return mFocus ? QWaylandWindow::fromWlSurface(mFocus) : nullptr; } QWaylandInputDevice::Pointer::Pointer(QWaylandInputDevice *seat) @@ -421,12 +421,6 @@ QWaylandInputDevice::Touch *QWaylandInputDevice::createTouch(QWaylandInputDevice return new Touch(device); } -void QWaylandInputDevice::handleWindowDestroyed(QWaylandWindow *window) -{ - if (mKeyboard && window == mKeyboard->mFocus) - mKeyboard->stopRepeat(); -} - void QWaylandInputDevice::handleEndDrag() { if (mTouch) @@ -470,7 +464,7 @@ QWaylandWindow *QWaylandInputDevice::pointerFocus() const QWaylandWindow *QWaylandInputDevice::keyboardFocus() const { - return mKeyboard ? mKeyboard->mFocus : nullptr; + return mKeyboard ? mKeyboard->focusWindow() : nullptr; } QWaylandWindow *QWaylandInputDevice::touchFocus() const @@ -767,12 +761,19 @@ void QWaylandInputDevice::Keyboard::keyboard_enter(uint32_t time, struct wl_surf Q_UNUSED(time); Q_UNUSED(keys); - if (!surface) + if (!surface) { + // Ignoring wl_keyboard.enter event with null surface. This is either a compositor bug, + // or it's a race with a wl_surface.destroy request. In either case, ignore the event. return; + } + if (mFocus) { + qCWarning(lcQpaWayland()) << "Unexpected wl_keyboard.enter event. Keyboard already has focus"; + disconnect(focusWindow(), &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); + } - QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); - mFocus = window; + mFocus = surface; + connect(focusWindow(), &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); mParent->mQDisplay->handleKeyboardFocusChanged(mParent); } @@ -780,18 +781,20 @@ void QWaylandInputDevice::Keyboard::keyboard_enter(uint32_t time, struct wl_surf void QWaylandInputDevice::Keyboard::keyboard_leave(uint32_t time, struct wl_surface *surface) { Q_UNUSED(time); - Q_UNUSED(surface); - if (surface) { - QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); - window->unfocus(); + if (!surface) { + // Either a compositor bug, or a race condition with wl_surface.destroy, ignore the event. + return; } - mFocus = nullptr; - - mParent->mQDisplay->handleKeyboardFocusChanged(mParent); - - mRepeatTimer.stop(); + if (surface != mFocus) { + qCWarning(lcQpaWayland) << "Ignoring unexpected wl_keyboard.leave event." + << "wl_surface argument does not match the current focus" + << "This is most likely a compositor bug"; + return; + } + disconnect(focusWindow(), &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); + handleFocusLost(); } static void sendKey(QWindow *tlw, ulong timestamp, QEvent::Type type, int key, Qt::KeyboardModifiers modifiers, @@ -816,7 +819,7 @@ static void sendKey(QWindow *tlw, ulong timestamp, QEvent::Type type, int key, Q void QWaylandInputDevice::Keyboard::keyboard_key(uint32_t serial, uint32_t time, uint32_t key, uint32_t state) { - QWaylandWindow *window = mFocus; + auto *window = focusWindow(); if (!window) { // We destroyed the keyboard focus surface, but the server didn't get the message yet... // or the server didn't send an enter event first. In either case, ignore the event. @@ -896,14 +899,15 @@ void QWaylandInputDevice::Keyboard::keyboard_key(uint32_t serial, uint32_t time, void QWaylandInputDevice::Keyboard::repeatKey() { - if (!mFocus) { + auto *window = focusWindow(); + if (!window) { // We destroyed the keyboard focus surface, but the server didn't get the message yet... // or the server didn't send an enter event first. return; } mRepeatTimer.setInterval(mRepeatRate); - sendKey(mFocus->window(), mRepeatTime, QEvent::KeyRelease, mRepeatKey, modifiers(), mRepeatCode, + sendKey(window->window(), mRepeatTime, QEvent::KeyRelease, mRepeatKey, modifiers(), mRepeatCode, #if QT_CONFIG(xkbcommon) mRepeatSym, mNativeModifiers, #else @@ -911,7 +915,7 @@ void QWaylandInputDevice::Keyboard::repeatKey() #endif mRepeatText, true); - sendKey(mFocus->window(), mRepeatTime, QEvent::KeyPress, mRepeatKey, modifiers(), mRepeatCode, + sendKey(window->window(), mRepeatTime, QEvent::KeyPress, mRepeatKey, modifiers(), mRepeatCode, #if QT_CONFIG(xkbcommon) mRepeatSym, mNativeModifiers, #else @@ -920,6 +924,27 @@ void QWaylandInputDevice::Keyboard::repeatKey() mRepeatText, true); } +void QWaylandInputDevice::Keyboard::handleFocusDestroyed() +{ + // The signal is emitted by QWaylandWindow, which is not necessarily destroyed along with the + // surface, so we still need to disconnect the signal + auto *window = qobject_cast(sender()); + disconnect(window, &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); + Q_ASSERT(window->object() == mFocus); + handleFocusLost(); +} + +void QWaylandInputDevice::Keyboard::handleFocusLost() +{ + mFocus = nullptr; +#if QT_CONFIG(clipboard) + if (auto *dataDevice = mParent->dataDevice()) + dataDevice->invalidateSelectionOffer(); +#endif + mParent->mQDisplay->handleKeyboardFocusChanged(mParent); + mRepeatTimer.stop(); +} + void QWaylandInputDevice::Keyboard::keyboard_modifiers(uint32_t serial, uint32_t mods_depressed, uint32_t mods_latched, @@ -1021,7 +1046,7 @@ void QWaylandInputDevice::handleTouchPoint(int id, double x, double y, Qt::Touch if (!win && mPointer) win = mPointer->mFocus; if (!win && mKeyboard) - win = mKeyboard->mFocus; + win = mKeyboard->focusWindow(); if (!win || !win->window()) return; diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h index e57d7f4fd53..cb382da31a3 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h +++ b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h @@ -113,7 +113,6 @@ public: #if QT_CONFIG(cursor) void setCursor(const QCursor *cursor, const QSharedPointer &cachedBuffer = {}, int fallbackOutputScale = 1); #endif - void handleWindowDestroyed(QWaylandWindow *window); void handleEndDrag(); #if QT_CONFIG(wayland_datadevice) @@ -193,7 +192,7 @@ public: Keyboard(QWaylandInputDevice *p); ~Keyboard() override; - void stopRepeat(); + QWaylandWindow *focusWindow() const; void keyboard_keymap(uint32_t format, int32_t fd, @@ -213,7 +212,7 @@ public: void keyboard_repeat_info(int32_t rate, int32_t delay) override; QWaylandInputDevice *mParent = nullptr; - QPointer mFocus; + ::wl_surface *mFocus = nullptr; #if QT_CONFIG(xkbcommon) xkb_context *mXkbContext = nullptr; xkb_keymap *mXkbMap = nullptr; @@ -238,6 +237,8 @@ public: private slots: void repeatKey(); + void handleFocusDestroyed(); + void handleFocusLost(); private: #if QT_CONFIG(xkbcommon) diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index ae4fc57720b..ca4d176fed6 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -51,11 +51,6 @@ #include "qwaylanddecorationfactory_p.h" #include "qwaylandshmbackingstore_p.h" -#if QT_CONFIG(wayland_datadevice) -#include "qwaylanddatadevice_p.h" -#endif - - #include #include #include @@ -95,10 +90,6 @@ QWaylandWindow::~QWaylandWindow() if (isInitialized()) reset(false); - QList inputDevices = mDisplay->inputDevices(); - for (int i = 0; i < inputDevices.size(); ++i) - inputDevices.at(i)->handleWindowDestroyed(this); - const QWindow *parent = window(); foreach (QWindow *w, QGuiApplication::topLevelWindows()) { if (w->transientParent() == parent) @@ -236,8 +227,10 @@ void QWaylandWindow::reset(bool sendDestroyEvent) mShellSurface = nullptr; delete mSubSurfaceWindow; mSubSurfaceWindow = nullptr; - if (isInitialized()) + if (isInitialized()) { + emit wlSurfaceDestroyed(); destroy(); + } mScreens.clear(); if (mFrameCallback) { @@ -972,16 +965,6 @@ void QWaylandWindow::requestActivateWindow() qCWarning(lcQpaWayland) << "Wayland does not support QWindow::requestActivate()"; } -void QWaylandWindow::unfocus() -{ -#if QT_CONFIG(clipboard) - QWaylandInputDevice *inputDevice = mDisplay->currentInputDevice(); - if (inputDevice && inputDevice->dataDevice()) { - inputDevice->dataDevice()->invalidateSelectionOffer(); - } -#endif -} - bool QWaylandWindow::isExposed() const { if (mShellSurface) diff --git a/src/plugins/platforms/wayland/qwaylandwindow_p.h b/src/plugins/platforms/wayland/qwaylandwindow_p.h index 0e573f350eb..8999682d91a 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow_p.h +++ b/src/plugins/platforms/wayland/qwaylandwindow_p.h @@ -153,7 +153,6 @@ public: void requestActivateWindow() override; bool isExposed() const override; bool isActive() const override; - void unfocus(); QWaylandAbstractDecoration *decoration() const; @@ -200,6 +199,9 @@ public: public slots: void applyConfigure(); +signals: + void wlSurfaceDestroyed(); + protected: void surface_enter(struct ::wl_output *output) override; void surface_leave(struct ::wl_output *output) override; From de2fd1db6be2a825481aef054e613b9f06246d26 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Thu, 14 Feb 2019 10:26:48 +0100 Subject: [PATCH 05/11] Client: Add test for wl_data_offer leaks Also refactors the mocking code for data device manager and implements its isClean method to verify there are no leaks after each test. MockCompositor::DataDevice now persists across get_data_device requests. Task-number: QTBUG-73825 Change-Id: Ib5866e0c54d021e12557f9a96f27950010f2611b Reviewed-by: Paul Olav Tvete --- .../wayland/datadevicev1/tst_datadevicev1.cpp | 90 ++++++++++++++++--- tests/auto/wayland/shared/coreprotocol.h | 3 - tests/auto/wayland/shared/datadevice.cpp | 54 +++++++++-- tests/auto/wayland/shared/datadevice.h | 36 ++++++-- 4 files changed, 153 insertions(+), 30 deletions(-) diff --git a/tests/auto/wayland/datadevicev1/tst_datadevicev1.cpp b/tests/auto/wayland/datadevicev1/tst_datadevicev1.cpp index a85b10c7490..fe68d520d59 100644 --- a/tests/auto/wayland/datadevicev1/tst_datadevicev1.cpp +++ b/tests/auto/wayland/datadevicev1/tst_datadevicev1.cpp @@ -47,7 +47,7 @@ public: add(dataDeviceVersion); }); } - DataDevice *dataDevice() { return get()->dataDevice(); } + DataDevice *dataDevice() { return get()->deviceFor(get()); } }; class tst_datadevicev1 : public QObject, private DataDeviceCompositor @@ -58,6 +58,8 @@ private slots: void initTestCase(); void pasteAscii(); void pasteUtf8(); + void destroysPreviousSelection(); + void destroysSelectionWithSurface(); }; void tst_datadevicev1::initTestCase() @@ -69,7 +71,8 @@ void tst_datadevicev1::initTestCase() QCOMPOSITOR_TRY_VERIFY(keyboard()); QCOMPOSITOR_TRY_VERIFY(dataDevice()); - QCOMPOSITOR_TRY_COMPARE(dataDevice()->resource()->version(), dataDeviceVersion); + QCOMPOSITOR_TRY_VERIFY(dataDevice()->resourceMap().contains(client())); + QCOMPOSITOR_TRY_COMPARE(dataDevice()->resourceMap().value(client())->version(), dataDeviceVersion); } void tst_datadevicev1::pasteAscii() @@ -86,7 +89,8 @@ void tst_datadevicev1::pasteAscii() QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); exec([&] { - auto *offer = new DataOffer(client(), dataDeviceVersion); // Cleaned up by destroy_resource + auto *client = xdgSurface()->resource()->client(); + auto *offer = dataDevice()->sendDataOffer(client, {"text/plain"}); connect(offer, &DataOffer::receive, [](QString mimeType, int fd) { QFile file; file.open(fd, QIODevice::WriteOnly, QFile::FileHandleFlag::AutoCloseHandle); @@ -94,16 +98,14 @@ void tst_datadevicev1::pasteAscii() file.write(QByteArray("normal ascii")); file.close(); }); - dataDevice()->sendDataOffer(offer); - offer->send_offer("text/plain"); dataDevice()->sendSelection(offer); auto *surface = xdgSurface()->m_surface; keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol pointer()->sendEnter(surface, {32, 32}); - pointer()->sendButton(client(), BTN_LEFT, 1); - pointer()->sendButton(client(), BTN_LEFT, 0); + pointer()->sendButton(client, BTN_LEFT, 1); + pointer()->sendButton(client, BTN_LEFT, 0); }); QTRY_COMPARE(window.m_text, "normal ascii"); } @@ -122,7 +124,8 @@ void tst_datadevicev1::pasteUtf8() QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); exec([&] { - auto *offer = new DataOffer(client(), dataDeviceVersion); // Cleaned up by destroy_resource + auto *client = xdgSurface()->resource()->client(); + auto *offer = dataDevice()->sendDataOffer(client, {"text/plain", "text/plain;charset=utf-8"}); connect(offer, &DataOffer::receive, [](QString mimeType, int fd) { QFile file; file.open(fd, QIODevice::WriteOnly, QFile::FileHandleFlag::AutoCloseHandle); @@ -130,20 +133,81 @@ void tst_datadevicev1::pasteUtf8() file.write(QByteArray("face with tears of joy: 😂")); file.close(); }); - dataDevice()->sendDataOffer(offer); - offer->send_offer("text/plain"); - offer->send_offer("text/plain;charset=utf-8"); dataDevice()->sendSelection(offer); auto *surface = xdgSurface()->m_surface; keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol pointer()->sendEnter(surface, {32, 32}); - pointer()->sendButton(client(), BTN_LEFT, 1); - pointer()->sendButton(client(), BTN_LEFT, 0); + pointer()->sendButton(client, BTN_LEFT, 1); + pointer()->sendButton(client, BTN_LEFT, 0); }); QTRY_COMPARE(window.m_text, "face with tears of joy: 😂"); } +void tst_datadevicev1::destroysPreviousSelection() +{ + QRasterWindow window; + window.resize(64, 64); + window.show(); + QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); + + // When the client receives a selection event, it is required to destroy the previous offer + exec([&] { + QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 0); + auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"}); + dataDevice()->sendSelection(offer); + auto *surface = xdgSurface()->m_surface; + keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol + QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 1); + }); + + exec([&] { + auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"}); + dataDevice()->sendSelection(offer); + QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 2); + }); + + // Verify the first offer gets destroyed + QCOMPOSITOR_TRY_COMPARE(dataDevice()->m_sentSelectionOffers.size(), 1); + + exec([&] { + auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"}); + dataDevice()->sendSelection(offer); + auto *surface = xdgSurface()->m_surface; + keyboard()->sendLeave(surface); + }); + + // Clients are required to destroy their offer when losing keyboard focus + QCOMPOSITOR_TRY_COMPARE(dataDevice()->m_sentSelectionOffers.size(), 0); +} + +void tst_datadevicev1::destroysSelectionWithSurface() +{ + auto *window = new QRasterWindow; + window->resize(64, 64); + window->show(); + + QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); + + // When the client receives a selection event, it is required to destroy the previous offer + exec([&] { + QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 0); + auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"}); + dataDevice()->sendSelection(offer); + auto *surface = xdgSurface()->m_surface; + keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol + QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 1); + }); + + // Ping to make sure we receive the wl_keyboard enter and leave events, before destroying the + // surface. Otherwise, the client will receive enter and leave events with a destroyed (null) + // surface, which is not what we are trying to test for here. + xdgPingAndWaitForPong(); + window->destroy(); + + QCOMPOSITOR_TRY_COMPARE(dataDevice()->m_sentSelectionOffers.size(), 0); +} + QCOMPOSITOR_TEST_MAIN(tst_datadevicev1) #include "tst_datadevicev1.moc" diff --git a/tests/auto/wayland/shared/coreprotocol.h b/tests/auto/wayland/shared/coreprotocol.h index 699dcbdeda7..5cef476c8f6 100644 --- a/tests/auto/wayland/shared/coreprotocol.h +++ b/tests/auto/wayland/shared/coreprotocol.h @@ -250,9 +250,6 @@ public: Keyboard* m_keyboard = nullptr; QVector m_oldKeyboards; - DataDevice *dataDevice() { return m_dataDevice.data(); } - QScopedPointer m_dataDevice; - uint m_capabilities = 0; protected: diff --git a/tests/auto/wayland/shared/datadevice.cpp b/tests/auto/wayland/shared/datadevice.cpp index c136c75968f..dfa18952c35 100644 --- a/tests/auto/wayland/shared/datadevice.cpp +++ b/tests/auto/wayland/shared/datadevice.cpp @@ -30,29 +30,60 @@ namespace MockCompositor { +bool DataDeviceManager::isClean() +{ + for (auto *device : qAsConst(m_dataDevices)) { + // The client should not leak selection offers, i.e. if this fails, there is a missing + // data_offer.destroy request + if (!device->m_sentSelectionOffers.empty()) + return false; + } + return true; +} + +DataDevice *DataDeviceManager::deviceFor(Seat *seat) +{ + Q_ASSERT(seat); + if (auto *device = m_dataDevices.value(seat, nullptr)) + return device; + + auto *device = new DataDevice(this, seat); + m_dataDevices[seat] = device; + return device; +} + void DataDeviceManager::data_device_manager_get_data_device(Resource *resource, uint32_t id, wl_resource *seatResource) { auto *seat = fromResource(seatResource); QVERIFY(seat); - QVERIFY(!seat->m_dataDevice); - seat->m_dataDevice.reset(new DataDevice(resource->client(), id, resource->version())); + auto *device = deviceFor(seat); + device->add(resource->client(), id, resource->version()); } DataDevice::~DataDevice() { - // If the client hasn't deleted the wayland object, just ignore subsequent events - if (auto *r = resource()->handle) - wl_resource_set_implementation(r, nullptr, nullptr, nullptr); + // If the client(s) hasn't deleted the wayland object, just ignore subsequent events + for (auto *r : resourceMap()) + wl_resource_set_implementation(r->handle, nullptr, nullptr, nullptr); } -void DataDevice::sendDataOffer(DataOffer *offer) +DataOffer *DataDevice::sendDataOffer(wl_client *client, const QStringList &mimeTypes) { - wl_data_device::send_data_offer(offer->resource()->handle); + Q_ASSERT(client); + auto *offer = new DataOffer(this, client, m_manager->m_version); + for (auto *resource : resourceMap().values(client)) + wl_data_device::send_data_offer(resource->handle, offer->resource()->handle); + for (const auto &mimeType : mimeTypes) + offer->send_offer(mimeType); + return offer; } void DataDevice::sendSelection(DataOffer *offer) { - wl_data_device::send_selection(offer->resource()->handle); + auto *client = offer->resource()->client(); + for (auto *resource : resourceMap().values(client)) + wl_data_device::send_selection(resource->handle, offer->resource()->handle); + m_sentSelectionOffers << offer; } void DataOffer::data_offer_destroy_resource(Resource *resource) @@ -67,4 +98,11 @@ void DataOffer::data_offer_receive(Resource *resource, const QString &mime_type, emit receive(mime_type, fd); } +void DataOffer::data_offer_destroy(QtWaylandServer::wl_data_offer::Resource *resource) +{ + bool removed = m_dataDevice->m_sentSelectionOffers.removeOne(this); + QVERIFY(removed); + wl_resource_destroy(resource->handle); +} + } // namespace MockCompositor diff --git a/tests/auto/wayland/shared/datadevice.h b/tests/auto/wayland/shared/datadevice.h index 4613db7761e..a96da86f081 100644 --- a/tests/auto/wayland/shared/datadevice.h +++ b/tests/auto/wayland/shared/datadevice.h @@ -42,8 +42,14 @@ class DataDeviceManager : public Global, public QtWaylandServer::wl_data_device_ public: explicit DataDeviceManager(CoreCompositor *compositor, int version = 1) : QtWaylandServer::wl_data_device_manager(compositor->m_display, version) + , m_version(version) {} - QVector m_dataDevices; + ~DataDeviceManager() override { qDeleteAll(m_dataDevices); } + bool isClean() override; + DataDevice *deviceFor(Seat *seat); + + int m_version = 1; // TODO: remove on libwayland upgrade + QMap m_dataDevices; protected: void data_device_manager_get_data_device(Resource *resource, uint32_t id, ::wl_resource *seatResource) override; @@ -52,24 +58,42 @@ protected: class DataDevice : public QtWaylandServer::wl_data_device { public: - explicit DataDevice(::wl_client *client, int id, int version) - : QtWaylandServer::wl_data_device(client, id, version) + explicit DataDevice(DataDeviceManager *manager, Seat *seat) + : m_manager(manager) + , m_seat(seat) {} ~DataDevice() override; void send_data_offer(::wl_resource *resource) = delete; - void sendDataOffer(DataOffer *offer); + DataOffer *sendDataOffer(::wl_client *client, const QStringList &mimeTypes = {}); + DataOffer *sendDataOffer(const QStringList &mimeTypes = {}); + void send_selection(::wl_resource *resource) = delete; void sendSelection(DataOffer *offer); + + DataDeviceManager *m_manager = nullptr; + Seat *m_seat = nullptr; + QVector m_sentSelectionOffers; + +protected: + void data_device_release(Resource *resource) override + { + int removed = m_manager->m_dataDevices.remove(m_seat); + QVERIFY(removed); + wl_resource_destroy(resource->handle); + } }; class DataOffer : public QObject, public QtWaylandServer::wl_data_offer { Q_OBJECT public: - explicit DataOffer(::wl_client *client, int version) + explicit DataOffer(DataDevice *dataDevice, ::wl_client *client, int version) : QtWaylandServer::wl_data_offer (client, 0, version) + , m_dataDevice(dataDevice) {} + DataDevice *m_dataDevice = nullptr; + signals: void receive(QString mimeType, int fd); @@ -77,7 +101,7 @@ protected: void data_offer_destroy_resource(Resource *resource) override; void data_offer_receive(Resource *resource, const QString &mime_type, int32_t fd) override; // void data_offer_accept(Resource *resource, uint32_t serial, const QString &mime_type) override; -// void data_offer_destroy(Resource *resource) override; + void data_offer_destroy(Resource *resource) override; }; } // namespace MockCompositor From f07397208c0c6846706929bad986f5d72aff36c1 Mon Sep 17 00:00:00 2001 From: Liang Qi Date: Fri, 22 Feb 2019 15:15:02 +0100 Subject: [PATCH 06/11] Fix the build with -no-gui Change-Id: I9ff6ac4e33e89691e06965515a54e91a831b7a0a Reviewed-by: Oswald Buddenhagen Reviewed-by: Joerg Bornemann --- src/plugins/platforms/wayland/configure.json | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/platforms/wayland/configure.json b/src/plugins/platforms/wayland/configure.json index 1f86a49360f..586da6f66a9 100644 --- a/src/plugins/platforms/wayland/configure.json +++ b/src/plugins/platforms/wayland/configure.json @@ -1,5 +1,6 @@ { "module": "waylandclient", + "condition": "module.gui", "depends": [ "gui-private" ], From 446a9753abfc09d394d8ac9c2300a98d05313102 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Thu, 24 Jan 2019 14:42:28 +0100 Subject: [PATCH 07/11] Client: Decrease buffer_scale for small cursor themes Not all setups or themes have cursors with a matching DPI. In such cases, wl_cursor_load_theme will then return a theme that is the closest resolution it can get. With the previous implementation, cursors themes without a high dpi version would become become really tiny on high DPI displays. This patch prevents it by setting a lower wl_surface.scale for those themes. This also implements proper tracking of the cursor surface's entered outputs (i.e. if the entered surface is destroyed, the scale is reset, and similarly the following sequence of events should also be handled: wl_surface.enter(wl_output@1) wl_surface.enter(wl_output@2) wl_surface.leave(wl_output@2) In the old implementation, we would be stuck with the scale from wl_output@2, but now we now should correctly get the scale of wl_output@1. [ChangeLog][QPA plugin] Cursors on high DPI screens are now scaled up if the cursor theme does not have an appropriate high resolution version. Change-Id: Ic87d00e35612b5afdf8c2e3a4463fcfef1f1f09d Reviewed-by: Giulio Camuffo --- .../platforms/wayland/qwaylandinputdevice.cpp | 52 +++++++++++++--- tests/auto/wayland/seatv4/tst_seatv4.cpp | 62 ++++++++++++++++++- 2 files changed, 103 insertions(+), 11 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp index 43ca6dbd018..65267869fba 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp @@ -188,7 +188,8 @@ QWaylandInputDevice::Pointer::~Pointer() #if QT_CONFIG(cursor) -class CursorSurface : public QtWayland::wl_surface { +class CursorSurface : public QObject, public QtWayland::wl_surface +{ public: explicit CursorSurface(QWaylandInputDevice::Pointer *pointer, QWaylandDisplay *display) : m_pointer(pointer) @@ -196,6 +197,14 @@ public: init(display->createSurface(this)); //TODO: When we upgrade to libwayland 1.10, use wl_surface_get_version instead. m_version = display->compositorVersion(); + connect(qApp, &QGuiApplication::screenRemoved, this, [this](QScreen *screen) { + int oldScale = outputScale(); + if (!m_screens.removeOne(static_cast(screen->handle()))) + return; + + if (outputScale() != oldScale) + m_pointer->updateCursor(); + }); } void hide() @@ -225,18 +234,38 @@ public: commit(); } - int outputScale() const { return m_outputScale; } + int outputScale() const + { + int scale = 0; + for (auto *screen : m_screens) + scale = qMax(scale, screen->scale()); + return scale; + } protected: void surface_enter(struct ::wl_output *output) override { - //TODO: Can be improved to keep track of all entered screens - int scale = QWaylandScreen::fromWlOutput(output)->scale(); - if (scale == m_outputScale) + int oldScale = outputScale(); + auto *screen = QWaylandScreen::fromWlOutput(output); + if (m_screens.contains(screen)) return; - m_outputScale = scale; - m_pointer->updateCursor(); + m_screens.append(screen); + + if (outputScale() != oldScale) + m_pointer->updateCursor(); + } + + void surface_leave(struct ::wl_output *output) override + { + int oldScale = outputScale(); + auto *screen = QWaylandScreen::fromWlOutput(output); + + if (!m_screens.removeOne(screen)) + return; + + if (outputScale() != oldScale) + m_pointer->updateCursor(); } private: @@ -244,7 +273,7 @@ private: uint m_version = 0; uint m_setSerial = 0; QPoint m_hotspot; - int m_outputScale = 0; + QVector m_screens; }; QString QWaylandInputDevice::Pointer::cursorThemeName() const @@ -280,6 +309,13 @@ void QWaylandInputDevice::Pointer::updateCursorTheme() int pixelSize = cursorSize() * scale; auto *display = seat()->mQDisplay; mCursor.theme = display->loadCursorTheme(cursorThemeName(), pixelSize); + if (auto *arrow = mCursor.theme->cursorImage(Qt::ArrowCursor)) { + int arrowPixelSize = qMax(arrow->width, arrow->height); // Not all cursor themes are square + while (scale > 1 && arrowPixelSize / scale < cursorSize()) + --scale; + } else { + qWarning(lcQpaWayland) << "Cursor theme does not support the arrow cursor"; + } mCursor.themeBufferScale = scale; } diff --git a/tests/auto/wayland/seatv4/tst_seatv4.cpp b/tests/auto/wayland/seatv4/tst_seatv4.cpp index bc768fa61d1..8fa81b608df 100644 --- a/tests/auto/wayland/seatv4/tst_seatv4.cpp +++ b/tests/auto/wayland/seatv4/tst_seatv4.cpp @@ -75,6 +75,7 @@ private slots: void simpleAxis(); void invalidPointerEvents(); void scaledCursor(); + void unscaledFallbackCursor(); void bitmapCursor(); void hidpiBitmapCursor(); void hidpiBitmapCursorNonInt(); @@ -290,7 +291,7 @@ void tst_seatv4::invalidPointerEvents() static bool supportsCursorSize(uint size, wl_shm *shm) { - auto *theme = wl_cursor_theme_load(nullptr, size, shm); + auto *theme = wl_cursor_theme_load(qgetenv("XCURSOR_THEME"), size, shm); if (!theme) return false; @@ -313,10 +314,16 @@ static bool supportsCursorSizes(const QVector &sizes) }); } +static uint defaultCursorSize() { + const int xCursorSize = qEnvironmentVariableIntValue("XCURSOR_SIZE"); + return xCursorSize > 0 ? uint(xCursorSize) : 32; +} + void tst_seatv4::scaledCursor() { - if (!supportsCursorSizes({32, 64})) - QSKIP("Cursor themes with sizes 32 and 64 not found."); + const uint defaultSize = defaultCursorSize(); + if (!supportsCursorSizes({defaultSize, defaultSize * 2})) + QSKIP("Cursor themes with default size and 2x default size not found."); // Add a highdpi output exec([&] { @@ -351,6 +358,55 @@ void tst_seatv4::scaledCursor() exec([&] { remove(output(1)); }); } +void tst_seatv4::unscaledFallbackCursor() +{ + const uint defaultSize = defaultCursorSize(); + if (!supportsCursorSizes({defaultSize})) + QSKIP("Default cursor size not supported"); + + const int screens = 4; // with scales 1, 2, 4, 8 + + exec([=] { + for (int i = 1; i < screens; ++i) { + OutputData d; + d.scale = int(qPow(2, i)); + d.position = {1920 * i, 0}; + add(d); + } + }); + + QRasterWindow window; + window.resize(64, 64); + 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_TRY_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 1); + QSize unscaledPixelSize = exec([=] { + return pointer()->cursorSurface()->m_committed.buffer->size(); + }); + + QCOMPARE(unscaledPixelSize.width(), int(defaultSize)); + QCOMPARE(unscaledPixelSize.height(), int(defaultSize)); + + for (int i = 1; i < screens; ++i) { + exec([=] { + auto *surface = pointer()->cursorSurface(); + surface->sendEnter(getAll()[i]); + surface->sendLeave(getAll()[i-1]); + }); + + xdgPingAndWaitForPong(); // Give the client a chance to mess up + + // Surface size (buffer size / scale) should stay constant + QCOMPOSITOR_TRY_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size() / pointer()->cursorSurface()->m_committed.bufferScale, unscaledPixelSize); + } + + // Remove the extra outputs to clean up for the next test + exec([&] { while (auto *o = output(1)) remove(o); }); +} + void tst_seatv4::bitmapCursor() { // Add a highdpi output From ecd0d448faaa4c92a712bb1f04a00e4c2811d45d Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 6 Feb 2019 09:31:41 +0100 Subject: [PATCH 08/11] Client tests: Add convenience for getting cursor surface Let's simplify all those pointer()->surfaceCursor() calls. Change-Id: I83c51f460fa2313a0b84c8c64509d48027156443 Reviewed-by: Giulio Camuffo --- tests/auto/wayland/seatv4/tst_seatv4.cpp | 62 +++++++++++----------- tests/auto/wayland/shared/mockcompositor.h | 1 + 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/tests/auto/wayland/seatv4/tst_seatv4.cpp b/tests/auto/wayland/seatv4/tst_seatv4.cpp index 8fa81b608df..771307d7e2d 100644 --- a/tests/auto/wayland/seatv4/tst_seatv4.cpp +++ b/tests/auto/wayland/seatv4/tst_seatv4.cpp @@ -133,7 +133,7 @@ void tst_seatv4::setsCursorOnEnter() QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); exec([=] { pointer()->sendEnter(xdgSurface()->m_surface, {32, 32}); }); - QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface()); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()); } void tst_seatv4::usesEnterSerial() @@ -339,20 +339,20 @@ void tst_seatv4::scaledCursor() 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_TRY_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 1); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()->m_committed.buffer); + QCOMPOSITOR_TRY_COMPARE(cursorSurface()->m_committed.bufferScale, 1); QSize unscaledPixelSize = exec([=] { - return pointer()->cursorSurface()->m_committed.buffer->size(); + return cursorSurface()->m_committed.buffer->size(); }); exec([=] { - auto *surface = pointer()->cursorSurface(); + auto *surface = cursorSurface(); surface->sendEnter(getAll()[1]); surface->sendLeave(getAll()[0]); }); - QCOMPOSITOR_TRY_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size(), unscaledPixelSize * 2); + QCOMPOSITOR_TRY_COMPARE(cursorSurface()->m_committed.buffer->size(), unscaledPixelSize * 2); // Remove the extra output to clean up for the next test exec([&] { remove(output(1)); }); @@ -380,11 +380,11 @@ void tst_seatv4::unscaledFallbackCursor() 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_TRY_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 1); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()->m_committed.buffer); + QCOMPOSITOR_TRY_COMPARE(cursorSurface()->m_committed.bufferScale, 1); QSize unscaledPixelSize = exec([=] { - return pointer()->cursorSurface()->m_committed.buffer->size(); + return cursorSurface()->m_committed.buffer->size(); }); QCOMPARE(unscaledPixelSize.width(), int(defaultSize)); @@ -392,7 +392,7 @@ void tst_seatv4::unscaledFallbackCursor() for (int i = 1; i < screens; ++i) { exec([=] { - auto *surface = pointer()->cursorSurface(); + auto *surface = cursorSurface(); surface->sendEnter(getAll()[i]); surface->sendLeave(getAll()[i-1]); }); @@ -400,7 +400,7 @@ void tst_seatv4::unscaledFallbackCursor() xdgPingAndWaitForPong(); // Give the client a chance to mess up // Surface size (buffer size / scale) should stay constant - QCOMPOSITOR_TRY_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size() / pointer()->cursorSurface()->m_committed.bufferScale, unscaledPixelSize); + QCOMPOSITOR_TRY_COMPARE(cursorSurface()->m_committed.buffer->size() / cursorSurface()->m_committed.bufferScale, unscaledPixelSize); } // Remove the extra outputs to clean up for the next test @@ -430,14 +430,14 @@ void tst_seatv4::bitmapCursor() 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(24, 24)); - QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 1); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()->m_committed.buffer); + QCOMPOSITOR_COMPARE(cursorSurface()->m_committed.buffer->size(), QSize(24, 24)); + QCOMPOSITOR_COMPARE(cursorSurface()->m_committed.bufferScale, 1); QCOMPOSITOR_COMPARE(pointer()->m_hotspot, QPoint(12, 12)); exec([=] { - auto *surface = pointer()->cursorSurface(); + auto *surface = cursorSurface(); surface->sendEnter(getAll()[1]); surface->sendLeave(getAll()[0]); }); @@ -445,8 +445,8 @@ void tst_seatv4::bitmapCursor() 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(cursorSurface()->m_committed.bufferScale, 1); + QCOMPOSITOR_COMPARE(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 @@ -476,22 +476,22 @@ void tst_seatv4::hidpiBitmapCursor() 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_TRY_VERIFY(cursorSurface()); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()->m_committed.buffer); + QCOMPOSITOR_COMPARE(cursorSurface()->m_committed.buffer->size(), QSize(48, 48)); + QCOMPOSITOR_COMPARE(cursorSurface()->m_committed.bufferScale, 2); QCOMPOSITOR_COMPARE(pointer()->m_hotspot, QPoint(12, 12)); exec([=] { - auto *surface = pointer()->cursorSurface(); + auto *surface = cursorSurface(); surface->sendEnter(getAll()[1]); surface->sendLeave(getAll()[0]); }); xdgPingAndWaitForPong(); - QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.bufferScale, 2); - QCOMPOSITOR_COMPARE(pointer()->cursorSurface()->m_committed.buffer->size(), QSize(48, 48)); + QCOMPOSITOR_COMPARE(cursorSurface()->m_committed.bufferScale, 2); + QCOMPOSITOR_COMPARE(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 @@ -513,10 +513,10 @@ void tst_seatv4::hidpiBitmapCursorNonInt() 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); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()->m_committed.buffer); + QCOMPOSITOR_COMPARE(cursorSurface()->m_committed.buffer->size(), QSize(100, 100)); + QCOMPOSITOR_COMPARE(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)); diff --git a/tests/auto/wayland/shared/mockcompositor.h b/tests/auto/wayland/shared/mockcompositor.h index ddbd7f34264..75ef1eaea1d 100644 --- a/tests/auto/wayland/shared/mockcompositor.h +++ b/tests/auto/wayland/shared/mockcompositor.h @@ -54,6 +54,7 @@ public: XdgToplevel *xdgToplevel(int i = 0) { return get()->toplevel(i); } XdgPopup *xdgPopup(int i = 0) { return get()->popup(i); } Pointer *pointer() { auto *seat = get(); Q_ASSERT(seat); return seat->m_pointer; } + Surface *cursorSurface() { auto *p = pointer(); return p ? p->cursorSurface() : nullptr; } Keyboard *keyboard() { auto *seat = get(); Q_ASSERT(seat); return seat->m_keyboard; } uint sendXdgShellPing(); void xdgPingAndWaitForPong(); From 7fa16ebff6ca429daf9cb0eddc2d8ff362423ba8 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 6 Feb 2019 09:30:09 +0100 Subject: [PATCH 09/11] Client: Don't send illegal wl_pointer.set_cursor requests When the cursor focus' wl_surface is destroyed, or the pointer leaves a surface, we have to reset the enter serial to avoid sending illegal set_cursor requests. Change-Id: I0c886e4123acb4aebd325b07bf15b9d3fa8589da Reviewed-by: Giulio Camuffo --- .../platforms/wayland/qwaylandinputdevice.cpp | 23 +++++++++++-- .../platforms/wayland/qwaylandinputdevice_p.h | 10 ++++-- tests/auto/wayland/seatv4/tst_seatv4.cpp | 34 ++++++++++++++++++- tests/auto/wayland/shared/coreprotocol.cpp | 6 ++-- 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp index 65267869fba..76000e54203 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp @@ -209,7 +209,9 @@ public: void hide() { - m_pointer->set_cursor(m_pointer->mEnterSerial, nullptr, 0, 0); + uint serial = m_pointer->mEnterSerial; + Q_ASSERT(serial); + m_pointer->set_cursor(serial, nullptr, 0, 0); m_setSerial = 0; } @@ -581,7 +583,16 @@ void QWaylandInputDevice::Pointer::pointer_enter(uint32_t serial, struct wl_surf return; QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); + + if (mFocus) { + qWarning(lcQpaWayland) << "The compositor sent a wl_pointer.enter event before sending a" + << "leave event first, this is not allowed by the wayland protocol" + << "attempting to work around it by invalidating the current focus"; + invalidateFocus(); + } mFocus = window; + connect(mFocus, &QWaylandWindow::wlSurfaceDestroyed, this, &Pointer::handleFocusDestroyed); + mSurfacePos = QPointF(wl_fixed_to_double(sx), wl_fixed_to_double(sy)); mGlobalPos = window->window()->mapToGlobal(mSurfacePos.toPoint()); @@ -611,7 +622,8 @@ void QWaylandInputDevice::Pointer::pointer_leave(uint32_t time, struct wl_surfac QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); window->handleMouseLeave(mParent); } - mFocus = nullptr; + + invalidateFocus(); mButtons = Qt::NoButton; mParent->mTime = time; @@ -714,6 +726,13 @@ void QWaylandInputDevice::Pointer::pointer_button(uint32_t serial, uint32_t time } } +void QWaylandInputDevice::Pointer::invalidateFocus() +{ + disconnect(mFocus, &QWaylandWindow::wlSurfaceDestroyed, this, &Pointer::handleFocusDestroyed); + mFocus = nullptr; + mEnterSerial = 0; +} + void QWaylandInputDevice::Pointer::releaseButtons() { mButtons = Qt::NoButton; diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h index cb382da31a3..50b1af385a9 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice_p.h +++ b/src/plugins/platforms/wayland/qwaylandinputdevice_p.h @@ -250,8 +250,9 @@ private: }; -class Q_WAYLAND_CLIENT_EXPORT QWaylandInputDevice::Pointer : public QtWayland::wl_pointer +class Q_WAYLAND_CLIENT_EXPORT QWaylandInputDevice::Pointer : public QObject, public QtWayland::wl_pointer { + Q_OBJECT public: explicit Pointer(QWaylandInputDevice *seat); ~Pointer() override; @@ -277,6 +278,12 @@ protected: uint32_t axis, wl_fixed_t value) override; +private slots: + void handleFocusDestroyed() { invalidateFocus(); } + +private: + void invalidateFocus(); + public: void releaseButtons(); @@ -285,7 +292,6 @@ public: uint32_t mEnterSerial = 0; #if QT_CONFIG(cursor) struct { - uint32_t serial = 0; QWaylandCursorTheme *theme = nullptr; int themeBufferScale = 0; QScopedPointer surface; diff --git a/tests/auto/wayland/seatv4/tst_seatv4.cpp b/tests/auto/wayland/seatv4/tst_seatv4.cpp index 771307d7e2d..7dc2e727a6a 100644 --- a/tests/auto/wayland/seatv4/tst_seatv4.cpp +++ b/tests/auto/wayland/seatv4/tst_seatv4.cpp @@ -70,6 +70,7 @@ private slots: void createsPointer(); void setsCursorOnEnter(); void usesEnterSerial(); + void focusDestruction(); void mousePress(); void simpleAxis_data(); void simpleAxis(); @@ -147,12 +148,43 @@ void tst_seatv4::usesEnterSerial() uint enterSerial = exec([=] { return pointer()->sendEnter(xdgSurface()->m_surface, {32, 32}); }); - QCOMPOSITOR_TRY_VERIFY(pointer()->cursorSurface()); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()); QTRY_COMPARE(setCursorSpy.count(), 1); QCOMPARE(setCursorSpy.takeFirst().at(0).toUInt(), enterSerial); } +void tst_seatv4::focusDestruction() +{ + QSignalSpy setCursorSpy(exec([=] { return pointer(); }), &Pointer::setCursor); + QRasterWindow window; + window.resize(64, 64); + window.show(); + QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); + // Setting a cursor now is not allowed since there has been no enter event + QCOMPARE(setCursorSpy.count(), 0); + + uint enterSerial = exec([=] { + return pointer()->sendEnter(xdgSurface()->m_surface, {32, 32}); + }); + QCOMPOSITOR_TRY_VERIFY(cursorSurface()); + QTRY_COMPARE(setCursorSpy.count(), 1); + QCOMPARE(setCursorSpy.takeFirst().at(0).toUInt(), enterSerial); + + // Destroy the focus + window.close(); + + QRasterWindow window2; + window2.resize(64, 64); + window2.show(); + window2.setCursor(Qt::WaitCursor); + QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); + + // Setting a cursor now is not allowed since there has been no enter event + xdgPingAndWaitForPong(); + QCOMPARE(setCursorSpy.count(), 0); +} + void tst_seatv4::mousePress() { class Window : public QRasterWindow { diff --git a/tests/auto/wayland/shared/coreprotocol.cpp b/tests/auto/wayland/shared/coreprotocol.cpp index c4dc3f34156..729d481f83b 100644 --- a/tests/auto/wayland/shared/coreprotocol.cpp +++ b/tests/auto/wayland/shared/coreprotocol.cpp @@ -261,6 +261,7 @@ uint Pointer::sendEnter(Surface *surface, const QPointF &position) uint serial = m_seat->m_compositor->nextSerial(); m_enterSerials << serial; + m_cursorRole = nullptr; // According to the protocol, the pointer image is undefined after enter wl_client *client = surface->resource()->client(); const auto pointerResources = resourceMap().values(client); @@ -320,9 +321,8 @@ void Pointer::pointer_set_cursor(Resource *resource, uint32_t serial, wl_resourc QVERIFY(s); if (s->m_role) { - auto *cursorRole = CursorRole::fromSurface(s); - QVERIFY(cursorRole); - QVERIFY(cursorRole == m_cursorRole); + m_cursorRole = CursorRole::fromSurface(s); + QVERIFY(m_cursorRole); } else { m_cursorRole = new CursorRole(s); //TODO: make sure we don't leak CursorRole s->m_role = m_cursorRole; From c104decfdd3a8fd679ca0fefef571f96b1cc2415 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 5 Feb 2019 10:07:56 +0100 Subject: [PATCH 10/11] Client xdg-shell: Add test for switching popups Task-number: QTBUG-73524 Change-Id: Ie9a13aeae52a7576699147e5515e2ed32a3a4d1c Reviewed-by: Giulio Camuffo --- tests/auto/wayland/shared/xdgshell.cpp | 1 + tests/auto/wayland/shared/xdgshell.h | 5 +- tests/auto/wayland/xdgshell/tst_xdgshell.cpp | 89 ++++++++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/tests/auto/wayland/shared/xdgshell.cpp b/tests/auto/wayland/shared/xdgshell.cpp index 9437688adf5..13acc01e219 100644 --- a/tests/auto/wayland/shared/xdgshell.cpp +++ b/tests/auto/wayland/shared/xdgshell.cpp @@ -236,6 +236,7 @@ void XdgPopup::xdg_popup_destroy(Resource *resource) { } m_xdgSurface->m_popup = nullptr; m_parentXdgSurface->m_popups.removeAll(this); + emit destroyRequested(); } } // namespace MockCompositor diff --git a/tests/auto/wayland/shared/xdgshell.h b/tests/auto/wayland/shared/xdgshell.h index ca19c293fc3..618babde73a 100644 --- a/tests/auto/wayland/shared/xdgshell.h +++ b/tests/auto/wayland/shared/xdgshell.h @@ -130,8 +130,9 @@ protected: void xdg_toplevel_set_min_size(Resource *resource, int32_t width, int32_t height) override; }; -class XdgPopup : public QtWaylandServer::xdg_popup +class XdgPopup : public QObject, public QtWaylandServer::xdg_popup { + Q_OBJECT public: explicit XdgPopup(XdgSurface *xdgSurface, XdgSurface *parent, int id, int version = 1); void sendConfigure(const QRect &geometry); @@ -141,6 +142,8 @@ public: XdgSurface *m_parentXdgSurface = nullptr; bool m_grabbed = false; uint m_grabSerial = 0; +signals: + void destroyRequested(); protected: void xdg_popup_grab(Resource *resource, ::wl_resource *seat, uint32_t serial) override; void xdg_popup_destroy(Resource *resource) override; diff --git a/tests/auto/wayland/xdgshell/tst_xdgshell.cpp b/tests/auto/wayland/xdgshell/tst_xdgshell.cpp index 32b62689e60..6efffc8a4b6 100644 --- a/tests/auto/wayland/xdgshell/tst_xdgshell.cpp +++ b/tests/auto/wayland/xdgshell/tst_xdgshell.cpp @@ -43,6 +43,7 @@ private slots: void configureStates(); void popup(); void tooltipOnPopup(); + void switchPopups(); void pongs(); void minMaxSize(); void windowGeometry(); @@ -332,6 +333,94 @@ void tst_xdgshell::tooltipOnPopup() QCOMPOSITOR_TRY_COMPARE(xdgPopup(0), nullptr); } +// QTBUG-65680 +void tst_xdgshell::switchPopups() +{ + class Popup : public QRasterWindow { + public: + explicit Popup(QWindow *parent) { + setTransientParent(parent); + setFlags(Qt::Popup); + resize(10, 10); + show(); + } + }; + + class Window : public QRasterWindow { + public: + void mousePressEvent(QMouseEvent *event) override { + QRasterWindow::mousePressEvent(event); + if (!m_popups.empty()) + m_popups.last()->setVisible(false); + } + // The bug this checks for, is the case where there is a flushWindowSystemEvents() call + // somewhere within setVisible(false) before the grab has been released. + // This leads to the the release event below—including its show() call—to be handled + // At a time where there is still an active grab, making it illegal for the new popup to + // grab. + void mouseReleaseEvent(QMouseEvent *event) override { + QRasterWindow::mouseReleaseEvent(event); + m_popups << new Popup(this); + } + ~Window() override { qDeleteAll(m_popups); } + QVector m_popups; + }; + + Window window; + window.resize(200, 200); + window.show(); + + QCOMPOSITOR_TRY_VERIFY(xdgToplevel()); + exec([=] { xdgToplevel()->sendCompleteConfigure(); }); + QCOMPOSITOR_TRY_VERIFY(xdgToplevel()->m_xdgSurface->m_committedConfigureSerial); + + exec([=] { + auto *surface = xdgToplevel()->surface(); + auto *p = pointer(); + p->sendEnter(surface, {100, 100}); +// p->sendFrame(); //TODO: uncomment when we support seat v5 + p->sendButton(client(), BTN_LEFT, Pointer::button_state_pressed); + p->sendButton(client(), BTN_LEFT, Pointer::button_state_released); +// p->sendFrame(); + p->sendLeave(surface); +// p->sendFrame(); + }); + + QCOMPOSITOR_TRY_VERIFY(xdgPopup()); + exec([=] { xdgPopup()->sendCompleteConfigure(QRect(100, 100, 100, 100)); }); + QCOMPOSITOR_TRY_VERIFY(xdgPopup()->m_xdgSurface->m_committedConfigureSerial); + QCOMPOSITOR_TRY_VERIFY(xdgPopup()->m_grabbed); + + QSignalSpy firstDestroyed(exec([=] { return xdgPopup(); }), &XdgPopup::destroyRequested); + + exec([=] { + auto *surface = xdgToplevel()->surface(); + auto *p = pointer(); + p->sendEnter(surface, {100, 100}); +// p->sendFrame(); + p->sendButton(client(), BTN_LEFT, Pointer::button_state_pressed); + p->sendButton(client(), BTN_LEFT, Pointer::button_state_released); +// p->sendFrame(); + }); + + // The client will now hide one popup and then show another + firstDestroyed.wait(); + + QCOMPOSITOR_TRY_VERIFY(xdgPopup()); + QCOMPOSITOR_TRY_VERIFY(!xdgPopup(1)); + + // Verify we still grabbed in case the client has checks to avoid illegal grabs + QCOMPOSITOR_TRY_VERIFY(xdgPopup()->m_grabbed); + + // Sometimes the popup will select another parent if one is illegal at the time it tries to + // grab. So we verify we got the intended parent on the compositor side. + QCOMPOSITOR_TRY_VERIFY(xdgPopup()->m_parentXdgSurface == xdgToplevel()->m_xdgSurface); + + // For good measure just check that configuring works as usual + exec([=] { xdgPopup()->sendCompleteConfigure(QRect(100, 100, 100, 100)); }); + QCOMPOSITOR_TRY_VERIFY(xdgPopup()->m_xdgSurface->m_committedConfigureSerial); +} + void tst_xdgshell::pongs() { QSignalSpy pongSpy(exec([=] { return get(); }), &XdgWmBase::pong); From 11e0fc58b6d065d56669ccbb9fe340a500fb466d Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Thu, 14 Mar 2019 09:37:41 +0100 Subject: [PATCH 11/11] Client: Fix incorrect damage region for window decorations The previous implementation had gaps in several places. [ChangeLog][QPA plugin] Fixed a bug where the window decoration's damaged area didn't cover the entire decoration. This meant some compositors would not redraw those areas. Change-Id: Ic72663dde301936635b2a1cfa90570a53227e8ea Fixes: QTBUG-74341 Reviewed-by: Paul Olav Tvete Reviewed-by: Pier Luigi Fiorini --- .../wayland/qwaylandabstractdecoration.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandabstractdecoration.cpp b/src/plugins/platforms/wayland/qwaylandabstractdecoration.cpp index 6a8f1d92d92..98a0b17d5e1 100644 --- a/src/plugins/platforms/wayland/qwaylandabstractdecoration.cpp +++ b/src/plugins/platforms/wayland/qwaylandabstractdecoration.cpp @@ -100,14 +100,19 @@ void QWaylandAbstractDecoration::setWaylandWindow(QWaylandWindow *window) d->m_wayland_window = window; } -// \a size is without margins +// Creates regions like this on the outside of a rectangle with inner size \a size +// ----- +// | | +// ----- +// I.e. the top and bottom extends into the corners static QRegion marginsRegion(const QSize &size, const QMargins &margins) { QRegion r; - r += QRect(0, 0, size.width(), margins.top()); // top - r += QRect(0, size.height()+margins.top(), size.width(), margins.bottom()); //bottom - r += QRect(0, 0, margins.left(), size.height()); //left - r += QRect(size.width()+margins.left(), 0, margins.right(), size.height()); // right + const int widthWithMargins = margins.left() + size.width() + margins.right(); + r += QRect(0, 0, widthWithMargins, margins.top()); // top + r += QRect(0, size.height()+margins.top(), widthWithMargins, margins.bottom()); //bottom + r += QRect(0, margins.top(), margins.left(), size.height()); //left + r += QRect(size.width()+margins.left(), margins.top(), margins.right(), size.height()); // right return r; }