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
7867a683fc.

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>
This commit is contained in:
Shawn Rutledge 2023-03-31 20:45:04 +02:00
parent 79d61b3ab7
commit 07aaa7c1b6
6 changed files with 79 additions and 39 deletions

View File

@ -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

View File

@ -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;
};

View File

@ -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());

View File

@ -35,7 +35,14 @@ Rectangle {
}
}
Text { color: "magenta"; z: 1; text: "scale: " + blackRect.scale}
Text {
color: "magenta"
z: 1
text: "scale: " + blackRect.scale +
"\npos: " + blackRect.x.toFixed(2) + ", " + blackRect.y.toFixed(2) +
"\ntranslation: active " + pincharea.activeTranslation.x.toFixed(2) + ", " + pincharea.activeTranslation.y.toFixed(2) +
"\n persistent " + pincharea.persistentTranslation.x.toFixed(2) + ", " + pincharea.persistentTranslation.y.toFixed(2)
}
Rectangle {
id: blackRect

View File

@ -557,16 +557,20 @@ void tst_QQuickPinchHandler::cumulativeNativeGestures_data()
QTest::addColumn<const QPointingDevice*>("device");
QTest::addColumn<Qt::NativeGestureType>("gesture");
QTest::addColumn<qreal>("value");
QTest::addColumn<QList<QPoint>>("expectedTargetTranslations");
const auto *touchpadDevice = touchpad.get();
const auto *mouse = QPointingDevice::primaryPointingDevice();
QTest::newRow("touchpad: rotate") << touchpadDevice << Qt::RotateNativeGesture << 5.0;
QTest::newRow("touchpad: scale") << touchpadDevice << Qt::ZoomNativeGesture << 0.1;
QTest::newRow("touchpad: rotate") << touchpadDevice << Qt::RotateNativeGesture << 5.0
<< QList<QPoint>{{-2, 2}, {-5, 4}, {-7, 6}, {-10, 7}};
QTest::newRow("touchpad: scale") << touchpadDevice << Qt::ZoomNativeGesture << 0.1
<< QList<QPoint>{{3, 3}, {5, 5}, {8, 8}, {12, 12}};
if (mouse->type() == QInputDevice::DeviceType::Mouse) {
QTest::newRow("mouse: rotate") << mouse << Qt::RotateNativeGesture << 5.0;
QTest::newRow("mouse: scale") << mouse << Qt::ZoomNativeGesture << 0.1;
QTest::newRow("mouse: rotate") << mouse << Qt::RotateNativeGesture << 5.0
<< QList<QPoint>{{-2, 2}, {-5, 4}, {-7, 6}, {-10, 7}};
QTest::newRow("mouse: scale") << mouse << Qt::ZoomNativeGesture << 0.1
<< QList<QPoint>{{3, 3}, {5, 5}, {8, 8}, {12, 12}};
} else {
qCWarning(lcPointerTests) << "skipping mouse tests: primary device is not a mouse" << mouse;
}
@ -577,6 +581,9 @@ void tst_QQuickPinchHandler::cumulativeNativeGestures()
QFETCH(const QPointingDevice*, device);
QFETCH(Qt::NativeGestureType, gesture);
QFETCH(qreal, value);
QFETCH(QList<QPoint>, expectedTargetTranslations);
QCOMPARE(expectedTargetTranslations.size(), 4);
QQuickView window;
QVERIFY(QQuickTest::showView(window, testFileUrl("pinchproperties.qml")));
@ -589,15 +596,17 @@ void tst_QQuickPinchHandler::cumulativeNativeGestures()
QVERIFY(pinchHandler != nullptr);
QQuickItem *target = root->findChild<QQuickItem*>("blackrect");
QVERIFY(target != nullptr);
QCOMPARE(pinchHandler->target(), target);
ulong ts = 1;
qreal expectedScale = 1;
qreal expectedRotation = 0;
QPointF pinchPos(75, 75);
const QPointF initialTargetPos(target->position());
QWindowSystemInterface::handleGestureEvent(&window, ts++, device,
Qt::BeginNativeGesture, pinchPos, pinchPos);
if (lcPointerTests().isDebugEnabled()) QTest::qWait(500);
for (int i = 1; i <= 4; ++i) {
if (lcPointerTests().isDebugEnabled()) QTest::qWait(500);
QWindowSystemInterface::handleGestureEventWithRealValue(&window, ts++, device,
gesture, value, pinchPos, pinchPos);
qApp->processEvents();
@ -612,9 +621,10 @@ void tst_QQuickPinchHandler::cumulativeNativeGestures()
break; // PinchHandler doesn't react to the others
}
qCDebug(lcPointerTests) << gesture << "with value" << value
qCDebug(lcPointerTests) << i << gesture << "with value" << value
<< ": scale" << target->scale() << "expected" << expectedScale
<< ": rotation" << target->rotation() << "expected" << expectedRotation;
if (lcPointerTests().isDebugEnabled()) QTest::qWait(500);
QCOMPARE(target->scale(), expectedScale);
QCOMPARE(target->rotation(), expectedRotation);
QCOMPARE(pinchHandler->persistentScale(), expectedScale);
@ -625,7 +635,20 @@ void tst_QQuickPinchHandler::cumulativeNativeGestures()
QCOMPARE(pinchHandler->activeRotation(), expectedRotation);
QCOMPARE(pinchHandler->rotationAxis()->persistentValue(), expectedRotation);
QCOMPARE(pinchHandler->rotationAxis()->activeValue(), expectedRotation);
// The target gets transformed around the gesture position, for which
// QQuickItemPrivate::adjustedPosForTransform() computes its new position to compensate.
QPointF delta = target->position() - initialTargetPos;
qCDebug(lcPointerTests) << "target moved by" << delta << "to" << target->position()
<< "active trans" << pinchHandler->activeTranslation()
<< "perst trans" << pinchHandler->persistentTranslation();
QCOMPARE_NE(target->position(), initialTargetPos);
QCOMPARE(delta.toPoint(), expectedTargetTranslations.at(i - 1));
// The native pinch gesture cannot include a translation component (and
// the cursor doesn't move while you are performing the gesture on a touchpad).
QCOMPARE(pinchHandler->activeTranslation(), QPointF());
// The target only moves to compensate for scale and rotation changes, and that's
// not reflected in PinchHandler.persistentTranslation.
QCOMPARE(pinchHandler->persistentTranslation(), QPointF());
}
QCOMPARE(pinchHandler->active(), true);
qCDebug(lcPointerTests) << "centroid: local" << pinchHandler->centroid().position()
@ -647,6 +670,7 @@ void tst_QQuickPinchHandler::cumulativeNativeGestures()
QCOMPARE(pinchHandler->rotationAxis()->persistentValue(), expectedRotation);
QCOMPARE(pinchHandler->rotationAxis()->activeValue(), 0);
QCOMPARE(pinchHandler->activeTranslation(), QPointF());
QCOMPARE(pinchHandler->persistentTranslation(), QPointF());
}
void tst_QQuickPinchHandler::pan()

