Lmms: Clone Track Bug: Misplaces notes on GUI + Instruments Unusable in a Beat/Baseline

[som1chan]

_Edited, @Sawuare, fixed steps to reproduce._

I tried to make this as quick as possible, so I didn't experiment too much, but here it is.

1. Create a VeSTige track in a BB, and give it a VST (I chose the VSCO Community Sketch Piano for my test). The track you create should be the first one.
2. Put TCOs of the BB in the song editor.
3. Play the song, and while the BB TCO is being played, clone the track in the BB. This will cause the bug.
4. Create a new BB, and you will get a blank space where you cannot input any notes, until you restart.

This bug makes two things happen. The first would be that it would render editing the steps or piano roll impossible, because the positions between the thing. The second is that if you create a new Beat/Baseline, you won't be able to use any of those instruments, unless you reload and this will fix this all from my testing.

Quick video demonstrating some weird effects while it's active. https://streamable.com/r8vhk

I tried replicating this with the Kicker, but I didn't get it to work.

Sorry if this was poorly made, I was in a rush and this is my first time dealing with such things.

[Sawuare]

The steps to reproduce you provided don't work and they don't match what you demonstrated in the video.
However, I'm able to reproduce what happens in the video and I fixed your steps to reproduce.

This is a VST-specific bug. CC @DomClark

[DomClark]

I can reproduce this too. It looks similar to #3598 visually, perhaps they are related? This doesn't seem like a bug in the VST interface itself though, more like there's some special case regarding VeSTige within LMMS, so it's not necessarily my "area of speciality" so to speak, but I can take a look when I have time.

[DomClark]

The reason this happens with VST plugins only is that they process events while loading. The following sequence of events seems to be what happens:

• Track::create calls Track::createTCOsForBB to create a TCO for the cloned track for BB0
• Track::loadSettings deletes the TCO, then calls loadTrackSpecificSettings which in InstrumentTrack loads the instrument
• Loading the VST instrument calls QCoreApplication::processEvents while waiting for a response from the remote plugin
• A paint event is fired which is processed during the wait
• BBTCOView::paintEvent needs to know the length of the BB track, so calls BBTrackContainer::lengthOfBB which iterates through the list of instruments to find the maximum length of a TCO for BB0
• The cloned track does not currently have a TCO for BB0, so Track::getTCO creates one
• Now the plugin is loaded, so the cloned track loading continues
• The TCO for BB0 from the original track is cloned, now the cloned track has two TCOs for BB0
• The newly created TCO calls Pattern::resizeToFirstTrack, and being the second TCO in the cloned track, it looks for the length of the second TCO in the original track
• The original track does not have a second TCO, so Track::getTCO creates one
• Now both VeSTige tracks have two TCOs for BB0
• When BB1 is created, no TCOs are created for it as both tracks already have two TCOs
• TrackContentWidget::changePosition hides both TCOs for both tracks as they are all for BB0

I'm not sure what the best approach is here to fix this; perhaps load VSTs in a new thread so we don't need to process events? This might also improve project loading speed with many VSTs and keep LMMS responsive while VSTs are being loaded. Does anyone else have any ideas?

[musikBear]

..Idk
Should the operation
" Dragging copies in song-editor during replay "
Be possible?
Imo it would be valid to stop transport, as dragging on a instrument-button takes place. The play-head should be parked in current position, allowing the user to continue replay from that position, after he has done the drag or clone event.

[PhysSong]

@DomClark Thank you for the investigation! I'll look into this while fixing #3598.

[PhysSong]

It is the simplest way to make BBTrackContainer::lengthOfBB don't create TCOs.
It is also possible to change getTCO to return nullptr when the TCO we're looking for doesn't exist. However, this may cause crashes if something goes wrong, and thus not suitable for stable-1.2.
I'll add a fix for this before the 1.2.0 release.