Fix draghandler to respect axis constraints

..while its (ancestor) coordinate system has changed during the drag.

For example, ensure that a DragHandler-based Slider keeps its knob centered.
If the Slider is used on a Flickable which you are flicking with a second
finger, then the coordinate system is changing underneath the Slider.

The problem was that DragHandler stored the initial drag position of the
target when the target item was pressed, and used that throughout the
whole drag operation. Unfortunately if the target item was inside a
Flickable that got flicked during a drag operation, that initial position
was not updated (and thus, incorrect).

Instead of storing the initial target position in scene coordinates, we
now store the position that got pressed in local target coordinates, and
ensure that in any further updates the touchpoint have the same local
position (by moving the target).

Task-number: QTBUG-64852
Change-Id: I25012d34d88f45c7eb9c711db0037d530cf10854
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

3 files changed, 148 insertions, 36 deletions

diff --git a/src/quick/handlers/qquickdraghandler.cpp b/src/quick/handlers/qquickdraghandler.cpp
index d4a8f06db8..5180a840ca 100644
--- a/src/quick/handlers/qquickdraghandler.cpp
+++ b/src/quick/handlers/qquickdraghandler.cpp
@@ -91,20 +91,44 @@ bool QQuickDragHandler::wantsEventPoint(QQuickEventPoint *point)
             || QQuickSinglePointHandler::wantsEventPoint(point));
 }
 
+bool QQuickDragHandler::targetContains(QQuickEventPoint *point)
+{
+    Q_ASSERT(parentItem() && target());
+    return target()->contains(localTargetPosition(point));
+}
+
+QPointF QQuickDragHandler::localTargetPosition(QQuickEventPoint *point)
+{
+    QPointF pos = point->position();
+    if (target() != parentItem())
+        pos = parentItem()->mapToItem(target(), pos);
+    return pos;
+}
+
 void QQuickDragHandler::onGrabChanged(QQuickPointerHandler *grabber, QQuickEventPoint::GrabState stateChange, QQuickEventPoint *point)
 {
-    if (grabber == this && stateChange == QQuickEventPoint::GrabExclusive && m_targetStartPos.isNull())
-        // In case the grab got handled over from another grabber, we might not get the Press.
-        // Therefore, prefer the m_targetStartPos we got when it got Pressed.
-        initializeTargetStartPos(point);
-    enforceConstraints();
+    if (!target() || !target()->parentItem())
+        return;
+    if (grabber == this && stateChange == QQuickEventPoint::GrabExclusive) {
+        // In case the grab got handed over from another grabber, we might not get the Press.
+        if (!m_pressedInsideTarget) {
+            m_pressTargetPos = QPointF(target()->width(), target()->height()) / 2;
+            m_pressScenePos = point->scenePosition();
+        } else if (m_pressTargetPos.isNull()) {
+            m_pressTargetPos = localTargetPosition(point);
+            m_pressScenePos = point->scenePosition();
+        }
+    }
     QQuickSinglePointHandler::onGrabChanged(grabber, stateChange, point);
 }
 
 void QQuickDragHandler::onActiveChanged()
 {
-    if (!active())
-        m_targetStartPos = QPointF();
+    if (!active()) {
+        m_pressTargetPos = QPointF();
+        m_pressScenePos = m_pressTargetPos;
+        m_pressedInsideTarget = false;
+    }
 }
 
 void QQuickDragHandler::handleEventPoint(QQuickEventPoint *point)
