You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, PianoRoll::getSelectedNotes loops over all notes to find the ones where isSelected is true, adds them to a NoteVector, and returns that. This is done every time the function is called, which is not efficient. @szeli1 brought this to my attention in #7759.
This is mostly a TODO issue to remind me to do this, but if anyone else wants to try to tackle it, feel free!
Implementation Ideas
The selected notes should probably be cached in some kind of vector.
Since the piano roll is afaik the only class which actually interacts with the isSelected state of the notes, perhaps for all the occurances of a note being selected/deslected, we could add a line of code which adds/removes that note from the vector. We would have to be very careful to address all the occurances, and also repopulate the vector whenever a new clip is opened in the piano roll (Since selections of notes persist within each MidiClip; selecting some notes in one clip, opening another clip, and opening the old clip will retain the selection).
Or, we could implement that logic within Note::setSelected. However, this would probably require either giving Note access to the piano roll or having some kind of static vector, which may not be desirable.
The text was updated successfully, but these errors were encountered:
In places where we only need to loop over certain elements that meet some condition and don't need to store the results, we could make use of C++20 ranges, specifically std::ranges::filter_view.
I recently used it with success to remove some dynamic allocations in real-time code here: messmerd@f2637d4
Maybe I could refactor PianoRoll::getSelectedNotes as well, then open a PR.
This is done every time the function is called, which is not efficient.
That is of cause true, but it is also not in a efficiency-critical situation, because it does not happen during playback. What i mean is that this is not broken, maybe a bit clonky but only under the 'hood' and it works.
There are a lot that is missing where some are crippling and severe. One such example could be "Cut-your-self" for overlapping notes, a feature-miss that causes disharmony and make it difficult to get clean mixes. Eg important features that is missing, and make LMMS perform inferior compared to other DAWs.
Imho it would be better to spend dev-time on issues like thát and leave non-critical efficiency features that also works fine, until the 2.0 total rewrite.
Well just imo.
Currently,
PianoRoll::getSelectedNotes
loops over all notes to find the ones whereisSelected
is true, adds them to aNoteVector
, and returns that. This is done every time the function is called, which is not efficient. @szeli1 brought this to my attention in #7759.This is mostly a TODO issue to remind me to do this, but if anyone else wants to try to tackle it, feel free!
Implementation Ideas
isSelected
state of the notes, perhaps for all the occurances of a note being selected/deslected, we could add a line of code which adds/removes that note from the vector. We would have to be very careful to address all the occurances, and also repopulate the vector whenever a new clip is opened in the piano roll (Since selections of notes persist within eachMidiClip
; selecting some notes in one clip, opening another clip, and opening the old clip will retain the selection).Note::setSelected
. However, this would probably require either givingNote
access to the piano roll or having some kind of static vector, which may not be desirable.The text was updated successfully, but these errors were encountered: