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

Midi display #96

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Midi display #96

wants to merge 7 commits into from

Conversation

josmithiii
Copy link

Adds a simple MidiDisplay GuiItem similar to MidiLearn, which displays the last MIDI NoteOn parameters as well as MIDI controller, like most DAWs:
image
Also, if MidiLearn is repeated, it deletes the previous assignment, giving a simple delete mechanism for the mapping.

@josmithiii
Copy link
Author

josmithiii commented Aug 4, 2023

I was missing a "git add" - fixed.

Copy link
Owner

@ffAudio ffAudio left a 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
Copy link
Owner

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?

Copy link
Author

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);
Copy link
Owner

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...

Copy link
Author

@josmithiii josmithiii Aug 5, 2023

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.

Copy link
Author

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()) {
Copy link
Owner

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.

Copy link
Author

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!

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

Successfully merging this pull request may close these issues.

2 participants