make ControlPotmeter's '_set_default' an in/out CO

[ronso0]

inspired by the GUI lag probably caused by <Transform><IsEqual>0.5</IsEqual></Transform> when using a high-res pitch slider
https://mixxx.discourse.group/t/mixtrack-pro-3-tempo-slider-lag-on-w10/21835/18

<Connection>
    <ConfigKey><Variable name="Group"/>,rate</ConfigKey>
    <Transform><IsEqual>0.5</IsEqual></Transform>
    <BindProperty>highlight</BindProperty>
</Connection>

It's not yet verified that this would fix the lag, but
it's handy anyway to have group, control _set_default with read/write capability.

TODO

• Document new control in the manual

[ronso0]

@NotYourAverageAl
Please test if the PR build fixes the GUI lag for you.

[NotYourAverageAl]

@ronso0 thanks, I can confirm this fixes the lag issue completely.

[ronso0]

okay, great!

Though, this is just a workaround.
I don't know (and am not going to investigate) if the lag @NotYourAverageAl discovered with the mixtrack Pro 3 pitch fader is

• a bug with with some Qt or other depencies on windows or
• <Transform><IsEqual>... is simply not designed/efficient for non-integer compare values.

[ronso0]

Documentation is in mixxxdj/manual#374

[Swiftb0y]

not sure if this is bikeshedding, but I'd suggest naming the CO rate_neutral as it conveys the intent better and makes more sense when used in non-gui contexts like controller scripting.

[Holzhaus]

not sure if this is bikeshedding, but I'd suggest naming the CO rate_neutral as it conveys the intent better and makes more sense when used in non-gui contexts like controller scripting.

or rate_original?

[uklotzde]

Or simply name it by what it stands for, i.e. rate_zero which is unambiguous and universal. We do not need to invent a new term if none exists already.

[JoergAtGithub]

Isn't a rate of zero standstill? If the slider is in the center position it's rate one.

[uklotzde]

Isn't a rate of zero standstill? If the slider is in the center position it's rate one.

It's a relative rate.

[Holzhaus]

rate_adjust_zero to make it clearer? Idc ;-)

[ronso0]

thanks for the proposals, nothing really convinced me, except maybe rate_original -_o
but -for me- that refers to a non-scale value that cannot easily be restored, like a string or a number for example.
rate_zero is technically correct but we talk about 'speed' so _zero sounds fatal.. ; )

the new CO is a read-only state so if we want to get away from the slider reference it should be rate_reset but that is already taken by the _reset action of controlpotmeter.
rate_is_reset ?
rate_restored ?

[ronso0]

let's vote 😬 😁

[ronso0]

sry, just kidding...
I hope this has not become a 'naming issue' by now..

The more I think about it I think the original quick shot rate_centered makes sense, because also in non-gui (scripting) contexts COs relate to DJ hardware: basically in any DJ app and any DJ hardware the speed control is represented by a slider (except portable turntables which may have a rotary knob instead -- which can also be centered for neutral zero original rate)
I strained myself but don't see how _centered can be mistaken for something else.

Accordingly we also have orientation_center to center the crossfader.

[daschuer]

Just out of interest, can we explain the GUI lag now? Maybe we have discovered an endless loop somewhere. Something like two rounding functions that have two ideas of rounding.
To pixels vs midi values or such.

I can confirm that such a center indicator is usefull. So nothing holding back this PR.

[daschuer]

Wait ... does this apply to all sliders?
How about re-usung the "_reset" contol for it as a general solution? _reset

[Swiftb0y]

I'm sorry, I really did not intend to create a bikeshedding discussion, though my opinion on rate_centered vs rate_neutral vs rate_original is still strong. I prefer rate_neutral because just looking at the name is self-explanatory. rate_centered would have to be something like rate_slider_centered. rate_original suggests some kind of timeline which doesn't exist.

If I still didn't convince you, I'd be ok with rate_centered just for the sake of consistency with orientation_center.

[daschuer]

The existing Co is "rate_set_default"
Not an ideal name for this CO. ... Mmm
It could become a center indicator/button.

What do you think?

[Swiftb0y]

The existing Co is "rate_set_default"
Not an ideal name for this CO. ... Mmm

Those are COs added because rate is a ControlPotMeter, so the _set_default part is a generic suffix intended to work for all kinds of controls. rate_centeredis just an extra CO for the rate independent of the ControlPotMeter feature.

[daschuer]

Yes, right. Currently it is only used the receive updates. It would be no deal to move the logic introduced here to this control and let all sliders an knobs benefit from it.

[ronso0]

@daschuer You're right, all potmeter controls should have this status CO.
I didn't remember we have _set_default. Complement would be _is_default.

"no big deal" may be correct if you've been working on the control system for years. I haven't, but I made it. Please test.

[daschuer]

"no big deal" may be correct

Oh sorry, this was not meant to talk the work small. Every change is a significant effort and thank you for picking it up.

[daschuer]

I am unsure about the following:

From the naming, it would be the least confusing solution to introduce the complement CO _is_default to _set_default.
When we look to the entire CO system we have push button controls that reflect the button LED state as well as receiving press and release actions from the controller. From this point of view it would be less confusing to follow it and make _set_default such a push button control. We may also add a new in/out co for this and alias/deprecate the _set_default.

All this works for me, I just want to make sure we make an informed decision.

[Swiftb0y]

We may also add a new in/out co for this and alias/deprecate the _set_default.

I'm in favor of this solution and deprecating _set_default.

[ronso0]

We may also add a new in/out co for this and alias/deprecate the _set_default.

Sounds reasonable.

To clarify the purpose and urgency of this PR: avoid GUI lags @NotYourAverageAl encountered.
Can we merge the current solution? The rate center indicator is the first and only (GUI) control to make use of such an indicator. Something similar may also be helpful for key but that's working entirely different.
I'm about to finish branches more relevant for 2.3, so any further optimization of the controls can be done in a follow-up PR by someone familiar with the contorls system.

[ronso0]

If the current implementation is okay I'll update mixxxdj/manual#374

[ronso0]

"no big deal" may be correct

Oh sorry, this was not meant to talk the work small. Every change is a significant effort and thank you for picking it up.

no prob, I didn't understand it that way, it's just not "swapping a few lines" only, as the perfomrmance issue indicates..

[daschuer]

ronso0#24 is a improved version.
I was about to create an alias, but than I have noticed that the name _set_default is not too bad, given that it is already there.
It suits to make a touch friendly default button and the name fits for this purpose.

The original issue is hopefully solved with #3806

We can create an alias at any time later, while undoing the alias is hard. ...

[ronso0]

Thanks @daschuer !

[daschuer]

@Swiftb0y Do you want to have a final look?

[Swiftb0y]

Can you also document that reading from _set_default indicates whether the controls value is currently its default value in the manual?

[ronso0]

As soon as all reviewers will have approved the changes I'd like to squash the c++ and and the skin changes into two commits.
The current log looks like back and forth and contains too much noise for the actual outcome.
Agreed?

[daschuer]

Yes, thank you.

[ronso0]

Nice, done.
Thanks everyone!

[Holzhaus]

Thanks. Please update the manual accordingly.