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

Automation editor still displays mouse cursor frequency linear. #664

Closed
JohannesLorenz opened this issue Apr 30, 2014 · 27 comments
Closed

Comments

@JohannesLorenz
Copy link
Contributor

Even if the model is logarithmic, the mouse cursor displays a value as if handled linearly.

@tobydox For fixing this, the AutomationEditor needs to know the AutomatableModel. Should it find that in AutomationPattern::m_objects (or in a similar place?), or should the model be passed from outside?

@tobydox
Copy link
Member

tobydox commented Apr 30, 2014

Good question. AutomationPattern could provide an hasLogarithmicObjects() hint (or similiar name) which retrieves it from the connected object.

@JohannesLorenz
Copy link
Contributor Author

Am Mittwoch, 30. April 2014, 14:34:42 schrieb Tobias Doerffel:

Good question. AutomationPattern could provide an hasLogarithmicObjects()
hint (or similiar name) which retrieves it from the connected object.

IIRC, the problem is that the user could drag a logarithmic and a linear knob
both into the same pattern. I wonder how much sense this feature makes. I just
dragged a Pan-Knob onto a LP2's automation. The result is that you can pan up
to 20000 Hz ;)

Also, the window title is not updated correctly for this case.

Maybe we could delete this feature. Or better: restrict the user to only allow
automation track sharing for models that equal each other (where we'd need a
special function that determines what equals means). If you still want to
share an automation pattern to two knobs, you can simply copy and paste the
pattern, I tried it multiple times.

What do you think?

@diizy
Copy link
Contributor

diizy commented May 1, 2014

On 05/01/2014 01:56 PM, JohannesLorenz wrote:

Am Mittwoch, 30. April 2014, 14:34:42 schrieb Tobias Doerffel:

Good question. AutomationPattern could provide an
hasLogarithmicObjects()
hint (or similiar name) which retrieves it from the connected object.

IIRC, the problem is that the user could drag a logarithmic and a
linear knob
both into the same pattern. I wonder how much sense this feature
makes. I just
dragged a Pan-Knob onto a LP2's automation. The result is that you can
pan up
to 20000 Hz ;)

The scale may show that you can pan "up to 20000 hz" but that's just
numbers, the automation still works as intended. I think the feature is
mainly useful in that if you want the same shape of automation, the same
curve, for multiple controls - it isn't meant for cases where you need
automation with accurate values.

Also, the window title is not updated correctly for this case.

Maybe we could delete this feature. Or better: restrict the user to
only allow
automation track sharing for models that equal each other (where we'd
need a
special function that determines what equals means). If you still want to
share an automation pattern to two knobs, you can simply copy and
paste the
pattern, I tried it multiple times.

What do you think?

Those kinds of restrictions would break backwards compatibility, and I
don't really see much need for them...

I think it'd be easier to simply determine whether the automation
pattern should be logarithmic by the first object dragged to it. This
should probably cover 99% of use cases. So if you first drag a
logarithmic control to the pattern, then it's logarithmic, no matter
what kind the 2nd, 3rd, etc.. control are.

@JohannesLorenz
Copy link
Contributor Author

I think it'd be easier to simply determine whether the automation
pattern should be logarithmic by the first object dragged to it. This
should probably cover 99% of use cases. So if you first drag a
logarithmic control to the pattern, then it's logarithmic, no matter
what kind the 2nd, 3rd, etc.. control are.
Ok, I agree in that this would be better, because it does not break backwards
compatibility.

The scale may show that you can pan "up to 20000 hz" but that's just
numbers, the automation still works as intended. I think the feature is
mainly useful in that if you want the same shape of automation, the same
curve, for multiple controls - it isn't meant for cases where you need
automation with accurate values.
Still, I think that the only useful case where the user would like to share
the shape is where the minimum, maximum and step size are equal. I just can
not imagine that someone wants to increase the volume by "20" and raise the
frequency by 20 Hz at the same time. oO

@musikBear
Copy link

