qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 d02131e743 the
continuation of release event delivery after the delayed press seems to
have been more limited in scope, to say the least.

8673ae8bb6 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 bfde65a818 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 d7623d79ef,
QQuickFlickable::mouseReleaseEvent() used an item-localized position
(leftover code from 8673ae8bb6)
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.

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 45d4ccc765)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit ca9e3ee30f)
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
This commit is contained in:
Shawn Rutledge 2023-10-25 19:13:28 -07:00
parent 107cb70b6d
commit f7c656af68
3 changed files with 76 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/quick/items/qquickflickable.cpp
View File

@ -1521,13 +1521,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
@ -1580,11 +1581,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());
}

12
tests/auto/quick/qquickflickable/data/pressDelay.qml
View File

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

102
tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
View File

@ -165,6 +165,7 @@ private slots:
void rebound();
void maximumFlickVelocity();
void flickDeceleration();
void pressDelay_data();
void pressDelay();
void nestedPressDelay();
void filterReplayedPress();
@ -229,6 +230,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()
@ -238,6 +243,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()
@ -523,45 +530,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->pressed(), false);
// But, it should occur eventually
QTRY_VERIFY(mouseArea->property("pressed").toBool());
QTRY_VERIFY(mouseArea->pressed());
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());
@ -569,38 +585,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(mouseArea->pressed(), false);
QCOMPARE(clickedSpy.size(), 0);
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());
QCOMPARE(mouseArea->pressed(), false);
QQuickTest::pointerMove(device, &window, 0, p);
QTest::mouseMove(window.data(), QPoint(150, 190));
// Since we moved past the drag threshold, we should never receive the press
QCOMPARE(mouseArea->pressed(), false);
QTRY_COMPARE(mouseArea->pressed(), false);
// As we moved pass the drag threshold, we should never receive the press
QVERIFY(!mouseArea->property("pressed").toBool());
QTRY_VERIFY(!mouseArea->property("pressed").toBool());
// 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
@ -1951,7 +1973,7 @@ void tst_qquickflickable::nestedStopAtBounds()
// drag toward the aligned boundary. Outer flickable dragged.
moveAndPress(&view, position);
if (waitForPressDelay) {
QVERIFY(innerFiltering); // isPressed will never be true if the mouse area isn't enabled.
QVERIFY(innerFiltering); // pressed will never be true if the mouse area isn't enabled.
QTRY_VERIFY(mouseArea->pressed());
}