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 <martin.jones@nokia.com>

diff --git a/src/quick/items/qquickdrag.cpp b/src/quick/items/qquickdrag.cpp
index ca7b716..bc9732b 100644 (file)
--- a/src/quick/items/qquickdrag.cpp
+++ b/src/quick/items/qquickdrag.cpp
@@ -45,6 +45,7 @@
 #include <QtQuick/private/qquickevents_p_p.h>
 #include <private/qquickitemchangelistener_p.h>
 #include <private/qv8engine_p.h>
+#include <QtCore/qcoreapplication.h>
 #include <QtQml/qqmlinfo.h>
 #include <QtGui/qevent.h>
 
@@ -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 (file)
--- 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 (file)
--- 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<QObject *>(item, "Drag.target"), static_cast<QObject *>(0));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(0));
     QCOMPARE(dropTarget.enterEvents, 0); QCOMPARE(dropTarget.leaveEvents, 0);
+
+    evaluate<void>(item, "Drag.active = false");
+    dropTarget.setEnabled(true);
+    dropTarget.reset();
+
+    // Queued move events are discarded if the drag is cancelled.
+    evaluate<void>(item, "Drag.active = true");
+    QCOMPARE(evaluate<bool>(item, "Drag.active"), true);
+    QCOMPARE(evaluate<bool>(item, "dragActive"), true);
+    QCOMPARE(dropTarget.enterEvents, 1); QCOMPARE(dropTarget.leaveEvents, 0); QCOMPARE(dropTarget.moveEvents, 0);
+
+    dropTarget.reset();
+    item->setPos(QPointF(80, 80));
+    QCOMPARE(evaluate<bool>(item, "Drag.active"), true);
+    QCOMPARE(evaluate<bool>(item, "dragActive"), true);
+    QCOMPARE(dropTarget.enterEvents, 0); QCOMPARE(dropTarget.leaveEvents, 0); QCOMPARE(dropTarget.moveEvents, 0);
+
+    evaluate<void>(item, "Drag.active = false");
+    QCOMPARE(evaluate<bool>(item, "Drag.active"), false);
+    QCOMPARE(evaluate<bool>(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<QObject *>(item, "dragTarget"), static_cast<QObject *>(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<void>(item, "Drag.active = true");
+    item->setPos(QPointF(80, 80));
+    evaluate<void>(item, "Drag.drop()");
+    QCOMPARE(evaluate<bool>(item, "Drag.active"), false);
+    QCOMPARE(evaluate<bool>(item, "dragActive"), false);
+    QCOMPARE(evaluate<QObject *>(item, "Drag.target"), static_cast<QObject *>(&outerTarget));
+    QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&outerTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&rightTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&leftTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&leftTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(0));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&outerTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&outerTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&outerTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<QObject *>(item, "Drag.target"), static_cast<QObject *>(&outerTarget));
     QCOMPARE(evaluate<QObject *>(item, "dragTarget"), static_cast<QObject *>(&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<qreal>(item, "Drag.hotSpot.y"), qreal(10));
     QCOMPARE(evaluate<qreal>(item, "hotSpotX"), qreal(10));
     QCOMPARE(evaluate<qreal>(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<void>(item, "{ Drag.hotSpot.x = 10; Drag.hotSpot.y = 10 }");
 }
 
 void tst_QQuickDrag::supportedActions()
@@ -758,6 +825,7 @@ void tst_QQuickDrag::proposedAction()
     QCOMPARE(evaluate<bool>(item, "Drag.proposedAction == Qt.MoveAction"), true);
     QCOMPARE(evaluate<bool>(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<QQuickItem *>(object.data());
     QVERIFY(item);
 
-//    QCOMPARE(evaluate<QStringList>(item, "Drag.keys"), QStringList());
-//    QCOMPARE(evaluate<QStringList>(item, "keys"), QStringList());
+    QCOMPARE(evaluate<QStringList>(item, "Drag.keys"), QStringList());
+    QCOMPARE(evaluate<QStringList>(item, "keys"), QStringList());
     QCOMPARE(item->property("keys").toStringList(), QStringList());
 
     evaluate<void>(item, "Drag.keys = [\"red\", \"blue\"]");
-//    QCOMPARE(evaluate<QStringList>(item, "Drag.keys"), QStringList() << "red" << "blue");
-//    QCOMPARE(evaluate<QStringList>(item, "keys"), QStringList() << "red" << "blue");
+    QCOMPARE(evaluate<QStringList>(item, "Drag.keys"), QStringList() << "red" << "blue");
+    QCOMPARE(evaluate<QStringList>(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<void>(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 (file)
--- 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<int>(dropArea, "exitEvents"), 0);
 
     dragItem->setPos(QPointF(50, 50));
+    QCoreApplication::processEvents();
     QCOMPARE(evaluate<bool>(dropArea, "containsDrag"), true);
     QCOMPARE(evaluate<bool>(dropArea, "hasDrag"), true);
     QCOMPARE(evaluate<int>(dropArea, "enterEvents"), 1);
@@ -160,6 +161,7 @@ void tst_QQuickDropArea::containsDrag_internal()
 
     evaluate<void>(dropArea, "{ enterEvents = 0; exitEvents = 0 }");
     dragItem->setPos(QPointF(150, 50));
+    QCoreApplication::processEvents();
 
     QCOMPARE(evaluate<bool>(dropArea, "containsDrag"), false);
     QCOMPARE(evaluate<bool>(dropArea, "hasDrag"), false);
@@ -543,6 +545,7 @@ void tst_QQuickDropArea::position_internal()
 
     evaluate<void>(dropArea, "{ enterEvents = 0; moveEvents = 0; eventX = -1; eventY = -1 }");
     dragItem->setPos(QPointF(40, 50));
+    QCoreApplication::processEvents();
     QCOMPARE(evaluate<int>(dropArea, "enterEvents"), 0);
     QCOMPARE(evaluate<int>(dropArea, "moveEvents"), 1);
     QCOMPARE(evaluate<qreal>(dropArea, "drag.x"), qreal(40));
@@ -554,6 +557,7 @@ void tst_QQuickDropArea::position_internal()
 
     evaluate<void>(dropArea, "{ enterEvents = 0; moveEvents = 0; eventX = -1; eventY = -1 }");
     dragItem->setPos(QPointF(75, 25));
+    QCoreApplication::processEvents();
     QCOMPARE(evaluate<int>(dropArea, "enterEvents"), 0);
     QCOMPARE(evaluate<int>(dropArea, "moveEvents"), 1);
     QCOMPARE(evaluate<qreal>(dropArea, "drag.x"), qreal(75));