aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFabian Kosmale <fabian.kosmale@qt.io>2019-10-09 15:34:01 +0200
committerFabian Kosmale <fabian.kosmale@qt.io>2019-10-11 16:51:49 +0200
commit0d403890a40983b0bfa5c3640e20f6943492a6f6 (patch)
treec80a2e82269b3440fb326faa15e26e1ca84ebd32
parent66db74e53e3fa4aedeb519b8c8626066e1d6a1ce (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.cpp89
-rw-r--r--tests/auto/quick/qquickstates/data/parentChangeCorrectReversal.qml72
-rw-r--r--tests/auto/quick/qquickstates/tst_qquickstates.cpp17
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)