-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Support disabled state in control system and widgets. #7037
Comments
Commented by: rryan I wonder how this should interact with Qt's "enabled" state for widgets.
We could use this to our advantage if every widget supported rendering a "disabled" state version of itself. Today only WDisplay/WKnob support this. If we wanted to disable portion of the GUI, like the EQs or the mixer, we could simply have a control that indicates the enabled state and connect that control to the enabled state of the WWidgetGroup containing all of those controls. The whole section would automatically switch its enabled/disabled state with changes of that control. |
Commented by: daschuer Here some thoughts about it (please correct me If I am wrong):
For now, I can see two use cases:
I think some of the issues of 1. are part of the cdj branch #65 The branch issues a slit of a control into a Control and a Control Indicator. A "play_disable" control for instance has the disadvantage that all indicating devices have to calculate what to show from the original control and the disable control. For use case 2. we have already solutions in 1.11 eg. the talk over button. |
Commented by: ywwg Why not have enabled/disabled be a built-in property of the CO? |
Commented by: rryan Sorry for not being super clear. The bug title suggests I wanted to put a disabled state on each CO but I agree that that is not a good fix. I was talking about connecting indicator COs (e.g. [Channel1],passthrough) to the enabled state of widgets or groups of widgets. I'm talking about solving the GUI feedback problem of representing that the entire mixer section is not used if no master or headphone output is selected. Or that the deck controls have no effect when passthrough is enabled. Or that EQs have no effect if the user disables EQs (not something we support yet) or disabling the low-filter knob when the low-filter-kill is active. If skin writers could hook a [Master],mixer_enabled CO to the enabled QWidget property of the WWidgetGroup that contains all the mixer widgets then all the mixer widgets would automatically become disabled when the mixer_enabled CO is 0. We will also support hiding the mixer section entirely but that I think should be a user selected action rather than an automatic one. Hiding widgets isn't always an option. This a real usability issue -- for example users being confused that the microphone talkover button doesn't work when they haven't configured a mic input. The widgets should render in a disabled state. We would get far fewer confused people on the forums. WKnob/WDisplay already support the WWidget disabled state. I'm talking about combining the WWidget disabled state with the QWidget enabled property and extending support for the enabled/disabled state to WWidgetGroup/WWidgetStack/WKnobComposed/WSliderComposed/WStatusLight/WLabel/etc. This leaves representing the disabled states as an option to the skin writer. Similarly, the controller preset writer also has the option to represent the disabled states if their controller has the option. |
Commented by: rryan
I think you may have misunderstood me -- I'm talking about making a way for Mixxx to communicate to the user that a control is disabled -- not the other way around. But your 2 use-cases are accurate -- although I would re-phrase your use cases as:
Whether or not to show/hide widgets or to represent them in a disabled state (greyed out) is a choice the skin writer makes, not us. Hiding widgets may not always be desirable. An example of use case 1 would be loop_in and loop_out COs. Clicking loop_out has no effect if there is no loop start position set or if the loop start position is after the current play position. Creating a disabled state or CO for the loop_out CO would allow the Mixxx engine to set its disabled state to true if there is no start position or if the current position is before the current loop start position. Whether such a control is a one-off "enabled" indicator control (as hotcue_X_enabled is) or an "enabled" state built into ControlDoublePrivate is a question. The enabled state would be for consumers of the CO, not owners of the CO so we wouldn't have to spend extra CPU time checking the disabled state on every access in the engine. This way, skin writers or preset authors could make the skin or controller give visual feedback for that case. Currently there is no way to know other than hard-coding the logic in your controller script or making a custom widget.
I'm not sure what you mean -- the talkover button for the mic is exactly an example of where we fail today! You can click it regardless of whether there is a mic connected to Mixxx or not. There is another issue which has solidified my feeling that we need to use the QWidget "enabled" state: how to combine different enabled states. For example, if you tie the enabled state of the filterLowKill to filterLow, the widget will be enabled. However, if the user disables EQs entirely or some other state causes EQs to be disabled (e.g. no track is loaded to a deck) then we want the widget to disable regardless of the state of filterLowKill. The QWidget enabled state gives us exactly that through the hierarchical nature of the enabled state. A parent widget being disabled disables all of its children regardless of their individual enabled states. |
Commented by: rryan
We could do this via the Mixxx engine itself but this would be kind of onerous. For example, if passthrough is enabled then we would have to set every single EngineBuffer CO to disabled by hand. The skin writer, on the other hand, could just attach the passthrough indicator variable to the widget group containing. Alternatively, we could create a parent hierarchy within the control system for doing this. |
Commented by: daschuer Hi RJ, So let me collect the use cases first:
Examples:
Possible Solutions: for 1) We have already a Request/Confirm path to reject co changes before the are adopted. Combined with the new indicator control, this looks like a suitable solution. Btw. this is the way the controller manufactures think. E.G Denon uses blink patterns for many controls like play and looping controls. It is fully suitable for midi controllers as well. We have also discussed the option for showing a static disabled image instead of blink patterns. for 2) I am not sure if we need a special solution for this. For our EQ example, you have already a visual feedback that the killswitch is enabled.The RQ knob is still working and this is fine to me. for 3) Currently we use generic Skin controls for this. This is a fine solution for now. We may think about making the Skin defined controls more flexible later. for 4) Here Your Idea of introducing new control to be tied to the disable state perfectly fits. |
Commented by: ywwg I agree this would be very useful -- maybe we could even fake up "disabled" pixmaps by taking the existing pixmap and applying a saturation and brightness transform -- that might be good enough for 90% of widgets and would save jus time in creating a whole new raft of pixmaps. |
Commented by: daschuer This is now solved in many cases. |
Issue closed with status Fix Released. |
Reported by: rryan
Date: 2013-05-16T15:39:30Z
Status: Fix Released
Importance: Wishlist
Launchpad Issue: lp1180872
Tags: skin
Master sync and other feature requests would benefit from a way to mark a control as disabled for when its input is being ignored by the Mixxx engine. This requires updating the widget system to allow the skin to specify a disabled pixmap (maybe just an overlay?). Not sure whether the widget should reject mouse input or not but in either case the associated control could ignore the input.
Adding the disabled state is super easy given the work in the atomic-co branch.
The text was updated successfully, but these errors were encountered: