Fix std::vector refactor mistake (#6836)

* Use std::vector const reference instead of copying

* Add assertions

* Simplification

include/MidiClip.h

@@ -79,7 +79,7 @@ class LMMS_EXPORT MidiClip : public Clip
 	void setStep( int step, bool enabled );
 
 	// Split the list of notes on the given position
-	void splitNotes(NoteVector notes, TimePos pos);
+	void splitNotes(const NoteVector& notes, TimePos pos);
 
 	// clip-type stuff
 	inline Type type() const

include/PianoRoll.h

@@ -308,9 +308,9 @@ protected slots:
 	TimePos newNoteLen() const;
 
 	void shiftPos(int amount);
-	void shiftPos(NoteVector notes, int amount);
+	void shiftPos(const NoteVector& notes, int amount);
 	void shiftSemiTone(int amount);
-	void shiftSemiTone(NoteVector notes, int amount);
+	void shiftSemiTone(const NoteVector& notes, int amount);
 	bool isSelection() const;
 	int selectionCount() const;
 	void testPlayNote( Note * n );

src/core/EffectChain.cpp

@@ -25,6 +25,7 @@
 
 
 #include <QDomElement>
+#include <cassert>
 
 #include "EffectChain.h"
 #include "Effect.h"
@@ -162,6 +163,7 @@ void EffectChain::moveDown( Effect * _effect )
 	if (_effect != m_effects.back())
 	{
 		auto it = std::find(m_effects.begin(), m_effects.end(), _effect);
+		assert(it != m_effects.end());
 		std::swap(*std::next(it), *it);
 	}
 }
@@ -174,6 +176,7 @@ void EffectChain::moveUp( Effect * _effect )
 	if (_effect != m_effects.front())
 	{
 		auto it = std::find(m_effects.begin(), m_effects.end(), _effect);
+		assert(it != m_effects.end());
 		std::swap(*std::prev(it), *it);
 	}
 }

src/core/Mixer.cpp

@@ -394,8 +394,8 @@ void Mixer::moveChannelLeft( int index )
 	else if (m_lastSoloed == b) { m_lastSoloed = a; }
 
 	// go through every instrument and adjust for the channel index change
