Detach QEventPoint instances during touch compression; test & docs

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 <shawn.rutledge@qt.io>
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
(cherry picked from commit d08038ba7015dc681c95654f7fe5e54b60c2e55c)

4 files changed, 112 insertions, 2 deletions

diff --git a/src/quick/util/qquickdeliveryagent.cpp b/src/quick/util/qquickdeliveryagent.cpp
index 6c40551a47..680398e2e9 100644
--- a/src/quick/util/qquickdeliveryagent.cpp
+++ b/src/quick/util/qquickdeliveryagent.cpp
@@ -1204,6 +1204,7 @@ void QQuickDeliveryAgentPrivate::deliverDelayedTouchEvent()
     // event loop recursions (e.g if it the touch starts a dnd session).
     QScopedPointer<QTouchEvent> e(delayedTouch.take());
     qCDebug(lcTouchCmprs) << "delivering" << e.data();
+    compressedTouchCount = 0;
     deliverPointerEvent(e.data());
 }
 
@@ -1360,10 +1361,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;
     }
@@ -1371,7 +1397,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::from(tp).detach();
+        }
+        ++compressedTouchCount;
+        qCDebug(lcTouchCmprs) << "delayed" << compressedTouchCount << delayedTouch.data();
         if (QQuickWindow *window = rootItem->window())
             window->maybeUpdate();
         return true;
@@ -1405,7 +1436,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::from(tp).detach();
+            }
+            ++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 461884f75b..94ce27afd2 100644
--- a/tests/auto/quick/qquickdeliveryagent/tst_qquickdeliveryagent.cpp
+++ b/tests/auto/quick/qquickdeliveryagent/tst_qquickdeliveryagent.cpp
@@ -35,6 +35,7 @@
 #include <QtQuick/QQuickView>
 #include <QtQuick/QQuickWindow>
 #include <QtQuick/private/qquickflickable_p.h>
+#include <QtQuick/private/qquickpointhandler_p.h>
 #include <QtQuick/private/qquickshadereffectsource_p.h>
 #include <QtQuick/private/qquicktaphandler_p.h>
 #include <QtQuick/private/qquickwindow_p.h>
@@ -133,6 +134,7 @@ public:
 private slots:
     void passiveGrabberOrder();
     void tapHandlerDoesntOverrideSubsceneGrabber();
+    void touchCompression();
 
 private:
     QScopedPointer<QPointingDevice> touchDevice = QScopedPointer<QPointingDevice>(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 *>(QQuickDeliveryAgentPrivate::get(windowAgent));
+    QQuickItem *root = qobject_cast<QQuickItem*>(window.rootObject());
+    QVERIFY(root);
+    QQuickPointHandler *rootHandler = root->findChild<QQuickPointHandler *>();
+    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"