Fix PinchHandler.persistentTranslation; test cumulative native gestures

Since we do not want persistentTranslation to be always the same as
target.position, clearly we cannot use xAxis/yAxis to store the initial
target position: thus the startPos() function was wrong, and is now
removed. We need to store it in a separate m_startTargetPos variable
like DragHandler does, and as PinchHandler did before
7867a683fcb938939fb2837a26ac8e1941e3fe08.

Add an internal doc comment to clarify the arguments to
QQuickItemPrivate::adjustedPosForTransform().

tst_QQuickPinchHandler::cumulativeNativeGestures() now checks the result
of adjustedPosForTransform(): how far the target item moved.

Pick-to: 6.5
Fixes: QTBUG-111220
Change-Id: I04237cb82a1abaaeab873a0d887acaf322f262ce
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

3 files changed, 37 insertions, 30 deletions

diff --git a/src/quick/handlers/qquickpinchhandler.cpp b/src/quick/handlers/qquickpinchhandler.cpp
index 8d7b36ee0d..197bf31783 100644
--- a/src/quick/handlers/qquickpinchhandler.cpp
+++ b/src/quick/handlers/qquickpinchhandler.cpp
@@ -478,12 +478,6 @@ void QQuickPinchHandler::onActiveChanged()
 {
     QQuickMultiPointHandler::onActiveChanged();
     const bool curActive = active();
-    if (const QQuickItem *t = target(); curActive && t) {
-        m_xAxis.m_accumulatedValue = t->position().x();
-        m_yAxis.m_accumulatedValue = t->position().y();
-        m_scaleAxis.m_accumulatedValue = t->scale();
-        m_rotationAxis.m_accumulatedValue = t->rotation();
-    }
     m_xAxis.onActiveChanged(curActive, 0);
     m_yAxis.onActiveChanged(curActive, 0);
     m_scaleAxis.onActiveChanged(curActive, 1);
@@ -492,9 +486,12 @@ void QQuickPinchHandler::onActiveChanged()
     if (curActive) {
         m_startAngles = angles(centroid().sceneGrabPosition());
         m_startDistance = averageTouchPointDistance(centroid().sceneGrabPosition());
+        m_startTargetPos = target() ? target()->position() : QPointF();
         qCDebug(lcPinchHandler) << "activated with starting scale" << m_scaleAxis.m_startValue
-                                << "target scale" << m_scaleAxis.m_startValue << "rotation" << m_rotationAxis.m_startValue;
+                                << "target scale" << m_scaleAxis.m_startValue << "rotation" << m_rotationAxis.m_startValue
+                                << "target pos" << m_startTargetPos;
     } else {
+        m_startTargetPos = QPointF();
         qCDebug(lcPinchHandler) << "deactivated with scale" << m_scaleAxis.m_activeValue << "rotation" << m_rotationAxis.m_activeValue;
     }
 }
@@ -677,17 +674,16 @@ void QQuickPinchHandler::handlePointerEventImpl(QPointerEvent *event)
 
 
     if (target() && target()->parentItem()) {
-        const QPointF centroidParentPos = target()->parentItem()->mapFromScene(centroid().scenePosition());
+        auto *t = target();
+        const QPointF centroidParentPos = t->parentItem()->mapFromScene(centroid().scenePosition());
         // 3. Drag/translate
-        const QPointF centroidStartParentPos = target()->parentItem()->mapFromScene(centroid().sceneGrabPosition());
+        const QPointF centroidStartParentPos = t->parentItem()->mapFromScene(centroid().sceneGrabPosition());
         auto activeTranslation = centroidParentPos - centroidStartParentPos;
         // apply rotation + scaling around the centroid - then apply translation.
-        QPointF pos = QQuickItemPrivate::get(target())->adjustedPosForTransform(centroidParentPos,
-                                                                                startPos(), QVector2D(activeTranslation),
-                                                                                m_scaleAxis.m_startValue,
-                                                                                m_scaleAxis.persistentValue() / m_scaleAxis.m_startValue,
-                                                                                m_rotationAxis.m_startValue,
-                                                                                m_rotationAxis.persistentValue() - m_rotationAxis.m_startValue);
+        QPointF pos = QQuickItemPrivate::get(t)->adjustedPosForTransform(centroidParentPos,
+                m_startTargetPos, QVector2D(activeTranslation),
+                t->scale(), m_scaleAxis.persistentValue() / m_scaleAxis.m_startValue,
+                t->rotation(), m_rotationAxis.persistentValue() - m_rotationAxis.m_startValue);
 
         if (xAxis()->enabled())
             pos.setX(qBound(xAxis()->minimum(), pos.x(), xAxis()->maximum()));
@@ -700,12 +696,12 @@ void QQuickPinchHandler::handlePointerEventImpl(QPointerEvent *event)
 
         const QVector2D delta(activeTranslation.x() - m_xAxis.activeValue(),
                               activeTranslation.y() - m_yAxis.activeValue());
-        m_xAxis.updateValue(activeTranslation.x(), pos.x(), delta.x());
-        m_yAxis.updateValue(activeTranslation.y(), pos.y(), delta.y());
+        m_xAxis.updateValue(activeTranslation.x(), m_xAxis.persistentValue() + delta.x(), delta.x());
+        m_yAxis.updateValue(activeTranslation.y(), m_yAxis.persistentValue() + delta.y(), delta.y());
         emit translationChanged(delta);
-        target()->setPosition(QPointF(m_xAxis.persistentValue(), m_yAxis.persistentValue()));
-        target()->setRotation(m_rotationAxis.persistentValue());
-        target()->setScale(m_scaleAxis.persistentValue());
+        t->setPosition(pos);
+        t->setRotation(m_rotationAxis.persistentValue());
+        t->setScale(m_scaleAxis.persistentValue());
     } else {
         auto activeTranslation = centroid().scenePosition() - centroid().scenePressPosition();
         auto accumulated = QPointF(m_xAxis.m_startValue, m_yAxis.m_startValue) + activeTranslation;
@@ -726,11 +722,6 @@ void QQuickPinchHandler::handlePointerEventImpl(QPointerEvent *event)
     emit updated();
 }
 
