Fix possible jump in animation timer.

If both a stop and start happen within an event loop, ensure they are
processed in order.

Based on a patch from Charles Yin.

Task-number: QTBUG-22865
Change-Id: I6131bd43a6ba5ad4fa37c863a9f4598bf2ac0e01
Reviewed-by: Leonardo Sobral Cunha <leo.cunha@nokia.com>
Reviewed-by: Martin Jones <martin.jones@nokia.com>

diff --git a/src/corelib/animation/qabstractanimation.cpp b/src/corelib/animation/qabstractanimation.cpp
index b529359..d852fee 100644 (file)
--- a/src/corelib/animation/qabstractanimation.cpp
+++ b/src/corelib/animation/qabstractanimation.cpp
@@ -168,6 +168,7 @@ Q_GLOBAL_STATIC(QThreadStorage<QUnifiedTimer *>, unifiedTimer)
 QUnifiedTimer::QUnifiedTimer() :
     QObject(), defaultDriver(this), lastTick(0), timingInterval(DEFAULT_TIMER_INTERVAL),
     currentAnimationIdx(0), insideTick(false), consistentTiming(false), slowMode(false),
+    startAnimationPending(false), stopTimerPending(false),
     slowdownFactor(5.0f), isPauseTimerActive(false), runningLeafAnimations(0), profilerCallback(0)
 {
     time.invalidate();
@@ -284,30 +285,41 @@ void QUnifiedTimer::setTimingInterval(int interval)
     }
 }
 
