allanger/ardour
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Activity

do not use shared_ptr<T> to manage GUI objects

Browse Source 
This fixes a design mistake made way back in 2009 (80e3845982) that for
reasons now unknown used std::shared_ptr<T> to manage sysex and patch change
canvas items. This is incompatible with the overall (current) design of the
canvas, and in particular the idea that a container can be told to delete
its children.

This supplements the reversion done in e664fa5e63 and fixes crashes
caused by double-free's in sessions with sysex and patch changes displayed
in pianorolls
This commit is contained in:
Paul Davis
2026-01-08 11:38:13 -07:00
parent 7379cec4ef
commit 183d4741e8
2 changed files with 30 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
gtk2_ardour/midi_view.cc
View File

@@ -1105,7 +1105,7 @@ MidiView::find_canvas_note (Evoral::event_id_t id)
return nullptr;
}
std::shared_ptr<PatchChange>
PatchChange*
MidiView::find_canvas_patch_change (MidiModel::PatchChangePtr p)
{
PatchChanges::const_iterator f = _patch_changes.find (p);
@@ -1114,10 +1114,10 @@ MidiView::find_canvas_patch_change (MidiModel::PatchChangePtr p)
return f->second;
}
return std::shared_ptr<PatchChange>();
return nullptr;
}
std::shared_ptr<SysEx>
SysEx*
MidiView::find_canvas_sys_ex (MidiModel::SysExPtr s)
{
SysExes::const_iterator f = _sys_exes.find (s);
@@ -1126,7 +1126,7 @@ MidiView::find_canvas_sys_ex (MidiModel::SysExPtr s)
return f->second;
}
return std::shared_ptr<SysEx>();
return nullptr;
}
void
@@ -1442,7 +1442,7 @@ MidiView::display_patch_changes_on_channel (uint8_t channel, bool active_channel
}
for (MidiModel::PatchChanges::const_iterator i = _model->patch_changes().begin(); i != _model->patch_changes().end(); ++i) {
std::shared_ptr<PatchChange> p;
PatchChange* p;
if ((*i)->channel() != channel) {
continue;
@@ -1478,23 +1478,21 @@ MidiView::update_patch_changes ()
return;
}
for (PatchChanges::iterator p = _patch_changes.begin(); p != _patch_changes.end(); ++p) {
for (auto & [model,gui] : _patch_changes) {
std::shared_ptr<PatchChange> pc (p->second);
const timepos_t region_time (_midi_region->source_beats_to_region_time (p->first->time()));
const timepos_t region_time (_midi_region->source_beats_to_region_time (model->time()));
if (region_time < timepos_t() || region_time >= _midi_region->length()) {
pc->hide();
gui->hide();
} else {
const timepos_t flag_time = _midi_region->source_beats_to_absolute_time (p->first->time());
const timepos_t flag_time = _midi_region->source_beats_to_absolute_time (model->time());
const double flag_x = _editing_context.time_to_pixel (flag_time);
const double region_x = _editing_context.time_to_pixel (_midi_region->position());
pc->canvas_item()->set_position (ArdourCanvas::Duple (flag_x-region_x, 1.0));
pc->update_name ();
pc->show();
gui->canvas_item()->set_position (ArdourCanvas::Duple (flag_x-region_x, 1.0));
gui->update_name ();
gui->show();
}
}
}
@@ -1569,10 +1567,10 @@ MidiView::display_sysexes()
// CAIROCANVAS: no longer passing *i (the sysex event) to the
// SysEx canvas object!!!
std::shared_ptr<SysEx> sysex = find_canvas_sys_ex (sysex_ptr);
SysEx* sysex = find_canvas_sys_ex (sysex_ptr);
if (!sysex) {
sysex = std::shared_ptr<SysEx>(new SysEx (*this, _note_group, text, height, x, 1.0, sysex_ptr));
sysex = new SysEx (*this, _note_group, text, height, x, 1.0, sysex_ptr);
_sys_exes.insert (make_pair (sysex_ptr, sysex));
} else {
sysex->set_height (height);
@@ -1597,23 +1595,22 @@ MidiView::update_sysexes ()
int height = _midi_context.contents_height();
for (SysExes::iterator s = _sys_exes.begin(); s != _sys_exes.end(); ++s) {
for (auto & [model,gui] : _sys_exes) {
const timepos_t time (s->first->time());
std::shared_ptr<SysEx> sysex (s->second);
const timepos_t time (model->time());
// Show unless message is beyond the region bounds
if (_midi_region->source_relative_position (time) >= _midi_region->length() || time < _midi_region->start()) {
sysex->hide();
gui->hide();
continue;
} else {
sysex->show();
gui->show();
}
const double x = _editing_context.time_to_pixel (_midi_region->source_beats_to_region_time (time.beats()));
sysex->set_height (height);
sysex->item().set_position (ArdourCanvas::Duple (x, 1.0));
gui->set_height (height);
gui->item().set_position (ArdourCanvas::Duple (x, 1.0));
}
}
@@ -2198,14 +2195,12 @@ MidiView::add_canvas_patch_change (MidiModel::PatchChangePtr patch)
// so we need to do something more sophisticated to keep its color
// appearance (MidiPatchChangeFill/MidiPatchChangeInactiveChannelFill)
// up to date.
std::shared_ptr<PatchChange> patch_change = std::shared_ptr<PatchChange>(
new PatchChange(*this, _note_group->parent(),
height, x, 1.0,
_midi_track->instrument_info(),
patch,
_patch_change_outline,
_patch_change_fill)
);
PatchChange* patch_change = new PatchChange (*this, _note_group->parent(),
height, x, 1.0,
_midi_track->instrument_info(),
patch,
_patch_change_outline,
_patch_change_fill);
_patch_changes.insert (make_pair (patch, patch_change));
}

8
gtk2_ardour/midi_view.h
View File

@@ -500,8 +500,8 @@ class MidiView : public virtual sigc::trackable, public LineMerger
uint8_t get_channel_for_add (ARDOUR::MidiModel::TimeType time) const;
typedef std::unordered_map<std::shared_ptr<NoteType>, NoteBase*> Events;
typedef std::unordered_map<ARDOUR::MidiModel::PatchChangePtr, std::shared_ptr<PatchChange> > PatchChanges;
typedef std::unordered_map<ARDOUR::MidiModel::constSysExPtr, std::shared_ptr<SysEx> > SysExes;
typedef std::unordered_map<ARDOUR::MidiModel::PatchChangePtr, PatchChange*> PatchChanges;
typedef std::unordered_map<ARDOUR::MidiModel::constSysExPtr, SysEx*> SysExes;
typedef std::vector<NoteBase*> CopyDragEvents;
std::shared_ptr<ARDOUR::MidiTrack> _midi_track;
@@ -557,8 +557,8 @@ class MidiView : public virtual sigc::trackable, public LineMerger
NoteBase* find_canvas_note (Evoral::event_id_t id);
Events::iterator _optimization_iterator;
std::shared_ptr<PatchChange> find_canvas_patch_change (ARDOUR::MidiModel::PatchChangePtr p);
std::shared_ptr<SysEx> find_canvas_sys_ex (ARDOUR::MidiModel::SysExPtr s);
PatchChange* find_canvas_patch_change (ARDOUR::MidiModel::PatchChangePtr p);
SysEx* find_canvas_sys_ex (ARDOUR::MidiModel::SysExPtr s);
friend class VelocityDisplay;
void sync_velocity_drag (double factor);