-QPointF QQuickPinchHandler::startPos()
-{
-    return {m_xAxis.m_startValue, m_yAxis.m_startValue};
-}
-
 /*!
     \internal
     \qmlproperty flags QtQuick::PinchHandler::acceptedButtons
diff --git a/src/quick/handlers/qquickpinchhandler_p.h b/src/quick/handlers/qquickpinchhandler_p.h
index 7ed8f292ac..3a11933d1f 100644
--- a/src/quick/handlers/qquickpinchhandler_p.h
+++ b/src/quick/handlers/qquickpinchhandler_p.h
@@ -112,8 +112,6 @@ protected:
     void onActiveChanged() override;
     void handlePointerEventImpl(QPointerEvent *event) override;
 
-    QPointF startPos();
-
 private:
     QQuickDragAxis m_xAxis = {this, u"x"_s};
     QQuickDragAxis m_yAxis = {this, u"y"_s};
@@ -123,6 +121,7 @@ private:
     // internal
     qreal m_startDistance = 0;
     qreal m_accumulatedStartCentroidDistance = 0;
+    QPointF m_startTargetPos;
     QVector<PointData> m_startAngles;
     QQuickMatrix4x4 m_transform;
 };
diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index e745e4d110..ba161f5b27 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -5337,13 +5337,30 @@ bool QQuickItemPrivate::transformChanged(QQuickItem *transformedItem)
     return ret;
 }
 
+/*! \internal
+    Returns the new position (proposed values for the x and y properties)
+    to which this item should be moved to compensate for the given change
+    in scale from \a startScale to \a activeScale and in rotation from
+    \a startRotation to \a activeRotation. \a centroidParentPos is the
+    point that we wish to hold in place (and then apply \a activeTranslation to),
+    in this item's parent's coordinate system. \a startPos is this item's
+    position in its parent's coordinate system when the gesture began.
+    \a activeTranslation is the amount of translation that should be added to
+    the return value, i.e. the displacement by which the centroid is expected
+    to move.
+
+    If \a activeTranslation is \c (0, 0) the centroid is to be held in place.
+    If \a activeScale is \c 1, it means scale is intended to be held constant,
+    the same as \a startScale. If \a activeRotation is \c 0, it means rotation
+    is intended to be held constant, the same as \a startRotation.
+*/
 QPointF QQuickItemPrivate::adjustedPosForTransform(const QPointF &centroidParentPos,
                                                    const QPointF &startPos,
-                                                   const QVector2D &activeTranslation,  //[0,0] means no additional translation from startPos
+                                                   const QVector2D &activeTranslation,
                                                    qreal startScale,
-                                                   qreal activeScale,                   // 1.0 means no additional scale from startScale
+                                                   qreal activeScale,
                                                    qreal startRotation,
-                                                   qreal activeRotation)                // 0.0 means no additional rotation from startRotation
+                                                   qreal activeRotation)
 {
     Q_Q(QQuickItem);
     QVector3D xformOrigin(q->transformOriginPoint());