From 7f626e206614f6030ee0fc994eb9b3602f133d87 Mon Sep 17 00:00:00 2001 From: Andrew den Exter Date: Fri, 2 Mar 2012 12:58:45 +1000 Subject: [PATCH] Don't deliver drag move events immediately. Rather than delivering drag move events everytime the position of an item changes queue up an event to process in the event loop. This filters out noisy intermediate positions particularly where the x and y values are set independently and better insulates against feedback. Change-Id: I5d787d63ed01441a9080d0daaee9db1373d5f073 Reviewed-by: Martin Jones --- src/quick/items/qquickdrag.cpp | 50 +++++++++++- src/quick/items/qquickdrag_p.h | 2 + tests/auto/quick/qquickdrag/tst_qquickdrag.cpp | 83 ++++++++++++++++++-- .../quick/qquickdroparea/tst_qquickdroparea.cpp | 4 + 4 files changed, 130 insertions(+), 9 deletions(-) diff --git a/src/quick/items/qquickdrag.cpp b/src/quick/items/qquickdrag.cpp index ca7b716..bc9732b 100644 --- a/src/quick/items/qquickdrag.cpp +++ b/src/quick/items/qquickdrag.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -65,10 +66,13 @@ public: , active(false) , listening(false) , inEvent(false) + , itemMoved(false) + , eventQueued(false) { } void itemGeometryChanged(QQuickItem *, const QRectF &, const QRectF &); + void deliverMoveEvent(); void deliverEvent(QQuickCanvas *canvas, QEvent *event); void start() { start(supportedActions); } void start(Qt::DropActions supportedActions); @@ -85,6 +89,8 @@ public: bool active : 1; bool listening : 1; bool inEvent : 1; + bool itemMoved : 1; + bool eventQueued : 1; QPointF hotSpot; QStringList keys; }; @@ -119,9 +125,22 @@ public: void QQuickDragAttachedPrivate::itemGeometryChanged(QQuickItem *, const QRectF &newGeometry, const QRectF &oldGeometry) { Q_Q(QQuickDragAttached); - if (newGeometry.topLeft() == oldGeometry.topLeft() || !active || inEvent) + if (newGeometry.topLeft() == oldGeometry.topLeft() || !active || itemMoved) return; + itemMoved = true; + + if (!eventQueued) { + eventQueued = true; + QCoreApplication::postEvent(q, new QEvent(QEvent::User)); + } +} + +void QQuickDragAttachedPrivate::deliverMoveEvent() +{ + Q_Q(QQuickDragAttached); + + itemMoved = false; if (QQuickCanvas *canvas = attachedItem->canvas()) { QPoint scenePos = attachedItem->mapToScene(hotSpot).toPoint(); QDragMoveEvent event(scenePos, mimeData->m_supportedActions, mimeData, Qt::NoButton, Qt::NoModifier); @@ -142,6 +161,20 @@ void QQuickDragAttachedPrivate::deliverEvent(QQuickCanvas *canvas, QEvent *event inEvent = false; } +bool QQuickDragAttached::event(QEvent *event) +{ + Q_D(QQuickDragAttached); + + if (event->type() == QEvent::User) { + d->eventQueued = false; + if (d->itemMoved) + d->deliverMoveEvent(); + return true; + } else { + return QObject::event(event); + } +} + QQuickDragAttached::QQuickDragAttached(QObject *parent) : QObject(*new QQuickDragAttachedPrivate, parent) { @@ -260,8 +293,17 @@ void QQuickDragAttached::setHotSpot(const QPointF &hotSpot) Q_D(QQuickDragAttached); if (d->hotSpot != hotSpot) { d->hotSpot = hotSpot; + + if (d->active) { + d->itemMoved = true; + + if (!d->eventQueued) { + d->eventQueued = true; + QCoreApplication::postEvent(this, new QEvent(QEvent::User)); + } + } + emit hotSpotChanged(); - // Send a move event if active? } } @@ -428,6 +470,9 @@ int QQuickDragAttached::drop() return acceptedAction; } + if (d->itemMoved) + d->deliverMoveEvent(); + if (!d->active) return acceptedAction; d->active = false; @@ -475,6 +520,7 @@ void QQuickDragAttached::cancel() if (!d->active) return; d->active = false; + d->itemMoved = false; if (QQuickCanvas *canvas = d->attachedItem->canvas()) { QDragLeaveEvent event; diff --git a/src/quick/items/qquickdrag_p.h b/src/quick/items/qquickdrag_p.h index ac19c02..7209469 100644 --- a/src/quick/items/qquickdrag_p.h +++ b/src/quick/items/qquickdrag_p.h @@ -182,6 +182,8 @@ public: Q_INVOKABLE int drop(); + bool event(QEvent *event); + public Q_SLOTS: void start(QQmlV8Function *); void cancel(); diff --git a/tests/auto/quick/qquickdrag/tst_qquickdrag.cpp b/tests/auto/quick/qquickdrag/tst_qquickdrag.cpp index 37fb6d2..0f8449f 100644 --- a/tests/auto/quick/qquickdrag/tst_qquickdrag.cpp +++ b/tests/auto/quick/qquickdrag/tst_qquickdrag.cpp @@ -359,6 +359,31 @@ void tst_QQuickDrag::active() QCOMPARE(evaluate(item, "Drag.target"), static_cast(0)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(0)); QCOMPARE(dropTarget.enterEvents, 0); QCOMPARE(dropTarget.leaveEvents, 0); + + evaluate(item, "Drag.active = false"); + dropTarget.setEnabled(true); + dropTarget.reset(); + + // Queued move events are discarded if the drag is cancelled. + evaluate(item, "Drag.active = true"); + QCOMPARE(evaluate(item, "Drag.active"), true); + QCOMPARE(evaluate(item, "dragActive"), true); + QCOMPARE(dropTarget.enterEvents, 1); QCOMPARE(dropTarget.leaveEvents, 0); QCOMPARE(dropTarget.moveEvents, 0); + + dropTarget.reset(); + item->setPos(QPointF(80, 80)); + QCOMPARE(evaluate(item, "Drag.active"), true); + QCOMPARE(evaluate(item, "dragActive"), true); + QCOMPARE(dropTarget.enterEvents, 0); QCOMPARE(dropTarget.leaveEvents, 0); QCOMPARE(dropTarget.moveEvents, 0); + + evaluate(item, "Drag.active = false"); + QCOMPARE(evaluate(item, "Drag.active"), false); + QCOMPARE(evaluate(item, "dragActive"), false); + QCOMPARE(dropTarget.enterEvents, 0); QCOMPARE(dropTarget.leaveEvents, 1); QCOMPARE(dropTarget.moveEvents, 0); + + dropTarget.reset(); + QCoreApplication::processEvents(); + QCOMPARE(dropTarget.enterEvents, 0); QCOMPARE(dropTarget.leaveEvents, 0); QCOMPARE(dropTarget.moveEvents, 0); } void tst_QQuickDrag::drop() @@ -478,6 +503,23 @@ void tst_QQuickDrag::drop() QCOMPARE(evaluate(item, "dragTarget"), static_cast(0)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.dropEvents, 0); QCOMPARE(innerTarget.enterEvents, 0); QCOMPARE(innerTarget.leaveEvents, 0); QCOMPARE(innerTarget.dropEvents, 0); + + // Queued move event is delivered before a drop event. + innerTarget.reset(); outerTarget.reset(); + evaluate(item, "Drag.active = true"); + item->setPos(QPointF(80, 80)); + evaluate(item, "Drag.drop()"); + QCOMPARE(evaluate(item, "Drag.active"), false); + QCOMPARE(evaluate(item, "dragActive"), false); + QCOMPARE(evaluate(item, "Drag.target"), static_cast(&outerTarget)); + QCOMPARE(evaluate(item, "dragTarget"), static_cast(&outerTarget)); + QCOMPARE(outerTarget.enterEvents, 1); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.dropEvents, 1); QCOMPARE(outerTarget.moveEvents, 1); + QCOMPARE(innerTarget.enterEvents, 0); QCOMPARE(innerTarget.leaveEvents, 0); QCOMPARE(innerTarget.dropEvents, 0); QCOMPARE(innerTarget.moveEvents, 0); + + innerTarget.reset(); outerTarget.reset(); + QCoreApplication::processEvents(); + QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.dropEvents, 0); QCOMPARE(outerTarget.moveEvents, 0); + QCOMPARE(innerTarget.enterEvents, 0); QCOMPARE(innerTarget.leaveEvents, 0); QCOMPARE(innerTarget.dropEvents, 0); QCOMPARE(innerTarget.moveEvents, 0); } void tst_QQuickDrag::move() @@ -518,6 +560,11 @@ void tst_QQuickDrag::move() // Move within the outer target. outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(60, 50)); + // Move event is delivered in the event loop. + QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 0); + QCOMPARE(leftTarget .enterEvents, 0); QCOMPARE(leftTarget .leaveEvents, 0); QCOMPARE(leftTarget .moveEvents, 0); + QCOMPARE(rightTarget.enterEvents, 0); QCOMPARE(rightTarget.leaveEvents, 0); QCOMPARE(rightTarget.moveEvents, 0); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&outerTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&outerTarget)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 1); @@ -527,7 +574,13 @@ void tst_QQuickDrag::move() // Move into the right target. outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); - item->setPos(QPointF(75, 50)); + // Setting X and Y individually should still only generate on move. + item->setX(75); + item->setY(50); + QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 0); + QCOMPARE(leftTarget .enterEvents, 0); QCOMPARE(leftTarget .leaveEvents, 0); QCOMPARE(leftTarget .moveEvents, 0); + QCOMPARE(rightTarget.enterEvents, 0); QCOMPARE(rightTarget.leaveEvents, 0); QCOMPARE(rightTarget.moveEvents, 0); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&rightTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&rightTarget)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 1); @@ -539,6 +592,7 @@ void tst_QQuickDrag::move() // Move into the left target. outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(25, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&leftTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&leftTarget)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 1); @@ -550,6 +604,7 @@ void tst_QQuickDrag::move() // Move within the left target. outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(25, 40)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&leftTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&leftTarget)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 1); @@ -561,6 +616,7 @@ void tst_QQuickDrag::move() // Move out of all targets. outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(110, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(0)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(0)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 1); QCOMPARE(outerTarget.moveEvents, 0); @@ -572,6 +628,7 @@ void tst_QQuickDrag::move() outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(80, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&outerTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&outerTarget)); QCOMPARE(outerTarget.enterEvents, 1); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 0); @@ -584,6 +641,7 @@ void tst_QQuickDrag::move() outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(60, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&outerTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&outerTarget)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 1); @@ -596,6 +654,7 @@ void tst_QQuickDrag::move() outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(40, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&outerTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&outerTarget)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 1); @@ -608,6 +667,7 @@ void tst_QQuickDrag::move() outerTarget.reset(); leftTarget.reset(); rightTarget.reset(); item->setPos(QPointF(25, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(item, "Drag.target"), static_cast(&outerTarget)); QCOMPARE(evaluate(item, "dragTarget"), static_cast(&outerTarget)); QCOMPARE(outerTarget.enterEvents, 0); QCOMPARE(outerTarget.leaveEvents, 0); QCOMPARE(outerTarget.moveEvents, 1); @@ -616,7 +676,6 @@ void tst_QQuickDrag::move() QCOMPARE(outerTarget.position.x(), qreal(25)); QCOMPARE(outerTarget.position.y(), qreal(50)); } - void tst_QQuickDrag::hotSpot() { QQuickCanvas canvas; @@ -656,6 +715,7 @@ void tst_QQuickDrag::hotSpot() QCOMPARE(dropTarget.position.y(), qreal(55)); item->setPos(QPointF(30, 20)); + QCoreApplication::processEvents(); QCOMPARE(dropTarget.position.x(), qreal(35)); QCOMPARE(dropTarget.position.y(), qreal(25)); @@ -664,13 +724,20 @@ void tst_QQuickDrag::hotSpot() QCOMPARE(evaluate(item, "Drag.hotSpot.y"), qreal(10)); QCOMPARE(evaluate(item, "hotSpotX"), qreal(10)); QCOMPARE(evaluate(item, "hotSpotY"), qreal(10)); - // Changing the hotSpot won't generate a move event so the position is unchanged. Should it? + + // Setting the hotSpot will deliver a move event in the event loop. QCOMPARE(dropTarget.position.x(), qreal(35)); QCOMPARE(dropTarget.position.y(), qreal(25)); + QCoreApplication::processEvents(); + QCOMPARE(dropTarget.position.x(), qreal(40)); + QCOMPARE(dropTarget.position.y(), qreal(30)); item->setPos(QPointF(10, 20)); + QCoreApplication::processEvents(); QCOMPARE(dropTarget.position.x(), qreal(20)); QCOMPARE(dropTarget.position.y(), qreal(30)); + + evaluate(item, "{ Drag.hotSpot.x = 10; Drag.hotSpot.y = 10 }"); } void tst_QQuickDrag::supportedActions() @@ -758,6 +825,7 @@ void tst_QQuickDrag::proposedAction() QCOMPARE(evaluate(item, "Drag.proposedAction == Qt.MoveAction"), true); QCOMPARE(evaluate(item, "proposedAction == Qt.MoveAction"), true); item->setPos(QPointF(60, 60)); + QCoreApplication::processEvents(); QCOMPARE(dropTarget.defaultAction, Qt::MoveAction); QCOMPARE(dropTarget.proposedAction, Qt::MoveAction); @@ -783,13 +851,13 @@ void tst_QQuickDrag::keys() QQuickItem *item = qobject_cast(object.data()); QVERIFY(item); -// QCOMPARE(evaluate(item, "Drag.keys"), QStringList()); -// QCOMPARE(evaluate(item, "keys"), QStringList()); + QCOMPARE(evaluate(item, "Drag.keys"), QStringList()); + QCOMPARE(evaluate(item, "keys"), QStringList()); QCOMPARE(item->property("keys").toStringList(), QStringList()); evaluate(item, "Drag.keys = [\"red\", \"blue\"]"); -// QCOMPARE(evaluate(item, "Drag.keys"), QStringList() << "red" << "blue"); -// QCOMPARE(evaluate(item, "keys"), QStringList() << "red" << "blue"); + QCOMPARE(evaluate(item, "Drag.keys"), QStringList() << "red" << "blue"); + QCOMPARE(evaluate(item, "keys"), QStringList() << "red" << "blue"); QCOMPARE(item->property("keys").toStringList(), QStringList() << "red" << "blue"); } @@ -1008,6 +1076,7 @@ void tst_QQuickDrag::recursion() QCOMPARE(dropTarget.leaveEvents, 0); evaluate(item, "y = 15"); + QCoreApplication::processEvents(); QCOMPARE(dropTarget.enterEvents, 1); QCOMPARE(dropTarget.moveEvents, 1); QCOMPARE(dropTarget.dropEvents, 0); diff --git a/tests/auto/quick/qquickdroparea/tst_qquickdroparea.cpp b/tests/auto/quick/qquickdroparea/tst_qquickdroparea.cpp index 8b4b63f..b59e767 100644 --- a/tests/auto/quick/qquickdroparea/tst_qquickdroparea.cpp +++ b/tests/auto/quick/qquickdroparea/tst_qquickdroparea.cpp @@ -153,6 +153,7 @@ void tst_QQuickDropArea::containsDrag_internal() QCOMPARE(evaluate(dropArea, "exitEvents"), 0); dragItem->setPos(QPointF(50, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(dropArea, "containsDrag"), true); QCOMPARE(evaluate(dropArea, "hasDrag"), true); QCOMPARE(evaluate(dropArea, "enterEvents"), 1); @@ -160,6 +161,7 @@ void tst_QQuickDropArea::containsDrag_internal() evaluate(dropArea, "{ enterEvents = 0; exitEvents = 0 }"); dragItem->setPos(QPointF(150, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(dropArea, "containsDrag"), false); QCOMPARE(evaluate(dropArea, "hasDrag"), false); @@ -543,6 +545,7 @@ void tst_QQuickDropArea::position_internal() evaluate(dropArea, "{ enterEvents = 0; moveEvents = 0; eventX = -1; eventY = -1 }"); dragItem->setPos(QPointF(40, 50)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(dropArea, "enterEvents"), 0); QCOMPARE(evaluate(dropArea, "moveEvents"), 1); QCOMPARE(evaluate(dropArea, "drag.x"), qreal(40)); @@ -554,6 +557,7 @@ void tst_QQuickDropArea::position_internal() evaluate(dropArea, "{ enterEvents = 0; moveEvents = 0; eventX = -1; eventY = -1 }"); dragItem->setPos(QPointF(75, 25)); + QCoreApplication::processEvents(); QCOMPARE(evaluate(dropArea, "enterEvents"), 0); QCOMPARE(evaluate(dropArea, "moveEvents"), 1); QCOMPARE(evaluate(dropArea, "drag.x"), qreal(75)); -- 1.7.2.5