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>

4 files changed, 130 insertions, 9 deletions

diff --git a/src/quick/items/qquickdrag.cpp b/src/quick/items/qquickdrag.cpp
index ca7b716ef2..bc9732b6b3 100644
--- 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 ac19c021e0..720946924f 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 37fb6d2d1f..0f8449fff5 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<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 8b4b63f19f..b59e767be9 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<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));