diff options
author | Jan Holthuis <jan.holthuis@ruhr-uni-bochum.de> | 2020-07-08 11:33:52 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-07-08 11:33:52 +0200 |
commit | d967b0ee7b416be34a6c5774385b9157a00a2156 (patch) | |
tree | a9dd7212ed4c8ae5a75e883a11191c7fc041a193 /src/control | |
parent | 89bd191bae5d03a0257cdc5407e8d25f77de36d9 (diff) | |
parent | 04017eb9ac296b5dcf1afa02d32929ec79a3e724 (diff) |
Merge pull request #2918 from uklotzde/controlobject
ControlObject/ControlPrivateDouble: Various Fixes
Diffstat (limited to 'src/control')
-rw-r--r-- | src/control/control.cpp | 141 | ||||
-rw-r--r-- | src/control/control.h | 90 | ||||
-rw-r--r-- | src/control/controlobject.cpp | 4 | ||||
-rw-r--r-- | src/control/controlobject.h | 5 |
4 files changed, 142 insertions, 98 deletions
diff --git a/src/control/control.cpp b/src/control/control.cpp index d46fcb1d78..99f72ce37f 100644 --- a/src/control/control.cpp +++ b/src/control/control.cpp @@ -1,46 +1,38 @@ -#include <QtDebug> -#include <QSharedPointer> - #include "control/control.h" +#include "control/controlobject.h" #include "util/stat.h" -// Static member variable definition +//static UserSettingsPointer ControlDoublePrivate::s_pUserConfig; -QHash<ConfigKey, QWeakPointer<ControlDoublePrivate> > ControlDoublePrivate::s_qCOHash -GUARDED_BY(ControlDoublePrivate::s_qCOHashMutex); +//static +QHash<ConfigKey, QWeakPointer<ControlDoublePrivate>> ControlDoublePrivate::s_qCOHash + GUARDED_BY(ControlDoublePrivate::s_qCOHashMutex); +//static QHash<ConfigKey, ConfigKey> ControlDoublePrivate::s_qCOAliasHash -GUARDED_BY(ControlDoublePrivate::s_qCOHashMutex); + GUARDED_BY(ControlDoublePrivate::s_qCOHashMutex); +//static MMutex ControlDoublePrivate::s_qCOHashMutex; -/* -ControlDoublePrivate::ControlDoublePrivate() - : m_bIgnoreNops(true), - m_bTrack(false), - m_trackType(Stat::UNSPECIFIED), - m_trackFlags(Stat::COUNT | Stat::SUM | Stat::AVERAGE | - Stat::SAMPLE_VARIANCE | Stat::MIN | Stat::MAX), - m_confirmRequired(false) { - initialize(); -} -*/ - -ControlDoublePrivate::ControlDoublePrivate(ConfigKey key, - ControlObject* pCreatorCO, - bool bIgnoreNops, bool bTrack, - bool bPersist, double defaultValue) +ControlDoublePrivate::ControlDoublePrivate( + ConfigKey key, + ControlObject* pCreatorCO, + bool bIgnoreNops, + bool bTrack, + bool bPersist, + double defaultValue) : m_key(key), + m_pCreatorCO(pCreatorCO), m_bPersistInConfiguration(bPersist), m_bIgnoreNops(bIgnoreNops), m_bTrack(bTrack), m_trackType(Stat::UNSPECIFIED), m_trackFlags(Stat::COUNT | Stat::SUM | Stat::AVERAGE | - Stat::SAMPLE_VARIANCE | Stat::MIN | Stat::MAX), - m_confirmRequired(false), - m_pCreatorCO(pCreatorCO) { + Stat::SAMPLE_VARIANCE | Stat::MIN | Stat::MAX), + m_confirmRequired(false) { initialize(defaultValue); } @@ -112,59 +104,88 @@ QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl( VERIFY_OR_DEBUG_ASSERT(!key.isEmpty()) { qWarning() << "ControlDoublePrivate::getControl returning NULL" << "for empty ConfigKey."; - return QSharedPointer<ControlDoublePrivate>(); + return nullptr; } - QSharedPointer<ControlDoublePrivate> pControl; // Scope for MMutexLocker. { - MMutexLocker locker(&s_qCOHashMutex); - auto it = s_qCOHash.constFind(key); - if (it != s_qCOHash.constEnd()) { - if (pCreatorCO) { - qWarning() << "ControlObject" << key.group << key.item << "already created"; - DEBUG_ASSERT(!"ControlObject already created"); + const MMutexLocker locker(&s_qCOHashMutex); + const auto it = s_qCOHash.find(key); + if (it != s_qCOHash.end()) { + auto pControl = it.value().lock(); + if (pControl) { + // Control object already exists + VERIFY_OR_DEBUG_ASSERT(!pCreatorCO) { + qWarning() + << "ControlObject" + << key.group << key.item + << "already created"; + return nullptr; + } + return pControl; } else { - pControl = it.value(); + // The weak pointer has become invalid and can be cleaned up + s_qCOHash.erase(it); } } } - if (pControl == NULL) { - if (pCreatorCO) { - pControl = QSharedPointer<ControlDoublePrivate>( - new ControlDoublePrivate(key, pCreatorCO, bIgnoreNops, - bTrack, bPersist, defaultValue)); - MMutexLocker locker(&s_qCOHashMutex); - //qDebug() << "ControlDoublePrivate::s_qCOHash.insert(" << key.group << "," << key.item << ")"; - s_qCOHash.insert(key, pControl); - } else if (!flags.testFlag(ControlFlag::NoWarnIfMissing)) { - qWarning() << "ControlDoublePrivate::getControl returning NULL for (" - << key.group << "," << key.item << ")"; - DEBUG_ASSERT(flags.testFlag(ControlFlag::NoAssertIfMissing)); - } + if (pCreatorCO) { + auto pControl = QSharedPointer<ControlDoublePrivate>( + new ControlDoublePrivate(key, + pCreatorCO, + bIgnoreNops, + bTrack, + bPersist, + defaultValue)); + const MMutexLocker locker(&s_qCOHashMutex); + //qDebug() << "ControlDoublePrivate::s_qCOHash.insert(" << key.group << "," << key.item << ")"; + s_qCOHash.insert(key, pControl); + return pControl; + } + + if (!flags.testFlag(ControlFlag::NoWarnIfMissing)) { + qWarning() << "ControlDoublePrivate::getControl returning NULL for (" + << key.group << "," << key.item << ")"; + DEBUG_ASSERT(flags.testFlag(ControlFlag::NoAssertIfMissing)); } - return pControl; + return nullptr; } // static -void ControlDoublePrivate::getControls( - QList<QSharedPointer<ControlDoublePrivate> >* pControlList) { - s_qCOHashMutex.lock(); - pControlList->clear(); - for (auto it = s_qCOHash.constBegin(); it != s_qCOHash.constEnd(); ++it) { - QSharedPointer<ControlDoublePrivate> pControl = it.value(); - if (!pControl.isNull()) { - pControlList->push_back(pControl); +QList<QSharedPointer<ControlDoublePrivate>> ControlDoublePrivate::getAllInstances() { + QList<QSharedPointer<ControlDoublePrivate>> result; + MMutexLocker locker(&s_qCOHashMutex); + result.reserve(s_qCOHash.size()); + for (auto it = s_qCOHash.begin(); it != s_qCOHash.end(); ++it) { + auto pControl = it.value().lock(); + if (pControl) { + result.append(std::move(pControl)); + } else { + // The weak pointer has become invalid and can be cleaned up + s_qCOHash.erase(it); } } - s_qCOHashMutex.unlock(); + return result; } // static -QHash<ConfigKey, ConfigKey> ControlDoublePrivate::getControlAliases() { +QList<QSharedPointer<ControlDoublePrivate>> ControlDoublePrivate::takeAllInstances() { + QList<QSharedPointer<ControlDoublePrivate>> result; MMutexLocker locker(&s_qCOHashMutex); - return s_qCOAliasHash; + result.reserve(s_qCOHash.size()); + for (auto it = s_qCOHash.begin(); it != s_qCOHash.end(); ++it) { + auto pControl = it.value().lock(); + if (pControl) { + result.append(std::move(pControl)); + } + } + s_qCOHash.clear(); + return result; +} + +void ControlDoublePrivate::deleteCreatorCO() { + delete m_pCreatorCO.fetchAndStoreOrdered(nullptr); } void ControlDoublePrivate::reset() { diff --git a/src/control/control.h b/src/control/control.h index 56df7d5ae2..5ff49bcd4b 100644 --- a/src/control/control.h +++ b/src/control/control.h @@ -1,10 +1,10 @@ -#ifndef CONTROL_H -#define CONTROL_H +#pragma once +#include <QAtomicPointer> #include <QHash> -#include <QString> #include <QObject> -#include <QAtomicPointer> +#include <QSharedPointer> +#include <QString> #include "control/controlbehavior.h" #include "control/controlvalue.h" @@ -26,7 +26,7 @@ Q_DECLARE_OPERATORS_FOR_FLAGS(ControlFlags) class ControlDoublePrivate : public QObject { Q_OBJECT public: - virtual ~ControlDoublePrivate(); + ~ControlDoublePrivate() override; // Used to implement control persistence. All controls that are marked // "persist in user config" get and set their value on creation/deletion @@ -43,18 +43,24 @@ class ControlDoublePrivate : public QObject { // Gets the ControlDoublePrivate matching the given ConfigKey. If pCreatorCO // is non-NULL, allocates a new ControlDoublePrivate for the ConfigKey if // one does not exist. - static QSharedPointer<ControlDoublePrivate> getControl(const ConfigKey& key, + static QSharedPointer<ControlDoublePrivate> getControl( + const ConfigKey& key, ControlFlags flags = ControlFlag::None, - ControlObject* pCreatorCO = NULL, + ControlObject* pCreatorCO = nullptr, bool bIgnoreNops = true, bool bTrack = false, bool bPersist = false, double defaultValue = 0.0); - // Adds all ControlDoublePrivate that currently exist to pControlList - static void getControls(QList<QSharedPointer<ControlDoublePrivate> >* pControlsList); + // Returns a list of all existing instances. + static QList<QSharedPointer<ControlDoublePrivate>> getAllInstances(); + // Clears all existing instances and returns them as a list. + static QList<QSharedPointer<ControlDoublePrivate>> takeAllInstances(); - static QHash<ConfigKey, ConfigKey> getControlAliases(); + static QHash<ConfigKey, ConfigKey> getControlAliases() { + // Implicitly shared classes can safely be copied across threads + return s_qCOAliasHash; + } const QString& name() const { return m_name; @@ -78,7 +84,7 @@ class ControlDoublePrivate : public QObject { // ValueChangeRequest slot. void setAndConfirm(double value, QObject* pSender); // Gets the control value. - inline double get() const { + double get() const { return m_value.getValue(); } // Resets the control value to its default. @@ -86,8 +92,10 @@ class ControlDoublePrivate : public QObject { // Set the behavior to be used when setting values and translating between // parameter and value space. Returns the previously set behavior (if any). - // The caller must not delete the behavior at any time. The memory is managed - // by this function. + // Callers must allocate the passed behavior using new and ownership to this + // memory is passed with the function call!! + // TODO: Pass a std::unique_ptr instead of a plain pointer to ensure this + // transfer of ownership. void setBehavior(ControlNumericBehavior* pBehavior); void setParameter(double dParam, QObject* pSender); @@ -98,27 +106,28 @@ class ControlDoublePrivate : public QObject { void setValueFromMidi(MidiOpCode opcode, double dParam); double getMidiParameter() const; - inline bool ignoreNops() const { + bool ignoreNops() const { return m_bIgnoreNops; } - inline void setDefaultValue(double dValue) { + void setDefaultValue(double dValue) { m_defaultValue.setValue(dValue); } - inline double defaultValue() const { + double defaultValue() const { return m_defaultValue.getValue(); } - inline ControlObject* getCreatorCO() const { - return m_pCreatorCO; + ControlObject* getCreatorCO() const { + return m_pCreatorCO.loadAcquire(); } - inline void removeCreatorCO() { - m_pCreatorCO = NULL; + bool resetCreatorCO(ControlObject* pCreatorCO) { + return m_pCreatorCO.testAndSetOrdered(pCreatorCO, nullptr); } + void deleteCreatorCO(); - inline ConfigKey getKey() { + ConfigKey getKey() { return m_key; } @@ -144,25 +153,30 @@ class ControlDoublePrivate : public QObject { void valueChangeRequest(double value); private: - ControlDoublePrivate(ConfigKey key, ControlObject* pCreatorCO, - bool bIgnoreNops, bool bTrack, bool bPersist, - double defaultValue); + ControlDoublePrivate( + ConfigKey key, + ControlObject* pCreatorCO, + bool bIgnoreNops, + bool bTrack, + bool bPersist, + double defaultValue); + ControlDoublePrivate(ControlDoublePrivate&&) = delete; + ControlDoublePrivate(const ControlDoublePrivate&) = delete; + ControlDoublePrivate& operator=(ControlDoublePrivate&&) = delete; + ControlDoublePrivate& operator=(const ControlDoublePrivate&) = delete; + void initialize(double defaultValue); void setInner(double value, QObject* pSender); - ConfigKey m_key; + const ConfigKey m_key; + + QAtomicPointer<ControlObject> m_pCreatorCO; // Whether the control should persist in the Mixxx user configuration. The // value is loaded from configuration when the control is created and // written to the configuration when the control is deleted. bool m_bPersistInConfiguration; - // User-visible, i18n name for what the control is. - QString m_name; - - // User-visible, i18n description for what the control does. - QString m_description; - // Whether to ignore sets which would have no effect. bool m_bIgnoreNops; @@ -173,6 +187,12 @@ class ControlDoublePrivate : public QObject { int m_trackFlags; bool m_confirmRequired; + // User-visible, i18n name for what the control is. + QString m_name; + + // User-visible, i18n description for what the control does. + QString m_description; + // The control value. ControlValueAtomic<double> m_value; // The default control value. @@ -180,8 +200,6 @@ class ControlDoublePrivate : public QObject { QSharedPointer<ControlNumericBehavior> m_pBehavior; - ControlObject* m_pCreatorCO; - // Hack to implement persistent controls. This is a pointer to the current // user configuration object (if one exists). In general, we do not want the // user configuration to be a singleton -- objects that need access to it @@ -191,7 +209,8 @@ class ControlDoublePrivate : public QObject { static UserSettingsPointer s_pUserConfig; // Hash of ControlDoublePrivate instantiations. - static QHash<ConfigKey, QWeakPointer<ControlDoublePrivate> > s_qCOHash; + static QHash<ConfigKey, QWeakPointer<ControlDoublePrivate>> s_qCOHash; + // Hash of aliases between ConfigKeys. Solely used for looking up the first // alias associated with a key. static QHash<ConfigKey, ConfigKey> s_qCOAliasHash; @@ -199,6 +218,3 @@ class ControlDoublePrivate : public QObject { // Mutex guarding access to s_qCOHash and s_qCOAliasHash. static MMutex s_qCOHashMutex; }; - - -#endif /* CONTROL_H */ diff --git a/src/control/controlobject.cpp b/src/control/controlobject.cpp index 2de1c38683..988681cc50 100644 --- a/src/control/controlobject.cpp +++ b/src/control/controlobject.cpp @@ -54,7 +54,9 @@ ControlObject::ControlObject(ConfigKey key, bool bIgnoreNops, bool bTrack, ControlObject::~ControlObject() { if (m_pControl) { - m_pControl->removeCreatorCO(); + const bool success = m_pControl->resetCreatorCO(this); + Q_UNUSED(success); + DEBUG_ASSERT(success); } } diff --git a/src/control/controlobject.h b/src/control/controlobject.h index 56af1aefd5..a3e247f846 100644 --- a/src/control/controlobject.h +++ b/src/control/controlobject.h @@ -180,6 +180,11 @@ class ControlObject : public QObject { void readOnlyHandler(double v); private: + ControlObject(ControlObject&&) = delete; + ControlObject(const ControlObject&) = delete; + ControlObject& operator=(ControlObject&&) = delete; + ControlObject& operator=(const ControlObject&) = delete; + inline bool ignoreNops() const { return m_pControl ? m_pControl->ignoreNops() : true; } |