View File

@ -25,7 +25,9 @@ Rectangle {
function getTransformationDetails(item, pinchhandler) {
return "\n\npinch.scale:" + pinchhandler.scale.toFixed(2)
+ "\npinch.rotation:" + pinchhandler.rotation.toFixed(2)
+ "°\npinch.translation:" + "(" + pinchhandler.translation.x.toFixed(2) + "," + pinchhandler.translation.y.toFixed(2) + ")"
+ "°\npinch.activeTranslation:" + "(" + pinchhandler.activeTranslation.x.toFixed(2) + "," + pinchhandler.activeTranslation.y.toFixed(2) + ")"
+ "\npinch.persistentTranslation:" + "(" + pinchhandler.persistentTranslation.x.toFixed(2) + "," + pinchhandler.persistentTranslation.y.toFixed(2) + ")"
+ " item pos " + "(" + transformable.x.toFixed(2) + "," + transformable.y.toFixed(2) + ")"
+ "\nscale wheel.rotation:" + scaleWheelHandler.rotation.toFixed(2)
+ "°\nhorizontal wheel.rotation:" + horizontalRotationWheelHandler.rotation.toFixed(2)
+ "°\ncontrol-rotation wheel.rotation:" + controlRotationWheelHandler.rotation.toFixed(2)