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

Fix Envelope/LFO knob scaling and add Tempo Sync #7811

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

regulus79
Copy link
Contributor

This PR fixes the scaling of the knobs in the envelope/LFO tab of instruments to give the knobs meaningful values. Previously, the value in the knobs were first squared and then multiplied by a constant before used in calculating the envelope time, which is not intuitive. I have changed it so that the value shown on the knob is exactly how long, in seconds, that part of the envelope will last. This also means that the knobs can be tempo-synced, so that users can precisely time the lengths of the attack/decay to match the beat.

Preview

Knobs values are in seconds now:

image

And you can tempo-sync them:

image

NOTE!

  • I updated the initial env/lfo values to give the same audio result as before. However, this means that their current values seem a little random. If anyone wants to change the default values, just let me know!
  • Because the quadratic knob scaling is removed, the env graph may feel a bit weird if you are used to it before. I tried to change the scaling of the graph so that it looks okay, but I would love to hear your opinions.
  • I made the knobs logarithmic by default. Is that okay with everyone?

Changes

  • Removed the calls to expKnobVal() within EnvelopeAndLfoParameters.cpp, and reworked the calculations to use raw seconds. This required changing the model min/max's to allow for the same range.
  • Changed most of the env/lfo knobs to be TempoSyncKnobs/TempoSyncFloatModels, and set them to logarithmic by default.
  • Changed the scaling of the env/lfo graphs to feel better after the change.
  • Added an upgrade routine to recalculate the env/lfo knob values.

Also why is expKnobVal used in the filter cutoff calculation? Like why? Doesn't that mean the filter cutoff isn't linearly moving with the envelope, unlike the volume or resonance? (???)

Notes about testing

This PR needs to be tested with old projects to make sure everything sounds the same! I tried my best to make sure the upgrade routine works correctly, but it is possible I could have made a mistake somewhere. If the envelope/lfos of old instruments aren't behaving the same as they did, please let me know!

@tresf
Copy link
Member

tresf commented Mar 24, 2025

I tested this PR with demos/JousBoxx-BuzzerBeater and it sounds OK on playback.

However, the sensitivity of the knobs I believe to be a regression in this PR. Now, adjusting the parameters is much less fine-grained.

Here's a comparison video.

envelope.mov

Copy link
Member

@tresf tresf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request for knob sensitivity to be reconsidered.

@regulus79
Copy link
Contributor Author

Request for knob sensitivity to be reconsidered.

TLDR It should be fixed now.


I think the behavior you are experiencing is due to old project knobs being updated to the correct values, but the knobs still staying in linear scale, rather than logarithmic (on the gui side). Since the range of possible values for something like attack in master goes from 0 to 20 seconds, I made the knobs go from 0 to 20, but this is a very large range which most users will not need, so I also made the knobs logarithmic by default (on the gui side of course; the actual value of the knob does not change, just the sensitivity scaling)

But, knobs in old projects which were set to linear would keep their scaling and not update to be logarithmic. (Or at least that's what I thought; it's a bit more complicated than that since there are two ways knobs can be stored in the xml, either as attributes or as child nodes; only knobs stored as child nodes can have special things like scale types or automation. Then I realized that the upgrade routine didn't account for that, so reworked it so that it should upgrade the knobs correctly even if they are stored as child nodes. Aaand just because I could, I decided to also force the old env/lfo knobs to be set to logarithmic by default, which should hopefully fix the issue you are experiencing and make it easier to use)

I still don't know what we should do about upgrading automated knobs.

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

Successfully merging this pull request may close these issues.

2 participants