diff options
author | Fabian Kosmale <fabian.kosmale@qt.io> | 2019-10-09 15:34:01 +0200 |
---|---|---|
committer | Fabian Kosmale <fabian.kosmale@qt.io> | 2019-10-11 16:51:49 +0200 |
commit | 0d403890a40983b0bfa5c3640e20f6943492a6f6 (patch) | |
tree | c80a2e82269b3440fb326faa15e26e1ca84ebd32 | |
parent | 66db74e53e3fa4aedeb519b8c8626066e1d6a1ce (diff) |
ParentChange: Restore old position correctly
ParentChange does floating point math in doChange to calculate the
correct position. Unfortunately, while doing the conversion, we
accumulate rounding errors in the presence of rotations.
Those can lead to visual glitches, as observed in QTBUG-68176.
This patch avoids the issue by storing the old values and resetting to
them in restore.
Fixes: QTBUG-68176
Change-Id: I6ebe1ccbc601838aa664cdc723e0cd58c95e785a
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
-rw-r--r-- | src/quick/items/qquickstateoperations.cpp | 89 | ||||
-rw-r--r-- | tests/auto/quick/qquickstates/data/parentChangeCorrectReversal.qml | 72 | ||||
-rw-r--r-- | tests/auto/quick/qquickstates/tst_qquickstates.cpp | 17 |
3 files changed, 140 insertions, 38 deletions
diff --git a/src/quick/items/qquickstateoperations.cpp b/src/quick/items/qquickstateoperations.cpp index 31d1c91649..07767d377d 100644 --- a/src/quick/items/qquickstateoperations.cpp +++ b/src/quick/items/qquickstateoperations.cpp @@ -44,6 +44,7 @@ #include <QtQml/qqmlinfo.h> #include <QtCore/qmath.h> +#include <memory> QT_BEGIN_NAMESPACE @@ -51,15 +52,17 @@ class QQuickParentChangePrivate : public QQuickStateOperationPrivate { Q_DECLARE_PUBLIC(QQuickParentChange) public: - QQuickParentChangePrivate() : target(nullptr), parent(nullptr), origParent(nullptr), origStackBefore(nullptr), - rewindParent(nullptr), rewindStackBefore(nullptr) {} - - QQuickItem *target; + QQuickItem *target = nullptr; QPointer<QQuickItem> parent; - QPointer<QQuickItem> origParent; - QPointer<QQuickItem> origStackBefore; - QQuickItem *rewindParent; - QQuickItem *rewindStackBefore; + + struct StateSnapshot { + QPointer<QQuickItem> parent; + QPointer<QQuickItem> stackBefore; + qreal x = 0, y = 0, width = 0, height = 0, scale = 0, rotation = 0; + }; + + std::unique_ptr<StateSnapshot> orig; + std::unique_ptr<StateSnapshot> rewind; QQmlNullableValue<QQmlScriptString> xString; QQmlNullableValue<QQmlScriptString> yString; @@ -68,10 +71,11 @@ public: QQmlNullableValue<QQmlScriptString> scaleString; QQmlNullableValue<QQmlScriptString> rotationString; - void doChange(QQuickItem *targetParent, QQuickItem *stackBefore = nullptr); + void doChange(QQuickItem *targetParent); + void reverseRewindHelper(const std::unique_ptr<StateSnapshot> &snapshot); }; -void QQuickParentChangePrivate::doChange(QQuickItem *targetParent, QQuickItem *stackBefore) +void QQuickParentChangePrivate::doChange(QQuickItem *targetParent) { if (targetParent && target && target->parentItem()) { Q_Q(QQuickParentChange); @@ -137,11 +141,6 @@ void QQuickParentChangePrivate::doChange(QQuickItem *targetParent, QQuickItem *s } else if (target) { target->setParentItem(targetParent); } - - //restore the original stack position. - //### if stackBefore has also been reparented this won't work - if (target && stackBefore) - target->stackBefore(stackBefore); } /*! @@ -305,7 +304,7 @@ bool QQuickParentChange::rotationIsSet() const QQuickItem *QQuickParentChange::originalParent() const { Q_D(const QQuickParentChange); - return d->origParent; + return d->orig ? d->orig->parent : nullptr; } /*! @@ -473,21 +472,11 @@ void QQuickParentChange::saveOriginals() { Q_D(QQuickParentChange); saveCurrentValues(); - d->origParent = d->rewindParent; - d->origStackBefore = d->rewindStackBefore; + if (!d->orig) + d->orig.reset(new QQuickParentChangePrivate::StateSnapshot); + *d->orig = *d->rewind; } -/*void QQuickParentChange::copyOriginals(QQuickStateActionEvent *other) -{ - Q_D(QQuickParentChange); - QQuickParentChange *pc = static_cast<QQuickParentChange*>(other); - - d->origParent = pc->d_func()->rewindParent; - d->origStackBefore = pc->d_func()->rewindStackBefore; - - saveCurrentValues(); -}*/ - void QQuickParentChange::execute() { Q_D(QQuickParentChange); @@ -499,10 +488,26 @@ bool QQuickParentChange::isReversable() return true; } +void QQuickParentChangePrivate::reverseRewindHelper(const std::unique_ptr<QQuickParentChangePrivate::StateSnapshot> &snapshot) +{ + if (!target || !snapshot) + return; + target->setX(snapshot->x); + target->setY(snapshot->y); + target->setScale(snapshot->scale); + target->setWidth(snapshot->width); + target->setHeight(snapshot->height); + target->setRotation(snapshot->rotation); + target->setParentItem(snapshot->parent); + if (snapshot->stackBefore) + target->stackBefore(snapshot->stackBefore); +} + + void QQuickParentChange::reverse() { Q_D(QQuickParentChange); - d->doChange(d->origParent, d->origStackBefore); + d->reverseRewindHelper(d->orig); } QQuickStateActionEvent::EventType QQuickParentChange::type() const @@ -524,21 +529,28 @@ void QQuickParentChange::saveCurrentValues() { Q_D(QQuickParentChange); if (!d->target) { - d->rewindParent = nullptr; - d->rewindStackBefore = nullptr; + d->rewind = nullptr; return; } - d->rewindParent = d->target->parentItem(); - d->rewindStackBefore = nullptr; + d->rewind.reset(new QQuickParentChangePrivate::StateSnapshot); + d->rewind->x = d->target->x(); + d->rewind->y = d->target->y(); + d->rewind->scale = d->target->scale(); + d->rewind->width = d->target->width(); + d->rewind->height = d->target->height(); + d->rewind->rotation = d->target->rotation(); + + d->rewind->parent = d->target->parentItem(); + d->rewind->stackBefore = nullptr; - if (!d->rewindParent) + if (!d->rewind->parent) return; - QList<QQuickItem *> children = d->rewindParent->childItems(); + QList<QQuickItem *> children = d->rewind->parent->childItems(); for (int ii = 0; ii < children.count() - 1; ++ii) { if (children.at(ii) == d->target) { - d->rewindStackBefore = children.at(ii + 1); + d->rewind->stackBefore = children.at(ii + 1); break; } } @@ -547,7 +559,8 @@ void QQuickParentChange::saveCurrentValues() void QQuickParentChange::rewind() { Q_D(QQuickParentChange); - d->doChange(d->rewindParent, d->rewindStackBefore); + d->reverseRewindHelper(d->rewind); + d->rewind.reset(); } /*! diff --git a/tests/auto/quick/qquickstates/data/parentChangeCorrectReversal.qml b/tests/auto/quick/qquickstates/data/parentChangeCorrectReversal.qml new file mode 100644 index 0000000000..3d38fa4046 --- /dev/null +++ b/tests/auto/quick/qquickstates/data/parentChangeCorrectReversal.qml @@ -0,0 +1,72 @@ +import QtQuick 2.10 +import QtQuick.Layouts 1.3 + +Item { + id: root + + visible: true + + width: 400 + height: 200 + property bool switchToRight: false + property alias stayingRectX: stayingRect.x + + RowLayout { + id: topLayout + + anchors.fill: parent + + Item { + Layout.fillWidth: true + Layout.fillHeight: true + + Rectangle { + id: leftRect + + width: parent.width*(2/3) + height: width + anchors.centerIn: parent + + color: "red" + + Rectangle { + id: stayingRect + + x: 70; y: 70 + width: 50; height: 50 + + color: "yellow" + } + } + } + + Item { + Layout.fillWidth: true + Layout.fillHeight: true + + Rectangle { + id: rightRect + + width: parent.height*(2/3) + height: width + anchors.centerIn: parent + + color: "green" + rotation: 45 + } + } + } + + states: State { + name: "switchToRight" + + ParentChange { + target: stayingRect + parent: rightRect + width: 70 + } + + } + + state: root.switchToRight ? "switchToRight" : "" +} diff --git a/tests/auto/quick/qquickstates/tst_qquickstates.cpp b/tests/auto/quick/qquickstates/tst_qquickstates.cpp index 1eb797f54f..d5fea3cb28 100644 --- a/tests/auto/quick/qquickstates/tst_qquickstates.cpp +++ b/tests/auto/quick/qquickstates/tst_qquickstates.cpp @@ -139,6 +139,7 @@ private slots: void revertListMemoryLeak(); void duplicateStateName(); void trivialWhen(); + void parentChangeCorrectReversal(); }; void tst_qquickstates::initTestCase() @@ -1675,6 +1676,22 @@ void tst_qquickstates::trivialWhen() QVERIFY(c.create()); } +void tst_qquickstates::parentChangeCorrectReversal() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("parentChangeCorrectReversal.qml")); + QScopedPointer<QObject> root {c.create()}; + QVERIFY(root); + QQmlProperty stayingRectX(root.get(), "stayingRectX"); + qreal oldX = stayingRectX.read().toDouble(); + QQmlProperty switchToRight(root.get(), "switchToRight"); + switchToRight.write(true); + qreal newX = stayingRectX.read().toDouble(); + QVERIFY(newX != oldX); + switchToRight.write(false); + QCOMPARE(oldX, stayingRectX.read().toDouble()); +} + QTEST_MAIN(tst_qquickstates) |