From 3bd3b82a2f6e61bb9f5bcc095ef96b39d8e19b80 Mon Sep 17 00:00:00 2001 From: Michael Brasser Date: Fri, 9 Dec 2011 12:39:53 +1000 Subject: 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 Reviewed-by: Martin Jones --- src/corelib/animation/qabstractanimation.cpp | 64 ++++++++++++++-------- src/corelib/animation/qabstractanimation_p.h | 7 +++ .../qabstractanimation/tst_qabstractanimation.cpp | 46 +++++++++++++++- 3 files changed, 92 insertions(+), 25 deletions(-) diff --git a/src/corelib/animation/qabstractanimation.cpp b/src/corelib/animation/qabstractanimation.cpp index b529359f71..d852fee7f0 100644 --- a/src/corelib/animation/qabstractanimation.cpp +++ b/src/corelib/animation/qabstractanimation.cpp @@ -168,6 +168,7 @@ Q_GLOBAL_STATIC(QThreadStorage, 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 7e7571bc58..61a68a7032 100644 --- 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 8fd5237606..083324bdd8 100644 --- 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" -- cgit v1.2.3