Fix knob linking / refactor linking (#7883)

Closes #7869

This PR aims to fix linking bugs and it aims to make linking code faster. In the future I would like to replace controller and automation code with linked models, so it is essential for this feature to work as efficiently as possible.

Before this PR:

- AutomatableModels store a list of AutomatableModels that they are linked to.
- setValue() and other functions make recursive calls to themself resulting in the #7869 crash.
- Each AutomatableModel can unlink from other AutomatableModels, unlinking is the inverse operation to linking.

After this PR:

- AutomatableModels store a pointer to an other AutomatableModel making a linked list. The end is connected to the first element resulting in a "ring".
- setValue() and others are now recursion free, the code runs for every linked model, more efficiently than before.
- Each AutomatableModel can NOT unlink form other AutomatableModels, unlinking is NOT the inverse operation to linking. AutomatableModels can unlink themself from the linked list, they can not unlink themself from single models.

---------

Co-authored-by: allejok96 <allejok96@gmail.com>

Author: szeli1

data/themes/classic/edit_unlink.svg

@@ -77,8 +77,6 @@ class LMMS_EXPORT AutomatableModel : public Model, public JournallingObject
 {
 	Q_OBJECT
 public:
-	using AutoModelVector = std::vector<AutomatableModel*>;
-
 	enum class ScaleType
 	{
 		Linear,
@@ -150,22 +148,26 @@ class LMMS_EXPORT AutomatableModel : public Model, public JournallingObject
 	template<class T>
 	inline T value( int frameOffset = 0 ) const
 	{
-		if (m_controllerConnection)
+		// TODO
+		// The `m_value` should only be updated whenever the Controller value changes,
+		// instead of the Model calling `controller->currentValue()` every time.
+		// This becomes even worse in the case of linked Models, where it has to
+		// loop through the list of all links.
+
+		if (m_useControllerValue)
 		{
-			if (!m_useControllerValue)
+			if (m_controllerConnection)
 			{
-				return castValue<T>(m_value);
+				return castValue<T>(controllerValue(frameOffset));
 			}
-			else
+			for (auto next = m_nextLink; next != this; next = next->m_nextLink)
 			{
-				return castValue<T>(controllerValue(frameOffset));
+				if (next->controllerConnection() && next->useControllerValue())
+				{
+					return castValue<T>(fittedValue(next->controllerValue(frameOffset)));
+				}
 			}
 		}
-		else if (hasLinkedModels())
-		{
-			return castValue<T>( controllerValue( frameOffset ) );
-		}
-
 		return castValue<T>( m_value );
 	}
 
@@ -211,8 +213,7 @@ class LMMS_EXPORT AutomatableModel : public Model, public JournallingObject
 
 	void setInitValue( const float value );
 
-	void setAutomatedValue( const float value );
-	void setValue( const float value );
+	void setValue(const float value, const bool isAutomated = false);
 
 	void incValue( int steps )
 	{
@@ -249,11 +250,10 @@ class LMMS_EXPORT AutomatableModel : public Model, public JournallingObject
 		m_centerValue = centerVal;
 	}
 
-	//! link @p m1 and @p m2, let @p m1 take the values of @p m2
-	static void linkModels( AutomatableModel* m1, AutomatableModel* m2 );
-	static void unlinkModels( AutomatableModel* m1, AutomatableModel* m2 );
-
-	void unlinkAllModels();
+	//! link this to @p model, copying the value from @p model
+	void linkToModel(AutomatableModel* model);
+	//! @return number of other models linked to this
+	size_t countLinks() const;
 
 	/**
 	 * @brief Saves settings (value, automation links and controller connections) of AutomatableModel into
@@ -276,9 +276,9 @@ class LMMS_EXPORT AutomatableModel : public Model, public JournallingObject
 
 	virtual QString displayValue( const float val ) const = 0;
 
-	bool hasLinkedModels() const
+	bool isLinked() const
 	{
-		return !m_linkedModels.empty();
+		return m_nextLink != this;
 	}
 
 	// a way to track changed values in the model and avoid using signals/slots - useful for speed-critical code.
@@ -311,13 +311,14 @@ class LMMS_EXPORT AutomatableModel : public Model, public JournallingObject
 		s_periodCounter = 0;
 	}
 
-	bool useControllerValue()
+	bool useControllerValue() const
 	{
 		return m_useControllerValue;
 	}
 
 public slots:
 	virtual void reset();
+	void unlink();
 	void unlinkControllerConnection();
 	void setUseControllerValue(bool b = true);
 
@@ -367,9 +368,15 @@ public slots:
 		loadSettings( element, "value" );
 	}
 
-	void linkModel( AutomatableModel* model );
-	void unlinkModel( AutomatableModel* model );
+	void setValueInternal(const float value);
 
+	//! linking is stored in a linked list ring
+	//! @return the model whose `m_nextLink` is `this`,
+	//! or `this` if there are no linked models
+	AutomatableModel* getLastLinkedModel() const;
+	//! @return true if the `model` is in the linked list
+	bool isLinkedToModel(AutomatableModel* model) const;
+	
 	//! @brief Scales @value from linear to logarithmic.
 	//! Value should be within [0,1]
 	template<class T> T logToLinearScale( T value ) const;
@@ -389,16 +396,15 @@ public slots:
 	float m_centerValue;
 
 	bool m_valueChanged;
-
-	// currently unused?
-	float m_oldValue;
-	int m_setValueDepth;
+	float m_oldValue; //!< used by valueBuffer for interpolation
 
 	// used to determine if step size should be applied strictly (ie. always)
 	// or only when value set from gui (default)
 	bool m_hasStrictStepSize;
 
-	AutoModelVector m_linkedModels;
+	//! an `AutomatableModel` can be linked together with others in a linked list
+	//! the list has no end, the last model is connected to the first forming a ring
+	AutomatableModel* m_nextLink;
 
 
 	//! NULL if not appended to controller, otherwise connection info

data/themes/default/edit_unlink.svg

@@ -98,7 +98,6 @@ public slots:
 	void execConnectionDialog();
 	void removeConnection();
 	void editSongGlobalAutomation();
-	void unlinkAllModels();
 	void removeSongGlobalAutomation();
 
 private slots:

include/AutomatableModel.h

@@ -1052,8 +1052,8 @@ void ManageVestigeInstrumentView::syncPlugin( void )
 			std::snprintf(paramStr.data(), paramStr.size(), "param%d", i);
 			s_dumpValues = dump[paramStr.data()].split(":");
 			float f_value = LocaleHelper::toFloat(s_dumpValues.at(2));
-			m_vi->knobFModel[ i ]->setAutomatedValue( f_value );
-			m_vi->knobFModel[ i ]->setInitValue( f_value );
+			m_vi->knobFModel[i]->setValue(f_value, true);
+			m_vi->knobFModel[i]->setInitValue(f_value);
 		}
 	}
 	syncParameterText();

include/AutomatableModelView.h

@@ -455,8 +455,8 @@ void ManageVSTEffectView::syncPlugin()
 			std::snprintf(paramStr.data(), paramStr.size(), "param%d", i);
 			s_dumpValues = dump[paramStr.data()].split(":");
 			float f_value = LocaleHelper::toFloat(s_dumpValues.at(2));
-			m_vi2->knobFModel[ i ]->setAutomatedValue( f_value );
-			m_vi2->knobFModel[ i ]->setInitValue( f_value );
+			m_vi2->knobFModel[i]->setValue(f_value, true);
+			m_vi2->knobFModel[i]->setInitValue(f_value);
 		}
 	}
 	syncParameterText();