From 9ef8d99892425d3a4494614f54738da810620c46 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Wed, 19 Mar 2025 20:18:52 -0600 Subject: [PATCH] bring order to the chaos of pianoroll automation visibility/editing --- gtk2_ardour/pianoroll.cc | 98 ++++++------ gtk2_ardour/pianoroll_midi_view.cc | 229 ++++++++++++++--------------- gtk2_ardour/pianoroll_midi_view.h | 6 +- 3 files changed, 160 insertions(+), 173 deletions(-) diff --git a/gtk2_ardour/pianoroll.cc b/gtk2_ardour/pianoroll.cc index ccd466e35c..7c843e29a1 100644 --- a/gtk2_ardour/pianoroll.cc +++ b/gtk2_ardour/pianoroll.cc @@ -282,10 +282,6 @@ Pianoroll::build_lower_toolbar () cc_dropdown2->add_elements (ArdourButton::Indicator); cc_dropdown3->add_elements (ArdourButton::Indicator); - cc_dropdown1->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_led_click), cc_dropdown1)); - cc_dropdown2->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_led_click), cc_dropdown2)); - cc_dropdown3->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_led_click), cc_dropdown3)); - rebuild_parameter_button_map (); /* Only need to do this once because i->first is the actual button, @@ -317,16 +313,20 @@ Pianoroll::build_lower_toolbar () modulation_button->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::automation_button_event), ARDOUR::MidiCCAutomation, MIDI_CTL_MSB_MODWHEEL)); expression_button->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::automation_button_event), ARDOUR::MidiCCAutomation, MIDI_CTL_MSB_EXPRESSION)); - cc_dropdown1->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_automation_button_event), cc_dropdown1), false); - cc_dropdown2->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_automation_button_event), cc_dropdown2), false); - cc_dropdown3->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_automation_button_event), cc_dropdown3), false); - velocity_button->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::automation_led_click), ARDOUR::MidiVelocityAutomation, 0)); pressure_button->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::automation_led_click), ARDOUR::MidiChannelPressureAutomation, 0)); bender_button->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::automation_led_click), ARDOUR::MidiPitchBenderAutomation, 0)); modulation_button->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::automation_led_click), ARDOUR::MidiCCAutomation, MIDI_CTL_MSB_MODWHEEL)); expression_button->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::automation_led_click), ARDOUR::MidiCCAutomation, MIDI_CTL_MSB_EXPRESSION)); + cc_dropdown1->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_automation_button_event), cc_dropdown1), false); + cc_dropdown2->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_automation_button_event), cc_dropdown2), false); + cc_dropdown3->signal_button_release_event().connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_automation_button_event), cc_dropdown3), false); + + cc_dropdown1->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_led_click), cc_dropdown1)); + cc_dropdown2->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_led_click), cc_dropdown2)); + cc_dropdown3->signal_led_clicked.connect (sigc::bind (sigc::mem_fun (*this, &Pianoroll::user_led_click), cc_dropdown3)); + _toolbox.pack_start (button_bar, false, false); } @@ -885,6 +885,10 @@ Pianoroll::trigger_rec_enable_change (ARDOUR::Trigger const & t) bool Pianoroll::button_press_handler (ArdourCanvas::Item* item, GdkEvent* event, ItemType item_type) { + if (event->type != GDK_BUTTON_PRESS) { + return false; + } + switch (event->button.button) { case 1: return button_press_handler_1 (item, event, item_type); @@ -2389,44 +2393,20 @@ Pianoroll::user_automation_button_event (GdkEventButton* ev, MetaButton* mb) } if (mb->is_led_click (ev)) { - user_led_click (ev, mb); - return true; + return false; } ParameterButtonMap::iterator i = parameter_button_map.find (mb); + if (i == parameter_button_map.end()) { return false; } - return automation_button_event (ev, i->second.type(), i->second.id()); -} - -bool -Pianoroll::automation_button_event (GdkEventButton* ev, Evoral::ParameterType type, int id) -{ - if (ev->button != 1) { - return false; + if (view) { + view->set_active_automation (i->second); } - SelectionOperation op = ArdourKeyboard::selection_type (ev->state); - - switch (ev->type) { - case GDK_BUTTON_RELEASE: - if (view) { - Evoral::Parameter param (type, _visible_channel, id); - - if (view->is_visible_automation (param) && (op == SelectionSet)) { - op = SelectionToggle; - } - - view->update_automation_display (param, op); - } - return true; - default: - break; - } - - return false; + return true; } void @@ -2436,22 +2416,27 @@ Pianoroll::user_led_click (GdkEventButton* ev, MetaButton* metabutton) return; } - /* Find the parameter */ - - ParameterButtonMap::iterator i; - for (i = parameter_button_map.begin(); i != parameter_button_map.end(); ++i) { - if (i->first == metabutton) { - break; - } - } + ParameterButtonMap::iterator i = parameter_button_map.find (metabutton); if (i == parameter_button_map.end()) { return; } - if (view) { - view->set_active_automation (i->second); + automation_button_event (ev, i->second.type(), i->second.id()); +} + +bool +Pianoroll::automation_button_event (GdkEventButton* ev, Evoral::ParameterType type, int id) +{ + if (ev->button != 1) { + return false; } + + if (view) { + view->set_active_automation (Evoral::Parameter (type, _visible_channel, id)); + } + + return true; } void @@ -2461,8 +2446,15 @@ Pianoroll::automation_led_click (GdkEventButton* ev, Evoral::ParameterType type, return; } - if (view) { - view->set_active_automation (Evoral::Parameter (type, _visible_channel, id)); + switch (ev->type) { + case GDK_BUTTON_RELEASE: + if (view) { + Evoral::Parameter param (type, _visible_channel, id); + view->toggle_visibility (param); + } + break; + default: + break; } } @@ -2474,17 +2466,17 @@ Pianoroll::automation_state_changed () for (ParameterButtonMap::iterator i = parameter_button_map.begin(); i != parameter_button_map.end(); ++i) { std::string str (ARDOUR::EventTypeMap::instance().to_symbol (i->second)); - /* Indicate visible automation state with selected/not-selected visual state */ + /* Indicate active automation state with selected/not-selected visual state */ - if (view->is_visible_automation (i->second)) { + if (view->is_active_automation (i->second)) { i->first->set_visual_state (Gtkmm2ext::Selected); } else { i->first->set_visual_state (Gtkmm2ext::NoVisualState); } - /* Indicate active automation state with explicit widget active state (LED) */ + /* Indicate visible automation state with explicit widget active state (LED) */ - if (view->is_active_automation (i->second)) { + if (view->is_visible_automation (i->second)) { i->first->set_active_state (Gtkmm2ext::ExplicitActive); } else { i->first->set_active_state (Gtkmm2ext::Off); diff --git a/gtk2_ardour/pianoroll_midi_view.cc b/gtk2_ardour/pianoroll_midi_view.cc index bcd7837ac6..bbc3e4d22a 100644 --- a/gtk2_ardour/pianoroll_midi_view.cc +++ b/gtk2_ardour/pianoroll_midi_view.cc @@ -327,7 +327,7 @@ PianorollMidiView::swap_automation_channel (int new_channel) /* Create the new */ for (auto const & p : new_params) { - update_automation_display (p, SelectionAdd); + toggle_visibility (p); } if (have_active) { @@ -359,8 +359,73 @@ PianorollMidiView::line_color_for (Evoral::Parameter const & param) return 0xff0000ff; } + +PianorollMidiView::AutomationDisplayState* +PianorollMidiView::find_or_create_automation_display_state (Evoral::Parameter const & param) +{ + CueAutomationMap::iterator i = automation_map.find (param); + + /* Step one: find the AutomationDisplayState object for this parameter, + * or create it if it does not already exist. + */ + + if (i != automation_map.end()) { + return &i->second; + } + + AutomationDisplayState* ads = nullptr; + bool was_empty = automation_map.empty(); + + if (param.type() == MidiVelocityAutomation) { + + if (!velocity_display) { + + /* Create and add to automation display map */ + + velocity_display = new PianorollVelocityDisplay (editing_context(), midi_context(), *this, *automation_group, 0x312244ff); + auto res = automation_map.insert (std::make_pair (param, AutomationDisplayState (*velocity_display, false))); + + ads = &((*res.first).second); + + for (auto & ev : _events) { + velocity_display->add_note (ev.second); + } + } + + } else { + + std::shared_ptr control = _midi_region->model()->control (param, true); + CueAutomationControl ac = std::dynamic_pointer_cast (control); + + if (!ac) { + return nullptr; + } + + CueAutomationLine line (new PianorollAutomationLine (ARDOUR::EventTypeMap::instance().to_symbol (param), + _editing_context, + *automation_group, + automation_group, + ac->alist(), + ac->desc())); + + line->set_insensitive_line_color (line_color_for (param)); + + AutomationDisplayState cad (ac, line, false); + + auto res = automation_map.insert (std::make_pair (param, cad)); + + ads = &((*res.first).second); + } + + if (was_empty) { + set_height (_height); + } + + return ads; +} + void -PianorollMidiView::update_automation_display (Evoral::Parameter const & param, SelectionOperation op) +PianorollMidiView::toggle_visibility (Evoral::Parameter const & param) { using namespace ARDOUR; @@ -381,111 +446,34 @@ PianorollMidiView::update_automation_display (Evoral::Parameter const & param, S return; } - CueAutomationMap::iterator i = automation_map.find (param); - AutomationDisplayState* ads = nullptr; + AutomationDisplayState* ads = find_or_create_automation_display_state (param); - if (i != automation_map.end()) { - - ads = &i->second; - - } else { - - if (op == SelectionRemove) { - /* remove it, but it doesn't exist yet, no worries */ - return; - } - - if (param.type() == MidiVelocityAutomation) { - - if (!velocity_display) { - - /* Create and add to automation display map */ - - velocity_display = new PianorollVelocityDisplay (editing_context(), midi_context(), *this, *automation_group, 0x312244ff); - auto res = automation_map.insert (std::make_pair (param, AutomationDisplayState (*velocity_display, false))); - - ads = &((*res.first).second); - - for (auto & ev : _events) { - velocity_display->add_note (ev.second); - } - } - - } else { - - std::shared_ptr control = _midi_region->model()->control (param, true); - CueAutomationControl ac = std::dynamic_pointer_cast (control); - - if (!ac) { - return; - } - - CueAutomationLine line (new PianorollAutomationLine (ARDOUR::EventTypeMap::instance().to_symbol (param), - _editing_context, - *automation_group, - automation_group, - ac->alist(), - ac->desc())); - - line->set_insensitive_line_color (line_color_for (param)); - - AutomationDisplayState cad (ac, line, false); - - auto res = automation_map.insert (std::make_pair (param, cad)); - - ads = &((*res.first).second); - } + if (!ads) { + return; } - switch (op) { - case SelectionSet: - /* hide the rest */ - for (auto & as : automation_map) { - as.second.hide (); - } - ads->set_height (automation_group->get().height()); - ads->show (); - internal_set_active_automation (param); - break; - - case SelectionAdd: - ads->set_height (automation_group->get().height()); - ads->show (); - break; - - case SelectionRemove: + if (ads->visible) { ads->hide (); - if (active_automation == ads) { + if (ads == active_automation) { unset_active_automation (); + /* no need to set height or emit signal */ + return; } - break; - - case SelectionToggle: - if (ads->visible) { - ads->hide (); - if (active_automation == ads) { - unset_active_automation (); - } - } else { - ads->set_height (automation_group->get().height()); - ads->show (); - internal_set_active_automation (param); - } - break; - - case SelectionExtend: - /* undefined in this context */ - break; + } else { + ads->show (); } set_height (_height); + AutomationStateChange (); /* EMIT SIGNAL */ } void PianorollMidiView::set_active_automation (Evoral::Parameter const & param) { - if (!internal_set_active_automation (param)) { - update_automation_display (param, SelectionSet); + AutomationDisplayState* ads = find_or_create_automation_display_state (param); + + if (ads) { + internal_set_active_automation (*ads); } } @@ -504,35 +492,31 @@ PianorollMidiView::unset_active_automation () AutomationStateChange(); /* EMIT SIGNAL */ } -bool -PianorollMidiView::internal_set_active_automation (Evoral::Parameter const & param) +void +PianorollMidiView::internal_set_active_automation (AutomationDisplayState& ads) { - bool exists = false; + if (active_automation == &ads) { + return; + } - for (auto & iter : automation_map) { - if (iter.first == param) { - if (iter.second.line) { - /* velocity does not have a line */ - iter.second.line->set_sensitive (true); - } else { - iter.second.velocity_display->set_sensitive (true); - } - active_automation = &iter.second; - exists = true; - } else { - if (iter.second.line) { - iter.second.line->set_sensitive (false); - } else { - iter.second.velocity_display->set_sensitive (false); - } + /* active automation MUST be visible and sensitive */ + + ads.set_sensitive (true); + ads.set_height (automation_group->get().height()); + ads.show (); + active_automation = &ads; + + /* Now desensitize the rest */ + + for (auto & [param,ds] : automation_map) { + if (&ds == &ads) { + continue; } + + ds.set_sensitive (false); } - if (exists) { - AutomationStateChange(); /* EMIT SIGNAL */ - } - - return exists; + AutomationStateChange(); /* EMIT SIGNAL */ } bool @@ -574,7 +558,6 @@ MergeableLine* PianorollMidiView::make_merger () { if (active_automation && active_automation->line) { - std::cerr << "Mergeable will use active automation @ " << active_automation << std::endl; return new MergeableLine (active_automation->line, active_automation->control, [](Temporal::timepos_t const& t) { return t; }, @@ -627,6 +610,16 @@ PianorollMidiView::AutomationDisplayState::hide () visible = false; } +void +PianorollMidiView::AutomationDisplayState::set_sensitive (bool yn) +{ + if (velocity_display) { + velocity_display->set_sensitive (yn); + } else if (line) { + line->set_sensitive (yn); + } +} + void PianorollMidiView::AutomationDisplayState::show () { diff --git a/gtk2_ardour/pianoroll_midi_view.h b/gtk2_ardour/pianoroll_midi_view.h index ef932f7fac..819e6de355 100644 --- a/gtk2_ardour/pianoroll_midi_view.h +++ b/gtk2_ardour/pianoroll_midi_view.h @@ -60,7 +60,7 @@ class PianorollMidiView : public MidiView void ghost_add_note (NoteBase*); void ghost_sync_selection (NoteBase*); - void update_automation_display (Evoral::Parameter const & param, ARDOUR::SelectionOperation); + void toggle_visibility (Evoral::Parameter const & param); void swap_automation_channel (int); void set_active_automation (Evoral::Parameter const &); bool is_active_automation (Evoral::Parameter const &) const; @@ -110,6 +110,7 @@ class PianorollMidiView : public MidiView void hide (); void show (); + void set_sensitive (bool); void set_height (double); }; @@ -128,7 +129,8 @@ class PianorollMidiView : public MidiView double _height; - bool internal_set_active_automation (Evoral::Parameter const &); + AutomationDisplayState* find_or_create_automation_display_state (Evoral::Parameter const &); + void internal_set_active_automation (AutomationDisplayState&); void unset_active_automation (); bool midi_canvas_group_event (GdkEvent*);