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

Add support for cycling through available hotcue colors #2384

Merged
merged 6 commits into from
Jan 9, 2020

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Dec 6, 2019

This adds two three new control objects:

hotcue_focus
hotcue_focus_color_prev
hotcue_focus_color_next

The first one "remembers" the hotcue number of the most recently activated/set hotcue. The other two allow users to cycle through the available colors in their palette and makes it possible to modify colors through controller mappings.

For the Roland DJ-505, I mapped the (previously unused) PARAMETER 2 +/- buttons to make assigning colors to cue points much more straightforward on this controller.

This adds two new control objects:

    hotcue_X_color_prev
    hotcue_X_color_next

These allow users to cycle through the available colors in their
palette and makes it possible to modify colors through controller
mappings (e.g. by mapping the parameter +/- buttons).
This makes assigning colors to cue points much more straightforward on
this controller. When pressing a performance pad (e.g. to activate a
hotcue, the mapping "remembers" the hotcue number and allows cycling
through the available colors of that hotcue using the PARAMETER 2 +/-
buttons.
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. From the code it looks already good. However I like to discuss two issues, unsure.

This introduced a massive number of 5027 new COs.

The the new controls are not learn-able.

Both can be solved by moving the controller script part to the c++ domain.

@Holzhaus
Copy link
Member Author

@daschuer I moved the logic to the C++ domain, so that this only introduces 3 COs per channel.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good now, only a minor comment.

@Holzhaus
Copy link
Member Author

Holzhaus commented Jan 4, 2020

OK, I removed it.

@Holzhaus
Copy link
Member Author

Holzhaus commented Jan 9, 2020

Are there any issues remaining that you want me to fix before merging thi? If not, I'll merge this tomorrow.

@Holzhaus Holzhaus merged commit 1a186a7 into mixxxdj:master Jan 9, 2020
@Be-ing
Copy link
Contributor

Be-ing commented Jan 11, 2020

I don't understand why this was added. Couldn't this already be done entirely by a controller script? Why should Mixxx have a concept of a focused hotcue? Why doesn't the controller script maintain that state?

@Holzhaus
Copy link
Member Author

That was my initial implementation. However, as @daschuer pointed out this a) adds a lot of new COs for cycling through colors and b) is not learnable/mappable by plain XML mappings. Shall I revert the merge?

@Holzhaus
Copy link
Member Author

The reason why I wanted a CO to cycle through colors at all is that it would be very cumbersome to implement it in JS. We currently do not have a way to know how many colors are in the current palette. And even if we had that, a JS implementation that just increments/decrements the color_id would only work until we switch to the QColor/QRgb-based approach. Also, we'd also keep an eye on palette changes (when a user switches/modified the palette).

@Be-ing
Copy link
Contributor

Be-ing commented Jan 11, 2020

Yeah, that would take some work to implement in JS. I was just wondering whether we need to expose the color palette to the JS environment. With this new feature, I don't think we do. If we don't need that, it will make #2399 a bit easier.

@Holzhaus
Copy link
Member Author

Holzhaus commented Jan 11, 2020

With the color_prev/color_next CO we don't need to know anything about the palette in JS.

If we revert the last 3 commits of this PR, we'd get rid of the hotcue_focus CO and restore the original approach that has hotcue_X_color_prev/hotcue_X_color_next COs for each hotcue and requires keeping track of the focused hotcue in JS. But that would also result in a large number of COs and make it unmappable for non-JS mappings.

@Be-ing
Copy link
Contributor

Be-ing commented Jan 11, 2020

Without exposing the color palette to JS, we impose this one way of mapping how to choose hotcue colors as the only way it can be mapped. I am doubting this is a great way to map this functionality. Scrolling through an ordered list one by one, especially when you can't see what is next and previous, is cumbersome and inefficient. In Mixxx 2.0, that was the only way to load different effects and it was a pain. It would be easier to pick a hotcue whose color to change (perhaps by pressing it with some modifier button held) then show the color palette on the controller's pads so they are all visible and the user could immediately choose any of them without scrolling. This would require exposing the color palette to JS.

a large number of COs

Why would that be a problem?

make it unmappable for non-JS mappings

I'm not concerned about this. The approach in this PR is already unusable with XML. There is no way to choose the focused hotcue via XML.

@daschuer
Copy link
Member

I am still convinced that his PR is a good step forward.

a large number of COs

Why would that be a problem?

Because of the time it will take to fire up and shut down Mixxx.

make it unmappable for non-JS mappings

