From cf9b4816add835ff5eee561141eabec771a16c56 Mon Sep 17 00:00:00 2001
From: ronso0 <ronso0@mixxx.org>
Date: Sat, 27 Jan 2024 18:23:10 +0100
Subject: [PATCH] Samplers: don't create unneeded empty samplers during startup

---
 src/mixer/playermanager.cpp |  7 +++-
 src/mixer/samplerbank.cpp   | 65 +++++++++++++++++++++++--------------
 src/mixer/samplerbank.h     |  3 +-
 3 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/src/mixer/playermanager.cpp b/src/mixer/playermanager.cpp
index c8d162a5811..1f591dc036b 100644
--- a/src/mixer/playermanager.cpp
+++ b/src/mixer/playermanager.cpp
@@ -450,7 +450,12 @@ void PlayerManager::addDeckInner() {
 }
 
 void PlayerManager::loadSamplers() {
-    m_pSamplerBank->loadSamplerBankFromPath(getDefaultSamplerPath(m_pConfig));
+    // This is only called by CoreServices during startup to restore
+    // samplers from the previous session.
+    // We don't want it to create more players than necessary.
+    bool dontCreateEmptySamplers = true;
+    m_pSamplerBank->loadSamplerBankFromPath(getDefaultSamplerPath(m_pConfig),
+            dontCreateEmptySamplers);
 }
 
 void PlayerManager::addSampler() {
diff --git a/src/mixer/samplerbank.cpp b/src/mixer/samplerbank.cpp
index 267ad218321..647cc269634 100644
--- a/src/mixer/samplerbank.cpp
+++ b/src/mixer/samplerbank.cpp
@@ -165,7 +165,8 @@ void SamplerBank::slotLoadSamplerBank(double v) {
     }
 }
 
-bool SamplerBank::loadSamplerBankFromPath(const QString& samplerBankPath) {
+bool SamplerBank::loadSamplerBankFromPath(const QString& samplerBankPath,
+        bool dontCreateEmptySamplers) {
     // The user has picked a new directory via a file dialog. This means the
     // system sandboxer (if we are sandboxed) has granted us permission to this
     // folder. We don't need access to this file on a regular basis so we do not
@@ -195,33 +196,47 @@ bool SamplerBank::loadSamplerBankFromPath(const QString& samplerBankPath) {
         return false;
     }
 
-    QDomNode n = root.firstChild();
-
-    while (!n.isNull()) {
+    const auto samplerNodes = root.childNodes();
+    if (samplerNodes.isEmpty()) {
+        return true;
+    }
+    for (int i = 0; i < samplerNodes.size(); i++) {
+        QDomNode n = samplerNodes.at(i);
         QDomElement e = n.toElement();
+        if (e.isNull() || e.tagName() != "sampler") {
+            continue;
+        }
+
+        const QString group = e.attribute("group", "");
+        const QString location = e.attribute("location", "");
+        int samplerNum;
+
+        if (group.isEmpty() || !m_pPlayerManager->isSamplerGroup(group, &samplerNum)) {
+            continue;
+        }
+
+        // During startup we only increase the sampler count if there is
+        // a track to be loaded. This avoids
+        // * creating more sampler players than strictly necessary
+        // * an unnecessary large Load To > Sampler N submenu in the track menu
+        if (dontCreateEmptySamplers && location.isEmpty()) {
+            continue;
+        }
+
+        // Later on, when the user loads a samplers file manually, we
+        // want to eject loaded tracks if the file's sample slot is empty.
+        // We also create new players even if they are not present in the
+        // GUI to not drop tracks loaded to invisible samplers when saving
+        // the samplers file.
+        if (static_cast<int>(m_pPlayerManager->numSamplers()) < samplerNum) {
+            m_pCONumSamplers->set(samplerNum);
+        }
 
-        if (!e.isNull()) {
-            if (e.tagName() == "sampler") {
-                QString group = e.attribute("group", "");
-                QString location = e.attribute("location", "");
-                int samplerNum;
-
-                if (!group.isEmpty()
-                        && m_pPlayerManager->isSamplerGroup(group, &samplerNum)) {
-                    if (m_pPlayerManager->numSamplers() < (unsigned) samplerNum) {
-                        m_pCONumSamplers->set(samplerNum);
-                    }
-
-                    if (location.isEmpty()) {
-                        m_pPlayerManager->slotLoadTrackToPlayer(TrackPointer(), group, false);
-                    } else {
-                        m_pPlayerManager->slotLoadLocationToPlayer(location, group, false);
-                    }
-                }
-
-            }
+        if (location.isEmpty()) {
+            m_pPlayerManager->slotLoadTrackToPlayer(TrackPointer(), group, false);
+        } else {
+            m_pPlayerManager->slotLoadLocationToPlayer(location, group, false);
         }
-        n = n.nextSibling();
     }
 
     file.close();
diff --git a/src/mixer/samplerbank.h b/src/mixer/samplerbank.h
index 5234f045f71..0f787e089d2 100644
--- a/src/mixer/samplerbank.h
+++ b/src/mixer/samplerbank.h
@@ -19,7 +19,8 @@ class SamplerBank : public QObject {
             PlayerManager* pPlayerManager);
 
     bool saveSamplerBankToPath(const QString& samplerBankPath);
-    bool loadSamplerBankFromPath(const QString& samplerBankPath);
+    bool loadSamplerBankFromPath(const QString& samplerBankPath,
+            bool dontCreateEmptySamplers = false);
 
   private slots:
     void slotSaveSamplerBank(double v);