aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn Rutledge <shawn.rutledge@qt.io>2023-10-25 19:13:28 -0700
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2023-11-02 03:26:40 +0000
commitca9e3ee30fe6e1756c219d9383667c26ada97a60 (patch)
tree65c39f7c61cc4c237a79a382db97dcb5559ecabd
parent18eb16e42ff1a4783a6571aa6b948d43805e372d (diff)
Localize Flickable delayed release to scene, not to grabber
When pressDelay > 0, if Flickable receives mouse or touch release before it has sent the delayed press, it needs to send the delayed press before allowing the release to be seen, in some sense. Further back in d02131e743597b9bd3070d986c61a1c91ea8317a the continuation of release event delivery after the delayed press seems to have been more limited in scope, to say the least. 8673ae8bb6d4bac01cc54638a7d617072299a808 added localPos = window()->mouseGrabberItem()->mapFromScene(...) QQuickWindowPrivate::cloneMouseEvent(event, localPos) window()->sendEvent(window()->mouseGrabberItem(), mouseEvent) where QQuickWindow::sendEvent() was _not_ the same as sending the event to the whole window (the grabber is the intended recipient, but sendEvent() was taking care of child event filtering back then). But over time we became convinced that sendEvent() should not exist, because it was used for non-standard forms of event delivery, could be hacked over time to do arbitrary things, and bypassed any old-school QObject event filters that users might expect to work. In bfde65a8180e3dbf811e757f6d95f9554f622475 though, it was assumed that since the delayed press got delivered to the whole scene, the release should be too. Now it looks a bit redundant: one can see in the debugger that the release gets an extra nested delivery to the whole scene again, after being partially delivered before the delayed press is sent. But that might be risky to change right now. So at the time of writing d7623d79ef4bc9533fced027bf1d173d68b4eba6, QQuickFlickable::mouseReleaseEvent() used an item-localized position (leftover code from 8673ae8bb6d4bac01cc54638a7d617072299a808) and it seemed reasonable to follow that precedent to handle delivery of a delayed touch press and then the release; but in retrospect, 1) we're sending the event to a window, so we don't expect the grabber-localized position to be retained; 2) in fact, QQuickDeliveryAgentPrivate::translateTouchEvent() treats the local position as scene position. QTBUG-118069 occurred because of this assumption being violated. 3) Even for mouse release, it no longer makes sense to localize to the grabber, now that we are sending the event to the window rather than just to the grabber (for quite a number of years already). It will get relocalized during delivery to each item and handler visited. 4) Maybe it doesn't even matter whether there is a grabber or not: we could resend the release to the whole scene regardless. But this patch is conservative; and now we optimize slightly by using QObject::isQuickItemType() rather than qmlobject_cast. tst_qquickflickable::pressDelay() tests the same old scenarios as before, but now with both mouse and touch, and gets a general revamping while we're at it. Pick-to: 6.5 Fixes: QTBUG-118069 Change-Id: I0f33d23ac1eae9fd697f2eca315107169619706c Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io> Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io> (cherry picked from commit 45d4ccc765b7fae86aca749f7b0aabc9c4671b23) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--src/quick/items/qquickflickable.cpp23
-rw-r--r--tests/auto/quick/qquickflickable/data/pressDelay.qml12
-rw-r--r--tests/auto/quick/qquickflickable/tst_qquickflickable.cpp100
3 files changed, 75 insertions, 60 deletions
diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp
index d7a53bcb13..65a5a30205 100644
--- a/src/quick/items/qquickflickable.cpp
+++ b/src/quick/items/qquickflickable.cpp
@@ -1523,13 +1523,14 @@ void QQuickFlickable::mouseReleaseEvent(QMouseEvent *event)
if (d->delayedPressEvent) {
d->replayDelayedPress();
- // Now send the release
- if (auto grabber = qmlobject_cast<QQuickItem *>(event->exclusiveGrabber(event->point(0)))) {
- // not copying or detaching anything, so make sure we return the original event unchanged
- const auto oldPosition = event->point(0).position();
- QMutableEventPoint::setPosition(event->point(0), grabber->mapFromScene(event->scenePosition()));
+ auto &firstPoint = event->point(0);
+ if (const auto *grabber = event->exclusiveGrabber(firstPoint); grabber && grabber->isQuickItemType()) {
+ // Since we sent the delayed press to the window, we need to resend the release to the window too.
+ // We're not copying or detaching, so restore the original event position afterwards.
+ const auto oldPosition = firstPoint.position();
+ QMutableEventPoint::setPosition(firstPoint, event->scenePosition());
QCoreApplication::sendEvent(window(), event);
- QMutableEventPoint::setPosition(event->point(0), oldPosition);
+ QMutableEventPoint::setPosition(firstPoint, oldPosition);
}
// And the event has been consumed
@@ -1582,11 +1583,11 @@ void QQuickFlickable::touchEvent(QTouchEvent *event)
if (d->delayedPressEvent) {
d->replayDelayedPress();
- // Now send the release
- auto &firstPoint = event->point(0);
- if (auto grabber = qmlobject_cast<QQuickItem *>(event->exclusiveGrabber(firstPoint))) {
- const auto localPos = grabber->mapFromScene(firstPoint.scenePosition());
- QScopedPointer<QPointerEvent> localizedEvent(QQuickDeliveryAgentPrivate::clonePointerEvent(event, localPos));
+ const auto &firstPoint = event->point(0);
+ if (const auto *grabber = event->exclusiveGrabber(firstPoint); grabber && grabber->isQuickItemType()) {
+ // Since we sent the delayed press to the window, we need to resend the release to the window too.
+ QScopedPointer<QPointerEvent> localizedEvent(
+ QQuickDeliveryAgentPrivate::clonePointerEvent(event, firstPoint.scenePosition()));
QCoreApplication::sendEvent(window(), localizedEvent.data());
}
diff --git a/tests/auto/quick/qquickflickable/data/pressDelay.qml b/tests/auto/quick/qquickflickable/data/pressDelay.qml
index a69c4af6de..97bc6b794f 100644
--- a/tests/auto/quick/qquickflickable/data/pressDelay.qml
+++ b/tests/auto/quick/qquickflickable/data/pressDelay.qml
@@ -1,4 +1,4 @@
-import QtQuick 2.0
+import QtQuick
Flickable {
flickableDirection: Flickable.VerticalFlick
@@ -15,7 +15,6 @@ Flickable {
MouseArea {
id: ma
- objectName: "mouseArea"
y: 100
anchors.horizontalCenter: parent.horizontalCenter
width: 240
@@ -32,14 +31,7 @@ Flickable {
Rectangle {
anchors.fill: parent
- color: parent.pressed ? 'blue' : 'green'
-
- Text {
- anchors.fill: parent
- verticalAlignment: Text.AlignVCenter
- horizontalAlignment: Text.AlignHCenter
- text: 'Hello'
- }
+ color: parent.pressed ? 'blue' : 'lightsteelblue'
}
}
}
diff --git a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
index 5451e3466c..88d19eb6c6 100644
--- a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
+++ b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
@@ -167,6 +167,7 @@ private slots:
void rebound();
void maximumFlickVelocity();
void flickDeceleration();
+ void pressDelay_data();
void pressDelay();
void nestedPressDelay();
void filterReplayedPress();
@@ -233,6 +234,10 @@ private slots:
private:
void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to);
QPointingDevice *touchDevice = QTest::createTouchDevice();
+ const QPointingDevice *mouseDevice = new QPointingDevice(
+ "test mouse", 1000, QInputDevice::DeviceType::Mouse, QPointingDevice::PointerType::Generic,
+ QInputDevice::Capability::Position | QInputDevice::Capability::Hover | QInputDevice::Capability::Scroll,
+ 1, 5, QString(), QPointingDeviceUniqueId(), this);
};
void tst_qquickflickable::initTestCase()
@@ -242,6 +247,8 @@ void tst_qquickflickable::initTestCase()
#endif
QQmlDataTest::initTestCase();
qmlRegisterType<TouchDragArea>("Test",1,0,"TouchDragArea");
+ touchDevice->setParent(this); // avoid leak
+ QWindowSystemInterface::registerInputDevice(mouseDevice);
}
void tst_qquickflickable::cleanup()
@@ -527,45 +534,54 @@ void tst_qquickflickable::flickDeceleration()
delete flickable;
}
+void tst_qquickflickable::pressDelay_data()
+{
+ QTest::addColumn<const QPointingDevice *>("device");
+ const QPointingDevice *constTouchDevice = touchDevice;
+
+ QTest::newRow("mouse") << mouseDevice;
+ QTest::newRow("touch") << constTouchDevice;
+}
+
void tst_qquickflickable::pressDelay()
{
- QScopedPointer<QQuickView> window(new QQuickView);
- window->setSource(testFileUrl("pressDelay.qml"));
- QTRY_COMPARE(window->status(), QQuickView::Ready);
- QQuickViewTestUtils::centerOnScreen(window.data());
- QQuickViewTestUtils::moveMouseAway(window.data());
- window->show();
- QVERIFY(QTest::qWaitForWindowActive(window.data()));
- QVERIFY(window->rootObject() != nullptr);
+ QFETCH(const QPointingDevice *, device);
- QQuickFlickable *flickable = qobject_cast<QQuickFlickable*>(window->rootObject());
- QSignalSpy spy(flickable, SIGNAL(pressDelayChanged()));
+ QQuickView window;
+ QVERIFY(QQuickTest::showView(window, testFileUrl("pressDelay.qml")));
+ QQuickFlickable *flickable = qobject_cast<QQuickFlickable*>(window.rootObject());
QVERIFY(flickable);
- QCOMPARE(flickable->pressDelay(), 100);
+ QQuickMouseArea *mouseArea = flickable->findChild<QQuickMouseArea*>();
+ QSignalSpy clickedSpy(mouseArea, &QQuickMouseArea::clicked);
+ // Test the pressDelay property itself
+ QSignalSpy pressDelayChangedSpy(flickable, &QQuickFlickable::pressDelayChanged);
+ QCOMPARE(flickable->pressDelay(), 100);
flickable->setPressDelay(200);
QCOMPARE(flickable->pressDelay(), 200);
- QCOMPARE(spy.size(),1);
+ QCOMPARE(pressDelayChangedSpy.size(), 1);
flickable->setPressDelay(200);
- QCOMPARE(spy.size(),1);
+ QCOMPARE(pressDelayChangedSpy.size(), 1);
- QQuickItem *mouseArea = window->rootObject()->findChild<QQuickItem*>("mouseArea");
- QSignalSpy clickedSpy(mouseArea, SIGNAL(clicked(QQuickMouseEvent*)));
- moveAndPress(window.data(), QPoint(150, 150));
+ // Test the press delay
+ QPoint p(150, 150);
+ if (device->type() == QInputDevice::DeviceType::Mouse)
+ QQuickTest::pointerMove(device, &window, 0, p);
+ QQuickTest::pointerPress(device, &window, 0, p);
// The press should not occur immediately
- QVERIFY(!mouseArea->property("pressed").toBool());
+ QCOMPARE(mouseArea->isPressed(), false);
// But, it should occur eventually
- QTRY_VERIFY(mouseArea->property("pressed").toBool());
+ QTRY_VERIFY(mouseArea->isPressed());
- QCOMPARE(clickedSpy.size(),0);
+ QCOMPARE(clickedSpy.size(), 0);
// On release the clicked signal should be emitted
- QTest::mouseRelease(window.data(), Qt::LeftButton, Qt::NoModifier, QPoint(150, 150));
- QCOMPARE(clickedSpy.size(),1);
+ QQuickTest::pointerRelease(device, &window, 0, p);
+ QCOMPARE(clickedSpy.size(), 1);
// Press and release position should match
QCOMPARE(flickable->property("pressX").toReal(), flickable->property("releaseX").toReal());
@@ -573,38 +589,44 @@ void tst_qquickflickable::pressDelay()
// Test a quick tap within the pressDelay timeout
+ p = QPoint(180, 180);
clickedSpy.clear();
- moveAndPress(window.data(), QPoint(180, 180));
+ if (device->type() == QInputDevice::DeviceType::Mouse)
+ QQuickTest::pointerMove(device, &window, 0, p);
+ QQuickTest::pointerPress(device, &window, 0, p);
// The press should not occur immediately
- QVERIFY(!mouseArea->property("pressed").toBool());
-
- QCOMPARE(clickedSpy.size(),0);
+ QCOMPARE(mouseArea->isPressed(), false);
+ QCOMPARE(clickedSpy.size(), 0);
- // On release the press, release and clicked signal should be emitted
- QTest::mouseRelease(window.data(), Qt::LeftButton, Qt::NoModifier, QPoint(180, 180));
- QCOMPARE(clickedSpy.size(),1);
+ // On release, the press, release and clicked signal should be emitted
+ QQuickTest::pointerRelease(device, &window, 0, p);
+ QCOMPARE(clickedSpy.size(), 1);
// Press and release position should match
QCOMPARE(flickable->property("pressX").toReal(), flickable->property("releaseX").toReal());
QCOMPARE(flickable->property("pressY").toReal(), flickable->property("releaseY").toReal());
- // QTBUG-31168
- moveAndPress(window.data(), QPoint(150, 110));
+ // Test flick after press (QTBUG-31168)
+ QPoint startPosition(150, 110);
+ p = QPoint(150, 190);
+ clickedSpy.clear();
+ if (device->type() == QInputDevice::DeviceType::Mouse)
+ QQuickTest::pointerMove(device, &window, 0, startPosition);
+ QQuickTest::pointerPress(device, &window, 0, startPosition);
// The press should not occur immediately
- QVERIFY(!mouseArea->property("pressed").toBool());
-
- QTest::mouseMove(window.data(), QPoint(150, 190));
+ QCOMPARE(mouseArea->isPressed(), false);
+ QQuickTest::pointerMove(device, &window, 0, p);
- // As we moved pass the drag threshold, we should never receive the press
- QVERIFY(!mouseArea->property("pressed").toBool());
- QTRY_VERIFY(!mouseArea->property("pressed").toBool());
+ // Since we moved past the drag threshold, we should never receive the press
+ QCOMPARE(mouseArea->isPressed(), false);
+ QTRY_COMPARE(mouseArea->isPressed(), false);
- // On release the clicked signal should *not* be emitted
- QTest::mouseRelease(window.data(), Qt::LeftButton, Qt::NoModifier, QPoint(150, 190));
- QCOMPARE(clickedSpy.size(),1);
+ // On release, the clicked signal should *not* be emitted
+ QQuickTest::pointerRelease(device, &window, 0, p);
+ QCOMPARE(clickedSpy.size(), 0);
}
// QTBUG-17361