do not clamp out-of-bounds ControlPotmeter parameters when they are allowed #1237
Conversation
getting the normalized value should reflect the actual value of the ControlPotmeter if out-of-bounds values are allowed
|
Sliders with out of bounds values can still be operated by mouse by clicking on the slider, which makes the value jump in bounds. While it is weird to have the handle disappear, it may be helpful to have this clear visual indication that the value is out of bounds. Thoughts? |
|
It works for me. I think it its finally up to the widget/skin to deal with this situation. Without this patch they they can't. LGTM. |
|
I think it's okay to go ahead and merge this. If people do not like slider handles disappearing I do not think it will be difficult to later implement clamping the parameter for display purposes in the widget. |
|
Thank you! |
Yes, please! |
|
@Be-ing Please clamp parameters for sliders! |
How do you expect this to work? Previously, when sync caused the tempo to go outside its normal range, if the controller mapping used soft takeover, the hardware slider could be moved to the appropriate limit of its range, but then moving it from there would cause a big tempo jump to the edge of the slider's normal range. I don't think I'd call that rock solid. It seems to me (and I may be wrong) that the best way to handle this is to make tempo sliders work as relative controls like they can in Traktor with a preference option or like they do in Serato. To implement this in JavaScript as I did in #1227 requires engine.getParameter to return the actual position of the slider even if it is out of its normal boundaries. Perhaps this shouldn't be handled by controller mappings and should be handled by Mixxx? Personally my controller uses infinitely turning encoders for tempo so this does not become an issue for me. |
|
Basically I'm accepting the offer you made in the first post -_o
Sure, sounds great! But I guess(!) it takes longer to implement, and I'm afraid users get disappointed by 'broken features' from 2.0 in the mean time. Regarding #1227 I agree with @daschuer 's concerns. What you implemented in maybe handy but is far from self-explaining (physical control doesn't match its representation on screen), if then Shift is involved as well it's beyond of what a beginner DJ can/wants to handle, I guess. |
|
This PR is unfortunately only the half way to a solution. The other half was somehow fallen into oblivion. Do we have a bug for this? It looks like it is high prio. @ronso0 , do you have fun and time to look into it? We need to stop the handle from disappearing as a first step. Extra feature: Another Extra feature: |
This is incompatible with the requirement for engine.getParameter returning the actual value for implementing relative control in JavaScript. Changing the skin widget would not change the issue with controllers' tempo sliders losing control. Perhaps the implementation of a relative mode could be done in C++. The existing "rate" Control could become like a facade that implements either the relative mode or soft takeover logic toggled by a preference option. The engine and other parts of Mixxx would be switched to use a true or internal rate that controllers would not interact with. But this is a considerably more complicated solution. I personally don't feel a pressing need to implement it at the moment, so unless anyone else wants to take that up now, it would probably be best to revert 9ee2d1c, remove the relative mode for Pots from midi-components-0.0.js, and leave it as a wishlist item.
It seems to work well for Traktor and Serato. It is an alternative to dealing with the confusion of soft takeover. IMO the problem is really in the pervasive use of finite range potentiometers for controlling tempo on controllers. It does not make much sense to me to have both a finite range tempo control and a sync button as inevitably that creates this confusing situation that has no perfect solution. But alas I cannot simply change the design of most controllers. There are only a few controllers that have infinite range encoders for tempo (Hercules P32, A&H Xone K2, Kontrol S5, and Kontrol S8 are all that come to mind). |
https://bugs.launchpad.net/mixxx/+bug/1696265 I'm afraid I don't have much time in the next month, and I still don't have a proper IDE, sorry. The situation: Sync deck2 to 80 BPM, now rate2 is below lower end of rate range, -20%
@daschuer It's what you're referring to? Regarding relative mode:
@daschuer That's what you mean? |
Yes |
|
The first step just not hiding the slider seams to be a one liner. |
|
No, changing the skin widget would not fix the issue of controllers' tempo sliders losing control when the value goes out of bounds. |
…lowed" This reverts commit 9ee2d1c, which made it so controllers not mapped using the new relative mode Pot in Components lost control of the tempo fader when sync made it go out of bounds, as reported here: mixxxdj#1237 (comment)
implementing this in JavaScript required making a change in C++ that broke old controller mappings as discussed on GitHub: mixxxdj#1237 (comment)
|
Ah Ok, I understand. This means we need to fix softtakover for to not loose control if the parameter cannot reached. Right? |
That could work, but I'm not sure it would be the best solution. I'm not sure what the best solution would be... |
implementing this in JavaScript required making a change in C++ that broke old controller mappings as discussed on GitHub: mixxxdj#1237 (comment)
Getting the normalized value should reflect the actual value of the ControlPotmeter if out-of-bounds values are allowed, as discovered in #1227.
This has a weird side effect with WSliderComposed:
If this is not desired, I could make WSliderComposed and WKnobComposed clamp parameters.