From b8b8c1ce5acceab8751f5c192314e305565503f0 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Thu, 28 Nov 2019 02:31:17 +0100 Subject: [PATCH 1/2] Avoid animating single frame cursors Currently to determine if a cursor is animated or not we check the cursor theme delay. This doesn't work in practice as by default many cursor themes have a delay of 50 set even if they don't animate. This comes from xcursorgen which specifies a delay of 50ms if there isn't anything set in the config. (https://github.com/freedesktop/xcursorgen/blob/master/xcursorgen.c#L92) Given many themes will have a delay we should also check the number of images in a given cursor. In order to do that without a double lookup QWaylandCursor needed to return the native wl_cursor, not wl_cursor_image and move the relevant logic. Change-Id: Ie782ace8054910ae76e61cab33ceca0377194929 Reviewed-by: Johan Helsing --- src/plugins/platforms/wayland/qwaylandcursor.cpp | 12 ++---------- src/plugins/platforms/wayland/qwaylandcursor_p.h | 3 +-- .../platforms/wayland/qwaylandinputdevice.cpp | 16 ++++++++++++---- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandcursor.cpp b/src/plugins/platforms/wayland/qwaylandcursor.cpp index 4356b23a0a8..1d3d88bea3a 100644 --- a/src/plugins/platforms/wayland/qwaylandcursor.cpp +++ b/src/plugins/platforms/wayland/qwaylandcursor.cpp @@ -219,7 +219,7 @@ wl_cursor *QWaylandCursorTheme::requestCursor(WaylandCursor shape) return nullptr; } -::wl_cursor_image *QWaylandCursorTheme::cursorImage(Qt::CursorShape shape, uint millisecondsIntoAnimation) +::wl_cursor *QWaylandCursorTheme::cursor(Qt::CursorShape shape) { struct wl_cursor *waylandCursor = nullptr; @@ -237,15 +237,7 @@ wl_cursor *QWaylandCursorTheme::requestCursor(WaylandCursor shape) return nullptr; } - int frame = wl_cursor_frame(waylandCursor, millisecondsIntoAnimation); - ::wl_cursor_image *image = waylandCursor->images[frame]; - ::wl_buffer *buffer = wl_cursor_image_get_buffer(image); - if (!buffer) { - qCWarning(lcQpaWayland) << "Could not find buffer for cursor"; - return nullptr; - } - - return image; + return waylandCursor; } QWaylandCursor::QWaylandCursor(QWaylandDisplay *display) diff --git a/src/plugins/platforms/wayland/qwaylandcursor_p.h b/src/plugins/platforms/wayland/qwaylandcursor_p.h index a4605f3d275..751ffa68b71 100644 --- a/src/plugins/platforms/wayland/qwaylandcursor_p.h +++ b/src/plugins/platforms/wayland/qwaylandcursor_p.h @@ -75,7 +75,7 @@ class Q_WAYLAND_CLIENT_EXPORT QWaylandCursorTheme public: static QWaylandCursorTheme *create(QWaylandShm *shm, int size, const QString &themeName); ~QWaylandCursorTheme(); - ::wl_cursor_image *cursorImage(Qt::CursorShape shape, uint millisecondsIntoAnimation = 0); + ::wl_cursor *cursor(Qt::CursorShape shape); private: enum WaylandCursor { @@ -129,7 +129,6 @@ public: void setPos(const QPoint &pos) override; static QSharedPointer cursorBitmapBuffer(QWaylandDisplay *display, const QCursor *cursor); - struct wl_cursor_image *cursorImage(Qt::CursorShape shape); private: QWaylandDisplay *mDisplay = nullptr; diff --git a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp index a4098edd3a7..d812918e72a 100644 --- a/src/plugins/platforms/wayland/qwaylandinputdevice.cpp +++ b/src/plugins/platforms/wayland/qwaylandinputdevice.cpp @@ -283,8 +283,8 @@ void QWaylandInputDevice::Pointer::updateCursorTheme() if (!mCursor.theme) return; // A warning has already been printed in loadCursorTheme - if (auto *arrow = mCursor.theme->cursorImage(Qt::ArrowCursor)) { - int arrowPixelSize = qMax(arrow->width, arrow->height); // Not all cursor themes are square + if (auto *arrow = mCursor.theme->cursor(Qt::ArrowCursor)) { + int arrowPixelSize = qMax(arrow->images[0]->width, arrow->images[0]->height); // Not all cursor themes are square while (scale > 1 && arrowPixelSize / scale < cursorSize()) --scale; } else { @@ -326,12 +326,20 @@ void QWaylandInputDevice::Pointer::updateCursor() // Set from shape using theme uint time = seat()->mCursor.animationTimer.elapsed(); - if (struct ::wl_cursor_image *image = mCursor.theme->cursorImage(shape, time)) { + + if (struct ::wl_cursor *waylandCursor = mCursor.theme->cursor(shape)) { + int frame = wl_cursor_frame(waylandCursor, time); + ::wl_cursor_image *image = waylandCursor->images[frame]; + struct wl_buffer *buffer = wl_cursor_image_get_buffer(image); + if (!buffer) { + qCWarning(lcQpaWayland) << "Could not find buffer for cursor" << shape; + return; + } int bufferScale = mCursor.themeBufferScale; QPoint hotspot = QPoint(image->hotspot_x, image->hotspot_y) / bufferScale; QSize size = QSize(image->width, image->height) / bufferScale; - bool animated = image->delay > 0; + bool animated = waylandCursor->image_count > 1 && image->delay > 0; getOrCreateCursorSurface()->update(buffer, hotspot, size, bufferScale, animated); return; } From d9b7325cc030080cc42542b26392e68c153c02da Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 4 Nov 2019 14:21:51 +0100 Subject: [PATCH 2/2] Add client test for touch down and motion in same frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task-number: QTBUG-79744 Change-Id: Ibb8239b4f53a345105bee3cc7a0fb4b777cabf9b Reviewed-by: Jan Arve Sæther Reviewed-by: Paul Olav Tvete --- tests/auto/wayland/seatv5/tst_seatv5.cpp | 33 ++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/auto/wayland/seatv5/tst_seatv5.cpp b/tests/auto/wayland/seatv5/tst_seatv5.cpp index 636f260815a..e333082ecc7 100644 --- a/tests/auto/wayland/seatv5/tst_seatv5.cpp +++ b/tests/auto/wayland/seatv5/tst_seatv5.cpp @@ -71,6 +71,7 @@ private slots: void singleTapFloat(); void multiTouch(); void multiTouchUpAndMotionFrame(); + void tapAndMoveInSameFrame(); }; void tst_seatv5::bindsToSeat() @@ -586,5 +587,37 @@ void tst_seatv5::multiTouchUpAndMotionFrame() QVERIFY(window.m_events.empty()); } +void tst_seatv5::tapAndMoveInSameFrame() +{ + TouchWindow window; + QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial); + + exec([=] { + auto *t = touch(); + auto *c = client(); + + t->sendDown(xdgToplevel()->surface(), {32, 32}, 0); + t->sendMotion(c, {33, 33}, 0); + t->sendFrame(c); + + // Don't leave touch in a weird state + t->sendUp(c, 0); + t->sendFrame(c); + }); + + QTRY_VERIFY(!window.m_events.empty()); + { + auto e = window.m_events.takeFirst(); + QCOMPARE(e.type, QEvent::TouchBegin); + QCOMPARE(e.touchPoints.size(), 1); + QCOMPARE(e.touchPoints[0].state(), Qt::TouchPointState::TouchPointPressed); + // Position isn't that important, we just want to make sure we actually get the pressed event + } + + // Make sure we eventually release + QTRY_VERIFY(!window.m_events.empty()); + QTRY_COMPARE(window.m_events.last().touchPoints.first().state(), Qt::TouchPointState::TouchPointReleased); +} + QCOMPOSITOR_TEST_MAIN(tst_seatv5) #include "tst_seatv5.moc"