imo multiple controls in one auto-track are -so- rarely used, (like volume control for a group of instruments) that it would be a shame to make serious changes to a working model. If peeps drags weird controller combos together, would it be weird if they get weird results? -I think no.

@diizy
Copy link
Contributor

diizy commented May 1, 2014

On 05/01/2014 02:34 PM, JohannesLorenz wrote:

The scale may show that you can pan "up to 20000 hz" but that's just
numbers, the automation still works as intended. I think the feature is
mainly useful in that if you want the same shape of automation, the same
curve, for multiple controls - it isn't meant for cases where you need
automation with accurate values.
Still, I think that the only useful case where the user would like to
share
the shape is where the minimum, maximum and step size are equal. I
just can
not imagine that someone wants to increase the volume by "20" and
raise the
frequency by 20 Hz at the same time. oO

Well, not necessarily. The numbers on the ranges may be different, but
they're only numbers and don't tell anything as is...

For example: 100% volume is equivalent to 0dBV volume, or 1.0 gain...
without knowing the units (which LMMS doesn't, because it'd be way too
much complexity to keep track of all the billions of units used in
digital audio, also none of our plugin formats supports that kind of
functionality so it would be pretty limited in any case) the numbers
don't necessarily say much.

It's not just volume... for time/frequency as well: there's Hz, there's
BPM, and then there's inverse units: ms, s... all basically measure the
same thing (cycle length or frequency, which are inverses of each other)
but with very different values. And that's not even getting into
logarithmic units - seminote, cent, octave...

The user may, however, still want to apply the same curve shape to two
controls. The user may want to tweak two knobs from min to max at the
same rate, no matter what those knobs control - I don't think we should
even worry about the knob units or values in this case, or try to
normalize them to the same "scale" in any way - if you want accurate
values, use separate patterns... I think the flexibility of using the
patterns in creative ways is something we should keep here, even if all
use cases don't necessarily make "sense"...

@JohannesLorenz
Copy link
Contributor Author

I think the flexibility of using the
patterns in creative ways is something we should keep here, even if all
use cases don't necessarily make "sense"...

Ok, convinced. I'll write code that will just look at the first
AutomatableModel. This is also compliant to the window title, which also only
shows the first model's name.

@diizy
Copy link
Contributor

diizy commented May 1, 2014

On 05/01/2014 07:25 PM, JohannesLorenz wrote:

I think the flexibility of using the
patterns in creative ways is something we should keep here, even if all
use cases don't necessarily make "sense"...

Ok, convinced. I'll write code that will just look at the first
AutomatableModel. This is also compliant to the window title, which
also only
shows the first model's name.

Yeah, I think that's for the best. Thanks.

@JohannesLorenz
Copy link
Contributor Author

Ok, convinced. I'll write code that will just look at the first
AutomatableModel. This is also compliant to the window title, which
also only
shows the first model's name.

Yeah, I think that's for the best. Thanks.

Currently, I'm trying to set that number display in the AutomationEditor (that
one that tells you in a filter what the frequency is). If one wants to modify
this behaviour to be logarithmic, would it be correct to modify the
functionality of

AutomationEditor::getLevel ?

Or is this the wrong function?

Thanks on advance.

@unfa
Copy link
Contributor

unfa commented May 8, 2014

On 1 May 2014 17:40, "Vesa V" [email protected] wrote:

On 05/01/2014 02:34 PM, JohannesLorenz wrote:

The scale may show that you can pan "up to 20000 hz" but that's just
numbers, the automation still works as intended. I think the feature
is
mainly useful in that if you want the same shape of automation, the
same
curve, for multiple controls - it isn't meant for cases where you need
automation with accurate values.
Still, I think that the only useful case where the user would like to
share
the shape is where the minimum, maximum and step size are equal. I
just can
not imagine that someone wants to increase the volume by "20" and
raise the
frequency by 20 Hz at the same time. oO

