mixxx
mixxx copied to clipboard
mixxxdj
Add quick effect support on stem
https://github.com/mixxxdj/mixxx/assets/7086688/1c51ca82-d110-443b-8232-f1ecf0b356cc
Depends on #13086
Two minor remarks regarding the floating widgets:
- It's sometimes difficult to distinguish the colored widget from the waveform which has the same colors. A border in a different color might help.
- The effect toggle button is very small an difficult to hit with the mouse
Yeah, that widget definitely would need some refactoring, but I think the deal was to remove it before we merge that feature, and is only here to facilitate testing, thus I didn't want to spend too much time with it.
I suggested to move the floating widgets in an own PR, to not block merging this PR and the PRs before. I'm not against these widgets, I just expect a longer review discussion than for the other code.
Ah I had misunderstood the plan for that. Sounds good then, I try and see if I can use the effect state indicator used in the effect bar instead of the mixer one.
Please note that UI changes (1st and 3rd commits) will be moved to https://github.com/mixxxdj/mixxx/pull/13515 once this PR is ready to be merged!
Remark after Mixxxing a lot (and having lots of fun) As I feel (and the way I DJ) it would prefer to have the fx standard disabled when a track is loaded, when I switch a lot loading tracks, I forget an effect is enabled and pushing/turning one stemcontrol activates all the others. So analogous to the [ChannelXStemY],mute (that is disabled by default) ([QuickEffectRackX_[ChannelXStemY]],enabled) also on 0. Or is there a certain idea to put these on enabled buh default?
I have removed the UI widget, so this PR should be ready for merge. If you wish to test it with the UI widget, you can use the branch feat/add-quick-fx-on-stem-with-ui
fx standard disabled when a track is loaded
I will look into this. There is already a setting for mute/volume, so it makes sense to extend it to FX IMO.
fx standard disabled when a track is loaded
I will look into this. There is already a setting for mute/volume, so it makes sense to extend it to FX IMO.
Before you implement this, have a look on the discussion in PR https://github.com/mixxxdj/mixxx/pull/11198
How is the state of the QML GUI inegration? I see the controls, but they are reset immoderately. What is the best option to test it without?
QML GUI integration should be functional and I intend to have it fully usable there, so if it's not, it's a bug that I should fix!
Maybe this relates to my old Qt version 6.2. Is there a minimum requirement that we need to met?
@JoergAtGithub How does the testing succeeds on your side? Since we don't officially ship QML anyway and my Qt is outdated it shall not block the merge.
Which Qt versions are known working?
@daschuer I am also using Qt 6.2 - it turns out that something was broken on QML. The control should now work correctly. Also turned out that I had applied the iterator change and committed by accident.
Before you implement this, have a look on the discussion in PR #11198
Ah I have a pending review on that PR - I guess we could add the feature suggested by @Eve00000 as part of that change or once this PR is merged so we can use the same setting for it?
@JoergAtGithub How does the testing succeeds on your side? Since we don't officially ship QML anyway and my Qt is outdated it shall not block the merge.
My last tests before removing the non-QML GUI code was flawless. But I can't start QML anymore, neither with main nor this PR (#13600). As this is unrelated, and Daniel tested the QML changes successful on his system, this shouldn't block the merge of this PR.