@@ -112,25 +136,38 @@ void QQuickDragHandler::handleEventPoint(QQuickEventPoint *point)
     point->setAccepted();
     switch (point->state()) {
     case QQuickEventPoint::Pressed:
-        initializeTargetStartPos(point);
+        m_pressedInsideTarget = targetContains(point);
+        m_pressTargetPos = localTargetPosition(point);
+        m_pressScenePos = point->scenePosition();
         setPassiveGrab(point);
         break;
     case QQuickEventPoint::Updated: {
-        QPointF delta = point->scenePosition() - point->scenePressPosition();
-        if (!m_xAxis.enabled())
-            delta.setX(0);
-        if (!m_yAxis.enabled())
-            delta.setY(0);
+        QVector2D accumulatedDragDelta = QVector2D(point->scenePosition() - m_pressScenePos);
         if (active()) {
-            setTranslation(QVector2D(delta));
+            // update translation property. Make sure axis is respected for it.
+            if (!m_xAxis.enabled())
+                accumulatedDragDelta.setX(0);
+            if (!m_yAxis.enabled())
+                accumulatedDragDelta.setY(0);
+            setTranslation(accumulatedDragDelta);
+
             if (target() && target()->parentItem()) {
-                QPointF pos = target()->parentItem()->mapFromScene(m_targetStartPos + delta);
+                const QPointF newTargetTopLeft = localTargetPosition(point) - m_pressTargetPos;
+                const QPointF xformOrigin = target()->transformOriginPoint();
+                const QPointF targetXformOrigin = newTargetTopLeft + xformOrigin;
+                QPointF pos = target()->parentItem()->mapFromItem(target(), targetXformOrigin);
+                pos -= xformOrigin;
+                QPointF targetItemPos = target()->position();
+                if (!m_xAxis.enabled())
+                    pos.setX(targetItemPos.x());
+                if (!m_yAxis.enabled())
+                    pos.setY(targetItemPos.y());
                 enforceAxisConstraints(&pos);
                 moveTarget(pos, point);
             }
         } else if (!point->exclusiveGrabber() &&
-                   ((m_xAxis.enabled() && QQuickWindowPrivate::dragOverThreshold(delta.x(), Qt::XAxis, point)) ||
-                    (m_yAxis.enabled() && QQuickWindowPrivate::dragOverThreshold(delta.y(), Qt::YAxis, point)))) {
+                   ((m_xAxis.enabled() && QQuickWindowPrivate::dragOverThreshold(accumulatedDragDelta.x(), Qt::XAxis, point)) ||
+                    (m_yAxis.enabled() && QQuickWindowPrivate::dragOverThreshold(accumulatedDragDelta.y(), Qt::YAxis, point)))) {
             setExclusiveGrab(point);
             if (auto parent = parentItem()) {
                 if (point->pointerEvent()->asPointerTouchEvent())
@@ -166,23 +203,6 @@ void QQuickDragHandler::enforceAxisConstraints(QPointF *localPos)
         localPos->setY(qBound(m_yAxis.minimum(), localPos->y(), m_yAxis.maximum()));
 }
 
-void QQuickDragHandler::initializeTargetStartPos(QQuickEventPoint *point)
-{
-    if (target() && target()->parentItem()) {
-        m_targetStartPos = target()->parentItem()->mapToScene(target()->position());
-        if (!target()->contains(point->position())) {
-            // If pressed outside of target item, move the target item so that the touchpoint is in its center,
-            // while still respecting the axis constraints.
-            const QPointF center = target()->parentItem()->mapFromScene(point->scenePosition());
-            const QPointF pointCenteredInItemPos = target()->parentItem()->mapToScene(center - QPointF(target()->width(), target()->height())/2);
-            if (m_xAxis.enabled())
-                m_targetStartPos.setX(pointCenteredInItemPos.x());
-            if (m_yAxis.enabled())
-                m_targetStartPos.setY(pointCenteredInItemPos.y());
-        }
-    }
-}
-
 void QQuickDragHandler::setTranslation(const QVector2D &trans)
 {
     if (trans == m_translation) // fuzzy compare?
diff --git a/src/quick/handlers/qquickdraghandler_p.h b/src/quick/handlers/qquickdraghandler_p.h
index d10084c654..50f56d78a4 100644
--- a/src/quick/handlers/qquickdraghandler_p.h
+++ b/src/quick/handlers/qquickdraghandler_p.h
@@ -118,13 +118,19 @@ protected:
 private:
     void ungrab();
     void enforceAxisConstraints(QPointF *localPos);
-    void initializeTargetStartPos(QQuickEventPoint *point);
+    bool targetContains(QQuickEventPoint *point);
+    QPointF localTargetPosition(QQuickEventPoint *point);
 
 private:
-    QPointF m_targetStartPos;
+    QPointF m_pressScenePos;
+    QPointF m_pressTargetPos;   // We must also store the local targetPos, because we cannot deduce
+                                // the press target pos from the scene pos in case there was e.g a
+                                // flick in one of the ancestors during the drag.
     QVector2D m_translation;
+
     QQuickDragAxis m_xAxis;
     QQuickDragAxis m_yAxis;
+    bool m_pressedInsideTarget = false;
 
     friend class QQuickDragAxis;
 };
diff --git a/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp b/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp
index c0b34f8246..f3513881cd 100644
--- a/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp
+++ b/tests/auto/quick/pointerhandlers/flickableinterop/tst_flickableinterop.cpp
@@ -75,6 +75,7 @@ private slots:
     void touchDragFlickableBehindItemWithHandlers();
     void mouseDragFlickableBehindItemWithHandlers_data();
     void mouseDragFlickableBehindItemWithHandlers();
+    void touchDragSliderAndFlickable();
 
 private:
     void createView(QScopedPointer<QQuickView> &window, const char *fileName);
@@ -560,6 +561,91 @@ void tst_FlickableInterop::mouseDragFlickableBehindItemWithHandlers()
     QCOMPARE(originP1, rect->mapToScene(rect->clipRect().center()).toPoint());
 }
 
+void tst_FlickableInterop::touchDragSliderAndFlickable()
+{
+    const int dragThreshold = QGuiApplication::styleHints()->startDragDistance();
+    QScopedPointer<QQuickView> windowPtr;
+    createView(windowPtr, "flickableWithHandlers.qml");
+    QQuickView * window = windowPtr.data();
+
+    QQuickItem *slider = window->rootObject()->findChild<QQuickItem*>("Slider");
+    QVERIFY(slider);
+    QQuickDragHandler *drag = slider->findChild<QQuickDragHandler*>();
+    QVERIFY(drag);
+    QQuickItem *knob = slider->findChild<QQuickItem*>("Slider Knob");
+    QVERIFY(knob);
+    QQuickFlickable *flickable = window->rootObject()->findChild<QQuickFlickable*>();
+    QVERIFY(flickable);
+
+    // The knob is initially centered over the slider's "groove"
+    qreal initialXOffset = qAbs(knob->mapToScene(knob->clipRect().center()).x() - slider->mapToScene
+                                (slider->clipRect().center()).x());
+    QVERIFY(initialXOffset <= 1);
+
+    // Drag the slider in the allowed (vertical) direction with one finger
+    QPoint p1 = knob->mapToScene(knob->clipRect().center()).toPoint();
+    QTest::touchEvent(window, touchDevice).press(1, p1, window);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(0, dragThreshold);
+    QTest::touchEvent(window, touchDevice).move(1, p1, window);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(0, dragThreshold);
+    QTest::touchEvent(window, touchDevice).move(1, p1, window);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(0, dragThreshold);
+    QTest::touchEvent(window, touchDevice).move(1, p1, window);
+    QQuickTouchUtils::flush(window);
+    QTRY_VERIFY(slider->property("value").toInt() < 49);
+    QVERIFY(!flickable->isMoving());
+
+    // Drag the Flickable with a second finger
+    QPoint p2(300,300);
+    QTest::touchEvent(window, touchDevice).stationary(1).press(2, p2, window);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).move(1, p1, window).stationary(2);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).stationary(1).move(2, p2, window);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).move(1, p1, window).stationary(2);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).stationary(1).move(2, p2, window);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).move(1, p1, window).stationary(2);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).stationary(1).move(2, p2, window);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).move(1, p1, window).stationary(2);
+    QQuickTouchUtils::flush(window);
+    p1 += QPoint(-10, -10);
+    p2 += QPoint(dragThreshold, 0);
+    QTest::touchEvent(window, touchDevice).stationary(1).move(2, p2, window);
+    QQuickTouchUtils::flush(window);
+    QTRY_VERIFY(flickable->isMoving());
+    qreal knobSliderXOffset = qAbs(knob->mapToScene(knob->clipRect().center()).toPoint().x() -
+        slider->mapToScene(slider->clipRect().center()).toPoint().x()) - initialXOffset;
+    if (knobSliderXOffset > 1)
+        qDebug() << "knob has slipped out of groove by" << knobSliderXOffset << "pixels";
+    // See if the knob is still centered over the slider's "groove"
+    QVERIFY(qAbs(knobSliderXOffset) <= 1);
+
+    // Release
+    QTest::touchEvent(window, touchDevice).release(1, p1, window).release(2, p2, window);
+}
+
 QTEST_MAIN(tst_FlickableInterop)
 
 #include "tst_flickableinterop.moc"