From 0f3df189e57d4c2bddce09380bbed8e0ed1fe2b7 Mon Sep 17 00:00:00 2001 From: Piotr Mikolajczyk Date: Tue, 6 Oct 2020 10:27:20 +0200 Subject: Fix alwaysRunToEnd==true prevented complex Anim from stopping AnimatorProxyJob would not forward loopCount to the controlled job causing the sequential or parallel animation to go infinitely after attempt to stop Task-number: QTBUG-82890 Change-Id: I6a1ca787f06789064e05407bbe9ae5e5861f24d5 Reviewed-by: Ulf Hermann (cherry picked from commit d66d0540dc323e6a536b952acedcfda70cd90c0c) Reviewed-by: Qt Cherry-pick Bot --- src/qml/animations/qabstractanimationjob.cpp | 3 + src/qml/animations/qabstractanimationjob_p.h | 1 + src/quick/util/qquickanimatorjob.cpp | 9 ++ src/quick/util/qquickanimatorjob_p.h | 1 + .../alwaysRunToEndInSequentialAnimationBug.qml | 59 ++++++++++++ .../quick/qquickanimations/qquickanimations.pro | 1 + .../qquickanimations/tst_qquickanimations.cpp | 103 +++++++++++++++++++++ 7 files changed, 177 insertions(+) create mode 100644 tests/auto/quick/qquickanimations/data/alwaysRunToEndInSequentialAnimationBug.qml diff --git a/src/qml/animations/qabstractanimationjob.cpp b/src/qml/animations/qabstractanimationjob.cpp index 82f3e53d68..f3c12bce3f 100644 --- a/src/qml/animations/qabstractanimationjob.cpp +++ b/src/qml/animations/qabstractanimationjob.cpp @@ -416,7 +416,10 @@ void QAbstractAnimationJob::setDirection(Direction direction) void QAbstractAnimationJob::setLoopCount(int loopCount) { + if (m_loopCount == loopCount) + return; m_loopCount = loopCount; + updateLoopCount(loopCount); } int QAbstractAnimationJob::totalDuration() const diff --git a/src/qml/animations/qabstractanimationjob_p.h b/src/qml/animations/qabstractanimationjob_p.h index d046ce9def..9490070246 100644 --- a/src/qml/animations/qabstractanimationjob_p.h +++ b/src/qml/animations/qabstractanimationjob_p.h @@ -134,6 +134,7 @@ public: SelfDeletable m_selfDeletable; protected: virtual void updateCurrentTime(int) {} + virtual void updateLoopCount(int) {} virtual void updateState(QAbstractAnimationJob::State newState, QAbstractAnimationJob::State oldState); virtual void updateDirection(QAbstractAnimationJob::Direction direction); virtual void topLevelAnimationLoopChanged() {} diff --git a/src/quick/util/qquickanimatorjob.cpp b/src/quick/util/qquickanimatorjob.cpp index 767be96403..2ae8a5a2aa 100644 --- a/src/quick/util/qquickanimatorjob.cpp +++ b/src/quick/util/qquickanimatorjob.cpp @@ -123,6 +123,11 @@ QQuickAnimatorProxyJob::QQuickAnimatorProxyJob(QAbstractAnimationJob *job, QObje } } +void QQuickAnimatorProxyJob::updateLoopCount(int loopCount) +{ + m_job->setLoopCount(loopCount); +} + QQuickAnimatorProxyJob::~QQuickAnimatorProxyJob() { if (m_job && m_controller) @@ -143,6 +148,10 @@ void QQuickAnimatorProxyJob::updateCurrentTime(int) if (m_internalState != State_Running) return; + // Copy current loop number from the job + // we could make currentLoop() virtual but it would be less efficient + m_currentLoop = m_job->currentLoop(); + // A proxy which is being ticked should be associated with a window, (see // setWindow() below). If we get here when there is no more controller we // have a problem. diff --git a/src/quick/util/qquickanimatorjob_p.h b/src/quick/util/qquickanimatorjob_p.h index 74085526c0..522540bcbc 100644 --- a/src/quick/util/qquickanimatorjob_p.h +++ b/src/quick/util/qquickanimatorjob_p.h @@ -87,6 +87,7 @@ public: protected: void updateCurrentTime(int) override; + void updateLoopCount(int) override; void updateState(QAbstractAnimationJob::State newState, QAbstractAnimationJob::State oldState) override; void debugAnimation(QDebug d) const override; diff --git a/tests/auto/quick/qquickanimations/data/alwaysRunToEndInSequentialAnimationBug.qml b/tests/auto/quick/qquickanimations/data/alwaysRunToEndInSequentialAnimationBug.qml new file mode 100644 index 0000000000..a775e5d263 --- /dev/null +++ b/tests/auto/quick/qquickanimations/data/alwaysRunToEndInSequentialAnimationBug.qml @@ -0,0 +1,59 @@ +import QtQuick 2.15 +import QtQuick.Window 2.15 + +Rectangle { + width: 100 + height: 100 + visible: true + color: "blue" + property bool onFinishedCalled : false; + property bool onStoppedCalled : false; + property int loopsMade: 0; + + Rectangle { + id: whiteRect + objectName: "whiteRect" + anchors.fill: parent + color: "white" + opacity: 1 + + } + + property var seqAnim : SequentialAnimation { + loops: Animation.Infinite + alwaysRunToEnd: true + NumberAnimation { + target: whiteRect + properties: "opacity" + from: 1; + to: 0.1; + duration: 500 + } + NumberAnimation { + target: whiteRect + properties: "opacity" + from: 0.1; + to: 1; + duration: 500 + } + ScriptAction { + script: loopsMade++ + } + + onFinished: { + whiteRect.opacity = 1 + onFinishedCalled = true + } + + onStopped : { + whiteRect.opacity = 1 + onStoppedCalled = true + } + + onStarted:{ + onFinishedCalled = false + onStoppedCalled = false + loopsMade = 0 + } + } +} diff --git a/tests/auto/quick/qquickanimations/qquickanimations.pro b/tests/auto/quick/qquickanimations/qquickanimations.pro index 6998c023db..85a77ae620 100644 --- a/tests/auto/quick/qquickanimations/qquickanimations.pro +++ b/tests/auto/quick/qquickanimations/qquickanimations.pro @@ -12,6 +12,7 @@ QT += core-private gui-private qml-private quick-private testlib qmlmodels-priv DEFINES += QT_DISABLE_DEPRECATED_BEFORE=0 OTHER_FILES += \ + data/alwaysRunToEndInSequentialAnimationBug.qml \ data/animationJobSelfDestructionBug.qml\ data/attached.qml \ data/badproperty1.qml \ diff --git a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp index b4eb33eb7a..9b5710bad4 100644 --- a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp +++ b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp @@ -115,6 +115,7 @@ private slots: void animationJobSelfDestruction(); void fastFlickingBug(); void opacityAnimationFromZero(); + void alwaysRunToEndInSequentialAnimationBug(); }; #define QTIMED_COMPARE(lhs, rhs) do { \ @@ -1895,6 +1896,108 @@ void tst_qquickanimations::opacityAnimationFromZero() QTRY_VERIFY(!img.isNull() && img.pixel(100, 100) > qRgb(10, 10, 10)); } +void tst_qquickanimations::alwaysRunToEndInSequentialAnimationBug() +{ + QQuickView view(QUrl::fromLocalFile("data/alwaysRunToEndInSequentialAnimationBug.qml")); + view.show(); + + QVERIFY(QTest::qWaitForWindowExposed(&view)); + QVERIFY(view.rootObject()); + QObject *root = view.rootObject(); + QQuickAbstractAnimation* seqAnim = root->property("seqAnim").value(); + QObject *whiteRect = root->findChild("whiteRect"); + + // + // Tesing the SequentialAnimation with alwaysRunToEnd = true + // + + // + // Stopping in the first loop + + QVERIFY(whiteRect->property("opacity").value() == 1.0); + seqAnim->start(); + QTRY_VERIFY(seqAnim->isRunning() == true); + + seqAnim->stop(); + QTRY_VERIFY(seqAnim->isRunning() == false); + QTRY_VERIFY(!root->property("onStoppedCalled").value()); + QTRY_VERIFY(!root->property("onFinishedCalled").value()); + + //The animation should still be running + QTest::qWait(500); + QTRY_VERIFY(whiteRect->property("opacity").value() != 1.0); + + QTest::qWait(500); + + //The animation should now be stopped - after reaching its cycle end + QTRY_COMPARE(whiteRect->property("opacity").value(), 1.0); + QTRY_VERIFY(root->property("onStoppedCalled").value()); + QTRY_VERIFY(root->property("onFinishedCalled").value()); + + // + // Stopping in the second loop + + QVERIFY(whiteRect->property("opacity").value() == 1.0); + seqAnim->start(); + QTRY_VERIFY(seqAnim->isRunning() == true); + + QTRY_COMPARE(root->property("loopsMade").value(), 1); // Wait for cycle no 2 to start + + seqAnim->stop(); + QTRY_VERIFY(seqAnim->isRunning() == false); + QTRY_VERIFY(!root->property("onStoppedCalled").value()); + QTRY_VERIFY(!root->property("onFinishedCalled").value()); + + //The animation should still be running + QTest::qWait(500); + QTRY_VERIFY(whiteRect->property("opacity").value() != 1.0); + + QTest::qWait(500); + + //The animation should now be stopped - after reaching its cycle end + QTRY_COMPARE(whiteRect->property("opacity").value(), 1.0); + QTRY_VERIFY(root->property("onStoppedCalled").value()); + QTRY_VERIFY(root->property("onFinishedCalled").value()); + + + // + // Tesing the SequentialAnimation with alwaysRunToEnd = false + // + + // + // Stopping in the first loop + + seqAnim->setProperty("alwaysRunToEnd", false); + QVERIFY(whiteRect->property("opacity").value() == 1.0); + + seqAnim->start(); + QTRY_VERIFY(seqAnim->isRunning() == true); + + QTest::qWait(50); + seqAnim->stop(); + //The animation should be stopped immediately + QVERIFY(seqAnim->isRunning() == false); + QVERIFY(root->property("onStoppedCalled").value()); + QVERIFY(root->property("onFinishedCalled").value()); + QCOMPARE(whiteRect->property("opacity").value(), 1.0); + + // + // Stopping in the second loop + QVERIFY(whiteRect->property("opacity").value() == 1.0); + seqAnim->start(); + QTRY_VERIFY(seqAnim->isRunning() == true); + + QTRY_COMPARE(root->property("loopsMade").value(), 1); // Wait for cycle no 2 to start + + QTest::qWait(50); + seqAnim->stop(); + //The animation should be stopped immediately + QVERIFY(seqAnim->isRunning() == false); + QVERIFY(root->property("onStoppedCalled").value()); + QVERIFY(root->property("onFinishedCalled").value()); + QCOMPARE(whiteRect->property("opacity").value(),1.0); +} + QTEST_MAIN(tst_qquickanimations) #include "tst_qquickanimations.moc" -- cgit v1.2.3