diff options
author | Shawn Rutledge <shawn.rutledge@qt.io> | 2023-10-25 19:13:28 -0700 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2023-11-02 03:26:40 +0000 |
commit | ca9e3ee30fe6e1756c219d9383667c26ada97a60 (patch) | |
tree | 65c39f7c61cc4c237a79a382db97dcb5559ecabd | |
parent | 18eb16e42ff1a4783a6571aa6b948d43805e372d (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.cpp | 23 | ||||
-rw-r--r-- | tests/auto/quick/qquickflickable/data/pressDelay.qml | 12 | ||||
-rw-r--r-- | tests/auto/quick/qquickflickable/tst_qquickflickable.cpp | 100 |
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 |