Merge pull request mixxxdj#11926 from Swiftb0y/refactor/pref-sound-ta…

…b-order

Refactor/pref sound tab order

src/preferences/dialog/dlgprefsound.cpp

@@ -2,6 +2,8 @@
 
 #include <QMessageBox>
 #include <QtDebug>
+#include <algorithm>
+#include <vector>
 
 #include "control/controlproxy.h"
 #include "engine/enginebuffer.h"
@@ -13,6 +15,23 @@
 #include "util/rlimit.h"
 #include "util/scopedoverridecursor.h"
 
+namespace {
+
+bool soundItemAlreadyExists(const AudioPath& output, const QWidget& widget) {
+    for (const QObject* pObj : widget.children()) {
+        auto item = qobject_cast<const DlgPrefSoundItem*>(pObj);
+        if (!item || item->type() != output.getType()) {
+            continue;
+        }
+        if (!AudioPath::isIndexed(item->type()) || item->index() == output.getIndex()) {
+            return true;
+        }
+    }
+    return false;
+}
+
+} // namespace
+
 /**
  * Construct a new sound preferences pane. Initializes and populates all the
  * all the controls to the values obtained from SoundManager.
@@ -244,10 +263,6 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
                             MIXXX_WIKI_HARDWARE_COMPATIBILITY_URL)));
 }
 
-DlgPrefSound::~DlgPrefSound() {
-    delete m_pLatencyCompensation;
-}
-
 /**
  * Slot called when the preferences dialog is opened or this pane is
  * selected.
@@ -312,41 +327,39 @@ QUrl DlgPrefSound::helpUrl() const {
  * if necessary.
  */
 void DlgPrefSound::initializePaths() {
-    foreach (AudioOutput out, m_pSoundManager->registeredOutputs()) {
-        if (!out.isHidden()) {
-            addPath(out);
+    // Pre-sort paths so they're added in the order they'll appear later on
+    // so Tab key order matches order in layout:
+    // * by AudioPathType
+    // * identical types by index
+    auto sortFilterAdd = [this]<typename T>(const QList<T>& l) {
+        // we use a vec of ref_wrappers since copying the path is unnecessary
+        // and we really just want to change the order
+        auto ref_vec_to_sort = std::vector<std::reference_wrapper<const T>>(l.begin(), l.end());
+        std::sort(ref_vec_to_sort.begin(), ref_vec_to_sort.end());
+        for (const T& path : ref_vec_to_sort) {
+            if (!path.isHidden()) {
+                addPath(path);
+            }
         }
-    }
-    foreach (AudioInput in, m_pSoundManager->registeredInputs()) {
-        addPath(in);
-    }
+    };
+
+    sortFilterAdd(m_pSoundManager->registeredOutputs());
+    sortFilterAdd(m_pSoundManager->registeredInputs());
 }
 
 void DlgPrefSound::addPath(const AudioOutput& output) {
     // if we already know about this output, don't make a new entry
-    foreach (QObject *obj, outputTab->children()) {
-        DlgPrefSoundItem *item = qobject_cast<DlgPrefSoundItem*>(obj);
-        if (item) {
-            if (item->type() == output.getType()) {
-                if (AudioPath::isIndexed(item->type())) {
-                    if (item->index() == output.getIndex()) {
-                        return;
-                    }
-                } else {
-                    return;
-                }
-            }
-        }
-    }
 
-    DlgPrefSoundItem* pSoundItem;
-    AudioPathType type = output.getType();
-    if (AudioPath::isIndexed(type)) {
-        pSoundItem = new DlgPrefSoundItem(
-                outputTab, type, m_outputDevices, false, output.getIndex());
-    } else {
-        pSoundItem = new DlgPrefSoundItem(outputTab, type, m_outputDevices, false);
+    if (soundItemAlreadyExists(output, *outputTab)) {
+        return;
     }
+    AudioPathType type = output.getType();
+    // TODO who owns this?
+    DlgPrefSoundItem* pSoundItem = new DlgPrefSoundItem(outputTab,
+            type,
+            m_outputDevices,
+            false,
+            AudioPath::isIndexed(type) ? output.getIndex() : 0);
     connect(this,
             &DlgPrefSound::refreshOutputDevices,
             pSoundItem,
@@ -358,28 +371,17 @@ void DlgPrefSound::addPath(const AudioOutput& output) {
 }
 
 void DlgPrefSound::addPath(const AudioInput& input) {
-    DlgPrefSoundItem* pSoundItem;
-    // if we already know about this input, don't make a new entry
-    foreach (QObject *obj, inputTab->children()) {
-        DlgPrefSoundItem *item = qobject_cast<DlgPrefSoundItem*>(obj);
-        if (item) {
-            if (item->type() == input.getType()) {
-                if (AudioPath::isIndexed(item->type())) {
-                    if (item->index() == input.getIndex()) {
-                        return;
-                    }
-                } else {
-                    return;
-                }
-            }
-        }
+    if (soundItemAlreadyExists(input, *inputTab)) {
+        return;
     }
     AudioPathType type = input.getType();
-    if (AudioPath::isIndexed(type)) {
-        pSoundItem = new DlgPrefSoundItem(inputTab, type, m_inputDevices, true, input.getIndex());
-    } else {
-        pSoundItem = new DlgPrefSoundItem(inputTab, type, m_inputDevices, true);
-    }
+    // TODO: who owns this?
+    DlgPrefSoundItem* pSoundItem = new DlgPrefSoundItem(inputTab,
+            type,
+            m_inputDevices,
+            true,
+            AudioPath::isIndexed(type) ? input.getIndex() : 0);
+
     connect(this,
             &DlgPrefSound::refreshInputDevices,
             pSoundItem,

src/preferences/dialog/dlgprefsound.h

@@ -32,7 +32,6 @@ class DlgPrefSound : public DlgPreferencePage, public Ui::DlgPrefSoundDlg  {
     DlgPrefSound(QWidget* parent,
             std::shared_ptr<SoundManager> soundManager,
             UserSettingsPointer pSettings);
-    virtual ~DlgPrefSound();
 
     QUrl helpUrl() const override;
 

src/soundio/soundmanagerutil.cpp

@@ -312,11 +312,6 @@ QList<AudioPathType> AudioOutput::getSupportedTypes() {
     return types;
 }
 
-bool AudioOutput::isHidden() {
-    return m_type == RECORD_BROADCAST;
-}
-
-
 /**
  * Implements setting the type of an AudioOutput, using
  * AudioOutput::getSupportedTypes.

src/soundio/soundmanagerutil.h

@@ -102,27 +102,37 @@ class AudioPath {
         return qHash(path.hashValue(), seed);
     }
 
-    friend bool operator==(
-            const AudioPath& lhs,
-            const AudioPath& rhs) {
-        // Exclude m_channelGroup from comparison!
-        // See also: hashValue()/qHash()
-        // TODO: Why??
-        return lhs.m_type == rhs.m_type &&
-                lhs.m_index == rhs.m_index;
-    }
+    // CppCoreGuidelines C.161: Use non-member functions for symmetric operators
+    friend constexpr bool operator<(const AudioPath& lhs,
+            const AudioPath& rhs) noexcept;
+    friend constexpr bool operator==(const AudioPath& lhs,
+            const AudioPath& rhs) noexcept;
 
-protected:
+  protected:
     virtual void setType(AudioPathType type) = 0;
     ChannelGroup m_channelGroup;
     AudioPathType m_type;
     unsigned char m_index;
 };
 
-inline bool operator!=(
-        const AudioPath& lhs,
-        const AudioPath& rhs) {
-    return !(lhs == rhs);
+// TODO: turn this into operator<=> once all targets fully support that
+// XCode 14 probably and GCC 10
+constexpr bool operator<(const AudioPath& lhs,
+        const AudioPath& rhs) noexcept {
+    // Exclude m_channelGroup from comparison!
+    // See also: hashValue()/qHash()
+    // TODO: Why??
+    return std::tie(lhs.m_type, lhs.m_index) <
+            std::tie(rhs.m_type, rhs.m_index);
+}
+
+constexpr bool operator==(const AudioPath& lhs,
+        const AudioPath& rhs) noexcept {
+    // Exclude m_channelGroup from comparison!
+    // See also: hashValue()/qHash()
+    // TODO: Why??
+    return std::tie(lhs.m_type, lhs.m_index) ==
+            std::tie(rhs.m_type, rhs.m_index);
 }
 
 /// A source of audio in Mixxx that is to be output to a group of
@@ -136,7 +146,10 @@ class AudioOutput : public AudioPath {
     QDomElement toXML(QDomElement *element) const;
     static AudioOutput fromXML(const QDomElement &xml);
     static QList<AudioPathType> getSupportedTypes();
-    bool isHidden();
+    bool isHidden() const {
+        return m_type == RECORD_BROADCAST;
+    }
+
   protected:
     void setType(AudioPathType type) override;
 };
@@ -165,6 +178,11 @@ class AudioInput : public AudioPath {
     QDomElement toXML(QDomElement *element) const;
     static AudioInput fromXML(const QDomElement &xml);
     static QList<AudioPathType> getSupportedTypes();
+    // implemented for regularity with AudioOutput
+    bool isHidden() const {
+        return false;
+    }
+
   protected:
     void setType(AudioPathType type) override;
 };