From 8b389ee8296f62d4a6e2541b366e6e24ee0c8bf4 Mon Sep 17 00:00:00 2001
From: Paul Davis <paul@linuxaudiosystems.com>
Date: Tue, 25 Mar 2025 14:49:49 -0600
Subject: [PATCH] amend 365e3ef8e278d to be "more right"

NoteBase-derived note objects must delete their children, because
often they are deleted long before the parent (group) is. However,
in MidiView::clear_events() we used to call _note_group->clear (true)
first, which would delete the canvas items owned by these objects,
without them knowing about it. This made it dangerous for them
to delete those same items in their destructors.

This reverses the ordering so that NoteBase objects are deleted first
(along with their canvas items) and after that we clear _note_group
which will address any danging canvas items created there that are
not owned by a NoteBase-derived object
---
 gtk2_ardour/hit.cc       | 4 +---
 gtk2_ardour/midi_view.cc | 7 ++++---
 gtk2_ardour/note.cc      | 4 +---
 gtk2_ardour/note_base.cc | 2 +-
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/gtk2_ardour/hit.cc b/gtk2_ardour/hit.cc
index 7f2a32af5e..da6fcece21 100644
--- a/gtk2_ardour/hit.cc
+++ b/gtk2_ardour/hit.cc
@@ -40,9 +40,7 @@ Hit::Hit (MidiView& region, Item* parent, double size, const std::shared_ptr<Not
 
 Hit::~Hit ()
 {
-	/* do not delete the visual note here, because that will be handled by
-	 * the parent
-	 */
+	delete _polygon;
 }
 
 void
diff --git a/gtk2_ardour/midi_view.cc b/gtk2_ardour/midi_view.cc
index f6f257a1f0..54a22bf419 100644
--- a/gtk2_ardour/midi_view.cc
+++ b/gtk2_ardour/midi_view.cc
@@ -876,12 +876,13 @@ MidiView::clear_events ()
 	// clear selection without signaling or trying to change state of event objects
 	_selection.clear ();
 	clear_ghost_events ();
-	/* This will delete all the _visual_note members of the NoteBase* elements in the _events map */
-	_note_group->clear (true);
-	/* The above did not actually delete the the NoteBase* elements, so do that now  */
 	for (auto & [model_note,gui_note] : _events) {
 		delete gui_note;
 	}
+	/* This will delete any danging canvas items that were not owned by the
+	 * NoteBase objects we just deleted.
+	 */
+	_note_group->clear (true);
 	_events.clear();
 	_patch_changes.clear();
 	_sys_exes.clear();
diff --git a/gtk2_ardour/note.cc b/gtk2_ardour/note.cc
index 2fcf37f763..7ccba8b808 100644
--- a/gtk2_ardour/note.cc
+++ b/gtk2_ardour/note.cc
@@ -39,9 +39,7 @@ Note::Note (MidiView& region, ArdourCanvas::Item* parent, const std::shared_ptr<
 
 Note::~Note ()
 {
-	/* do not delete the canvas item here, because that will be handled by
-	 * the parent
-	 */
+	delete _visual_note;
 }
 
 void
diff --git a/gtk2_ardour/note_base.cc b/gtk2_ardour/note_base.cc
index deb9c2527e..4a5d7e6f04 100644
--- a/gtk2_ardour/note_base.cc
+++ b/gtk2_ardour/note_base.cc
@@ -88,7 +88,7 @@ NoteBase::NoteBase(MidiView& v, bool with_events, const std::shared_ptr<NoteType
 NoteBase::~NoteBase()
 {
 	_view.note_deleted (this);
-	/* do not delete _text, parent will do so */
+	delete _text;
 }
 
 void