Don't copy DetuningHelper in Note copy constructor

include/InlineAutomation.h

@@ -27,7 +27,6 @@
 
 #include "AutomationNode.h"
 #include "AutomationClip.h"
-#include "shared_object.h"
 
 namespace lmms
 {

include/Note.h

@@ -104,12 +104,15 @@ class LMMS_EXPORT Note : public SerializingObject
 		int key = DefaultKey,
 		volume_t volume = DefaultVolume,
 		panning_t panning = DefaultPanning,
-		DetuningHelper * detuning = nullptr );
+		std::shared_ptr<DetuningHelper> detuning = nullptr);
 	Note( const Note & note );
 	~Note() override;
 
 	Note& operator=(const Note& note);
 
+	//! Performs a deep copy and returns an owning raw pointer
+	Note* clone() const;
+
 	// Note types
 	enum class Type
 	{
@@ -237,10 +240,8 @@ class LMMS_EXPORT Note : public SerializingObject
 
 	static TimePos quantized( const TimePos & m, const int qGrid );
 
-	DetuningHelper * detuning() const
-	{
-		return m_detuning.get();
-	}
+	const std::shared_ptr<DetuningHelper>& detuning() const { return m_detuning; }
+
 	bool hasDetuningInfo() const;
 	bool withinRange(int tickStart, int tickEnd) const;
 
@@ -265,7 +266,7 @@ class LMMS_EXPORT Note : public SerializingObject
 	panning_t m_panning;
 	TimePos m_length;
 	TimePos m_pos;
-	std::unique_ptr<DetuningHelper> m_detuning;
+	std::shared_ptr<DetuningHelper> m_detuning;
 
 	Type m_type = Type::Regular;
 };

src/core/Note.cpp

@@ -36,7 +36,7 @@ namespace lmms
 
 Note::Note( const TimePos & length, const TimePos & pos,
 		int key, volume_t volume, panning_t panning,
-						DetuningHelper * detuning ) :
+						std::shared_ptr<DetuningHelper> detuning ) :
 	m_selected( false ),
 	m_oldKey(std::clamp(key, 0, NumKeys)),
 	m_oldPos( pos ),
@@ -46,13 +46,10 @@ Note::Note( const TimePos & length, const TimePos & pos,
 	m_volume(std::clamp(volume, MinVolume, MaxVolume)),
 	m_panning(std::clamp(panning, PanningLeft, PanningRight)),
 	m_length( length ),
-	m_pos( pos )
+	m_pos(pos),
+	m_detuning(std::move(detuning))
 {
-	if (detuning)
-	{
-		m_detuning = std::make_unique<DetuningHelper>(*detuning);
-	}
-	else
+	if (!detuning)
 	{
 		createDetuning();
 	}
@@ -73,12 +70,9 @@ Note::Note( const Note & note ) :
 	m_panning( note.m_panning ),
 	m_length( note.m_length ),
 	m_pos( note.m_pos ),
+	m_detuning(note.m_detuning),
 	m_type(note.m_type)
 {
-	if (note.m_detuning)
-	{
-		m_detuning = std::make_unique<DetuningHelper>(*note.m_detuning);
-	}
 }
 
 Note& Note::operator=(const Note& note)
@@ -94,16 +88,19 @@ Note& Note::operator=(const Note& note)
 	m_length = note.m_length;
 	m_pos = note.m_pos;
 	m_type = note.m_type;
-
-	if (note.m_detuning)
-	{
-		m_detuning = std::make_unique<DetuningHelper>(*note.m_detuning);
-	}
+	m_detuning = note.m_detuning;
 
 	return *this;
 }
 
 
+Note* Note::clone() const
+{
+	Note* newNote = new Note(*this);
+	newNote->m_detuning = std::make_shared<DetuningHelper>(*newNote->m_detuning);
+	return newNote;
+}
+
 
 
 Note::~Note()
@@ -234,7 +231,7 @@ void Note::createDetuning()
 {
 	if( m_detuning == nullptr )
 	{
-		m_detuning = std::make_unique<DetuningHelper>();
+		m_detuning = std::make_shared<DetuningHelper>();
 		(void) m_detuning->automationClip();
 		m_detuning->setRange( -MaxDetuning, MaxDetuning, 0.5f );
 		m_detuning->automationClip()->setProgressionType( AutomationClip::ProgressionType::Linear );

src/core/NotePlayHandle.cpp

@@ -54,7 +54,7 @@ NotePlayHandle::NotePlayHandle( InstrumentTrack* instrumentTrack,
 								int midiEventChannel,
 								Origin origin ) :
 	PlayHandle( PlayHandle::Type::NotePlayHandle, _offset ),
-	Note( n.length(), n.pos(), n.key(), n.getVolume(), n.getPanning(), n.detuning() ),
+	Note(n),
 	m_pluginData( nullptr ),
 	m_instrumentTrack( instrumentTrack ),
 	m_frames( 0 ),
@@ -84,7 +84,7 @@ NotePlayHandle::NotePlayHandle( InstrumentTrack* instrumentTrack,
 	lock();
 	if( hasParent() == false )
 	{
-		m_baseDetuning = new BaseDetuning( detuning() );
+		m_baseDetuning = new BaseDetuning(detuning().get());
 		m_instrumentTrack->m_processHandles.push_back( this );
 	}
 	else

src/tracks/MidiClip.cpp

@@ -68,7 +68,7 @@ MidiClip::MidiClip( const MidiClip& other ) :
 {
 	for (const auto& note : other.m_notes)
 	{
-		m_notes.push_back(new Note(*note));
+		m_notes.push_back(note->clone());
 	}
 
 	init();
@@ -197,7 +197,7 @@ TimePos MidiClip::beatClipLength() const
 
 Note * MidiClip::addNote( const Note & _new_note, const bool _quant_pos )
 {
-	auto new_note = new Note(_new_note);
+	auto new_note = _new_note.clone();
 	if (_quant_pos && gui::getGUI()->pianoRoll())
 	{
 		new_note->quantizePos(gui::getGUI()->pianoRoll()->quantization());

tests/src/tracks/AutomationTrackTest.cpp

@@ -163,8 +163,7 @@ private slots:
 		Note* note = midiClip.addNote(Note(TimePos(4, 0)), false);
 		note->createDetuning();
 
-		DetuningHelper* dh = note->detuning();
-		auto clip = dh->automationClip();
+		auto clip = note->detuning()->automationClip();
 		clip->setProgressionType( AutomationClip::ProgressionType::Linear );
 		clip->putValue(TimePos(0, 0), 0.0);
 		clip->putValue(TimePos(4, 0), 1.0);