-	TrackContainer::TrackList songTrackList = Engine::getSong()->tracks();
-	TrackContainer::TrackList patternTrackList = Engine::patternStore()->tracks();
+	const TrackContainer::TrackList& songTrackList = Engine::getSong()->tracks();
+	const TrackContainer::TrackList& patternTrackList = Engine::patternStore()->tracks();
 
 	for (const auto& trackList : {songTrackList, patternTrackList})
 	{

src/core/PatternStore.cpp

@@ -61,7 +61,7 @@ bool PatternStore::play(TimePos start, fpp_t frames, f_cnt_t offset, int clipNum
 
 	start = start % (lengthOfPattern(clipNum) * TimePos::ticksPerBar());
 
-	TrackList tl = tracks();
+	const TrackList& tl = tracks();
 	for (Track * t : tl)
 	{
 		if (t->play(start, frames, offset, clipNum))
@@ -117,7 +117,7 @@ int PatternStore::numOfPatterns() const
 
 void PatternStore::removePattern(int pattern)
 {
-	TrackList tl = tracks();
+	const TrackList& tl = tracks();
 	for (Track * t : tl)
 	{
 		delete t->getClip(pattern);
@@ -134,7 +134,7 @@ void PatternStore::removePattern(int pattern)
 
 void PatternStore::swapPattern(int pattern1, int pattern2)
 {
-	TrackList tl = tracks();
+	const TrackList& tl = tracks();
 	for (Track * t : tl)
 	{
 		t->swapPositionOfClips(pattern1, pattern2);
@@ -159,7 +159,7 @@ void PatternStore::updatePatternTrack(Clip* clip)
 
 void PatternStore::fixIncorrectPositions()
 {
-	TrackList tl = tracks();
+	const TrackList& tl = tracks();
 	for (Track * t : tl)
 	{
 		for (int i = 0; i < numOfPatterns(); ++i)
@@ -215,7 +215,7 @@ void PatternStore::updateComboBox()
 void PatternStore::currentPatternChanged()
 {
 	// now update all track-labels (the current one has to become white, the others gray)
-	TrackList tl = Engine::getSong()->tracks();
+	const TrackList& tl = Engine::getSong()->tracks();
 	for (Track * t : tl)
 	{
 		if (t->type() == Track::Type::Pattern)
@@ -230,7 +230,7 @@ void PatternStore::currentPatternChanged()
 
 void PatternStore::createClipsForPattern(int pattern)
 {
-	TrackList tl = tracks();
+	const TrackList& tl = tracks();
 	for (Track * t : tl)
 	{
 		t->createClipsForPattern(pattern);

src/core/RenderManager.cpp

@@ -97,7 +97,7 @@ void RenderManager::renderNextTrack()
 // Render the song into individual tracks
 void RenderManager::renderTracks()
 {
-	const TrackContainer::TrackList & tl = Engine::getSong()->tracks();
+	const TrackContainer::TrackList& tl = Engine::getSong()->tracks();
 
 	// find all currently unnmuted tracks -- we want to render these.
 	for (const auto& tk : tl)
@@ -112,7 +112,7 @@ void RenderManager::renderTracks()
 		}
 	}
 
-	const TrackContainer::TrackList t2 = Engine::patternStore()->tracks();
+	const TrackContainer::TrackList& t2 = Engine::patternStore()->tracks();
 	for (const auto& tk : t2)
 	{
 		Track::Type type = tk->type();

src/core/Song.cpp

@@ -383,7 +383,7 @@ void Song::processAutomations(const TrackList &tracklist, TimePos timeStart, fpp
 	}
 
 	values = container->automatedValuesAt(timeStart, clipNum);
-	TrackList tracks = container->tracks();
+	const TrackList& tracks = container->tracks();
 
 	Track::clipVector clips;
 	for (Track* track : tracks)

src/gui/MixerView.cpp

@@ -248,8 +248,8 @@ void MixerView::refreshDisplay()
 // update the and max. channel number for every instrument
 void MixerView::updateMaxChannelSelector()
 {
-	TrackContainer::TrackList songTracks = Engine::getSong()->tracks();
-	TrackContainer::TrackList patternStoreTracks = Engine::patternStore()->tracks();
+	const TrackContainer::TrackList& songTracks = Engine::getSong()->tracks();
+	const TrackContainer::TrackList& patternStoreTracks = Engine::patternStore()->tracks();
 
 	for (const auto& trackList : {songTracks, patternStoreTracks})
 	{

src/gui/clips/ClipView.cpp

@@ -25,6 +25,7 @@
 #include "ClipView.h"
 
 #include <set>
+#include <cassert>
 
 #include <QMenu>
 #include <QMouseEvent>
@@ -545,6 +546,7 @@ DataFile ClipView::createClipDataFiles(
 		// Insert into the dom under the "clips" element
 		Track* clipTrack = clipView->m_trackView->getTrack();
 		int trackIndex = std::distance(tc->tracks().begin(), std::find(tc->tracks().begin(), tc->tracks().end(), clipTrack));
+		assert(trackIndex != tc->tracks().size());
 		QDomElement clipElement = dataFile.createElement("clip");
 		clipElement.setAttribute( "trackIndex", trackIndex );
 		clipElement.setAttribute( "trackType", static_cast<int>(clipTrack->type()) );
@@ -1308,7 +1310,7 @@ void ClipView::mergeClips(QVector<ClipView*> clipvs)
 			continue;
 		}
 
-		NoteVector currentClipNotes = mcView->getMidiClip()->notes();
+		const NoteVector& currentClipNotes = mcView->getMidiClip()->notes();
 		TimePos mcViewPos = mcView->getMidiClip()->startPosition();
 
 		for (Note* note: currentClipNotes)

src/gui/editors/PatternEditor.cpp

@@ -69,7 +69,7 @@ void PatternEditor::cloneSteps()
 
 void PatternEditor::removeSteps()
 {
-	TrackContainer::TrackList tl = model()->tracks();
+	const TrackContainer::TrackList& tl = model()->tracks();
 
 	for (const auto& track : tl)
 	{
@@ -176,7 +176,7 @@ void PatternEditor::updatePosition()
 
 void PatternEditor::makeSteps( bool clone )
 {
-	TrackContainer::TrackList tl = model()->tracks();
+	const TrackContainer::TrackList& tl = model()->tracks();
 
 	for (const auto& track : tl)
 	{

src/gui/editors/PianoRoll.cpp

@@ -741,10 +741,10 @@ void PianoRoll::fitNoteLengths(bool fill)
 {
 	if (!hasValidMidiClip()) { return; }
 	m_midiClip->addJournalCheckPoint();
+	m_midiClip->rearrangeAllNotes();
 
 	// Reference notes
-	NoteVector refNotes = m_midiClip->notes();
-	std::sort(refNotes.begin(), refNotes.end(), Note::lessThan);
+	const NoteVector& refNotes = m_midiClip->notes();
 
 	// Notes to edit
 	NoteVector notes = getSelectedNotes();
@@ -762,7 +762,7 @@ void PianoRoll::fitNoteLengths(bool fill)
 	}
 
 	int length;
-	NoteVector::iterator ref = refNotes.begin();
+	auto ref = refNotes.begin();
 	for (Note* note : notes)
 	{
 		// Fast forward to next reference note
@@ -797,14 +797,11 @@ void PianoRoll::constrainNoteLengths(bool constrainMax)
 	if (!hasValidMidiClip()) { return; }
 	m_midiClip->addJournalCheckPoint();
 
-	NoteVector notes = getSelectedNotes();
-	if (notes.empty())
-	{
-		notes = m_midiClip->notes();
-	}
+	const NoteVector selectedNotes = getSelectedNotes();
+	const auto& notes = selectedNotes.empty() ? m_midiClip->notes() : selectedNotes;
 
-	TimePos bound = m_lenOfNewNotes;  // will be length of last note
-	for (Note* note : notes)
+	TimePos bound = m_lenOfNewNotes; // will be length of last note
+	for (auto note : notes)
 	{
 		if (constrainMax ? note->length() > bound : note->length() < bound)
 		{
@@ -1207,11 +1204,11 @@ void PianoRoll::shiftSemiTone(int amount) //Shift notes by amount semitones
 
 	auto selectedNotes = getSelectedNotes();
 	//If no notes are selected, shift all of them, otherwise shift selection
-	if (selectedNotes.empty()) { return shiftSemiTone(m_midiClip->notes(), amount); }
-	else { return shiftSemiTone(selectedNotes, amount); }
+	if (selectedNotes.empty()) { shiftSemiTone(m_midiClip->notes(), amount); }
+	else { shiftSemiTone(selectedNotes, amount); }
 }
 
-void PianoRoll::shiftSemiTone(NoteVector notes, int amount)
+void PianoRoll::shiftSemiTone(const NoteVector& notes, int amount)
 {
 	m_midiClip->addJournalCheckPoint();
 	for (Note *note : notes) { note->setKey( note->key() + amount ); }
@@ -1232,11 +1229,11 @@ void PianoRoll::shiftPos(int amount) //Shift notes pos by amount
 
 	auto selectedNotes = getSelectedNotes();
 	//If no notes are selected, shift all of them, otherwise shift selection
-	if (selectedNotes.empty()) { return shiftPos(m_midiClip->notes(), amount); }
-	else { return shiftPos(selectedNotes, amount); }
+	if (selectedNotes.empty()) { shiftPos(m_midiClip->notes(), amount); }
+	else { shiftPos(selectedNotes, amount); }
 }
 
-void PianoRoll::shiftPos(NoteVector notes, int amount)
+void PianoRoll::shiftPos(const NoteVector& notes, int amount)
 {
 	m_midiClip->addJournalCheckPoint();
 

src/gui/tracks/TrackContentWidget.cpp

@@ -344,7 +344,7 @@ bool TrackContentWidget::canPasteSelection( TimePos clipPos, const QMimeData* md
 	const int initialTrackIndex = tiAttr.value().toInt();
 
 	// Get the current track's index
-	const TrackContainer::TrackList tracks = t->trackContainer()->tracks();
+	const TrackContainer::TrackList& tracks = t->trackContainer()->tracks();
 	const auto currentTrackIt = std::find(tracks.begin(), tracks.end(), t);
 	const int currentTrackIndex = currentTrackIt != tracks.end() ? std::distance(tracks.begin(), currentTrackIt) : -1;
 
@@ -443,7 +443,7 @@ bool TrackContentWidget::pasteSelection( TimePos clipPos, const QMimeData * md,
 	TimePos grabbedClipPos = clipPosAttr.value().toInt();
 
 	// Snap the mouse position to the beginning of the dropped bar, in ticks
-	const TrackContainer::TrackList tracks = getTrack()->trackContainer()->tracks();
+	const TrackContainer::TrackList& tracks = getTrack()->trackContainer()->tracks();
 	const auto currentTrackIt = std::find(tracks.begin(), tracks.end(), getTrack());
 	const int currentTrackIndex = currentTrackIt != tracks.end() ? std::distance(tracks.begin(), currentTrackIt) : -1;
 

src/tracks/MidiClip.cpp

@@ -305,7 +305,7 @@ void MidiClip::setStep( int step, bool enabled )
 
 
 
-void MidiClip::splitNotes(NoteVector notes, TimePos pos)
+void MidiClip::splitNotes(const NoteVector& notes, TimePos pos)
 {
 	if (notes.empty()) { return; }
 
@@ -472,7 +472,7 @@ MidiClip *  MidiClip::nextMidiClip() const
 
 MidiClip * MidiClip::adjacentMidiClipByOffset(int offset) const
 {
-	std::vector<Clip *> clips = m_instrumentTrack->getClips();
+	auto& clips = m_instrumentTrack->getClips();
 	int clipNum = m_instrumentTrack->getClipNum(this);
 	if (clipNum < 0 || clipNum > clips.size() - 1) { return nullptr; }
 	return dynamic_cast<MidiClip*>(clips[clipNum + offset]);