-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Recording logarithmic automation hotfix #7621
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
Conversation
|
I believe this is ready to merge |
allejok96
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this merged yet?
|
The better solution would to give the automation editor a logarithmic Y axis when working with logarithmic models (possibly even an letting the user choose linear/log when drawing) and updating the draw code in AutomationClipView the same way. And then we'd need to do a update routine for older files... But until then let's merge this. |
I can't merge it until someone with write access approves. |
|
Pretty sure you have write access @allejok96? I haven't been able to test this, but if it fixed the issue it should be fine for now. |
|
Oh wait you aren't a member? Hm.. |
|
@bratpeki can you help quickly test this for me? The PR code wise is fine, it isn't too invasive, but testing it never hurts. I think the TODO comment also explains the situation quite well. |
sakertooth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eh.. I would also be fine if @regulus79 is sure about the fix. Sorry for all the @'s lol.
When you record an automation clip with a logarithmic knob (for example, a freq knob in the equalizer) the recorded values in the automation editor are correct.....
....But, for whatever reason, lmms tries to scale the recorded values back to linear when playing the automation, which causes the recorded frequency to be much lower.
This pr does not fix the underlying issue, but instead just scales the recorded values so that everything works.
Imo, the whole automation system needs some reorganizing, but that will have to wait. Just one example of the choas caused by swithcing between scaled/unscaled values: The automation editor conflicts with itself when showing scaled parameters. The tooltip shows the scaled value, while the actual stored value which you can edit when double-clicking on a node is unscaled. But again, that's for another day.
Fixes #6954 and #6209 and maybe #6490