I'm not concerned about this. The approach in this PR is already unusable with XML. There is no way to choose the focused hotcue via XML.

There is no need to, because it happens implicit by activating the desired hot cue.

Making Mixxx aware of the focused hot cue allows some more features. We may pop up the color selector when the controller switches the color. This way a user can see which color is next and can more easily scroll to it.

@Be-ing
Copy link
Contributor

Be-ing commented Jan 16, 2020

Ping @Holzhaus, thoughts on my comment above?

@Holzhaus
Copy link
Member Author

@Be-ing: Without exposing the color palette to JS, we impose this one way of mapping how to choose hotcue colors as the only way it can be mapped. I am doubting this is a great way to map this functionality.
Scrolling through an ordered list one by one, especially when you can't see what is next and previous, is cumbersome and inefficient.

It is, but it can somewhat be mitigated by popping up the color selector for a few seconds. Also, I fear this is the only way to map this properly on some controllers (see point 3 below).

It would be easier to pick a hotcue whose color to change (perhaps by pressing it with some modifier button held) then show the color palette on the controller's pads so they are all visible and the user could immediately choose any of them without scrolling. This would require exposing the color palette to JS.

Great idea!

Some thoughts:

  1. IMHO we still need a "focused" hotcue for this to work. The focus tracking does not necessarily have to happen in the C++ domain, but it could.
  2. As you pointed out, cycling through values is a pain for lists with many entries. But in contrast to the 20 effects, we only have 8 colors in the default palette. Also, the palette can be user modified so that common values can be moved to the front of the list. So that issue exists, but can be mitigated by the user to some degree.
  3. Your proposal only works on controllers that have hotcue pads with multi-color LEDs that also have "dim" versions of the colors (because otherwise it would be very confusing to see that you are in "color picker mode" and what color is currently selected. Other controllers still need a way to cycle through available colors IMHO. Same goes for users of controllers that do not have multi-color performance pads at all, but still want to assign colors for the on-screen feedback.
  4. The default palette only contains 8 values, but since users can use a custom palette, there needs to be a way to "scroll" through the palette if the number of palette colors is greater than the number of performance pads (e.g. by using the parameter buttons).
  5. The JS implementation of such a color picker would be quite complex (tracking palette changes at runtime, overriding behaviour of other hotcues, etc.). Since this color picker also changes the behaviour of all other performance pads, we can't simply cram it into the HotcueButton class, so we need to provide a dedicated component for this while still being flexible enough to allow custom hotcue behaviour (e.g. for CUE LOOP mode, LOOP mode, etc.).
  6. I'm not sure what would be the best way to map this: Obviously we cannot use SHIFT as modifier button. Idea: While the user holds the PARAM1+ button, the performance pads switch to color picker mode. Pressing PARAM1- (while still holding PARAM1+) will switch pages (if there are more colors in the palette than pads on the controller). Pressing the performance pads selects that color for the currently focused hotcue. Then release PARAM1+.
  7. In contrast to the current solution, all of this is only possible with JS mappings. However, I'd consider this an advanced feature. AFAIK, there is no way to change cue colors via controller in Serato for example. And almost none of the controller mappings that are included in Mixxx are XML-only, so I guess there's room for discussion on that issue.

@Be-ing
Copy link
Contributor

Be-ing commented Jan 17, 2020

Other controllers still need a way to cycle through available colors IMHO. Same goes for users of controllers that do not have multi-color performance pads at all, but still want to assign colors for the on-screen feedback.

I don't agree that this is necessary, nor does anyone else making DJ software (nor have I come across anyone else saying they want this from any DJ software). That's not to say it isn't a good idea and a neat feature, but I don't think there's much of a point if it is more cumbersome than using the mouse. I think scrolling through a list from a controller would be more cumbersome than using the mouse, even if there was on screen feedback. So I wouldn't suggest adding this to mappings for controllers that don't have multicolor LEDs.

The default palette only contains 8 values, but since users can use a custom palette, there needs to be a way to "scroll" through the palette if the number of palette colors is greater than the number of performance pads (e.g. by using the parameter buttons).

Do we really want to support this complexity? Currently only 8 colors are shown in the color picker popup. Letting users add an arbitrary number of colors to this seems like overkill to me.

The JS implementation of such a color picker would be quite complex (tracking palette changes at runtime, overriding behaviour of other hotcues, etc.). Since this color picker also changes the behaviour of all other performance pads, we can't simply cram it into the HotcueButton class, so we need to provide a dedicated component for this while still being flexible enough to allow custom hotcue behaviour (e.g. for CUE LOOP mode, LOOP mode, etc.).

Yes, the JS implementation would be a bit complex. We could change the HotcueButton class so that it can be switched to a mode for picking a color. Somehow all the HotcueButtons would need to be iterated over to enable that mode. I am thinking the color picking mode would be activated by pressing the hotcue button with a special modifier. I'm not exactly sure what would be the best way to organize the code would be without writing out the code, but I think we can take care of it (mostly) in the HotcueButton class.

I'm not sure what would be the best way to map this: Obviously we cannot use SHIFT as modifier button. Idea: While the user holds the PARAM1+ button, the performance pads switch to color picker mode. Pressing PARAM1- (while still holding PARAM1+) will switch pages (if there are more colors in the palette than pads on the controller). Pressing the performance pads selects that color for the currently focused hotcue. Then release PARAM1+.

This would require sticking with the "focused hotcue is the last activated hotcue" paradigm implemented in this PR. I do not think that is the best way to design this. It would prohibit changing the color of a cue on a playing deck unless the user interrupts playback by jumping to that cue, which I would think would not be desirable in some cases (or maybe most cases, depending on your workflow).

Instead, I propose holding some modifier and selecting which hotcue to pick the color for by pressing that hotcue button. Then the pads show the color palette, then switch back to showing the hotcues after a color is picked. No existing controller is designed for this, so it may be a challenge to squeeze it into mappings. Perhaps holding shift in addition to some infrequently used button could be used as the modifier.

In contrast to the current solution, all of this is only possible with JS mappings. However, I'd consider this an advanced feature. AFAIK, there is no way to change cue colors via controller in Serato for example. And almost none of the controller mappings that are included in Mixxx are XML-only, so I guess there's room for discussion on that issue.

I don't consider this a problem.

@Holzhaus
Copy link
Member Author

I think scrolling through a list from a controller would be more cumbersome than using the mouse, even if there was on screen feedback

Not always. The context switch is really bothersome IMHO. And most people would customize the palette that the most important 5 colors are near the front of the list. Setting a hotcue and pressing PARAM1+ 4 times is miles ahead of grabbing the mouse (or even the touchpad if there's none connected), searching the cue in the waveform, right clicking and picking a color in terms of speed and usability.

The default palette only contains 8 values, but since users can use a custom palette, there needs to be a way to "scroll" through the palette if the number of palette colors is greater than the number of performance pads (e.g. by using the parameter buttons).

Do we really want to support this complexity? Currently only 8 colors are shown in the color picker popup. Letting users add an arbitrary number of colors to this seems like overkill to me.

Maybe users like the Palette from Serato DJ Intro/Pro (18 Colors) or Rekordbox (16 colors). I think we should support that.

I'm not sure what would be the best way to map this: Obviously we cannot use SHIFT as modifier button. Idea: While the user holds the PARAM1+ button, the performance pads switch to color picker mode. Pressing PARAM1- (while still holding PARAM1+) will switch pages (if there are more colors in the palette than pads on the controller). Pressing the performance pads selects that color for the currently focused hotcue. Then release PARAM1+.

This would require sticking with the "focused hotcue is the last activated hotcue" paradigm implemented in this PR. I do not think that is the best way to design this. It would prohibit changing the color of a cue on a playing deck unless the user interrupts playback by jumping to that cue, which I would think would not be desirable in some cases (or maybe most cases, depending on your workflow).

I'm not sure that this is a problem. Usually I'm editing hotcues while I'm at home and practicing transitions. So It's very convenient to set/trigger a hotcue and then immediately edit its color. If it's not a new hotcue I want to listen to the part of the song it jumps to anyway, so that I know what kind of hotcue it is.

I don't think anybody edits hotcues during a live set while that track is playing on master out. Even if we implemented a hotcue picker instead of using the last hotcue, I wouldn't risk accidently jumping to a hotcue while editing colors because I release the modifier button too early or something like that.

By the way, that's another reason why I find it handy to map these color_prev/color_next buttons on PARAM1 -/+. You can safely use them during a live set, because there's no risk of accidently jumping to a hotcue involved.

Instead, I propose holding some modifier and selecting which hotcue to pick the color for by pressing that hotcue button. Then the pads show the color palette, then switch back to showing the hotcues after a color is picked. No existing controller is designed for this, so it may be a challenge to squeeze it into mappings. Perhaps holding shift in addition to some infrequently used button could be used as the modifier.

I think this is harder to implement and also less straightforward to use than just using the last activated hotcue.

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.

3 participants