-
Notifications
You must be signed in to change notification settings - Fork 48
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
Midi display #96
base: main
Are you sure you want to change the base?
Midi display #96
Conversation
… already exists (simple delete)
I was missing a "git add" - fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition, thanks!
Just a few nitpickings...
@@ -226,6 +226,21 @@ int MagicProcessorState::getLastController() const | |||
return midiMapper.getLastController(); | |||
} | |||
|
|||
int MagicProcessorState::getLastMidiChannel() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about exposing the midiMapper instead of cluttering the MagicProcessorState API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fine to me - I was just following the pattern I found in front of me, really shooting for "minimum perturbation".
auto cc = processorState->getLastController(); | ||
text += ("\nCC: " + (cc > 0 ? juce::String (cc) : "unknown")); | ||
|
||
g.setColour (juce::Colours::silver); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have those configurable...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the GuiItem should take care of that. Colo[u]r is something I've never spent any time on, but I'm sure it's easy to learn.
Also, font size and spacing et al. should be configurable in the GuiItem, maybe even the layout as well. It's hard to know where to stop on this stuff. What would be really cool would be a pop-up text-editor, analogous to the color inspector, that allows super general control like any good rich text editor. Then properties would have magic names like "$lastControllerColour" that would do the right thing, analogous to spreadsheet cell references.
For the short term, I'll take a look at what MIDI Learn offers and at least get it to that level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea: The MidiDisplayComponent should only display one item, and a pop-up list can support selecting which one. That way, each item can be dragged into a View and arranged via flexbox or in "contents mode" in the normal way.
Given that, why not simply use the LabelItem for this? It already has a value field, so it just becomes a matter of adding MIDI channel, noteNumber, velocity, etc., and lastController to its popup list.
@@ -90,6 +96,20 @@ void MidiParameterMapper::mapMidiController (int cc, const juce::String& paramet | |||
auto mappings = getMappingSettings(); | |||
|
|||
juce::ValueTree node { IDs::mapping, {{IDs::cc, cc}, {IDs::parameter, parameterID}} }; | |||
|
|||
// If mapping already there, remove it: | |||
if (mappings.isValid()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for my OCD, but please let's stay with 4 spaces ;-)
I intend to ad clang-format further down, so it is no longer a manual task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do try to keep to four spaces in your code, but sometimes I slip, because both Emacs and Xcode are set to two spaces for me. Yes, clang-format is a good answer!
Adds a simple MidiDisplay GuiItem similar to MidiLearn, which displays the last MIDI NoteOn parameters as well as MIDI controller, like most DAWs:
Also, if MidiLearn is repeated, it deletes the previous assignment, giving a simple delete mechanism for the mapping.