Fix mixer channel updates on solo/mute (#7055)

* Fix mixer channel updates on solo/mute

Fix the update of the mixer channels whenever a channel is soloed or muted.

The solution is rather "brutal" as it updates all mixer channel views when one of the models changes.

Introduce private helper method `MixerView::updateAllMixerChannels`. It calls the `update` method on each `MixerChannelView` so that they can get repainted.

Call `updateAllMixerChannels` at the end of the existing method `toggledSolo`.

Introduce a new method `MixerView::toggledMute` which is called whenever the mute model of a channel changes. It also updates all mixer channels.

Fixes #7054.

* Improve mixer channel update for mute events

Improve the mixer channel update for mute events by not delegating to `MixerView` like for the solo case. Instead the `MixerChannelView` now has its own `toggledMute` slot which simply updates the widget on changes to the mute model.

Also fix `MixerChannelView::setChannelIndex` by disconnecting from the signals of the previous mixer channel and connecting to the signals for the new one.

Remove `toggledMute` from `MixerView`.

The solo implementation is kept as is because it also triggers changes to the core model. So the chain seems to be:
* Solo button clicked in mixer channel view
* Button changes solo model
* Solo model signals to mixer view which was connected via the mixer channel view
* Mixer view performs changes in the other core models
* All mixer channels are updated.

Are better chain would first update the core models and then update the GUI from the changes:
* Solo button clicked in mixer channel view
* Button changes solo model
* Solo model signals to core mixer, i.e. not a view!
* Mixer view performs changes in the other core models
* Changed models emit signal to GUI elements

* Revert "Improve mixer channel update for mute events"

This reverts commit ede65963ea.

* Add comment

After the revert done with commit the code is more consistent again but not in a good way. Hence a comment is added which indicates that an inprovement is needed.

* Abstract mixer retrieval

Abstract the retrieval of the mixer behind the new method `getMixer`. This is done in preparation for some dependency injection so that the `MixerView` does not have to ask the `Engine` for the mixer but gets it injected, a.k.a. the "Hollywood principle": "Don't call us, we'll call you."

It's called `getMixer` and not just mixer because it allows for locale variables to be called `mixer` without confusing it with the method.

* Let MixerView connect directly to models

Let the `MixerView` connect directly to the solo and mute models it is connected with.

Remove the connections that are made in `MixerChannelView` which acted as a proxy which only complicated things.

Add `connectToSoloAndMute` which connects the `MixerView` to the solo and mute models of a given channel. Call it whenever a new channel is created.

Add `disconnectFromSoloAndMute` which disconnects the `MixerView` from the solo and mute models of a given channel. Call it when a channel is deleted.

* Code cleanup

Cleanup code related to the creation of the master channel view.

* Inject the Mixer into the MixerView

Inject the `Mixer` into the `MixerView` via the constructor. This makes it more explicit that the `Mixer` is the model of the view. It also implements the "Dependency Inversion Principle" in that the `MIxerView` does not have to ask the `Engine` for the `Mixer` anymore.

The current changes should be safe in that the `Mixer` instance is static and does not change.

* Fix connections on song load

Disconnect and reconnect in `MixerView::refreshDisplay` which is called when a song is loaded.

include/MixerView.h

@@ -38,6 +38,11 @@
 #include "embed.h"
 #include "EffectRackView.h"
 
+namespace lmms
+{
+	class Mixer;
+}
+
 namespace lmms::gui
 {
 class LMMS_EXPORT MixerView : public QWidget, public ModelView,
@@ -45,7 +50,7 @@ class LMMS_EXPORT MixerView : public QWidget, public ModelView,
 {
 	Q_OBJECT
 public:
-	MixerView();
+	MixerView(Mixer* mixer);
 	void keyPressEvent(QKeyEvent* e) override;
 
 	void saveSettings(QDomDocument& doc, QDomElement& domElement) override;
@@ -97,7 +102,17 @@ protected:
 
 private slots:
 	void updateFaders();
+	// TODO This should be improved. Currently the solo and mute models are connected via
+	// the MixerChannelView's constructor with the MixerView. It would already be an improvement
+	// if the MixerView connected itself to each new MixerChannel that it creates/handles.
 	void toggledSolo();
+	void toggledMute();
+
+private:
+	Mixer* getMixer() const;
+	void updateAllMixerChannels();
+	void connectToSoloAndMute(int channelIndex);
+	void disconnectFromSoloAndMute(int channelIndex);
 
 private:
 	QVector<MixerChannelView*> m_mixerChannelViews;
@@ -109,6 +124,7 @@ private:
 	QWidget* m_channelAreaWidget;
 	QStackedLayout* m_racksLayout;
 	QWidget* m_racksWidget;
+	Mixer* m_mixer;
 
 	void updateMaxChannelSelector();
 

src/gui/GuiApplication.cpp

@@ -148,7 +148,7 @@ GuiApplication::GuiApplication()
 	connect(m_songEditor, SIGNAL(destroyed(QObject*)), this, SLOT(childDestroyed(QObject*)));
 
 	displayInitProgress(tr("Preparing mixer"));
-	m_mixerView = new MixerView;
+	m_mixerView = new MixerView(Engine::mixer());
 	connect(m_mixerView, SIGNAL(destroyed(QObject*)), this, SLOT(childDestroyed(QObject*)));
 
 	displayInitProgress(tr("Preparing controller rack"));

src/gui/MixerChannelView.cpp

@@ -110,8 +110,7 @@ namespace lmms::gui
         m_soloButton->setActiveGraphic(embed::getIconPixmap("led_red"));
         m_soloButton->setInactiveGraphic(embed::getIconPixmap("led_off"));
         m_soloButton->setCheckable(true);
-        m_soloButton->setToolTip(tr("Solo this channel"));
-        connect(&mixerChannel->m_soloModel, &BoolModel::dataChanged, mixerView, &MixerView::toggledSolo, Qt::DirectConnection);
+        m_soloButton->setToolTip(tr("Solo this channel"));        
 
         QVBoxLayout* soloMuteLayout = new QVBoxLayout();
         soloMuteLayout->setContentsMargins(0, 0, 0, 0);

src/gui/MixerView.cpp

@@ -52,10 +52,11 @@ namespace lmms::gui
 {
 
 
-MixerView::MixerView() :
+MixerView::MixerView(Mixer* mixer) :
 	QWidget(),
 	ModelView(nullptr, this),
-	SerializingObjectHook()
+	SerializingObjectHook(),
+	m_mixer(mixer)
 {
 #if QT_VERSION < 0x50C00
 	// Workaround for a bug in Qt versions below 5.12,
@@ -67,8 +68,7 @@ MixerView::MixerView() :
 	using ::operator|;
 #endif
 
-	Mixer * m = Engine::mixer();
-	m->setHook(this);
+	mixer->setHook(this);
 
 	//QPalette pal = palette();
 	//pal.setColor(QPalette::Window, QColor(72, 76, 88));
@@ -100,12 +100,13 @@ MixerView::MixerView() :
 	m_racksWidget->setLayout(m_racksLayout);
 
 	// add master channel
-	m_mixerChannelViews.resize(m->numChannels());
-	m_mixerChannelViews[0] = new MixerChannelView(this, this, 0);
+	m_mixerChannelViews.resize(mixer->numChannels());
+	MixerChannelView * masterView = new MixerChannelView(this, this, 0);
+	connectToSoloAndMute(0);
+	m_mixerChannelViews[0] = masterView;
 
 	m_racksLayout->addWidget(m_mixerChannelViews[0]->m_effectRackView);
 
-	MixerChannelView * masterView = m_mixerChannelViews[0];
 	ml->addWidget(masterView, 0, Qt::AlignTop);
 
 	auto mixerChannelSize = masterView->sizeHint();
@@ -114,6 +115,7 @@ MixerView::MixerView() :
 	for (int i = 1; i < m_mixerChannelViews.size(); ++i)
 	{
 		m_mixerChannelViews[i] = new MixerChannelView(m_channelAreaWidget, this, i);
+		connectToSoloAndMute(i);
 		chLayout->addWidget(m_mixerChannelViews[i]);
 	}
 
@@ -175,7 +177,7 @@ MixerView::MixerView() :
 	parentWidget()->move(5, 310);
 
 	// we want to receive dataChanged-signals in order to update
-	setModel(m);
+	setModel(mixer);
 }
 
 
@@ -184,10 +186,11 @@ MixerView::MixerView() :
 int MixerView::addNewChannel()
 {
 	// add new mixer channel and redraw the form.
-	Mixer * mix = Engine::mixer();
+	Mixer * mix = getMixer();
 
 	int newChannelIndex = mix->createChannel();
 	m_mixerChannelViews.push_back(new MixerChannelView(m_channelAreaWidget, this, newChannelIndex));
+	connectToSoloAndMute(newChannelIndex);
 	chLayout->addWidget(m_mixerChannelViews[newChannelIndex]);
 	m_racksLayout->addWidget(m_mixerChannelViews[newChannelIndex]->m_effectRackView);
 
@@ -204,6 +207,9 @@ void MixerView::refreshDisplay()
 	// delete all views and re-add them
 	for (int i = 1; i<m_mixerChannelViews.size(); ++i)
 	{
+		// First disconnect from the solo/mute models.
+		disconnectFromSoloAndMute(i);
+
 		auto * mixerChannelView = m_mixerChannelViews[i];
 		chLayout->removeWidget(mixerChannelView);
 		m_racksLayout->removeWidget(mixerChannelView->m_effectRackView);
@@ -213,10 +219,12 @@ void MixerView::refreshDisplay()
 	m_channelAreaWidget->adjustSize();
 
 	// re-add the views
-	m_mixerChannelViews.resize(Engine::mixer()->numChannels());
+	m_mixerChannelViews.resize(getMixer()->numChannels());
 	for (int i = 1; i < m_mixerChannelViews.size(); ++i)
 	{
 		m_mixerChannelViews[i] = new MixerChannelView(m_channelAreaWidget, this, i);
+		connectToSoloAndMute(i);
+
 		chLayout->addWidget(m_mixerChannelViews[i]);
 		m_racksLayout->addWidget(m_mixerChannelViews[i]->m_effectRackView);
 	}
@@ -280,10 +288,46 @@ void MixerView::loadSettings(const QDomElement& domElement)
 
 void MixerView::toggledSolo()
 {
-	Engine::mixer()->toggledSolo();
+	getMixer()->toggledSolo();
+
+	updateAllMixerChannels();
 }
 
 
+void MixerView::toggledMute()
+{
+	updateAllMixerChannels();
+}
+
+Mixer* MixerView::getMixer() const
+{
+	return m_mixer;
+}
+
+void MixerView::updateAllMixerChannels()
+{
+	for (int i = 0; i < m_mixerChannelViews.size(); ++i)
+	{
+		m_mixerChannelViews[i]->update();
+	}
+}
+
+void MixerView::connectToSoloAndMute(int channelIndex)
+{
+	auto * mixerChannel = getMixer()->mixerChannel(channelIndex);
+
+	connect(&mixerChannel->m_muteModel, &BoolModel::dataChanged, this, &MixerView::toggledMute, Qt::DirectConnection);
+	connect(&mixerChannel->m_soloModel, &BoolModel::dataChanged, this, &MixerView::toggledSolo, Qt::DirectConnection);
+}
+
+void MixerView::disconnectFromSoloAndMute(int channelIndex)
+{
+	auto * mixerChannel = getMixer()->mixerChannel(channelIndex);
+
+	disconnect(&mixerChannel->m_muteModel, &BoolModel::dataChanged, this, &MixerView::toggledMute);
+	disconnect(&mixerChannel->m_soloModel, &BoolModel::dataChanged, this, &MixerView::toggledSolo);
+}
+
 
 void MixerView::setCurrentMixerChannel(MixerChannelView* channel)
 {
@@ -301,12 +345,12 @@ void MixerView::setCurrentMixerChannel(MixerChannelView* channel)
 
 void MixerView::updateMixerChannel(int index)
 {
-	Mixer * mix = Engine::mixer();
+	Mixer * mix = getMixer();
 
 	// does current channel send to this channel?
 	int selIndex = m_currentMixerChannel->channelIndex();
 	auto thisLine = m_mixerChannelViews[index];
-	thisLine->setToolTip(Engine::mixer()->mixerChannel(index)->m_name);
+	thisLine->setToolTip(getMixer()->mixerChannel(index)->m_name);
 
 	FloatModel * sendModel = mix->channelSendModel(selIndex, index);
 	if (sendModel == nullptr)
@@ -339,15 +383,19 @@ void MixerView::deleteChannel(int index)
 		return;
 	}
 
+	// Disconnect from the solo/mute models of the channel we are about to delete
+	disconnectFromSoloAndMute(index);
+
 	// remember selected line
 	int selLine = m_currentMixerChannel->channelIndex();
 
+	Mixer* mixer = getMixer();
 	// in case the deleted channel is soloed or the remaining
 	// channels will be left in a muted state
-	Engine::mixer()->clearChannel(index);
+	mixer->clearChannel(index);
 
 	// delete the real channel
-	Engine::mixer()->deleteChannel(index);
+	mixer->deleteChannel(index);
 
 	chLayout->removeWidget(m_mixerChannelViews[index]);
 	m_racksLayout->removeWidget(m_mixerChannelViews[index]);
@@ -379,7 +427,7 @@ bool MixerView::confirmRemoval(int index)
 	bool needConfirm = ConfigManager::inst()->value("ui", "mixerchanneldeletionwarning", "1").toInt();
 	if (!needConfirm) { return true; }
 
-	Mixer* mix = Engine::mixer();
+	Mixer* mix = getMixer();
 
 	if (!mix->isChannelInUse(index))
 	{
@@ -416,7 +464,7 @@ bool MixerView::confirmRemoval(int index)
 
 void MixerView::deleteUnusedChannels()
 {
-	Mixer* mix = Engine::mixer();
+	Mixer* mix = getMixer();
 
 	// Check all channels except master, delete those with no incoming sends
 	for (int i = m_mixerChannelViews.size() - 1; i > 0; --i)
@@ -435,7 +483,7 @@ void MixerView::moveChannelLeft(int index, int focusIndex)
 	// can't move master or first channel left or last channel right
 	if (index <= 1 || index >= m_mixerChannelViews.size()) return;
 
-	Mixer *m = Engine::mixer();
+	Mixer *m = getMixer();
 
 	// Move instruments channels
 	m->moveChannelLeft(index);
@@ -542,7 +590,7 @@ void MixerView::setCurrentMixerChannel(int channel)
 
 void MixerView::clear()
 {
-	Engine::mixer()->clear();
+	getMixer()->clear();
 
 	refreshDisplay();
 }
@@ -552,7 +600,7 @@ void MixerView::clear()
 
 void MixerView::updateFaders()
 {
-	Mixer * m = Engine::mixer();
+	Mixer * m = getMixer();
 
 	// apply master gain
 	m->mixerChannel(0)->m_peakLeft *= Engine::audioEngine()->masterGain();