Well, not necessarily. The numbers on the ranges may be different, but
they're only numbers and don't tell anything as is...

For example: 100% volume is equivalent to 0dBV volume, or 1.0 gain...
without knowing the units (which LMMS doesn't, because it'd be way too
much complexity to keep track of all the billions of units used in
digital audio, also none of our plugin formats supports that kind of
functionality so it would be pretty limited in any case) the numbers
don't necessarily say much.

It's not just volume... for time/frequency as well: there's Hz, there's
BPM, and then there's inverse units: ms, s... all basically measure the
same thing (cycle length or frequency, which are inverses of each other)
but with very different values. And that's not even getting into
logarithmic units - seminote, cent, octave...

The user may, however, still want to apply the same curve shape to two
controls. The user may want to tweak two knobs from min to max at the
same rate, no matter what those knobs control - I don't think we should
even worry about the knob units or values in this case, or try to
normalize them to the same "scale" in any way - if you want accurate
values, use separate patterns... I think the flexibility of using the
patterns in creative ways is something we should keep here, even if all
use cases don't necessarily make "sense"...
I agree.

I guess the % values could be applied to anything as a position on relation
to full scale. Sometimes 100% means 20kHz, sometimes +6dB, sometimes 1.0
and sometimes 'moog lowpass'.

I think the users will understand that once they try making some strange
combinations. No need to limit freedom :)


Reply to this email directly or view it on GitHub.

@JohannesLorenz
Copy link
Contributor Author

@tobydox I could need your help with my last question. Can you please try to answer it? Thanks.

@tobydox
Copy link
Member

tobydox commented May 22, 2014

I'm really sorry but I can't help here as I didn't develop the Automation Editor at all and I'm not very familiar with the code base - maybe someone else can help out here?

