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

new Deere effects units with meta knobs #1063

Merged
merged 22 commits into from
Feb 3, 2017
Merged

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Dec 5, 2016

This is a proof of concept for a GUI making use of per-effect meta knobs introduced in PR #1062. There is definitely room for improvement with the finer details of the layout, but the basic layout is there.

The attached screenshot shows an EffectUnit in expanded mode on the left and collapsed mode on the right. Expanded mode is similar to how it has been with the addition of meta knobs. Text sizes have been increased for readability. Button parameters and knob parameters have been separated so effects on top of each other are presented in a more visually cohesive way. Collapsed mode shows the meta knobs for each effect, plus enable, eject, previous, and next buttons.

deere-new-effects0

The effect chain name and next/previous chain buttons have been removed. These have been causing confusion because users press them and do not understand the difference between EffectUnit chains and individual effects. When support is added for saving customized chains, I think it would make sense to add those back.

There are only two EffectUnits shown. IMO 4 EffectUnits with 3 effects each is really overkill. AFAIK no controllers are designed for 4 effect units; they're typically designed for 2. Cutting the number of EffectUnits down to 2 makes space for increasing text sizes.

@sblaisot
Copy link
Member

sblaisot commented Dec 5, 2016

can you add a screen capture of the same effect racks using the french language ? (this is our test language for longest strings).
I think "Kill MID" will not feet (like today's design) :)
Probably the translation has been made without the interface layout in mind.

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 5, 2016

Yeah, the EQ kill button labels are cut off in French. If there are shorter words that could be used for that translation, I think that would be a better solution than making the layout wider. I haven't yet tested this on my netbook, but I'm doubtful any wider of labels would fit well on that screen. The graphic EQ is barely fitting on my screen as is.
deere-new-effects-french

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 5, 2016

Is there a consensus that this is generally a good way forward? I'd like to clarify that before I spend time refining the UI details.

@ferranpujolcamins , thoughts?

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 5, 2016

After some refinements and PR #1064:
deere-new-effects-1

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 5, 2016

Added buttons for inverting meta/superknob linking and increased font size for deck/master/headphone enable buttons. Bitcrusher's downsampling parameter shows hover color for the new button.
deere-new-effects-2

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 6, 2016

I am liking how this looks now. This is no longer just proof of concept.
deere-new-effects-3

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 6, 2016

For an example of how to map this to a controller, refer to the Hercules P32 mapping. I have created a generic ControlContainer object to make it easy to map controllers with similar controls for effects in the same way. It's really fun! :D

@daschuer
Copy link
Member

daschuer commented Dec 6, 2016

Thank you for your work here. I like it and it should work very nicely, especially with controllers that features an effect region per deck.

I am just writing, because I was involved in the discussion when we have originally introduced the effect units, and the ideas implemented here where explicit rejected because of future plans towards using EffectChains instead of single Effects and Effect Racks per deck instead of EffectUnits.

The original Idea was to have an offline effect chain editor, where you can combine some atomic effects to an EffectChain which can then be selected by the GUI/Controller. The LatNight Skin tries follows this route and consequently hides the single effects.

I am in doubt if the original plan still fits, but we should be clear that they will never fit if we continue here. Forced by the controller layouts, we are heading directly towards the effect model found in Trakt0r.

Is this OK for us?

@rryan, @ywwg ?

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 6, 2016

At this point, I don't understand the point of the EffectRacks separate from the EffectUnits.

EffectChains instead of single Effects and Effect Racks per deck instead of EffectUnits

If you don't get hung up on the terminology, I think that is what is going on here. All effects in chains, each deck can have its own. To use a single effect, only enable one of them in a chain.

The original Idea was to have an offline effect chain editor, where you can combine some atomic effects to an EffectChain which can then be selected by the GUI/Controller.

The expanded view here can serve as that offline editor. When the backend capability is ready, all that needs to be added are save and open buttons.

The LatNight Skin tries follows this route and consequently hides the single effects.

LateNight only shows single effects, confusing users and controller mappers, leading many to not understand they are separate EffectUnits that can have more than one effect under the hood.

Forced by the controller layouts, we are heading directly towards the effect model found in Trakt0r.

This is actually not exactly the same as Traktor. In Traktor's simpler effect unit view, it exposes the individual parameters for one effect. This shows the meta knob for 3 effects in a chain.

Controllers designed for Serato and Rekordbox also use similar controller layouts, replacing a dry/wet knob with an encoder labeled "Beats".

@daschuer
Copy link
Member

daschuer commented Dec 6, 2016

At this point, I don't understand the point of the EffectRacks separate from the EffectUnits.

Currently we have only one effect rack with four effect units. Since we have also a QuickEffectRack and an EqualizerRack it will be probably sufficient.

Looking at the H P32, it could have make also sense to map each effect control to a whole EffectUnit with the original super knob, and don't have single parameter knobs accessible. Since you can assign Each EffectChain freely to one or more positions in the signal path, it could make sense to have 6 or 8 of them.

But as said, I like the new, fixes deck association with an "on the fly" chain editor.

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 6, 2016

Currently we have only one effect rack with four effect units. Since we have also a QuickEffectRack and an EqualizerRack it will be probably sufficient.

I'd be in favor of getting rid of EffectRacks and aliasing [EffectRack1_... to just EffectUnits.

Looking at the H P32, it could have make also sense to map each effect control to a whole EffectUnit with the original super knob, and don't have single parameter knobs accessible.

That is what I helped @timrae come up with for the Denon MC4000, but it doesn't really make sense and provides only limited control. That's what inspired me to implement this approach.

Since you can assign Each EffectChain freely to one or more positions in the signal path, it could make sense to have 6 or 8 of them.

You can? I was not aware of that. AFAIK no skin makes this possible. Also, I am not sure how a controller could provide any control over individual parameters with that approach.

But as said, I like the new, fixes deck association with an "on the fly" chain editor.

Playing with the new P32 mapping is the most fun I've had with Mixxx in quite a while. :)

@daschuer
Copy link
Member

daschuer commented Dec 6, 2016

Since you can assign Each EffectChain freely to one or more positions in the signal path, it could make sense to have 6 or 8 of them.

You can? I was not aware of that. AFAIK no skin makes this possible. Also, I am not sure how a controller could provide any control over individual parameters with that approach.

I am sure you are. It is the "Master" "Head" "CH1" CH2" "CH4" button grid in LateNight and Deere.

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 6, 2016

I am sure you are. It is the "Master" "Head" "CH1" CH2" "CH4" button grid in LateNight and Deere.

Oh, I misunderstood what you meant by assigning effect chains freely in the signal path. I thought you were saying EffectUnits (which can have chains within them) could be chained together arbitrarily.

@daschuer
Copy link
Member

daschuer commented Dec 6, 2016

If you enable lets say CH1 an all four effectUnits, The containing chains are chaint to a kind of mega chain.

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 6, 2016

If you enable lets say CH1 an all four effectUnits, The containing chains are chaint to a kind of mega chain.

I was under the impression the EffectUnits work in parallel in this case, not in series.

@rryan
Copy link
Member

rryan commented Dec 7, 2016

My stance is pretty much the same as back in 2014. I prefer a per-effect meta knob to a "first effect parameter is the effect metaknob" approach and I'm still holding out for the dream that effect chains would be a fun and useful feature for power users. Though admittedly I haven't been pushing on making them real (i.e. the effect chain editor, load/save support). :-/

I agree the chain selectors in Deere are confusing -- removing them until chains are a real feature SGTM.

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 7, 2016

I prefer a per-effect meta knob to a "first effect parameter is the effect metaknob" approach

Likewise. This approach provides the same benefits but is more flexible.

@rryan
Copy link
Member

rryan commented Feb 2, 2017

Thanks for the changes -- @sblaisot could you try the latest changes on Windows just ot make sure it didn't break?

@sblaisot
Copy link
Member

sblaisot commented Feb 2, 2017

here it is:
image

image

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 2, 2017

Great, looks good to me!

@rryan
Copy link
Member

rryan commented Feb 2, 2017

The focus border suffers an refresh issue. It is sometimes not removed / drawn.
If I move the mouse onto it, the issue is fixed.

We don't do a polish/unpolish when the focus control is changed via MIDI controller, so I'm not surprised that it wouldn't refresh -- I am surprised that it's flaky though -- I would expect it to either happen or not happen consistently (assuming the mouse is not over it so that it gets restyled b/c of hover state changes)

I think BindProperty could use an additional option like "repolish" or something so that if we want to style something via a property, we can make sure that when the property is changed via a control connection, we also unpolish/polish the widget.

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 2, 2017

I have not experienced any issues with the focus border. If there are reproducable issues, they can be fixed later, but I don't think merging this should be delayed any longer. The new UX and changes to the effects need wider testing and controller mappings need to be updated. Keeping this unmerged makes that cumbersome.

@rryan
Copy link
Member

rryan commented Feb 3, 2017

Ok, @daschuer please file a bug with some repro steps.

@rryan rryan merged commit 3a3c819 into mixxxdj:master Feb 3, 2017
@Be-ing Be-ing deleted the deere_new_effects branch February 3, 2017 02:14
This was referenced Feb 3, 2017
@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 3, 2017

@daschuer do you want to update Shade with this interface?

@daschuer
Copy link
Member

daschuer commented Feb 4, 2017

I have thought about it and I am unsure what suites for Shade.
Currently the effect region is always visible above the mixer region. The expandable FX view is "just" the effect details mode, which may not be used during mixing, just to setup the effects.

Shade is targeted for a Netbook with 1024 x 600 and low power CPU. That was the reason, I haver reduced the options to just four effects for 2 decks only, but with the preview by headphone feature.

Adopting then new effect region, introduced here, will finally require to have the effect region always visible, which is not a good option on a Netbook.

How about this:

  • Do not show the mataknobs
  • Add the combo-box effect selectors
  • show the focus, expand and collapse the effect region along with the focus

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 4, 2017

Do not show the mataknobs

All skins must expose the same ControlObjects or controller mappings will continue to be tied to skins. The metaknobs allow the skin to show 3 effects in each chain without having to show the detail of all the parameters all the time. This will save space.

Shade is targeted for a Netbook with 1024 x 600 and low power CPU. That was the reason, I haver reduced the options to just four effects for 2 decks only, but with the preview by headphone feature.

The effects units introduced in this PR can fit into a small area about the same size as the current effect units in Shade:
small

With #940 Deere fits quite well into a netbook sized area. If the only reason for Shade is to have something that is usable on netbooks, we may consider retiring it, unless you (or another contributor) commits to maintaining it. What do you think?

show the focus, expand and collapse the effect region along with the focus

The focus is for controller mappings and should only be shown if a controller mapping makes use of it.

@daschuer
Copy link
Member

daschuer commented Feb 4, 2017

The issue here is that current Shade does not require to show the effect region at all, because it has an always visible effect region above the mixer. The current effect region of Shade is only used to setup single effects. So it is basically the single effect view of Deere. Not the meta knob view as shown in the screen-shot. In the screenshot you cannot alter the effects without showing the effect region.
In Shade you can. This is a major space saver I do not like to give up.

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 4, 2017

You could keep the deck assignment switches, superknob, and dry/wet knob in the mixer area. The effects section could be hideable as it is now. IMO that is not good UX to have some effects controls in a different part of the screen from other controls. If you think it makes sense that is okay, but I encourage you try to think of another way. What is important is that all the skins expose the same controls; how they are arranged and how they look is up to the skin designer.

@daschuer
Copy link
Member

daschuer commented Feb 6, 2017

You could keep the deck assignment switches, superknob, and dry/wet knob in the mixer area. The effects section could be hideable as it is now. IMO that is not good UX to have some effects controls in a different part of the screen from other controls. If you think it makes sense that is okay, but I encourage you try to think of another way. What is important is that all the skins expose the same controls; how they are arranged and how they look is up to the skin designer.

That bugs me as well, that the effect region above the mixers is a bit out of sight. I have unfortunately not a good idea how to fix it.

How would you continue with Shade? It should on one hand follow the unified Effect GUI, on the other hand it should be usable on a low end 1024 x 600 Netbook without a controller.

What is your ideal Shade Skin? Maybe you could Scribble an Idea?

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 7, 2017

You could try moving the dry/wet and superknobs to the bottom of the mixer to bring them closer to the effect units. It would probably look better to move the knobs in the top row below the EQs and volume faders together with that.

You could try arranging the effect units like Deere but with the metaknob next to the effect combobox instead of stacked vertically. Without having the superknob and mix knob in the effect unit, there might be enough horizontal space for that.

@Be-ing Be-ing mentioned this pull request Feb 9, 2017
@timrae
Copy link
Contributor

timrae commented Feb 11, 2017

This doesn't seem to work as expected when I run it on my old Ubuntu 14.04. Here's a screenshot, compiled with the latest master:

screenshot from 2017-02-11 09 17 14

and the run log:
runlog.txt

@timrae
Copy link
Contributor

timrae commented Feb 11, 2017

gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
Using Qt version 4.8.6 in /usr/lib/x86_64-linux-gnu

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 11, 2017

It looks you're using this skin with a version before #1135 was merged. Fetch master and recompile.

@timrae
Copy link
Contributor

timrae commented Feb 11, 2017 via email

@timrae
Copy link
Contributor

timrae commented Feb 11, 2017

Sorry maybe I screwed something up, when I compiled and ran the #1117 branch it looks as expected

@timrae
Copy link
Contributor

timrae commented Feb 12, 2017

@Be-ing
Is the FX unit supposed to take the keyboard focus? Previously we could use the keyboard to tab between the library panes, but now the FX drop down boxes are also focusable, and the "tab-chain" becomes long and unintuitive. If the change happened to be intentional it has pretty deep consequences as my new controls for the library rely on tab changing focus only between the library panels.

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 12, 2017

That was not intentional. I cannot reproduce the effect selectors getting focused with tab on GNU/Linux though. I don't think this is an issue with the skin. There is probably a way to prevent the effect selectors from being tab focusable by putting something in src/widgets/weffectselector.cpp. Or it might need some change in src/skin/legacyskinparser.cpp. I haven't worked with tab indexes in Qt before, so I am not sure what is needed.

@timrae
Copy link
Contributor

timrae commented Feb 12, 2017

This is on Ubuntu 16.10 (QT version 4.8.7). Here's a video (watch the FX drop downs from left to right while I'm mashing the up/down keys -- especially the left one from 14s - 16s)
tabfocus.ogv.zip

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 13, 2017

Oh, I can reproduce that now. I wasn't using the up/down arrows before. I had tried tabbing without going up/down and did not see a focus border for the effect selectors so I didn't notice they had tab focus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants