From 1fa666c090befbf2c75dbbb0885758a7da537f04 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Mon, 27 Oct 2025 09:27:13 -0600 Subject: [PATCH] refactoring of various clip GUI code This better defines when a region/trigger/track is being set and signals connected/disconnected to/from, along with some more code sharing between the MIDI (Pianoroll) and audio (AudioClipEditor) versions of things --- gtk2_ardour/audio_clip_editor.cc | 43 ++++---- gtk2_ardour/audio_clip_editor.h | 11 +- gtk2_ardour/cue_editor.cc | 178 +++++++++++++++++++------------ gtk2_ardour/cue_editor.h | 12 ++- gtk2_ardour/pianoroll.cc | 66 ++---------- gtk2_ardour/pianoroll.h | 5 +- 6 files changed, 155 insertions(+), 160 deletions(-) diff --git a/gtk2_ardour/audio_clip_editor.cc b/gtk2_ardour/audio_clip_editor.cc index 40009ce7e6..1ee43ef5d6 100644 --- a/gtk2_ardour/audio_clip_editor.cc +++ b/gtk2_ardour/audio_clip_editor.cc @@ -52,8 +52,9 @@ #include "editor.h" #include "keyboard.h" #include "region_view.h" -#include "verbose_cursor.h" +#include "timers.h" #include "ui_config.h" +#include "verbose_cursor.h" #include "pbd/i18n.h" @@ -468,24 +469,6 @@ AudioClipEditor::drop_waves () waves.clear (); } -void -AudioClipEditor::set_trigger (TriggerReference& tr) -{ - EC_LOCAL_TEMPO_SCOPE; - -// if (tr == ref) { -// return; -// } - - CueEditor::set_trigger (tr); - rec_box.show (); - - main_ruler->show (); - main_ruler->set_range (0, pixel_to_sample (_visible_canvas_width - 2.)); - - set_region (tr.trigger()->the_region()); -} - void AudioClipEditor::set_region (std::shared_ptr region) { @@ -791,16 +774,20 @@ AudioClipEditor::maybe_update () if (!playing_trigger) { - if (_drags->active() || !_region || !_track || !_track->triggerbox()) { + if (_drags->active() || !_track || !_track->triggerbox()) { return; } if (_track->triggerbox()->record_enabled() == Recording) { - _playhead_cursor->set_position (data_capture_duration); } + if (!_region) { + return; + } + } else { + if (playing_trigger->active ()) { if (playing_trigger->the_region()) { _playhead_cursor->set_position (playing_trigger->current_pos().samples() + playing_trigger->the_region()->start().samples()); @@ -809,6 +796,7 @@ AudioClipEditor::maybe_update () _playhead_cursor->set_position (0); } } + #if 0 } else if (view->midi_region()) { @@ -836,12 +824,20 @@ AudioClipEditor::maybe_update () } void -AudioClipEditor::unset (bool trigger_too) +AudioClipEditor::unset_region () { EC_LOCAL_TEMPO_SCOPE; drop_waves (); - CueEditor::unset (trigger_too); + + CueEditor::unset_region (); +} + +void +AudioClipEditor::unset_trigger () +{ + EC_LOCAL_TEMPO_SCOPE; + CueEditor::unset_trigger (); } Gdk::Cursor* @@ -897,4 +893,3 @@ AudioClipEditor::grid_type_chosen (Editing::GridType gt) grid_actions[Editing::GridTypeMinSec]->set_active (true); } } - diff --git a/gtk2_ardour/audio_clip_editor.h b/gtk2_ardour/audio_clip_editor.h index c132b74503..30790b7e1a 100644 --- a/gtk2_ardour/audio_clip_editor.h +++ b/gtk2_ardour/audio_clip_editor.h @@ -78,7 +78,6 @@ public: Gtk::Widget& contents (); - void set_trigger (ARDOUR::TriggerReference&); void set_region (std::shared_ptr r); void region_changed (const PBD::PropertyChange& what_changed); @@ -131,7 +130,11 @@ public: void maybe_update (); - bool idle_data_captured () { return false; } + bool idle_data_captured (); + + void set_overlay_text (std::string const &); + void hide_overlay_text (); + void show_overlay_text (); private: ArdourCanvas::Container* line_container; @@ -191,7 +194,9 @@ public: void show_count_in (std::string const &); void hide_count_in (); - void unset (bool trigger_too); + void unset_region (); + void unset_trigger (); + void load_shared_bindings (); void compute_fixed_ruler_scale (); diff --git a/gtk2_ardour/cue_editor.cc b/gtk2_ardour/cue_editor.cc index 5dbf560c46..b3e050c1ae 100644 --- a/gtk2_ardour/cue_editor.cc +++ b/gtk2_ardour/cue_editor.cc @@ -664,8 +664,6 @@ CueEditor::rec_enable_change () rec_blink_connection.disconnect (); count_in_connection.disconnect (); - std::cerr << "REC, state " << ref.box()->record_enabled() << std::endl; - switch (ref.box()->record_enabled()) { case Recording: rec_enable_button.set_active_state (Gtkmm2ext::ExplicitActive); @@ -1214,33 +1212,125 @@ CueEditor::set_track (std::shared_ptr t) { EC_LOCAL_TEMPO_SCOPE; + track_connections.drop_connections (); + _track = t; - _track->solo_control()->Changed.connect (object_connections, invalidator (*this), std::bind (&CueEditor::update_solo_display, this), gui_context()); + _track->solo_control()->Changed.connect (track_connections, invalidator (*this), std::bind (&CueEditor::update_solo_display, this), gui_context()); solo_button.set_sensitive (true); update_solo_display (); } +void +CueEditor::set_trigger (TriggerReference& tref) +{ + EC_LOCAL_TEMPO_SCOPE; + + unset_trigger (); + + ref = tref; + + TriggerPtr trigger (ref.trigger()); + + if (!trigger) { + return; + } + + rec_box.show (); + rec_enable_button.set_sensitive (true); + + idle_update_queued.store (0); + + ref.box()->Captured.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::data_captured, this, _1), gui_context()); + ref.box()->RecEnableChanged.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::rec_enable_change, this), gui_context()); + maybe_set_count_in (); + + Stripable* st = dynamic_cast (ref.box()->owner()); + assert (st); + + set_track (std::dynamic_pointer_cast (st->shared_from_this())); + + if (_track) { + _track->DropReferences.connect (track_connections, invalidator (*this), std::bind (&CueEditor::unset_trigger, this), gui_context()); + } + + trigger->PropertyChanged.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::trigger_prop_change, this, _1), gui_context()); + trigger->ArmChanged.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::trigger_arm_change, this), gui_context()); + + if (trigger) { + set_region (trigger->the_region()); + } else { + set_region (nullptr); + } + + _update_connection = Timers::super_rapid_connect (sigc::mem_fun (*this, &CueEditor::maybe_update)); +} + void CueEditor::set_region (std::shared_ptr r) { - unset (false); + unset_region (); _region = r; - if (_region) { - std::shared_ptr tmap = _region->tempo_map(); - if (tmap) { - start_local_tempo_map (tmap); - } - - if (!get_canvas()->is_visible()) { - _visible_pending_region = r; - } else { - _visible_pending_region.reset (); - } - } else { + if (!_region) { _visible_pending_region.reset (); + return; } + + + std::shared_ptr tmap = _region->tempo_map(); + + if (tmap) { + start_local_tempo_map (tmap); + } + + if (!get_canvas()->is_visible()) { + /* We can't really handle a region until we have a size, so + defer the rest of this until we do. + + XXX visibility is not a very good proxy for size (though + it's not terrible either. + */ + _visible_pending_region = r; + return; + } + + _visible_pending_region.reset (); + + r->DropReferences.connect (region_connections, invalidator (*this), std::bind (&CueEditor::unset_region, this), gui_context()); + r->PropertyChanged.connect (region_connections, invalidator (*this), std::bind (&CueEditor::region_prop_change, this, _1), gui_context()); +} + +void +CueEditor::unset_region () +{ + if (_local_tempo_map) { + end_local_tempo_map (); + } + + _history.clear (); + _update_connection.disconnect(); + region_connections.drop_connections (); + rec_blink_connection.disconnect (); + capture_connections.drop_connections (); + _region.reset (); +} + +void +CueEditor::unset_trigger () +{ + ref = TriggerReference (); + solo_button.set_active_state (Gtkmm2ext::Off); + solo_button.set_sensitive (false); + trigger_connections.drop_connections (); + track_connections.drop_connections (); + _track.reset (); + + /* Since set_trigger() calls set_region(), we need the symmetry here + * that calling unset_trigger() also calls unset_region(). + */ + + unset_region (); } void @@ -1284,29 +1374,6 @@ CueEditor::set_from_rsu (RegionUISettings& rsu) } } -void -CueEditor::set_trigger (TriggerReference& tref) -{ - EC_LOCAL_TEMPO_SCOPE; - - if (tref == ref) { - return; - } - - _update_connection.disconnect (); - object_connections.drop_connections (); - - ref = tref; - - Stripable* st = dynamic_cast (ref.box()->owner()); - assert (st); - - set_track (std::dynamic_pointer_cast (st->shared_from_this())); - set_region (ref.trigger()->the_region()); - - _update_connection = Timers::rapid_connect (sigc::mem_fun (*this, &CueEditor::maybe_update)); -} - void CueEditor::ruler_locate (GdkEventButton* ev) { @@ -1336,7 +1403,6 @@ CueEditor::maybe_set_count_in () EC_LOCAL_TEMPO_SCOPE; if (!ref.box()) { - std::cerr << "msci no box\n"; return; } @@ -1351,7 +1417,6 @@ CueEditor::maybe_set_count_in () count_in_to = ref.box()->start_time (valid); if (!valid) { - std::cerr << "no start time\n"; return; } @@ -1359,12 +1424,10 @@ CueEditor::maybe_set_count_in () Temporal::Beats const & a_q (tmap->quarters_at_sample (audible)); if ((count_in_to - a_q).get_beats() == 0) { - std::cerr << "not enough time\n"; return; } count_in_connection = ARDOUR_UI::Clock.connect (sigc::bind (sigc::mem_fun (*this, &CueEditor::count_in), ARDOUR_UI::clock_signal_interval())); - std::cerr << "count in started" << std::endl; } void @@ -1470,37 +1533,12 @@ CueEditor::idle_data_captured () return false; } -void -CueEditor::unset (bool trigger_too) -{ - if (_local_tempo_map) { - end_local_tempo_map (); - } - _history.clear (); - _update_connection.disconnect(); - object_connections.drop_connections (); - rec_blink_connection.disconnect (); - count_in_connection.disconnect (); - capture_connections.drop_connections (); - - _region.reset (); - - if (trigger_too) { - ref = TriggerReference (); - solo_button.set_active_state (Gtkmm2ext::Off); - solo_button.set_sensitive (false); - _track.reset (); - } else if (_track) { - /* re-subscribe to object_connections */ - set_track (_track); - } -} - void CueEditor::session_going_away () { EditingContext::session_going_away (); - unset (true); + unset_region (); + unset_trigger (); } void diff --git a/gtk2_ardour/cue_editor.h b/gtk2_ardour/cue_editor.h index 00041c984e..f11b96448f 100644 --- a/gtk2_ardour/cue_editor.h +++ b/gtk2_ardour/cue_editor.h @@ -214,12 +214,14 @@ class CueEditor : public EditingContext, public PBD::HistoryOwner void set_recording_length (Temporal::BBT_Offset bars); bool rec_button_press (GdkEventButton*); - void rec_enable_change (); + virtual void rec_enable_change (); void blink_rec_enable (bool); sigc::connection rec_blink_connection; sigc::connection _update_connection; - PBD::ScopedConnectionList object_connections; + PBD::ScopedConnectionList region_connections; + PBD::ScopedConnectionList trigger_connections; + PBD::ScopedConnectionList track_connections; void trigger_arm_change (); @@ -257,7 +259,8 @@ class CueEditor : public EditingContext, public PBD::HistoryOwner PBD::ScopedConnectionList capture_connections; samplecnt_t data_capture_duration; - virtual void unset (bool trigger_too); + virtual void unset_region (); + virtual void unset_trigger (); RegionUISettings region_ui_settings; void maybe_set_from_rsu (); @@ -268,4 +271,7 @@ class CueEditor : public EditingContext, public PBD::HistoryOwner bool _scroll_drag; bool hscroll_press (GdkEventButton*); bool hscroll_release (GdkEventButton*); + + virtual void region_prop_change (PBD::PropertyChange const & what_changed) {} + virtual void trigger_prop_change (PBD::PropertyChange const & what_changed) {} }; diff --git a/gtk2_ardour/pianoroll.cc b/gtk2_ardour/pianoroll.cc index 008f7e4881..7325be2c45 100644 --- a/gtk2_ardour/pianoroll.cc +++ b/gtk2_ardour/pianoroll.cc @@ -1350,54 +1350,6 @@ Pianoroll::trigger_prop_change (PBD::PropertyChange const & what_changed) } } -void -Pianoroll::region_prop_change (PBD::PropertyChange const & what_changed) -{ - EC_LOCAL_TEMPO_SCOPE; - - if (what_changed.contains (Properties::length)) { - /* XXX what, if anything, should we do here ? */ - } -} - -void -Pianoroll::set_trigger (TriggerReference & tref) -{ - EC_LOCAL_TEMPO_SCOPE; - - if (ref == tref) { - return; - } - - TriggerPtr trigger (tref.trigger()); - - CueEditor::set_trigger (tref); - - rec_box.show (); - rec_enable_button.set_sensitive (true); - - idle_update_queued.store (0); - - ref.box()->Captured.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::data_captured, this, _1), gui_context()); - ref.box()->RecEnableChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::rec_enable_change, this), gui_context()); - maybe_set_count_in (); - - if (_track) { - _track->DropReferences.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::unset, this, true), gui_context()); - } - - trigger->PropertyChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::trigger_prop_change, this, _1), gui_context()); - trigger->ArmChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::trigger_arm_change, this), gui_context()); - _update_connection.disconnect (); - _update_connection = Timers::super_rapid_connect (sigc::mem_fun (*this, &CueEditor::maybe_update)); - - if (trigger) { - set_region (trigger->the_region()); - } else { - set_region (nullptr); - } -} - void Pianoroll::make_a_region () { @@ -1426,12 +1378,18 @@ Pianoroll::make_a_region () } void -Pianoroll::unset (bool trigger_too) +Pianoroll::unset_region () { - CueEditor::unset (trigger_too); + CueEditor::unset_region (); view->set_region (nullptr); } +void +Pianoroll::unset_trigger () +{ + CueEditor::unset_trigger (); +} + void Pianoroll::build_cc_menu (ArdourWidgets::MetaButton* ccbtn) { @@ -1490,7 +1448,7 @@ Pianoroll::set_region (std::shared_ptr region) std::shared_ptr r (std::dynamic_pointer_cast (region)); - if (!r || !region) { + if (!r) { _update_connection.disconnect (); return; } @@ -1501,18 +1459,12 @@ Pianoroll::set_region (std::shared_ptr region) set_visible_channel (view->pick_visible_channel()); - r->DropReferences.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::unset, this, false), gui_context()); - r->PropertyChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::region_prop_change, this, _1), gui_context()); - /* Compute zoom level to show entire source plus some margin if possible */ zoom_to_show (timecnt_t (timepos_t (max_extents_scale() * max_zoom_extent ().second.samples()))); bg->display_region (*view); - _update_connection.disconnect (); - _update_connection = Timers::rapid_connect (sigc::mem_fun (*this, &Pianoroll::maybe_update)); - maybe_set_from_rsu (); } diff --git a/gtk2_ardour/pianoroll.h b/gtk2_ardour/pianoroll.h index 5d5d03a253..1784933119 100644 --- a/gtk2_ardour/pianoroll.h +++ b/gtk2_ardour/pianoroll.h @@ -78,7 +78,6 @@ class Pianoroll : public CueEditor int32_t get_grid_beat_divisions (Editing::GridType gt) const { return 1; } int32_t get_grid_music_divisions (Editing::GridType gt, uint32_t event_state) const { return 1; } - void set_trigger (ARDOUR::TriggerReference&); void set_region (std::shared_ptr); void set_track (std::shared_ptr); @@ -198,9 +197,9 @@ class Pianoroll : public CueEditor PBD::ScopedConnectionList view_connections; void maybe_update (); void trigger_prop_change (PBD::PropertyChange const &); - void region_prop_change (PBD::PropertyChange const &); - void unset (bool trigger_too); + void unset_region (); + void unset_trigger (); void bindings_changed ();