From what I can see getLevel() just does a linear mapping from screen coordinates (based on current zoom) settings to the current value. You probably have to apply the used function (such as log) afterwards, if possible using existing functionality (in AutomationPattern? I don't know, @diizy any clue?)

@diizy
Copy link
Contributor

diizy commented May 22, 2014

On 05/22/2014 12:14 PM, Tobias Doerffel wrote:

I'm really sorry but I can't help here as I didn't develop the
Automation Editor at all and I'm not very familiar with the code base

  • maybe someone else can help out here?

From what I can see getLevel() just does a linear mapping from screen
coordinates (based on current zoom) settings to the current value. You
probably have to apply the used function (such as log) afterwards, if
possible using existing functionality (in AutomationPattern? I don't
know, @diizy https://github.com/diizy any clue?)

I'm not really familiar with AutomationEditor either... haven't looked
at the codebase much. I think @softrabbit might have some ideas as he's
the one who's most recently edited the code here...

However:

I know that AutomationPattern doesn't have any code for converting
log/linear, that's all in AutomatableModel... but maybe the whole
log/linear conversion should be moved to lmms_math.h, so it could be
used anywhere.

From quick thinking, I don't think we need to touch the screen
coordinates here at all - we should just change the values being
displayed on the side... because we don't need to transform the values
themselves here, as they already get converted by the log/linear
conversion anyway when the values get written to the model from the
AutomationPattern... so all we really need in AutomationEditor is
display the scale correctly, right?

@JohannesLorenz
Copy link
Contributor Author

From quick thinking, I don't think we need to touch the screen
coordinates here at all - we should just change the values being
displayed on the side... because we don't need to transform the values
themselves here, as they already get converted by the log/linear
conversion anyway when the values get written to the model from the
AutomationPattern... so all we really need in AutomationEditor is
display the scale correctly, right?

It's indeed all about the displaying; the models itself works correct. The
values on the right need to be done (as you mentioned), but also the numbers
next to the mouse cursor. And I think that should already suffice.

Maybe one might, in the future, draw the horizontal lines with logarithmic
distance, but right now I think this would confuse me more than it would
clarify anything.

@softrabbit
Copy link
Member

Don't look at me... I've mostly tweaked the grid drawing on the time axis, haven't really gone deeper into the rest of the code.

@diizy
Copy link
Contributor

diizy commented May 22, 2014

@JohannesLorenz

Ok, from a cursory look, it seems there's only few changes required in
AutomationEditor.

The code for drawing the values on the side takes the values from
AutomatableModel::displayValue( float ), so for that you don't actually
need to modify AutomationEditor at all - you need to go back to
AutomatableModel and modify that to take the log/linear scale in account.

For the tooltip, it's a bit different - the level is taken from the
mouse coordinates, so you need to modify that to take the scale in
account. The code that sets the tooltip is starting from line 1328.

@JohannesLorenz
Copy link
Contributor Author

The code for drawing the values on the side takes the values from
AutomatableModel::displayValue( float ), so for that you don't actually
need to modify AutomationEditor at all - you need to go back to
AutomatableModel and modify that to take the log/linear scale in account.

For the tooltip, it's a bit different - the level is taken from the
mouse coordinates, so you need to modify that to take the scale in
account. The code that sets the tooltip is starting from line 1328.

Ah, it's drawCross()... I see. Thanks. I'll try to write the code next week.

@JohannesLorenz
Copy link
Contributor Author

I know that AutomationPattern doesn't have any code for converting
log/linear, that's all in AutomatableModel... but maybe the whole
log/linear conversion should be moved to lmms_math.h, so it could be
used anywhere.

I agree. @tobydox is it ok to put this into lmms_math.h?

@tobydox
Copy link
Member

tobydox commented May 30, 2014

Yes. Maybe we can even introduce a MathHelper class/namespace so the global namespace doesn't get polluted.

@JohannesLorenz
Copy link
Contributor Author

Am Freitag, 30. Mai 2014, 13:28:58 schrieb Tobias Doerffel:

Yes. Maybe we can even introduce a MathHelper class/namespace so the global
namespace doesn't get polluted.

Ok, you probably suggest a new namespace because it is not as often needed as
things from the math namespace, is that correct?

If so, I'd also add a new header for the new namespace. Hopefully, in the
future, we'll find a more precise name than "MathHelper", but for now, it
sounds good to me.

@diizy
Copy link
Contributor

diizy commented Jun 20, 2014

I'm working on fixing some bugs with logscales and I'll probably be able to do this too while I'm at it.

@JohannesLorenz
Copy link
Contributor Author

I'm working on fixing some bugs with logscales and I'll probably be able to do this too while I'm at it.

Ok, I originally wanted to do it, but didn't have time for it, so just go ahead.

@diizy
Copy link
Contributor

diizy commented Jun 20, 2014

On 06/20/2014 01:46 PM, JohannesLorenz wrote:

I'm working on fixing some bugs with logscales and I'll probably be
able to do this too while I'm at it.

Ok, I originally wanted to do it, but didn't have time for it, so just
go ahead.

Already done. Now I'm working on the knobs ;)

@tresf tresf added this to the 1.2.0 milestone Dec 23, 2014
@Umcaruje Umcaruje added the gui label Jun 29, 2015
@Umcaruje
Copy link
Member

Should we move this to the 1.3.0 milestone?

@JohannesLorenz
Copy link
Contributor Author

I'm not sure why this has not been finished. Actually I planned to do it, but then @diizy said he'd do it. What exactly is missing and why should it not be coded for an earlier version?

@Umcaruje
Copy link
Member

What exactly is missing and why should it not be coded for an earlier version?

@diizy hasn't been around the project for a while, and well no work has been done concerning this issue. We're short on devs, so if you would like to work on it, that would be really appreciated. If you want to work on it sometime soon and if it's not a too complex patch, please target it at the stable-1.2 branch :)

@JohannesLorenz
Copy link
Contributor Author

It could be confirmed that both mentioned issues (cursor tooltip in the automation editor and knob tooltip) are not present in the stable-1.2 branch (for whatever reason).

This bug is solved.

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

8 participants