diff options
author | Ahmad Samir <a.samirh78@gmail.com> | 2023-11-18 21:29:26 +0200 |
---|---|---|
committer | Ahmad Samir <a.samirh78@gmail.com> | 2024-03-03 19:56:55 +0200 |
commit | 4bc0834bc182335984431c6a1525782efc34368c (patch) | |
tree | 87db305cc2d9e0995cf46225b3a5d6dee93c9de3 | |
parent | 577a3dba521f7f69bf6129fcd28184ae288182d9 (diff) |
Timers: add Qt::TimerId enum class
Which will be used to represent timer IDs. Thanks to Marc for the idea
to use "a strongly typed int".
QTimer got a new id() method that returns Qt::TimerId (can't overload
timerId()). Various classes in qtbase have a member named timerId(), but
a new method is needed anyway in QTimer so id() it is (this is the
reason QChronoTimer only has id() and no timerId()). Besides
timer.timerId() has an extra "timer".
This commit fixes the inconsistency between QObject using `0` timer id
to indicate "failed to start", while QTimer::timerId() returned `-1` to
indicate "timer is inactive". QTimer::id(), being a new method and all,
now returns Qt::TimerId::Invalid, which has value `0`, so that the
values match between the two classes. Extend the unittests to ensure
QTimer::timerId()'s behavior is preserved.
[ChangeLog][Core][QObject] Added Qt::TimerId enum class, that is used to
represent timer IDs.
Change-Id: I0e8564c1461884106d8a797cc980a669035d480a
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
-rw-r--r-- | src/corelib/doc/snippets/code/src_corelib_kernel_qobject.cpp | 13 | ||||
-rw-r--r-- | src/corelib/global/qnamespace.h | 4 | ||||
-rw-r--r-- | src/corelib/global/qnamespace.qdoc | 20 | ||||
-rw-r--r-- | src/corelib/kernel/qabstracteventdispatcher_p.h | 4 | ||||
-rw-r--r-- | src/corelib/kernel/qchronotimer.cpp | 26 | ||||
-rw-r--r-- | src/corelib/kernel/qchronotimer.h | 2 | ||||
-rw-r--r-- | src/corelib/kernel/qobject.cpp | 29 | ||||
-rw-r--r-- | src/corelib/kernel/qobject.h | 1 | ||||
-rw-r--r-- | src/corelib/kernel/qobject_p.h | 2 | ||||
-rw-r--r-- | src/corelib/kernel/qsingleshottimer_p.h | 15 | ||||
-rw-r--r-- | src/corelib/kernel/qtimer.cpp | 35 | ||||
-rw-r--r-- | src/corelib/kernel/qtimer.h | 1 | ||||
-rw-r--r-- | src/corelib/kernel/qtimer_p.h | 15 | ||||
-rw-r--r-- | tests/auto/corelib/kernel/qchronotimer/tst_qchronotimer.cpp | 6 | ||||
-rw-r--r-- | tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp | 28 |
15 files changed, 147 insertions, 54 deletions
diff --git a/src/corelib/doc/snippets/code/src_corelib_kernel_qobject.cpp b/src/corelib/doc/snippets/code/src_corelib_kernel_qobject.cpp index 3e9f78c0ff..e3bbac7a3c 100644 --- a/src/corelib/doc/snippets/code/src_corelib_kernel_qobject.cpp +++ b/src/corelib/doc/snippets/code/src_corelib_kernel_qobject.cpp @@ -469,3 +469,16 @@ const bool wasBlocked = someQObject->blockSignals(true); // no signals here someQObject->blockSignals(wasBlocked); //! [54] + +{ +//! [invalid-timer-id] + QObject *obj; + ... + int id = obj->startTimer(100ms); + if (id > Qt::TimerId::Invalid) + // The timer has been started successfully + + if (id > 0) // Equivalent, albeit less readable + // The timer has been started successfully +//! [invalid-timer-id] +} diff --git a/src/corelib/global/qnamespace.h b/src/corelib/global/qnamespace.h index a8069c0c3b..33b2a69709 100644 --- a/src/corelib/global/qnamespace.h +++ b/src/corelib/global/qnamespace.h @@ -1675,6 +1675,10 @@ namespace Qt { VeryCoarseTimer }; + enum class TimerId { + Invalid = 0, + }; + enum ScrollPhase { NoScrollPhase = 0, ScrollBegin, diff --git a/src/corelib/global/qnamespace.qdoc b/src/corelib/global/qnamespace.qdoc index 7e1536a598..d90be59520 100644 --- a/src/corelib/global/qnamespace.qdoc +++ b/src/corelib/global/qnamespace.qdoc @@ -3230,6 +3230,26 @@ */ /*! + \enum class Qt::TimerId + \since 6.8 + \relates QObject + \relates QTimer + \relates QChronoTimer + + This is used to represent timer IDs (for example, QTimer and QChronoTimer). + The underlying type is \c int. You can use \l qToUnderlying() to convert + Qt::TimerId to \c int. + + \value Invalid Represents a no-op timer ID; it's usage depends on the + context, for example, this is the value returned by QObject::startTimer() + to indicate it failed to start a timer; whereas QChronoTimer::id() returns + this value when the timer is inactive, that is, \c timer.isActive() + returns \c false. + + \sa QTimer::id(), QChronoTimer::id(), QObject::startTimer() +*/ + +/*! \enum Qt::ScrollPhase \since 5.2 diff --git a/src/corelib/kernel/qabstracteventdispatcher_p.h b/src/corelib/kernel/qabstracteventdispatcher_p.h index 7d57fd0360..87afe6a191 100644 --- a/src/corelib/kernel/qabstracteventdispatcher_p.h +++ b/src/corelib/kernel/qabstracteventdispatcher_p.h @@ -16,7 +16,9 @@ // #include "QtCore/qabstracteventdispatcher.h" +#include "QtCore/qnamespace.h" #include "private/qobject_p.h" +#include "QtCore/qttypetraits.h" QT_BEGIN_NAMESPACE @@ -33,6 +35,8 @@ public: static int allocateTimerId(); static void releaseTimerId(int id); + static void releaseTimerId(Qt::TimerId id) + { releaseTimerId(qToUnderlying(id)); } }; QT_END_NAMESPACE diff --git a/src/corelib/kernel/qchronotimer.cpp b/src/corelib/kernel/qchronotimer.cpp index 4cc8ff59fc..ac799203d3 100644 --- a/src/corelib/kernel/qchronotimer.cpp +++ b/src/corelib/kernel/qchronotimer.cpp @@ -167,10 +167,12 @@ QBindable<bool> QChronoTimer::bindableActive() } /*! - Returns the ID of the timer if the timer is running; otherwise returns - -1. + Returns a Qt::TimerId representing the timer ID if the timer is running; + otherwise returns \c Qt::TimerId::Invalid. + + \sa Qt::TimerId */ -int QChronoTimer::id() const +Qt::TimerId QChronoTimer::id() const { return d_func()->id; } @@ -189,8 +191,8 @@ void QChronoTimer::start() auto *d = d_func(); if (d->isActive()) // stop running timer stop(); - const auto id = QObject::startTimer(d->intervalDuration, d->type); - if (id > 0) { + const auto id = Qt::TimerId{QObject::startTimer(d->intervalDuration, d->type)}; + if (id > Qt::TimerId::Invalid) { d->id = id; d->isActiveData.notify(); } @@ -206,7 +208,7 @@ void QChronoTimer::stop() auto *d = d_func(); if (d->isActive()) { QObject::killTimer(d->id); - d->id = QTimerPrivate::INV_TIMER; + d->id = Qt::TimerId::Invalid; d->isActiveData.notify(); } } @@ -217,7 +219,7 @@ void QChronoTimer::stop() void QChronoTimer::timerEvent(QTimerEvent *e) { auto *d = d_func(); - if (e->timerId() == d->id) { + if (Qt::TimerId{e->timerId()} == d->id) { if (d->single) stop(); Q_EMIT timeout(QPrivateSignal()); @@ -288,14 +290,14 @@ void QChronoTimer::setInterval(std::chrono::nanoseconds nsec) d->intervalDuration.setValueBypassingBindings(nsec); if (d->isActive()) { // Create new timer QObject::killTimer(d->id); // Restart timer - const auto id = QObject::startTimer(nsec, d->type); - if (id > 0) { + const auto newId = Qt::TimerId{QObject::startTimer(nsec, d->type)}; + if (newId > Qt::TimerId::Invalid) { // Restarted successfully. No need to update the active state. - d->id = id; + d->id = newId; } else { // Failed to start the timer. // Need to notify about active state change. - d->id = QTimerPrivate::INV_TIMER; + d->id = Qt::TimerId::Invalid; d->isActiveData.notify(); } } @@ -328,7 +330,7 @@ QBindable<std::chrono::nanoseconds> QChronoTimer::bindableInterval() std::chrono::nanoseconds QChronoTimer::remainingTime() const { if (isActive()) - return QAbstractEventDispatcher::instance()->remainingTime(d_func()->id) * 1ms; + return 1ms * QAbstractEventDispatcher::instance()->remainingTime(qToUnderlying(d_func()->id)); return std::chrono::nanoseconds::min(); } diff --git a/src/corelib/kernel/qchronotimer.h b/src/corelib/kernel/qchronotimer.h index 58735c46f2..79c475e93c 100644 --- a/src/corelib/kernel/qchronotimer.h +++ b/src/corelib/kernel/qchronotimer.h @@ -38,7 +38,7 @@ public: bool isActive() const; QBindable<bool> bindableActive(); - int id() const; + Qt::TimerId id() const; void setInterval(std::chrono::nanoseconds nsec); std::chrono::nanoseconds interval() const; diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index cbdb95079b..4fea4d0d3a 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -191,8 +191,8 @@ QObjectPrivate::~QObjectPrivate() thisThreadData->eventDispatcher.loadRelaxed()->unregisterTimers(q_ptr); // release the timer ids back to the pool - for (int i = 0; i < extraData->runningTimers.size(); ++i) - QAbstractEventDispatcherPrivate::releaseTimerId(extraData->runningTimers.at(i)); + for (auto id : std::as_const(extraData->runningTimers)) + QAbstractEventDispatcherPrivate::releaseTimerId(id); } else { qWarning("QObject::~QObject: Timers cannot be stopped from another thread"); } @@ -1888,6 +1888,13 @@ int QObject::startTimer(int interval, Qt::TimerType timerType) is \c std::chrono::nanoseconds, prior to that it was \c std::chrono::milliseconds. This change is backwards compatible with older releases of Qt. + + \note In Qt 6.8, QObject was changed to use Qt::TimerId to represent timer + IDs. This method converts the TimerId to int for backwards compatibility + reasons, however you can use Qt::TimerId to check the value returned by + this method, for example: + \snippet code/src_corelib_kernel_qobject.cpp invalid-timer-id + */ int QObject::startTimer(std::chrono::nanoseconds interval, Qt::TimerType timerType) { @@ -1914,7 +1921,7 @@ int QObject::startTimer(std::chrono::nanoseconds interval, Qt::TimerType timerTy const auto msecs = std::chrono::ceil<std::chrono::milliseconds>(interval); int timerId = dispatcher->registerTimer(msecs.count(), timerType, this); d->ensureExtraData(); - d->extraData->runningTimers.append(timerId); + d->extraData->runningTimers.append(Qt::TimerId{timerId}); return timerId; } @@ -1929,17 +1936,26 @@ int QObject::startTimer(std::chrono::nanoseconds interval, Qt::TimerType timerTy void QObject::killTimer(int id) { + killTimer(Qt::TimerId{id}); +} + +/*! + \since 6.8 + \overload +*/ +void QObject::killTimer(Qt::TimerId id) +{ Q_D(QObject); if (Q_UNLIKELY(thread() != QThread::currentThread())) { qWarning("QObject::killTimer: Timers cannot be stopped from another thread"); return; } - if (id) { + if (id > Qt::TimerId::Invalid) { int at = d->extraData ? d->extraData->runningTimers.indexOf(id) : -1; if (at == -1) { // timer isn't owned by this object qWarning("QObject::killTimer(): Error: timer id %d is not valid for object %p (%s, %ls), timer has not been killed", - id, + qToUnderlying(id), this, metaObject()->className(), qUtf16Printable(objectName())); @@ -1948,14 +1964,13 @@ void QObject::killTimer(int id) auto thisThreadData = d->threadData.loadRelaxed(); if (thisThreadData->hasEventDispatcher()) - thisThreadData->eventDispatcher.loadRelaxed()->unregisterTimer(id); + thisThreadData->eventDispatcher.loadRelaxed()->unregisterTimer(qToUnderlying(id)); d->extraData->runningTimers.remove(at); QAbstractEventDispatcherPrivate::releaseTimerId(id); } } - /*! \fn QObject *QObject::parent() const diff --git a/src/corelib/kernel/qobject.h b/src/corelib/kernel/qobject.h index 9d4f4bcbff..5145258029 100644 --- a/src/corelib/kernel/qobject.h +++ b/src/corelib/kernel/qobject.h @@ -149,6 +149,7 @@ public: int startTimer(std::chrono::nanoseconds time, Qt::TimerType timerType = Qt::CoarseTimer); void killTimer(int id); + void killTimer(Qt::TimerId id); template<typename T> T findChild(QAnyStringView aName, Qt::FindChildOptions options = Qt::FindChildrenRecursively) const diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index 68ca9f53a2..5464731385 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -91,7 +91,7 @@ public: QList<QByteArray> propertyNames; QList<QVariant> propertyValues; - QList<int> runningTimers; + QList<Qt::TimerId> runningTimers; QList<QPointer<QObject>> eventFilters; Q_OBJECT_COMPAT_PROPERTY(QObjectPrivate::ExtraData, QString, objectName, &QObjectPrivate::ExtraData::setObjectNameForwarder, diff --git a/src/corelib/kernel/qsingleshottimer_p.h b/src/corelib/kernel/qsingleshottimer_p.h index 06656ec79c..3ae207fcd4 100644 --- a/src/corelib/kernel/qsingleshottimer_p.h +++ b/src/corelib/kernel/qsingleshottimer_p.h @@ -27,7 +27,8 @@ QT_BEGIN_NAMESPACE class QSingleShotTimer : public QObject { Q_OBJECT - int timerId = -1; + + Qt::TimerId timerId = Qt::TimerId::Invalid; public: inline ~QSingleShotTimer(); @@ -68,7 +69,7 @@ QSingleShotTimer::QSingleShotTimer(std::chrono::nanoseconds interval, Qt::TimerT QSingleShotTimer::~QSingleShotTimer() { - if (timerId > 0) + if (timerId > Qt::TimerId::Invalid) killTimer(timerId); } @@ -93,13 +94,12 @@ void QSingleShotTimer::startTimerForReceiver(std::chrono::nanoseconds interval, if (deadline.hasExpired()) { Q_EMIT timeout(); } else { - auto nsecs = deadline.remainingTimeAsDuration(); - timerId = startTimer(nsecs, timerType); + timerId = Qt::TimerId{startTimer(deadline.remainingTimeAsDuration(), timerType)}; } }; QMetaObject::invokeMethod(this, invokable, Qt::QueuedConnection); } else { - timerId = startTimer(interval, timerType); + timerId = Qt::TimerId{startTimer(interval, timerType)}; } } @@ -107,9 +107,8 @@ void QSingleShotTimer::timerEvent(QTimerEvent *) { // need to kill the timer _before_ we emit timeout() in case the // slot connected to timeout calls processEvents() - if (timerId > 0) - killTimer(timerId); - timerId = -1; + if (timerId > Qt::TimerId::Invalid) + killTimer(std::exchange(timerId, Qt::TimerId::Invalid)); Q_EMIT timeout(); diff --git a/src/corelib/kernel/qtimer.cpp b/src/corelib/kernel/qtimer.cpp index e235e30a2d..5784d023ff 100644 --- a/src/corelib/kernel/qtimer.cpp +++ b/src/corelib/kernel/qtimer.cpp @@ -175,9 +175,21 @@ QBindable<bool> QTimer::bindableActive() */ int QTimer::timerId() const { - return d_func()->id; + auto v = qToUnderlying(id()); + return v == 0 ? -1 : v; } +/*! + \since 6.8 + Returns a Qt::TimerId representing the timer ID if the timer is running; + otherwise returns \c Qt::TimerId::Invalid. + + \sa Qt::TimerId +*/ +Qt::TimerId QTimer::id() const +{ + return d_func()->id; +} /*! \overload start() @@ -193,9 +205,10 @@ void QTimer::start() Q_D(QTimer); if (d->isActive()) // stop running timer stop(); - const int id = QObject::startTimer(std::chrono::milliseconds{d->inter}, d->type); - if (id > 0) { - d->id = id; + + const auto newId = Qt::TimerId{QObject::startTimer(d->inter * 1ms, d->type)}; + if (newId > Qt::TimerId::Invalid) { + d->id = newId; d->isActiveData.notify(); } } @@ -249,7 +262,7 @@ void QTimer::stop() Q_D(QTimer); if (d->isActive()) { QObject::killTimer(d->id); - d->id = QTimerPrivate::INV_TIMER; + d->id = Qt::TimerId::Invalid; d->isActiveData.notify(); } } @@ -261,7 +274,7 @@ void QTimer::stop() void QTimer::timerEvent(QTimerEvent *e) { Q_D(QTimer); - if (e->timerId() == d->id) { + if (Qt::TimerId{e->timerId()} == d->id) { if (d->single) stop(); emit timeout(QPrivateSignal()); @@ -572,14 +585,14 @@ void QTimer::setInterval(std::chrono::milliseconds interval) d->inter.setValueBypassingBindings(msec); if (d->isActive()) { // create new timer QObject::killTimer(d->id); // restart timer - const int id = QObject::startTimer(std::chrono::milliseconds{msec}, d->type); - if (id > 0) { + const auto newId = Qt::TimerId{QObject::startTimer(msec * 1ms, d->type)}; + if (newId > Qt::TimerId::Invalid) { // Restarted successfully. No need to update the active state. - d->id = id; + d->id = newId; } else { // Failed to start the timer. // Need to notify about active state change. - d->id = QTimerPrivate::INV_TIMER; + d->id = Qt::TimerId::Invalid; d->isActiveData.notify(); } } @@ -612,7 +625,7 @@ int QTimer::remainingTime() const { Q_D(const QTimer); if (d->isActive()) { - return QAbstractEventDispatcher::instance()->remainingTime(d->id); + return QAbstractEventDispatcher::instance()->remainingTime(qToUnderlying(d->id)); } return -1; diff --git a/src/corelib/kernel/qtimer.h b/src/corelib/kernel/qtimer.h index 9b59895e60..854d9072f2 100644 --- a/src/corelib/kernel/qtimer.h +++ b/src/corelib/kernel/qtimer.h @@ -31,6 +31,7 @@ public: bool isActive() const; QBindable<bool> bindableActive(); int timerId() const; + Qt::TimerId id() const; void setInterval(int msec); int interval() const; diff --git a/src/corelib/kernel/qtimer_p.h b/src/corelib/kernel/qtimer_p.h index 6b99d342f1..9347f6c241 100644 --- a/src/corelib/kernel/qtimer_p.h +++ b/src/corelib/kernel/qtimer_p.h @@ -12,11 +12,13 @@ // // We mean it. // -#include "qobject_p.h" -#include "qproperty_p.h" #include "qtimer.h" #include "qchronotimer.h" +#include "qobject_p.h" +#include "qproperty_p.h" +#include "qttypetraits.h" + QT_BEGIN_NAMESPACE class QTimerPrivate : public QObjectPrivate @@ -39,7 +41,7 @@ public: void setIntervalDuration(std::chrono::nanoseconds nsec) { if (isQTimer) { - const auto msec = std::chrono::duration_cast<std::chrono::milliseconds>(nsec); + const auto msec = std::chrono::ceil<std::chrono::milliseconds>(nsec); static_cast<QTimer *>(q)->setInterval(msec); } else { static_cast<QChronoTimer *>(q)->setInterval(nsec); @@ -52,9 +54,9 @@ public: static_cast<QTimer *>(q)->setInterval(msec); } - bool isActive() const { return id > 0; } + bool isActive() const { return id > Qt::TimerId::Invalid; } - int id = INV_TIMER; + Qt::TimerId id = Qt::TimerId::Invalid; Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QTimerPrivate, int, inter, &QTimerPrivate::setInterval, 0) Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QTimerPrivate, std::chrono::nanoseconds, intervalDuration, &QTimerPrivate::setIntervalDuration, @@ -64,8 +66,7 @@ public: Q_OBJECT_COMPUTED_PROPERTY(QTimerPrivate, bool, isActiveData, &QTimerPrivate::isActive) QObject *q; - // true if q is a QTimer*, false otherwise - const bool isQTimer = false; + const bool isQTimer = false; // true if q is a QTimer* }; QT_END_NAMESPACE diff --git a/tests/auto/corelib/kernel/qchronotimer/tst_qchronotimer.cpp b/tests/auto/corelib/kernel/qchronotimer/tst_qchronotimer.cpp index 2b137b06e3..6cbbd4e53b 100644 --- a/tests/auto/corelib/kernel/qchronotimer/tst_qchronotimer.cpp +++ b/tests/auto/corelib/kernel/qchronotimer/tst_qchronotimer.cpp @@ -576,6 +576,10 @@ void tst_QChronoTimer::deleteLaterOnQChronoTimer() QVERIFY(pointer.isNull()); } +namespace { +int operator&(Qt::TimerId id, int i) { return qToUnderlying(id) & i; } +} + static constexpr auto MoveToThread_Timeout = 200ms; static constexpr auto MoveToThread_Wait = 300ms; @@ -727,7 +731,7 @@ class TimerIdPersistsAfterThreadExitThread : public QThread { public: std::unique_ptr<QChronoTimer> timer; - int timerId = -1; + Qt::TimerId timerId = Qt::TimerId::Invalid; int returnValue = -1; void run() override diff --git a/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp b/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp index 467fc9abd7..40190ca465 100644 --- a/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp +++ b/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp @@ -98,6 +98,7 @@ private slots: void automatedBindingTests(); void negativeInterval(); + void testTimerId(); }; void tst_QTimer::zeroTimer() @@ -816,12 +817,10 @@ void tst_QTimer::timerFiresOnlyOncePerProcessEvents() class TimerIdPersistsAfterThreadExitThread : public QThread { public: - QTimer *timer; - int timerId, returnValue; + QTimer *timer = nullptr; + Qt::TimerId timerId = Qt::TimerId::Invalid; + int returnValue = -1; - TimerIdPersistsAfterThreadExitThread() - : QThread(), timer(0), timerId(-1), returnValue(-1) - { } ~TimerIdPersistsAfterThreadExitThread() { delete timer; @@ -833,11 +832,15 @@ public: timer = new QTimer; connect(timer, SIGNAL(timeout()), &eventLoop, SLOT(quit())); timer->start(100); - timerId = timer->timerId(); + timerId = timer->id(); returnValue = eventLoop.exec(); } }; +namespace { +int operator&(Qt::TimerId id, int i) { return qToUnderlying(id) & i; } +} + void tst_QTimer::timerIdPersistsAfterThreadExit() { TimerIdPersistsAfterThreadExitThread thread; @@ -863,6 +866,19 @@ void tst_QTimer::cancelLongTimer() QVERIFY(!timer.isActive()); } +void tst_QTimer::testTimerId() +{ + QTimer timer; + timer.start(100ms); + QVERIFY(timer.isActive()); + QCOMPARE_GT(timer.timerId(), 0); + QCOMPARE_GT(timer.id(), Qt::TimerId::Invalid); + timer.stop(); + QVERIFY(!timer.isActive()); + QCOMPARE(timer.timerId(), -1); + QCOMPARE(timer.id(), Qt::TimerId::Invalid); +} + class TimeoutCounter : public QObject { Q_OBJECT |