diff --git a/src/control/control.cpp b/src/control/control.cpp index d46fcb1d780..99f72ce37f5 100644 --- a/src/control/control.cpp +++ b/src/control/control.cpp @@ -1,46 +1,38 @@ -#include -#include - #include "control/control.h" +#include "control/controlobject.h" #include "util/stat.h" -// Static member variable definition +//static UserSettingsPointer ControlDoublePrivate::s_pUserConfig; -QHash > ControlDoublePrivate::s_qCOHash -GUARDED_BY(ControlDoublePrivate::s_qCOHashMutex); +//static +QHash> ControlDoublePrivate::s_qCOHash + GUARDED_BY(ControlDoublePrivate::s_qCOHashMutex); +//static QHash 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::getControl( VERIFY_OR_DEBUG_ASSERT(!key.isEmpty()) { qWarning() << "ControlDoublePrivate::getControl returning NULL" << "for empty ConfigKey."; - return QSharedPointer(); + return nullptr; } - QSharedPointer 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( - 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( + 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 >* pControlList) { - s_qCOHashMutex.lock(); - pControlList->clear(); - for (auto it = s_qCOHash.constBegin(); it != s_qCOHash.constEnd(); ++it) { - QSharedPointer pControl = it.value(); - if (!pControl.isNull()) { - pControlList->push_back(pControl); +QList> ControlDoublePrivate::getAllInstances() { + QList> 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 ControlDoublePrivate::getControlAliases() { +QList> ControlDoublePrivate::takeAllInstances() { + QList> 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 56df7d5ae2f..5ff49bcd4b6 100644 --- a/src/control/control.h +++ b/src/control/control.h @@ -1,10 +1,10 @@ -#ifndef CONTROL_H -#define CONTROL_H +#pragma once +#include #include -#include #include -#include +#include +#include #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 getControl(const ConfigKey& key, + static QSharedPointer 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 >* pControlsList); + // Returns a list of all existing instances. + static QList> getAllInstances(); + // Clears all existing instances and returns them as a list. + static QList> takeAllInstances(); - static QHash getControlAliases(); + static QHash 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 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 m_value; // The default control value. @@ -180,8 +200,6 @@ class ControlDoublePrivate : public QObject { QSharedPointer 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 > s_qCOHash; + static QHash> s_qCOHash; + // Hash of aliases between ConfigKeys. Solely used for looking up the first // alias associated with a key. static QHash 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 2de1c386838..988681cc50e 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 56af1aefd5c..a3e247f8463 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; } diff --git a/src/dialog/dlgdevelopertools.cpp b/src/dialog/dlgdevelopertools.cpp index 4642c649ccd..9e5b462e1c3 100644 --- a/src/dialog/dlgdevelopertools.cpp +++ b/src/dialog/dlgdevelopertools.cpp @@ -13,9 +13,9 @@ DlgDeveloperTools::DlgDeveloperTools(QWidget* pParent, m_pConfig(pConfig) { setupUi(this); - QList > controlsList; - ControlDoublePrivate::getControls(&controlsList); - QHash controlAliases = + const QList> controlsList = + ControlDoublePrivate::getAllInstances(); + const QHash controlAliases = ControlDoublePrivate::getControlAliases(); for (auto it = controlsList.constBegin(); @@ -144,8 +144,8 @@ void DlgDeveloperTools::slotControlDump() { return; } - QList > controlsList; - ControlDoublePrivate::getControls(&controlsList); + const QList> controlsList = + ControlDoublePrivate::getAllInstances(); for (auto it = controlsList.constBegin(); it != controlsList.constEnd(); ++it) { const QSharedPointer& pControl = *it; if (pControl) { diff --git a/src/mixxx.cpp b/src/mixxx.cpp index 0fd9dde2a6a..54b30bf3d2b 100644 --- a/src/mixxx.cpp +++ b/src/mixxx.cpp @@ -142,7 +142,6 @@ const int MixxxMainWindow::kAuxiliaryCount = 4; MixxxMainWindow::MixxxMainWindow(QApplication* pApp, const CmdlineArgs& args) : m_pWidgetParent(nullptr), m_pLaunchImage(nullptr), - m_pSettingsManager(nullptr), m_pEffectsManager(nullptr), m_pEngine(nullptr), m_pSkinLoader(nullptr), @@ -177,7 +176,7 @@ MixxxMainWindow::MixxxMainWindow(QApplication* pApp, const CmdlineArgs& args) StatsManager::createInstance(); } - m_pSettingsManager = new SettingsManager(this, args.getSettingsPath()); + m_pSettingsManager = std::make_unique(args.getSettingsPath()); initializeKeyboard(); @@ -285,10 +284,10 @@ void MixxxMainWindow::initialize(QApplication* pApp, const CmdlineArgs& args) { m_pRecordingManager = new RecordingManager(pConfig, m_pEngine); - #ifdef __BROADCAST__ - m_pBroadcastManager = new BroadcastManager(m_pSettingsManager, - m_pSoundManager); + m_pBroadcastManager = new BroadcastManager( + m_pSettingsManager.get(), + m_pSoundManager); #endif launchProgress(11); @@ -471,9 +470,17 @@ void MixxxMainWindow::initialize(QApplication* pApp, const CmdlineArgs& args) { } // Initialize preference dialog - m_pPrefDlg = new DlgPreferences(this, m_pSkinLoader, m_pSoundManager, m_pPlayerManager, - m_pControllerManager, m_pVCManager, pLV2Backend, m_pEffectsManager, - m_pSettingsManager, m_pLibrary); + m_pPrefDlg = new DlgPreferences( + this, + m_pSkinLoader, + m_pSoundManager, + m_pPlayerManager, + m_pControllerManager, + m_pVCManager, + pLV2Backend, + m_pEffectsManager, + m_pSettingsManager.get(), + m_pLibrary); m_pPrefDlg->setWindowIcon(QIcon(":/images/mixxx_icon.svg")); m_pPrefDlg->setHidden(true); @@ -777,42 +784,6 @@ void MixxxMainWindow::finalize() { delete m_pGuiTick; delete m_pVisualsManager; - // Check for leaked ControlObjects and give warnings. - QList > leakedControls; - QList leakedConfigKeys; - - ControlDoublePrivate::getControls(&leakedControls); - - if (leakedControls.size() > 0) { - qDebug() << "WARNING: The following" << leakedControls.size() - << "controls were leaked:"; - foreach (QSharedPointer pCDP, leakedControls) { - if (pCDP.isNull()) { - continue; - } - ConfigKey key = pCDP->getKey(); - qDebug() << key.group << key.item << pCDP->getCreatorCO(); - leakedConfigKeys.append(key); - } - - // Deleting leaked objects helps to satisfy valgrind. - // These delete calls could cause crashes if a destructor for a control - // we thought was leaked is triggered after this one exits. - // So, only delete so if developer mode is on. - if (CmdlineArgs::Instance().getDeveloper()) { - foreach (ConfigKey key, leakedConfigKeys) { - // A deletion early in the list may trigger a destructor - // for a control later in the list, so we check for a null - // pointer each time. - ControlObject* pCo = ControlObject::getControl(key, ControlFlag::NoAssertIfMissing); - if (pCo) { - delete pCo; - } - } - } - leakedControls.clear(); - } - // Delete the track collections after all internal track pointers // in other components have been released by deleting those components // beforehand! @@ -829,9 +800,34 @@ void MixxxMainWindow::finalize() { // at exit. m_pSettingsManager->save(); + // Check for leaked ControlObjects and give warnings. + { + QList> leakedControls = + ControlDoublePrivate::takeAllInstances(); + VERIFY_OR_DEBUG_ASSERT(leakedControls.isEmpty()) { + qWarning() + << "The following" + << leakedControls.size() + << "controls were leaked:"; + for (auto pCDP : leakedControls) { + ConfigKey key = pCDP->getKey(); + qWarning() << key.group << key.item << pCDP->getCreatorCO(); + // Deleting leaked objects helps to satisfy valgrind. + // These delete calls could cause crashes if a destructor for a control + // we thought was leaked is triggered after this one exits. + // So, only delete so if developer mode is on. + if (CmdlineArgs::Instance().getDeveloper()) { + pCDP->deleteCreatorCO(); + } + } + } + // Finally drop all shared pointers by exiting this scope + } + Sandbox::shutdown(); qDebug() << t.elapsed(false).debugMillisWithUnit() << "deleting SettingsManager"; + m_pSettingsManager.reset(); delete m_pKeyboard; delete m_pKbdConfig; diff --git a/src/mixxx.h b/src/mixxx.h index 765a7c065c5..f34157f888e 100644 --- a/src/mixxx.h +++ b/src/mixxx.h @@ -1,35 +1,19 @@ -/*************************************************************************** - mixxx.h - description - ------------------- - begin : Mon Feb 18 09:48:17 CET 2002 - copyright : (C) 2002 by Tue and Ken Haste Andersen - email : - ***************************************************************************/ - -/*************************************************************************** - * * - * This program is free software; you can redistribute it and/or modify * - * it under the terms of the GNU General Public License as published by * - * the Free Software Foundation; either version 2 of the License, or * - * (at your option) any later version. * - * * - ***************************************************************************/ - -#ifndef MIXXX_H -#define MIXXX_H +#pragma once #include #include #include +#include #include "preferences/configobject.h" -#include "preferences/usersettings.h" #include "preferences/constants.h" +#include "preferences/usersettings.h" +#include "soundio/sounddeviceerror.h" #include "track/track.h" #include "util/cmdlineargs.h" -#include "util/timer.h" #include "util/db/dbconnectionpool.h" -#include "soundio/sounddeviceerror.h" +#include "util/parented_ptr.h" +#include "util/timer.h" class ChannelHandleFactory; class ControlPushButton; @@ -142,7 +126,7 @@ class MixxxMainWindow : public QMainWindow { QWidget* m_pWidgetParent; LaunchImage* m_pLaunchImage; - SettingsManager* m_pSettingsManager; + std::unique_ptr m_pSettingsManager; // The effects processing system EffectsManager* m_pEffectsManager; @@ -203,5 +187,3 @@ class MixxxMainWindow : public QMainWindow { static const int kMicrophoneCount; static const int kAuxiliaryCount; }; - -#endif diff --git a/src/preferences/dialog/dlgprefdeck.cpp b/src/preferences/dialog/dlgprefdeck.cpp index 9ce38c81586..79799bd4ad5 100644 --- a/src/preferences/dialog/dlgprefdeck.cpp +++ b/src/preferences/dialog/dlgprefdeck.cpp @@ -34,22 +34,29 @@ constexpr int kDefaultRateRampSensitivity = 250; // to playermanager.cpp } -DlgPrefDeck::DlgPrefDeck(QWidget * parent, MixxxMainWindow * mixxx, - PlayerManager* pPlayerManager, - UserSettingsPointer pConfig) - : DlgPreferencePage(parent), - m_pConfig(pConfig), - m_mixxx(mixxx), - m_pPlayerManager(pPlayerManager), - m_iNumConfiguredDecks(0), - m_iNumConfiguredSamplers(0) { +DlgPrefDeck::DlgPrefDeck(QWidget* parent, + MixxxMainWindow* mixxx, + PlayerManager* pPlayerManager, + UserSettingsPointer pConfig) + : DlgPreferencePage(parent), + m_mixxx(mixxx), + m_pPlayerManager(pPlayerManager), + m_pConfig(pConfig), + m_pControlTrackTimeDisplay(std::make_unique( + ConfigKey("[Controls]", "ShowDurationRemaining"))), + m_pControlTrackTimeFormat(std::make_unique( + ConfigKey("[Controls]", "TimeFormat"))), + m_pNumDecks( + make_parented("[Master]", "num_decks", this)), + m_pNumSamplers(make_parented( + "[Master]", "num_samplers", this)), + m_iNumConfiguredDecks(0), + m_iNumConfiguredSamplers(0) { setupUi(this); - m_pNumDecks = new ControlProxy("[Master]", "num_decks", this); m_pNumDecks->connectValueChanged(this, [=](double value){slotNumDecksChanged(value);}); slotNumDecksChanged(m_pNumDecks->get(), true); - m_pNumSamplers = new ControlProxy("[Master]", "num_samplers", this); m_pNumSamplers->connectValueChanged(this, [=](double value){slotNumSamplersChanged(value);}); slotNumSamplersChanged(m_pNumSamplers->get(), true); @@ -74,10 +81,10 @@ DlgPrefDeck::DlgPrefDeck(QWidget * parent, MixxxMainWindow * mixxx, connect(ComboBoxCueMode, SIGNAL(activated(int)), this, SLOT(slotCueModeCombobox(int))); // Track time display configuration - m_pControlTrackTimeDisplay = new ControlObject( - ConfigKey("[Controls]", "ShowDurationRemaining")); - connect(m_pControlTrackTimeDisplay, SIGNAL(valueChanged(double)), - this, SLOT(slotSetTrackTimeDisplay(double))); + connect(m_pControlTrackTimeDisplay.get(), + &ControlObject::valueChanged, + this, + QOverload::of(&DlgPrefDeck::slotSetTrackTimeDisplay)); double positionDisplayType = m_pConfig->getValue( ConfigKey("[Controls]", "PositionDisplay"), @@ -101,10 +108,7 @@ DlgPrefDeck::DlgPrefDeck(QWidget * parent, MixxxMainWindow * mixxx, this, SLOT(slotSetTrackTimeDisplay(QAbstractButton *))); // display time format - - m_pControlTrackTimeFormat = new ControlObject( - ConfigKey("[Controls]", "TimeFormat")); - connect(m_pControlTrackTimeFormat, + connect(m_pControlTrackTimeFormat.get(), &ControlObject::valueChanged, this, &DlgPrefDeck::slotTimeFormatChanged); @@ -351,7 +355,6 @@ DlgPrefDeck::DlgPrefDeck(QWidget * parent, MixxxMainWindow * mixxx, } DlgPrefDeck::~DlgPrefDeck() { - delete m_pControlTrackTimeDisplay; qDeleteAll(m_rateControls); qDeleteAll(m_rateDirectionControls); qDeleteAll(m_cueControls); diff --git a/src/preferences/dialog/dlgprefdeck.h b/src/preferences/dialog/dlgprefdeck.h index 41788ab9cf2..dde7bff85a7 100644 --- a/src/preferences/dialog/dlgprefdeck.h +++ b/src/preferences/dialog/dlgprefdeck.h @@ -1,7 +1,7 @@ -#ifndef DLGPREFDECK_H -#define DLGPREFDECK_H +#pragma once #include +#include #include "engine/controls/cuecontrol.h" #include "engine/controls/ratecontrol.h" @@ -9,6 +9,7 @@ #include "preferences/dialog/ui_dlgprefdeckdlg.h" #include "preferences/dlgpreferencepage.h" #include "preferences/usersettings.h" +#include "util/parented_ptr.h" class ControlProxy; class ControlPotmeter; @@ -58,7 +59,7 @@ class DlgPrefDeck : public DlgPreferencePage, public Ui::DlgPrefDeckDlg { DlgPrefDeck(QWidget *parent, MixxxMainWindow *mixxx, PlayerManager* pPlayerManager, UserSettingsPointer pConfig); - virtual ~DlgPrefDeck(); + ~DlgPrefDeck() override; public slots: void slotUpdate() override; @@ -101,19 +102,22 @@ class DlgPrefDeck : public DlgPreferencePage, public Ui::DlgPrefDeckDlg { void setRateRangeForAllDecks(int rangePercent); void setRateDirectionForAllDecks(bool inverted); - UserSettingsPointer m_pConfig; - ControlObject* m_pControlTrackTimeDisplay; - ControlObject* m_pControlTrackTimeFormat; - ControlProxy* m_pNumDecks; - ControlProxy* m_pNumSamplers; + MixxxMainWindow* const m_mixxx; + PlayerManager* const m_pPlayerManager; + const UserSettingsPointer m_pConfig; + + const std::unique_ptr m_pControlTrackTimeDisplay; + const std::unique_ptr m_pControlTrackTimeFormat; + + const parented_ptr m_pNumDecks; + const parented_ptr m_pNumSamplers; + QList m_cueControls; QList m_rateControls; QList m_rateDirectionControls; QList m_rateRangeControls; QList m_keylockModeControls; QList m_keyunlockModeControls; - MixxxMainWindow *m_mixxx; - PlayerManager* m_pPlayerManager; int m_iNumConfiguredDecks; int m_iNumConfiguredSamplers; @@ -143,5 +147,3 @@ class DlgPrefDeck : public DlgPreferencePage, public Ui::DlgPrefDeckDlg { double m_dRatePermCoarse; double m_dRatePermFine; }; - -#endif diff --git a/src/preferences/settingsmanager.cpp b/src/preferences/settingsmanager.cpp index f5bb8c680f8..efae6089815 100644 --- a/src/preferences/settingsmanager.cpp +++ b/src/preferences/settingsmanager.cpp @@ -6,10 +6,8 @@ #include "preferences/upgrade.h" #include "util/assert.h" -SettingsManager::SettingsManager(QObject* pParent, - const QString& settingsPath) - : QObject(pParent), - m_bShouldRescanLibrary(false) { +SettingsManager::SettingsManager(const QString& settingsPath) + : m_bShouldRescanLibrary(false) { // First make sure the settings path exists. If we don't then other parts of // Mixxx (such as the library) will produce confusing errors. if (!QDir(settingsPath).exists()) { diff --git a/src/preferences/settingsmanager.h b/src/preferences/settingsmanager.h index d45ba67754c..2f9dcd9dfbd 100644 --- a/src/preferences/settingsmanager.h +++ b/src/preferences/settingsmanager.h @@ -1,17 +1,11 @@ -#ifndef PREFERENCES_SETTINGSMANAGER_H -#define PREFERENCES_SETTINGSMANAGER_H - -#include -#include -#include +#pragma once #include "preferences/broadcastsettings.h" #include "preferences/usersettings.h" -class SettingsManager : public QObject { - Q_OBJECT +class SettingsManager { public: - SettingsManager(QObject* pParent, const QString& settingsPath); + explicit SettingsManager(const QString& settingsPath); virtual ~SettingsManager(); UserSettingsPointer settings() const { @@ -37,5 +31,3 @@ class SettingsManager : public QObject { bool m_bShouldRescanLibrary; BroadcastSettingsPointer m_pBroadcastSettings; }; - -#endif /* PREFERENCES_SETTINGSMANAGER_H */ diff --git a/src/test/mixxxtest.cpp b/src/test/mixxxtest.cpp index f70495e2255..a57debe7c78 100644 --- a/src/test/mixxxtest.cpp +++ b/src/test/mixxxtest.cpp @@ -45,13 +45,7 @@ MixxxTest::MixxxTest() { MixxxTest::~MixxxTest() { // Mixxx leaks a ton of COs normally. To make new tests not affected by // previous tests, we clear our all COs after every MixxxTest completion. - QList> leakedControls; - ControlDoublePrivate::getControls(&leakedControls); - foreach (QSharedPointer pCDP, leakedControls) { - if (pCDP.isNull()) { - continue; - } - ConfigKey key = pCDP->getKey(); - delete pCDP->getCreatorCO(); + for (auto pControl : ControlDoublePrivate::takeAllInstances()) { + pControl->deleteCreatorCO(); } } diff --git a/src/util/widgethelper.cpp b/src/util/widgethelper.cpp index 6de2a7beb54..dc35c09bd60 100644 --- a/src/util/widgethelper.cpp +++ b/src/util/widgethelper.cpp @@ -13,7 +13,11 @@ QPoint mapPopupToScreen( const QWidget& widget, const QPoint& popupUpperLeft, const QSize& popupSize) { - const auto screenSize = widget.windowHandle()->screen()->size(); + const auto* pWindow = getWindow(widget); + if (!pWindow) { + return popupUpperLeft; + } + const auto screenSize = pWindow->screen()->size(); // math_clamp() cannot be used, because if the dimensions of // the popup menu are greater than the screen size a debug // assertion would be triggered! @@ -28,6 +32,17 @@ QPoint mapPopupToScreen( return QPoint(adjustedX, adjustedY); } +QWindow* getWindow( + const QWidget& widget) { + if (auto* window = widget.windowHandle()) { + return window; + } + if (auto* nativeParent = widget.nativeParentWidget()) { + return nativeParent->windowHandle(); + } + return nullptr; +} + } // namespace widgethelper } // namespace mixxx diff --git a/src/util/widgethelper.h b/src/util/widgethelper.h index 94a17b77d30..c6ce078a9c1 100644 --- a/src/util/widgethelper.h +++ b/src/util/widgethelper.h @@ -16,6 +16,14 @@ QPoint mapPopupToScreen( const QPoint& popupUpperLeft, const QSize& popupSize); +/// Obtains the corresponding window for the given widget. +/// +/// Might return nullptr if no window could be determined. +/// +/// Adopted from windowForWidget() in qtbase/src/widgets/kernel/qapplication_p.h +QWindow* getWindow( + const QWidget& widget); + } // namespace widgethelper } // namespace mixxx diff --git a/src/widget/wcuemenupopup.h b/src/widget/wcuemenupopup.h index d04e825f6f3..fdb265c4894 100644 --- a/src/widget/wcuemenupopup.h +++ b/src/widget/wcuemenupopup.h @@ -7,6 +7,7 @@ #include "preferences/colorpalettesettings.h" #include "track/cue.h" #include "track/track.h" +#include "util/widgethelper.h" #include "widget/wcolorpicker.h" class WCueMenuPopup : public QWidget { @@ -30,10 +31,10 @@ class WCueMenuPopup : public QWidget { } } - void popup(const QPoint& p, QAction* atAction = nullptr) { - Q_UNUSED(atAction); - qDebug() << "Showing menu at" << p; - move(p); + void popup(const QPoint& p) { + auto parentWidget = static_cast(parent()); + QPoint topLeft = mixxx::widgethelper::mapPopupToScreen(*parentWidget, p, size()); + move(topLeft); show(); } diff --git a/src/widget/woverview.cpp b/src/widget/woverview.cpp index cc43c830495..d9aac6b0a59 100644 --- a/src/widget/woverview.cpp +++ b/src/widget/woverview.cpp @@ -34,7 +34,6 @@ #include "util/math.h" #include "util/painterscope.h" #include "util/timer.h" -#include "util/widgethelper.h" #include "waveform/waveform.h" #include "waveform/waveformwidgetfactory.h" #include "widget/controlwidgetconnection.h" @@ -533,11 +532,7 @@ void WOverview::mousePressEvent(QMouseEvent* e) { } if (pHoveredCue != nullptr) { m_pCueMenuPopup->setTrackAndCue(m_pCurrentTrack, pHoveredCue); - QPoint cueMenuTopLeft = mixxx::widgethelper::mapPopupToScreen( - *this, - e->globalPos(), - m_pCueMenuPopup->size()); - m_pCueMenuPopup->popup(cueMenuTopLeft); + m_pCueMenuPopup->popup(e->globalPos()); } } } diff --git a/src/widget/wwaveformviewer.cpp b/src/widget/wwaveformviewer.cpp index 415bf3d8276..45a0e63a849 100644 --- a/src/widget/wwaveformviewer.cpp +++ b/src/widget/wwaveformviewer.cpp @@ -13,7 +13,6 @@ #include "track/track.h" #include "util/dnd.h" #include "util/math.h" -#include "util/widgethelper.h" #include "waveform/waveformwidgetfactory.h" #include "waveform/widgets/waveformwidgetabstract.h" @@ -88,11 +87,7 @@ void WWaveformViewer::mousePressEvent(QMouseEvent* event) { auto cueAtClickPos = getCuePointerFromCueMark(m_pHoveredMark); if (cueAtClickPos) { m_pCueMenuPopup->setTrackAndCue(currentTrack, cueAtClickPos); - QPoint cueMenuTopLeft = mixxx::widgethelper::mapPopupToScreen( - *this, - event->globalPos(), - m_pCueMenuPopup->size()); - m_pCueMenuPopup->popup(cueMenuTopLeft); + m_pCueMenuPopup->popup(event->globalPos()); } } else { // If we are scratching then disable and reset because the two shouldn't