Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support various behaviors when increasing sample length after drawing a waveform in BitInvader #3797

Closed
Sawuare opened this issue Sep 8, 2017 · 22 comments
Milestone

Comments

@Sawuare
Copy link
Member

Sawuare commented Sep 8, 2017

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:
image

The expected behavior:
capture20170908175912396

@PhysSong
Copy link
Member

PhysSong commented Sep 9, 2017

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
Copy link
Member

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. 👎
However someone might think other waveform(such as two periods of sine wave) should be there. So we need to discuss.

@musikBear
Copy link

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
Copy link
Member Author

Sawuare commented Oct 1, 2017

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.

@LostRobotMusic
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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

@musikBear
Copy link

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

@musikBear
Copy link

Bahh picture not showing :?

@PhysSong
Copy link
Member

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
12rc 113 extend

@PhysSong
Copy link
Member

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.

@LostRobotMusic
Copy link
Contributor

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.

@LostRobotMusic
Copy link
Contributor

LostRobotMusic commented Jan 22, 2019

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

@jasp00
Copy link
Member

jasp00 commented Jan 31, 2019

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

@jasp00
Copy link
Member

jasp00 commented Feb 5, 2019

Please let us move on; I will propose a decision. The "flat line" button (#3797 (comment)) and behavior "Alternative" (#3797 (comment)) 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.

@LostRobotMusic
Copy link
Contributor

LostRobotMusic commented Feb 5, 2019

@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
Copy link
Member

jasp00 commented Feb 5, 2019

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 PhysSong added this to the 1.2.0 milestone Feb 13, 2019
@PhysSong
Copy link
Member

@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.

@LostRobotMusic
Copy link
Contributor

LostRobotMusic commented Feb 13, 2019

@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.

@LostRobotMusic
Copy link
Contributor

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

@PhysSong
Copy link
Member

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

@PhysSong PhysSong added ux and removed bug labels Feb 15, 2019
@PhysSong PhysSong removed their assignment Feb 15, 2019
@PhysSong PhysSong changed the title Odd behavior when increasing sample length after drawing a waveform in BitInvader. Support various behaviors when increasing sample length after drawing a waveform in BitInvader Feb 15, 2019
@PhysSong PhysSong modified the milestones: 1.2.0, 1.3.0 Feb 15, 2019
@Sawuare Sawuare removed the ux label Jul 31, 2019
@zonkmachine
Copy link
Member

This was closed in #4824 and #5799 which restores the behavior from 1.1.3 .

The default sine wave that is created when a new wave is initialized fills the whole sample. When you generate/draw a wave that is shorter than this, the rest of the sample was unchanged. Its doesn't seem practical to make an option to leave this in nor does it seem very useful musically. There are wave table editors out there that have more advanced features. The best I saw was the standalone editor for wiard waveform city synth module http://www.wiard.com/modular/300series/waveformcity/index.htm but unfortunately it doesn't seem to be available any longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants