Note sorting algorithm rework

[zonkmachine]

Addresses some of the issues in #3880

Sorting after position and key. Probably only needed with the arpeggiator. This works well and doesn't seem to use a whole lot cpu cycles. The notes are sorted on input from mouse or keyboard in Pattern::addNote(). Actions on the notes in the Piano Roll also triggers a sort via Pattern::rearrangeAllNotes() with a helper function in note.h . The sorting is duplicated when adding notes in the Piano Roll and this has its own post in #3880 but is probably a wontfix.

I've pulled these against stable-1.2 but they can just as well go into master or be split over the two.

[zonkmachine]

Note that old patches will open up like before and all will be fine. The next time you edit a track however it will be sorted with the new method and this will probably be only an advantage. Probably. Maybe you like the notes to shift around a bit, and you'll only really notice this in arpeggiated tracks with more than two simultaneous notes and under sort mode.
If this is not wanted you can just open/press a note in the piano roll to trigger a sort/save the project in 1.1.3 and it'll go wonky again.

[zonkmachine]

Got a random crash. I don't think it's related to this PR though. The project had been running for maybe an hour or so. I was looping two 3osc tracks with an automation track for the Master pitch, two notes detuned in the one track and an LFO detuning the other synth.
m_frequencyNeedsUpdate = true; LFO ... or Note detune ... or Master pitch?

bt / bt full

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000062e0ea in NotePlayHandle::setFrequencyUpdate (this=0x81) at /home/zonkmachine/builds/lmms/lmms/include/NotePlayHandle.h:263
263			m_frequencyNeedsUpdate = true;
(gdb) bt
#0  0x000000000062e0ea in NotePlayHandle::setFrequencyUpdate (this=0x81) at /home/zonkmachine/builds/lmms/lmms/include/NotePlayHandle.h:263
#1  0x00000000006270a1 in InstrumentTrack::updateBaseNote (this=0x7fce29e77620) at /home/zonkmachine/builds/lmms/lmms/src/tracks/InstrumentTrack.cpp:543
#2  0x000000000062710a in InstrumentTrack::updatePitch (this=0x7fce29e77620) at /home/zonkmachine/builds/lmms/lmms/src/tracks/InstrumentTrack.cpp:552
#3  0x000000000063f89f in InstrumentTrack::qt_static_metacall (_o=0x7fce29e77620, _c=QMetaObject::InvokeMetaMethod, _id=6, _a=0x7fffe6625550)
    at /home/zonkmachine/builds/lmms/lmms/build/src/moc_InstrumentTrack.cpp:68
#4  0x00007fce35cf187a in QMetaObject::activate (sender=0x7fce29e78898, m=<optimized out>, local_signal_index=<optimized out>, argv=0x0)
    at kernel/qobject.cpp:3539
#5  0x0000000000642f4b in Model::dataChanged (this=0x7fce29e78898) at /home/zonkmachine/builds/lmms/lmms/build/src/moc_Model.cpp:102
#6  0x0000000000642e09 in Model::qt_static_metacall (_o=0x7fce29e78898, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0x7fffe66256b0)
    at /home/zonkmachine/builds/lmms/lmms/build/src/moc_Model.cpp:51
#7  0x00007fce35cf187a in QMetaObject::activate (sender=0x2ee0210, m=<optimized out>, local_signal_index=<optimized out>, argv=0x0)
    at kernel/qobject.cpp:3539
#8  0x000000000063b4ab in ControllerConnection::valueChanged (this=0x2ee0210)
    at /home/zonkmachine/builds/lmms/lmms/build/src/moc_ControllerConnection.cpp:104
#9  0x000000000063b354 in ControllerConnection::qt_static_metacall (_o=0x2ee0210, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0x7fcde40521b0)
    at /home/zonkmachine/builds/lmms/lmms/build/src/moc_ControllerConnection.cpp:52

[PhysSong]

NotePlayHandle::setFrequencyUpdate (this=0x81) was called in

lmms/src/tracks/InstrumentTrack.cpp

Line 543 in 54f3ecc

( *it )->setFrequencyUpdate();

and it may indicate an invalid reference of iterator occured.

[PhysSong]

m_processHandles is used in non-thread-safe way(i.e. without proper synchronization). (this=0x81) means *it == 0x81, which is an invalid address.
Anyway, I'm pretty sure it's unrelated with this PR. For this kind of crashes, I'll open a separate issue. :)

[gi0e5b06]

#3879 seems related to this NotePlayHandle race condition.

[zonkmachine]

This should go into master. I'll switch it over.

[zonkmachine]

I switched back to stable 1.2 and this is ready for a review. Switching sorting order was a matter of flipping over two 'lesser than' signs and this is now less buggy than before. See the note on backward compatibility here.

[lukas-w]

As far as I understand the code, all lines 216 to 245 do is insert the new note at the right position (sorted by the criteria in lessThan). If so, we can reduce this to a single line:

m_notes.insert(std::upper_bound(m_notes.begin(), m_notes.end(), new_note, Note:lessThan), new_note);

This reduces code complexity, improves performance by doing a binary search, and removes the duplicate compare code by using the lessThan method.

[zonkmachine]

As far as I understand the code, all lines 216 to 245 do is insert the new note at the right position (sorted by the criteria in lessThan).

Yes indeed. Oneliner, nice but I can't make it work.

Computer says no. 8)

/usr/include/c++/4.8/bits/stl_algo.h: In instantiation of ‘_FIter std::upper_bound(_FIter, _FIter, const _Tp&, _Compare) [with _FIter = Note**; _Tp = Note*; _Compare = bool (*)(Note*&, Note*&)]’:
/home/zonkmachine/builds/lmms/lmms/src/tracks/Pattern.cpp:216:90:   required from here
/usr/include/c++/4.8/bits/stl_algo.h:2543:31: error: invalid initialization of reference of type ‘Note*&’ from expression of type ‘Note* const’
    if (__comp(__val, *__middle))
                               ^

[lukas-w]

Ah, you'll have to change Note::lessThan to take const pointers:

static inline bool lessThan( const Note * lhs, const Note * rhs )

[zonkmachine]

Perfect, thanks!

[zonkmachine]

I took the opportunity to practice and updated the other sorting function to std::sort() as qSort() is obsolete.

Now I think I'm done. The notes are correctly sorted for both functions and there is no code changes needed to save/load the patterns correctly.