clearly-broken-software / ninjas2

Rewrite of Ninjas sample slicer
GNU General Public License v3.0
179 stars 13 forks source link

incorrect state restore and gfx bugs in LMMS #106

Open rghvdberg opened 4 years ago

rghvdberg commented 4 years ago

via discord bug in Win10 64bit LMMS 1.2

if you load a sample, slice it and use it, when you reload the project, some parts have reverse loop activated instead of normal playing of the sample. Also, if you have more than a ninjas 2 in your project usually only 1 display the sample sliced the otherones just give you a grey screen where it should be. Also i'm not sure because as i said i can't see the sample sliced, i feel like if i do a precision slice moving the vertica lines, i suspect they reset themselves

DomClark commented 3 years ago

when you reload the project, some parts have reverse loop activated instead of normal playing of the sample

DPF's VST2 wrapper loads states first, followed by parameters. The correct play mode is set when loading a program's state. However, Ninjas 2 has four parameters for play modes which are trigger parameters - setting these to some value doesn't change the play mode to that value, but changes the play mode to that associated with the parameter. Thus, when the parameters are loaded, the play mode of the currently selected slice is set to reverse loop, the last trigger parameter to be loaded. DPF ignores trigger parameters when restoring plugin state, but Ninjas 2's play mode parameters aren't flagged as such - paramOneShotForward, paramOneShotReverse, paramLoopForward, and paramLoopReverse should have their hints changed from kParameterIsBoolean to kParameterIsTrigger in NinjasPlugin::initParameter.

if you have more than a ninjas 2 in your project usually only 1 display the sample sliced the otherones just give you a grey screen where it should be

This isn't actually related to the number of Ninjas 2s in the project (although the bug is more likely to appear at all with more instances, of course). DPF's VST2 wrapper loads states in the order they're found in the VST data chunk. However, the order in which they're stored is not deterministic. DPF first puts the states in a map from state name to state value, then iterates over this map to serialise the states into a chunk. The map is a std::map<const DISTRHO::String, DISTRHO::String>; std::map is a sorted container, and by default uses operator< to sort its keys, but DISTRHO::String has no such operator. However, DISTRHO::String defines an implicit conversion to const char *, for which there is a built-in operator<, so the conversion is used and the strings are ordered by the location of their buffers in memory. This, of course, varies from run to run. Ninjas 2 has a flag sig_SampleLoaded in NinjasPlugin, which is communicated to the UI through a parameter and triggers regeneration of the waveform display. This is set when loading the filepathFromState and filepathFromUI states, and cleared when loading the sig_SampleLoaded state. If this last state happens to come later in the chunk than the other two, then the flag will be clear when all states have been loaded and the waveform will not be generated.

Another bug I came across (related to the previous one, but not mentioned in the original report) is that sometimes when restoring state, all the slices will be missing, with just one default slice stretching across the whole sample. This is because the filepathFromUI state clears all programs when loaded, so if a program comes before it in the chunk, that program will be overwritten by a default program on load.

rghvdberg commented 3 years ago

thanks you so much for investigating this let's try the trigger first :-)

pinging @falkTX because this might interest him