+void QUnifiedTimer::startAnimations()
+{
+    startAnimationPending = false;
+    //we transfer the waiting animations into the "really running" state
+    animations += animationsToStart;
+    animationsToStart.clear();
+    if (!animations.isEmpty()) {
+        restartAnimationTimer();
+        if (!time.isValid()) {
+            lastTick = 0;
+            time.start();
+        }
+    }
+}
+
+void QUnifiedTimer::stopTimer()
+{
+    stopTimerPending = false;
+    if (animations.isEmpty()) {
+        animationTimer.stop();
+        isPauseTimerActive = false;
+        // invalidate the start reference time
+        time.invalidate();
+    }
+}
 
 void QUnifiedTimer::timerEvent(QTimerEvent *event)
 {
     //in the case of consistent timing we make sure the orders in which events come is always the same
-   //for that purpose we do as if the startstoptimer would always fire before the animation timer
-    if ((consistentTiming && startStopAnimationTimer.isActive()) ||
-        event->timerId() == startStopAnimationTimer.timerId()) {
-        startStopAnimationTimer.stop();
-
-        //we transfer the waiting animations into the "really running" state
-        animations += animationsToStart;
-        animationsToStart.clear();
-        if (animations.isEmpty()) {
-            animationTimer.stop();
-            isPauseTimerActive = false;
-            // invalidate the start reference time
-            time.invalidate();
-        } else {
-            restartAnimationTimer();
-            if (!time.isValid()) {
-                lastTick = 0;
-                time.start();
-            }
-        }
+    //for that purpose we do as if the startstoptimer would always fire before the animation timer
+    if (consistentTiming) {
+        if (stopTimerPending)
+            stopTimer();
+        if (startAnimationPending)
+            startAnimations();
     }
 
     if (event->timerId() == animationTimer.timerId()) {
@@ -325,8 +337,10 @@ void QUnifiedTimer::registerAnimation(QAbstractAnimation *animation, bool isTopL
         Q_ASSERT(!QAbstractAnimationPrivate::get(animation)->hasRegisteredTimer);
         QAbstractAnimationPrivate::get(animation)->hasRegisteredTimer = true;
         inst->animationsToStart << animation;
-        if (!inst->startStopAnimationTimer.isActive())
-            inst->startStopAnimationTimer.start(STARTSTOP_TIMER_DELAY, inst);
+        if (!inst->startAnimationPending) {
+            inst->startAnimationPending = true;
+            QMetaObject::invokeMethod(inst, "startAnimations", Qt::QueuedConnection);
+        }
     }
 }
 
@@ -349,8 +363,10 @@ void QUnifiedTimer::unregisterAnimation(QAbstractAnimation *animation)
             if (idx <= inst->currentAnimationIdx)
                 --inst->currentAnimationIdx;
 
-            if (inst->animations.isEmpty() && !inst->startStopAnimationTimer.isActive())
-                inst->startStopAnimationTimer.start(STARTSTOP_TIMER_DELAY, inst);
+            if (inst->animations.isEmpty() && !inst->stopTimerPending) {
+                inst->stopTimerPending = true;
+                QMetaObject::invokeMethod(inst, "stopTimer", Qt::QueuedConnection);
+            }
         } else {
             inst->animationsToStart.removeOne(animation);
         }

diff --git a/src/corelib/animation/qabstractanimation_p.h b/src/corelib/animation/qabstractanimation_p.h
index 7e7571b..61a68a7 100644 (file)
--- a/src/corelib/animation/qabstractanimation_p.h
+++ b/src/corelib/animation/qabstractanimation_p.h
@@ -144,6 +144,7 @@ typedef QElapsedTimer ElapsedTimer;
 
 class Q_CORE_EXPORT QUnifiedTimer : public QObject
 {
+    Q_OBJECT
 private:
     QUnifiedTimer();
 
@@ -194,6 +195,10 @@ public:
 protected:
     void timerEvent(QTimerEvent *);
 
+private Q_SLOTS:
+    void startAnimations();
+    void stopTimer();
+
 private:
     friend class QDefaultAnimationDriver;
     friend class QAnimationDriver;
@@ -213,6 +218,8 @@ private:
     bool insideTick;
     bool consistentTiming;
     bool slowMode;
+    bool startAnimationPending;
+    bool stopTimerPending;
 
     // This factor will be used to divide the DEFAULT_TIMER_INTERVAL at each tick
     // when slowMode is enabled. Setting it to 0 or higher than DEFAULT_TIMER_INTERVAL (16)

diff --git a/tests/auto/corelib/animation/qabstractanimation/tst_qabstractanimation.cpp b/tests/auto/corelib/animation/qabstractanimation/tst_qabstractanimation.cpp
index 8fd5237..083324b 100644 (file)
--- a/tests/auto/corelib/animation/qabstractanimation/tst_qabstractanimation.cpp
+++ b/tests/auto/corelib/animation/qabstractanimation/tst_qabstractanimation.cpp
@@ -58,6 +58,8 @@ private slots:
     void loopCount();
     void state();
     void totalDuration();
+    void avoidJumpAtStart();
+    void avoidJumpAtStartWithStop();
 };
 
 class TestableQAbstractAnimation : public QAbstractAnimation
@@ -65,10 +67,15 @@ class TestableQAbstractAnimation : public QAbstractAnimation
     Q_OBJECT
 
 public:
+    TestableQAbstractAnimation() : m_duration(10) {}
     virtual ~TestableQAbstractAnimation() {};
 
-    int duration() const { return 10; }
+    int duration() const { return m_duration; }
     virtual void updateCurrentTime(int) {}
+
+    void setDuration(int duration) { m_duration = duration; }
+private:
+    int m_duration;
 };
 
 class DummyQAnimationGroup : public QAnimationGroup
@@ -150,6 +157,43 @@ void tst_QAbstractAnimation::totalDuration()
     QCOMPARE(anim.totalDuration(), 50);
 }
 
+void tst_QAbstractAnimation::avoidJumpAtStart()
+{
+    TestableQAbstractAnimation anim;
+    anim.setDuration(1000);
+
+    /*
+        the timer shouldn't actually start until we hit the event loop,
+        so the sleep should have no effect
+    */
+    anim.start();
+    QTest::qSleep(300);
+    QCoreApplication::processEvents();
+    QVERIFY(anim.currentTime() < 50);
+}
+
+void tst_QAbstractAnimation::avoidJumpAtStartWithStop()
+{
+    TestableQAbstractAnimation anim;
+    anim.setDuration(1000);
+
+    TestableQAbstractAnimation anim2;
+    anim2.setDuration(1000);
+
+    anim.start();
+    QTest::qWait(300);
+    anim.stop();
+
+    /*
+        same test as avoidJumpAtStart, but after there is a
+        running animation that is stopped
+    */
+    anim2.start();
+    QTest::qSleep(300);
+    QCoreApplication::processEvents();
+    QVERIFY(anim2.currentTime() < 50);
+}
+
 QTEST_MAIN(tst_QAbstractAnimation)
 
 #include "tst_qabstractanimation.moc"