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
Pick-to: 6.2 6.3
Change-Id: I6fd76651ca6fbf15169c44d4d9dbbeb7dc7e3a71
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
This commit is contained in:
Shawn Rutledge 2021-11-29 20:26:03 +01:00
parent 5ed37bbbf1
commit d08038ba70
4 changed files with 112 additions and 2 deletions

View File

@ -1209,6 +1209,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());
}
@ -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;
}
}

View File

@ -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;

View File

@ -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)
}
}

View File

@ -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"