Support disabled state in control system and widgets.

Bug #1180872 reported by RJ Skerry-Ryan
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mixxx
Fix Released
Wishlist
RJ Skerry-Ryan

Bug Description

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.

Tags: skin
RJ Skerry-Ryan (rryan)
Changed in mixxx:
milestone: none → 1.12.0
assignee: nobody → RJ Ryan (rryan)
importance: Undecided → Medium
importance: Medium → Wishlist
status: New → Confirmed
jus (jus)
tags: added: skin
Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

I wonder how this should interact with Qt's "enabled" state for widgets.

* Widgets that are not enabled do not receive mouse / other input events.
* Children of a disabled widget are also disabled.

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.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Here some thoughts about it (please correct me If I am wrong):
* Disabling control widgets will only effect the GUI, not the connected controllers.
* the "disable" CO will clutter the CO interface
* permanent disabled controls for all COs will clutter the GUI
* Disabled controls may need CPU time

- Is there a use case for the requirement to manually switch the disable state while paying life?
- Do we need a real control Object for disabling a CO like "play_disable"?
- Do we need to track CO changes while a CO is disabled?

For now, I can see two use cases:
1. Indicate that a control is temporary out of function
2. Hide some controls because they are not needed for the specific setup or mixing style

I think some of the issues of 1. are part of the cdj branch https://github.com/mixxxdj/mixxx/pull/65

The branch issues a slit of a control into a Control and a Control Indicator.
IMHO this is the best solution for use case 1. because it is useful for GUI and Controllers.
Currently it uses Blink patterns for indicate additional states. So it looks like we really need to find a way to extend this for less eye catching non blinking solutions inside the GUI.

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.
IMHO a pre-calculated indicator CO is much better, because it is easy to use and we can do the calculation inside Mixxx, able to adapt special cases like done in the cdj branch.

For use case 2. we have already solutions in 1.11 eg. the talk over button.

Revision history for this message
Owen Williams (ywwg) wrote :

Why not have enabled/disabled be a built-in property of the CO?

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

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.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

> Is there a use case for the requirement to manually switch the disable state while paying life?

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:

1) Indicate an individual control is enabled/disabled.
2) Indicate a group of controls are enabled/disabled.

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.

> For use case 2. we have already solutions in 1.11 eg. the talk over button.

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.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

> The QWidget enabled state gives us exactly that through the hierarchical nature of the enabled state.

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.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Hi RJ,
I think I got you point and I like it. But we have to be care full not to try to get confused with the different demands from that sort.

So let me collect the use cases first:

1) Indicate an individual control cannot accept changes (is fixed) because of a state inside Mixxx (read only)
2) Indicate (1 .. n) controls are not evaluated my Mixxx because of a state inside Mixxx (read only)
3) Drawer: Hide (1..n) controls because they are currently not needed for the specific setup or mixing style (read+write)
4) Hide or draw disabled (depending on Skin design) for (1..n) controls because they are not used in the specific setup (preferences)

Examples:
* Master Sync switch for an external clock source
    4) if no external clock source is configured
    1) if the external clock source is configured but not working
* EQs
    4) if EQ is bypassed
    2) if the Killswitch is enabled
* Mixer
    4) if an external Mixer is configured
* Play control
    1) if no track is loaded
* passthrough
    4) if no vinyl control is configured
* Deck 3+4
    4) if only two decks are configured
    3) if the decks are temporary not needed or they consume to much space.
* loop_in and loop_out
    1) if no loop is possible due to internal states
    3) if the DJ does not need Loops
* talkover button
    1) if mic is configured but not working
    4) if no mic is configured

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.
I think his can be achieved by extending the toggle buttons with a third state "disabled" or by allowing to read the current blink pattern from the indicator control. Still unsure what is better.

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.

Revision history for this message
Owen Williams (ywwg) wrote :

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.

Changed in mixxx:
milestone: 1.12.0 → none
Revision history for this message
Daniel Schürmann (daschuer) wrote :

This is now solved in many cases.
If a case is missing in a special skin, please file a new bug.

Changed in mixxx:
status: Confirmed → Fix Released
Revision history for this message
Swiftb0y (swiftb0y) wrote :

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/7037

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.