From ee68d19b1dc786eba66629b63648d93d60b18c9d Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 29 Nov 2021 20:26:03 +0100 Subject: Detach QEventPoint instances during touch compression; test & docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we don't detach, they could be modified between the time that the event is stored (delayed) until it's delivered. QQuickDeliveryAgentPrivate::compressTouchEvent() had no explicit test coverage until now; in fact, most tests call QQuickTouchUtils::flush() after every touch event to ensure that it gets delivered immediately. Also add internal docs. Fixes: QTBUG-97185 Fixes: QTBUG-98486 Fixes: QTBUG-98543 Change-Id: I6fd76651ca6fbf15169c44d4d9dbbeb7dc7e3a71 Reviewed-by: Shawn Rutledge Reviewed-by: Jan Arve Sæther (cherry picked from commit d08038ba7015dc681c95654f7fe5e54b60c2e55c) Reviewed-by: Qt Cherry-pick Bot --- src/quick/util/qquickdeliveryagent.cpp | 42 ++++++++++++++++- src/quick/util/qquickdeliveryagent_p_p.h | 1 + .../qquickdeliveryagent/data/pointHandler.qml | 17 +++++++ .../tst_qquickdeliveryagent.cpp | 54 ++++++++++++++++++++++ 4 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 tests/auto/quick/qquickdeliveryagent/data/pointHandler.qml diff --git a/src/quick/util/qquickdeliveryagent.cpp b/src/quick/util/qquickdeliveryagent.cpp index 931d27726b..643225ed8e 100644 --- a/src/quick/util/qquickdeliveryagent.cpp +++ b/src/quick/util/qquickdeliveryagent.cpp @@ -1209,6 +1209,7 @@ void QQuickDeliveryAgentPrivate::deliverDelayedTouchEvent() // event loop recursions (e.g if it the touch starts a dnd session). QScopedPointer e(delayedTouch.take()); qCDebug(lcTouchCmprs) << "delivering" << e.data(); + compressedTouchCount = 0; deliverPointerEvent(e.data()); } @@ -1365,10 +1366,35 @@ QQuickPointingDeviceExtra *QQuickDeliveryAgentPrivate::deviceExtra(const QInputD return extra; } +/*! + \internal + This function is called from handleTouchEvent() in case a series of touch + events containing only \c Updated and \c Stationary points arrives within a + short period of time. (Some touchscreens are more "jittery" than others.) + + It would be a waste of CPU time to deliver events and have items in the + scene getting modified more often than once per frame; so here we try to + coalesce the series of updates into a single event containing all updates + that occur within one frame period, and deliverDelayedTouchEvent() is + called from flushFrameSynchronousEvents() to send that single event. This + is the reason why touch compression lives here so far, instead of in a + lower layer: the render loop updates the scene in sync with the screen's + vsync, and flushFrameSynchronousEvents() is called from there (for example + from QSGThreadedRenderLoop::polishAndSync(), and equivalent places in other + render loops). It would be preferable to move this code down to a lower + level eventually, though, because it's not fundamentally a Qt Quick concern. + + This optimization can be turned off by setting the environment variable + \c QML_NO_TOUCH_COMPRESSION. + + Returns \c true if "done", \c false if the caller needs to finish the + \a event delivery. +*/ bool QQuickDeliveryAgentPrivate::compressTouchEvent(QTouchEvent *event) { QEventPoint::States states = event->touchPointStates(); if (states.testFlag(QEventPoint::State::Pressed) || states.testFlag(QEventPoint::State::Released)) { + qCDebug(lcTouchCmprs) << "no compression" << event; // we can only compress an event that doesn't include any pressed or released points return false; } @@ -1376,7 +1402,12 @@ bool QQuickDeliveryAgentPrivate::compressTouchEvent(QTouchEvent *event) if (!delayedTouch) { delayedTouch.reset(new QMutableTouchEvent(event->type(), event->pointingDevice(), event->modifiers(), event->points())); delayedTouch->setTimestamp(event->timestamp()); - qCDebug(lcTouchCmprs) << "delayed" << delayedTouch.data(); + for (qsizetype i = 0; i < delayedTouch->pointCount(); ++i) { + auto &tp = delayedTouch->point(i); + QMutableEventPoint::detach(tp); + } + ++compressedTouchCount; + qCDebug(lcTouchCmprs) << "delayed" << compressedTouchCount << delayedTouch.data(); if (QQuickWindow *window = rootItem->window()) window->maybeUpdate(); return true; @@ -1410,7 +1441,14 @@ bool QQuickDeliveryAgentPrivate::compressTouchEvent(QTouchEvent *event) // TODO optimize, or move event compression elsewhere delayedTouch.reset(new QMutableTouchEvent(event->type(), event->pointingDevice(), event->modifiers(), tpts)); delayedTouch->setTimestamp(event->timestamp()); - qCDebug(lcTouchCmprs) << "coalesced" << delayedTouch.data(); + for (qsizetype i = 0; i < delayedTouch->pointCount(); ++i) { + auto &tp = delayedTouch->point(i); + QMutableEventPoint::detach(tp); + } + ++compressedTouchCount; + qCDebug(lcTouchCmprs) << "coalesced" << compressedTouchCount << delayedTouch.data(); + if (QQuickWindow *window = rootItem->window()) + window->maybeUpdate(); return true; } } diff --git a/src/quick/util/qquickdeliveryagent_p_p.h b/src/quick/util/qquickdeliveryagent_p_p.h index 3ee295f133..3e551beb27 100644 --- a/src/quick/util/qquickdeliveryagent_p_p.h +++ b/src/quick/util/qquickdeliveryagent_p_p.h @@ -116,6 +116,7 @@ public: #if QT_CONFIG(wheelevent) uint lastWheelEventAccepted = 0; #endif + uchar compressedTouchCount = 0; bool allowChildEventFiltering = true; bool allowDoubleClick = true; bool frameSynchronousHoverEnabled = true; diff --git a/tests/auto/quick/qquickdeliveryagent/data/pointHandler.qml b/tests/auto/quick/qquickdeliveryagent/data/pointHandler.qml new file mode 100644 index 0000000000..a8afa8f78a --- /dev/null +++ b/tests/auto/quick/qquickdeliveryagent/data/pointHandler.qml @@ -0,0 +1,17 @@ +import QtQuick + +Rectangle { + id: root + objectName: "root" + color: ph.active ? "coral" : "cadetblue" + border.color: "black" + width: 100; height: 100 + PointHandler { + id: ph + objectName: root.objectName + "PointHandler" + } + Text { + anchors.centerIn: parent + text: ph.point.position.x.toFixed(1) + ", " + ph.point.position.y.toFixed(1) + } +} diff --git a/tests/auto/quick/qquickdeliveryagent/tst_qquickdeliveryagent.cpp b/tests/auto/quick/qquickdeliveryagent/tst_qquickdeliveryagent.cpp index 2d127203d9..2de8f403d1 100644 --- a/tests/auto/quick/qquickdeliveryagent/tst_qquickdeliveryagent.cpp +++ b/tests/auto/quick/qquickdeliveryagent/tst_qquickdeliveryagent.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -133,6 +134,7 @@ public: private slots: void passiveGrabberOrder(); void tapHandlerDoesntOverrideSubsceneGrabber(); + void touchCompression(); private: QScopedPointer touchDevice = QScopedPointer(QTest::createTouchDevice()); @@ -224,6 +226,58 @@ void tst_qquickdeliveryagent::tapHandlerDoesntOverrideSubsceneGrabber() // QTBUG QCOMPARE(clickSpy.count(), 0); // doesn't tap } +void tst_qquickdeliveryagent::touchCompression() +{ + QQuickView window; + // avoid interference from X11 window managers, so we can look at eventpoint globalPosition + window.setFlag(Qt::FramelessWindowHint); +#ifdef DISABLE_HOVER_IN_IRRELEVANT_TESTS + QQuickWindowPrivate::get(&window)->deliveryAgentPrivate()->frameSynchronousHoverEnabled = false; +#endif + QVERIFY(QQuickTest::showView(window, testFileUrl("pointHandler.qml"))); + QQuickDeliveryAgent *windowAgent = QQuickWindowPrivate::get(&window)->deliveryAgent; + QQuickDeliveryAgentPrivate *agentPriv = static_cast(QQuickDeliveryAgentPrivate::get(windowAgent)); + QQuickItem *root = qobject_cast(window.rootObject()); + QVERIFY(root); + QQuickPointHandler *rootHandler = root->findChild(); + QVERIFY(rootHandler); + QTest::QTouchEventSequence touch = QTest::touchEvent(&window, touchDevice.data()); + QPoint pt1(30, 50); + QPoint pt2(70, 50); + // Press and drag fast, alternating moving and stationary points + touch.press(11, pt1).press(12, pt2).commit(); + QQuickTouchUtils::flush(&window); + QTest::qWait(50); // not critical, but let it hopefully render a frame or two + QCOMPARE(agentPriv->compressedTouchCount, 0); + for (int m = 1; m < 4; ++m) { + pt1 += {0, 1}; + pt2 -= {0, 1}; + if (m % 2) + touch.move(11, pt1).stationary(12).commit(); + else + touch.stationary(11).move(12, pt2).commit(); + // don't call QQuickTouchUtils::flush() here: we want to see the compression happen + if (agentPriv->compressedTouchCount) { + if (m % 2) { + QCOMPARE(agentPriv->delayedTouch->point(0).position().toPoint(), pt1); + QCOMPARE(agentPriv->delayedTouch->point(0).globalPosition().toPoint(), root->mapToGlobal(pt1).toPoint()); + } else { + QCOMPARE(agentPriv->delayedTouch->point(1).position().toPoint(), pt2); + QCOMPARE(agentPriv->delayedTouch->point(1).globalPosition().toPoint(), root->mapToGlobal(pt2).toPoint()); + } + } + // we can't guarantee that a CI VM is fast enough, but usually compressedTouchCount == m + qCDebug(lcTests) << "compressedTouchCount" << agentPriv->compressedTouchCount << "expected" << m; + qCDebug(lcTests) << "PointHandler still sees" << rootHandler->point().position() << "while" << pt1 << "was likely not yet delivered"; + } + QTRY_COMPARE(rootHandler->point().position().toPoint(), pt1); + touch.release(11, pt1).release(12, pt2).commit(); + // should be delivered, bypassing compression; when PointHandler gets the release, it will reset its point + QTRY_COMPARE(rootHandler->active(), false); + QCOMPARE(rootHandler->point().position(), QPointF()); + QCOMPARE(agentPriv->compressedTouchCount, 0); +} + QTEST_MAIN(tst_qquickdeliveryagent) #include "tst_qquickdeliveryagent.moc" -- cgit v1.2.3