Lmms: Support various behaviors when increasing sample length after drawing a waveform in BitInvader

[Sawuare]

To re-produce:

1. Load a default BitInvader.
2. Decrease the sample length (e.g., to 64).
3. Draw a waveform or click any waveform button (e.g., sine wave button).
4. Increase the sample length (e.g., to 128).

The odd behavior:

The expected behavior:

[PhysSong]

Introduced in #1362, from changes in Graph.cpp. After that PR, the waveform is preserved on resizing, but doesn't clear its invisible "tail" part on waveform change.
See differences for details.

[PhysSong]

Before we fix this, we should discuss what is the desired behavior. I think the second picture is what the BitInvader supposed to behave. Since the second behavior was in 1.1 and there were no related bug reports as far as I know. And, current behavior is very weird. :-1:
However someone might think other waveform(such as two periods of sine wave) should be there. So we need to discuss.

[musikBear]

someone might think other waveform(such as two periods of sine wave) should be there

@PhysSong -Yes, periodic repeats would imo be the solution to go for. The reason would be, that perfect periodic repeats are very difficult to draw manually, whereas a simple strait line (img 2) is very easy to make manually, so imo: let the computer do the difficult thing.

[Sawuare]

Periodically repeating a wave is just increasing its frequency/pitch...
IMO, this is useless, as there is already a pitch parameter.

perfect periodic repeats are very difficult to draw manually, whereas a simple strait line (img 2) is very easy to make manually

Drawing a straight line at a level of zero (silence) is hard.

IMO, adding silence when increasing sample length is the best behavior.

[DouglasDGI]

In #4705 I made it so changing the sample length changes the sound realtime. I think the current graph behavior should be preserved, so that users may be able to move the sample length knob to change their sound in interesting ways.

If we instead make it so anything off the edge of the graph is changed to silence, then the graph would have to be redrawn whenever the sample length knob is automated, which I don't think is the wanted behavior. It would also render #4705 pointless.

One or the other would have to be chosen, and I think #4705 would be the wanted change, since #4705 adds sound design capabilities and #3797 removes some slightly. What do you all think?

[zonkmachine]

If we instead make it so anything off the edge of the graph is changed to silence, then the graph would have to be redrawn whenever the sample length knob is automated, which I don't think is the wanted behavior. It would also render #4705 pointless.

Can't we add a button to toggle the 'clearing of the end' on/off?

[DomClark]

Perhaps we could keep the current behaviour and add a "flat line" waveform, for a more consistent way to achieve both results.

[musikBear]

I like Zonks idea
Besides This i still think is more difficult than a strait line

[musikBear]

Bahh picture not showing :?

[PhysSong]

Suppose I'm doing this:

1. Increase the length to 192
2. Set the waveform to sine wave
3. Decrease the length to 128
4. Set the waveform to sine wave
5. Increase the length to 192 again

Then I can think of those three behaviors:

|1.2.0-RC7|1.1.3|Alternative|
|---|---|---|
||||

[PhysSong]

If we instead make it so anything off the edge of the graph is changed to silence, then the graph would have to be redrawn whenever the sample length knob is automated, which I don't think is the wanted behavior.

@DouglasDGI It doesn't affect changing the length of the graph. Instead, it affects how the 'invisible' part changes when selecting a waveform.
Also, the graph will be redrawn when automating the length, regardless of the tail's behavior.

[DouglasDGI]

Oh, so you're talking about the changes that are made when a waveform is selected, not when the length is changed. Gotcha.

In that case, I honestly prefer the 1.2 one since it makes it a bit easier to make smoothly-curved waveforms (by decreasing the length, choosing another waveform, repeat), but the 1.1.3 one would probably be more expected.

[DouglasDGI]

Whatever the final decision is, I'd probably be willing to code it. I don't think it would be too difficult.

[jasp00]

If the three behaviors PhysSong has represented are desired, all of them should be available as a "resize behavior" input.

[jasp00]

Please let us move on; I will propose a decision. The "flat line" button (https://github.com/LMMS/lmms/issues/3797#issuecomment-454145152) and behavior "Alternative" (https://github.com/LMMS/lmms/issues/3797#issuecomment-454657241) should be discarded, since they are new features that could be added later. Behavior "1.1.3" should be restored and be the default. If DouglasDGI thinks behavior "1.2.0-RC7" is useful, he should be the coder and add a button that enables this behavior; extra points for coding upgrade steps.

[DouglasDGI]

@jasp00

extra points for coding upgrade steps.

What do you mean by this?

Also, instead of a button I could use a simple dropdown box (like the one in AudioFileProcessor) to choose between the modes, so there isn't any hassle if we want more than just two behaviors implemented in the future.

[jasp00]

What do you mean by this?

Regarding upgrade steps, src/core/DataFile.cpp is in charge of upgrading old projects. You should add an upgrade step for projects before 1.2.0-rc7 and another one for 1.2.0-rc7.

instead of a button I could use a simple dropdown box

With two options only, a button would be less intrusive. However, use your judgment.

[PhysSong]

@DouglasDGI Do you still want to work on this? For 1.2, I think restoring 1.1's behavior is fine. Then we need to change graphModel::setWaveToSine and its siblings.
Also, I bumped this issue to 1.2.0. If this one blocks releasing 1.2.0, please feel free to bump it to 1.2.1 or later.

[DouglasDGI]

@PhysSong I'll see if I can finish it today.

Then we need to change graphModel::setWaveToSine and its siblings.

I'd rather avoid doing that, as that would require a loss of off-graph information for any synthesizers that need to keep the off-graph information (including Microwave actually, that synthesizer I'm programming).

Instead, I'll add graphModel::clearAll, which clears all samples regardless of whether they're visible. Then I'll put that in the functions BitInvader runs when a waveform button is clicked. I just did that and tested it out, and it works just fine. Would that be alright?

Edit: I'm instead doing graphModel::clearInvisible which clears all samples that aren't displayed.

[DouglasDGI]

If that's okay then I can probably go ahead and make a PR right now.

[PhysSong]

The original issue is fixed via 8d707df171fa89c8426dca2d0d42de744d5db365. Now changing this issue as an enhancement for 1.3.0.