search

LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
7.98k stars 995 forks source link

Piano views don't visualize playing of successive same-key notes. #3919

Open Sawuare opened 6 years ago

Sawuare commented 6 years ago

wow

qnebra commented 6 years ago

It looks like minor overseight.

PhysSong commented 6 years ago

Does it happen on piano views on instrument windows?

Sawuare commented 6 years ago

Does it happen on piano views on instrument windows?

Yes.

musikBear commented 6 years ago

@Sawuare if you magnify to max, are the notes

-or are they

Sawuare commented 6 years ago

@musikBear The notes are snapped.

PhysSong commented 6 years ago

I think it's not hard to fix. I think it would be work correctly if we set the state based on the number of running keys instead of MIDI events.

DomClark commented 6 years ago

It seems to be sufficient just to move m_piano.setKeyState into the if statement here: https://github.com/LMMS/lmms/blob/0f3b41f590ed8d573ce3891f6502f211d1be176a/src/tracks/InstrumentTrack.cpp#L411-L420

Sawuare commented 4 years ago

Seems fixed on master and 1.2.0.

zonkmachine commented 4 years ago

This was fixed by @DomClark in https://github.com/LMMS/lmms/pull/4908.

DomClark commented 4 years ago

The bug certainly seems to have disappeared, but I'd hesitate to call that PR a proper fix for this issue.

Prior to it, note-on messages were sent when NotePlayHandles were constructed during track processing, and then cancelled out by note-off messages sent from old NotePlayHandles during play handle processing. Now that the note-on messages are also sent during play handle processing, they don't get cancelled out since the new NotePlayHandles get put at the end of the list of play handles to process. However, the processing of play handles is handled by multiple threads, so it is technically possible for one of these later handles to send its note-on before the note-off of a previous handle, which would trigger the bug again.

Since this doesn't seem to happen in practise, and this is only a visual bug, I'm happy for this to stay closed, but anyone wanting to fix this properly should look at the suggestions mentioned earlier in this issue.

Sawuare commented 3 years ago

Reopening because #6025 recently reported what I think is the same bug.