Fix note velocity editing.

Don't abuse/leak selection when editing velocity (fix editing velocity of a single note actually editing velocity of every note who's velocity had previously been edited).
Properly preserve selection for MIDI operations in general.
Less crap method of delineating scroll events to canvas items (no exhaustive type cases needed in editor_canvas_events.cc).
Fix silly comment style in midi_region_view.h (hans: please note this and follow in the future).


git-svn-id: svn://localhost/ardour2/branches/3.0@4343 d708f5d6-7413-0410-9779-e7cbd77b26cf

7 files changed, 111 insertions, 105 deletions

diff --git a/gtk2_ardour/canvas-note-event.cc b/gtk2_ardour/canvas-note-event.cc
index bb369c7998..e4bbe038c0 100644
--- a/gtk2_ardour/canvas-note-event.cc
+++ b/gtk2_ardour/canvas-note-event.cc
@@ -198,20 +198,10 @@ CanvasNoteEvent::on_event(GdkEvent* ev)
 		}
 
 		if (ev->scroll.direction == GDK_SCROLL_UP) {
-			_region.note_selected(this, true);
-			if (_region.mouse_state() == MidiRegionView::SelectTouchDragging) {
-				// TODO: absolute velocity
-			} else {
-				_region.change_velocity(d_velocity, true);
-			}
+			_region.change_velocity(this, d_velocity, true);
 			return true;
 		} else if (ev->scroll.direction == GDK_SCROLL_DOWN) {
-			_region.note_selected(this, true);
-			if (_region.mouse_state() == MidiRegionView::SelectTouchDragging) {
-				// TODO: absolute velocity
-			} else {
-				_region.change_velocity(-d_velocity, true);
-			}
+			_region.change_velocity(this, -d_velocity, true);
 			return true;
 		} else {
 			return false;
diff --git a/gtk2_ardour/canvas-note-event.h b/gtk2_ardour/canvas-note-event.h
index 45403ed664..efb717823c 100644
--- a/gtk2_ardour/canvas-note-event.h
+++ b/gtk2_ardour/canvas-note-event.h
@@ -20,20 +20,20 @@
 #ifndef __gtk_ardour_canvas_midi_event_h__
 #define __gtk_ardour_canvas_midi_event_h__
 
-
+#include <boost/shared_ptr.hpp>
 #include <libgnomecanvasmm/text.h>
 #include <libgnomecanvasmm/widget.h>
-#include <ardour/midi_model.h>
 
 #include "rgb_macros.h"
 #include "ardour_ui.h"
 #include "ui_config.h"
-#include "simplerect.h"
-#include "midi_channel_selector.h"
+#include "interactive-item.h"
 
 class Editor;
 class MidiRegionView;
 
+namespace Evoral { class Note; }
+
 namespace Gnome {
 namespace Canvas {
 
@@ -49,7 +49,7 @@ namespace Canvas {
  *
  * A newer, better canvas should remove the need for all the ugly here.
  */
-class CanvasNoteEvent : public sigc::trackable {
+class CanvasNoteEvent : public sigc::trackable, public InteractiveItem {
 public:
 	CanvasNoteEvent(
 			MidiRegionView&                       region,
@@ -75,7 +75,6 @@ public:
 	void on_channel_selection_change(uint16_t selection);
 	
 	void show_channel_selector();
-	
 	void hide_channel_selector();
 
 	virtual void set_outline_color(uint32_t c) = 0;
diff --git a/gtk2_ardour/canvas-note.cc b/gtk2_ardour/canvas-note.cc
index 89916dbf4a..f505504198 100644
--- a/gtk2_ardour/canvas-note.cc
+++ b/gtk2_ardour/canvas-note.cc
@@ -17,7 +17,7 @@ CanvasNote::on_event(GdkEvent* ev)
 	static NoteEnd  note_end;
 	Editing::MidiEditMode edit_mode = _region.get_trackview().editor.current_midi_edit_mode();
 
-	switch(ev->type) {
+	switch (ev->type) {
 	case GDK_BUTTON_PRESS:
 		if (ev->button.button == 2 ||
 				(ev->button.button == 1 &&
@@ -26,7 +26,7 @@ CanvasNote::on_event(GdkEvent* ev)
 			event_x = ev->button.x;
 			middle_point = region_start + x1() + (x2() - x1()) / 2.0L;
 
-			if(event_x <= middle_point) {
+			if (event_x <= middle_point) {
 				cursor = Gdk::Cursor(Gdk::LEFT_SIDE);
 				note_end = NOTE_ON;
 			} else {
diff --git a/gtk2_ardour/editor_canvas_events.cc b/gtk2_ardour/editor_canvas_events.cc
index f821220cf3..7767eeeb8b 100644
--- a/gtk2_ardour/editor_canvas_events.cc
+++ b/gtk2_ardour/editor_canvas_events.cc
@@ -20,6 +20,7 @@
 #include <cstdlib>
 #include <cmath>
 #include <algorithm>
+#include <typeinfo>
 
 #include <pbd/stacktrace.h>
 
@@ -40,8 +41,7 @@
 #include "control_point.h"
 #include "canvas_impl.h"
 #include "simplerect.h"
-#include "canvas-note-event.h"
-#include "canvas-program-change.h"
+#include "interactive-item.h"
 
 #include "i18n.h"
 
@@ -59,9 +59,12 @@ Editor::track_canvas_scroll (GdkEventScroll* ev)
 	double wx, wy;
 	nframes64_t xdelta;
 	int direction = ev->direction;
-	CanvasNoteEvent     *midi_event = dynamic_cast<CanvasNoteEvent *>(track_canvas->get_item_at(ev->x, ev->y));
-	CanvasFlagRect      *flag_rect  = dynamic_cast<CanvasFlagRect *>(track_canvas->get_item_at(ev->x, ev->y));
-	CanvasFlagText      *flag_text  = dynamic_cast<CanvasFlagText *>(track_canvas->get_item_at(ev->x, ev->y));
+	
+	Gnome::Canvas::Item* item = track_canvas->get_item_at(ev->x, ev->y);
+	InteractiveItem* interactive_item = dynamic_cast<InteractiveItem*>(item);
+	if (interactive_item && interactive_item->on_event(reinterpret_cast<GdkEvent*>(ev))) {
+		return true;
+	}
 
   retry:
 	switch (direction) {
@@ -98,13 +101,6 @@ Editor::track_canvas_scroll (GdkEventScroll* ev)
 			current_stepping_trackview->step_height (true);
 			return true;
 		} else {
-			if(midi_event) {
-				return midi_event->on_event(reinterpret_cast<GdkEvent *>(ev));
-			} else if (flag_rect) {
-				return flag_rect->on_event(reinterpret_cast<GdkEvent *>(ev));				
-			} else if (flag_text) {
-				return flag_text->on_event(reinterpret_cast<GdkEvent *>(ev));				
-			}
 			scroll_tracks_up_line ();
 			return true;
 		}
@@ -138,13 +134,6 @@ Editor::track_canvas_scroll (GdkEventScroll* ev)
 			current_stepping_trackview->step_height (false);
 			return true;
 		} else {
-			if(midi_event) {
-				return midi_event->on_event(reinterpret_cast<GdkEvent *>(ev));
-			} else if (flag_rect) {
-				return flag_rect->on_event(reinterpret_cast<GdkEvent *>(ev));				
-			} else if (flag_text) {
-				return flag_text->on_event(reinterpret_cast<GdkEvent *>(ev));				
-			}
 			scroll_tracks_down_line ();
 			return true;
 		}
diff --git a/gtk2_ardour/interactive-item.h b/gtk2_ardour/interactive-item.h
new file mode 100644
index 0000000000..1e8db12e4b
--- /dev/null
+++ b/gtk2_ardour/interactive-item.h
@@ -0,0 +1,41 @@
+/*
+    Copyright (C) 2008 Paul Davis
+    Author: Dave Robillard
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#ifndef __ardour_interactive_item_h__
+#define __ardour_interactive_item_h__
+
+namespace Gnome {
+namespace Canvas {
+
+
+/** A canvas item that handles events.
+ * This is required so ardour can custom deliver events to specific items
+ * (e.g. to delineate scroll events) since Gnome::Canvas::Item::on_event
+ * is protected.
+ */
+class InteractiveItem {
+public:
+	virtual bool on_event(GdkEvent* ev) = 0;
+};
+
+
+} /* namespace Canvas */
+} /* namespace Gnome */
+
+#endif /* __ardour_interactive_item_h__ */
diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index f176f10367..cff4d358a8 100644
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -1338,7 +1338,7 @@ MidiRegionView::note_dropped(CanvasNoteEvent* ev, double dt, uint8_t dnote)
 			copy->set_note(new_pitch);
 			
 			command_remove_note(*i);
-			command_add_note(copy, true);
+			command_add_note(copy, (*i)->selected());
 
 			i = next;
 		}
@@ -1386,9 +1386,9 @@ MidiRegionView::snap_to_pixel(double x)
 }
 
 double
-MidiRegionView::get_position_pixels(void)
+MidiRegionView::get_position_pixels()
 {
-	nframes64_t  region_frame  = get_position();
+	nframes64_t region_frame = get_position();
 	return trackview.editor.frame_to_pixel(region_frame);
 }
 
@@ -1519,30 +1519,34 @@ MidiRegionView::commit_resizing(CanvasNote::NoteEnd note_end, double event_x, bo
 	apply_command();
 }
 
+void
+MidiRegionView::change_note_velocity(CanvasNoteEvent* event, int8_t velocity, bool relative)
+{
+	const boost::shared_ptr<Evoral::Note> copy(new Evoral::Note(*(event->note().get())));
+
+	if (relative) {
+		uint8_t new_velocity = copy->velocity() + velocity;
+		clamp_0_to_127(new_velocity);
+		copy->set_velocity(new_velocity);
+	} else {
+		copy->set_velocity(velocity);			
+	}
+
+	command_remove_note(event);
+	command_add_note(copy, event->selected());
+}
 
 void
-MidiRegionView::change_velocity(uint8_t velocity, bool relative)
+MidiRegionView::change_velocity(CanvasNoteEvent* ev, int8_t velocity, bool relative)
 {
 	start_delta_command(_("change velocity"));
+	
+	change_note_velocity(ev, velocity, relative);
+
 	for (Selection::iterator i = _selection.begin(); i != _selection.end();) {
 		Selection::iterator next = i;
 		++next;
-
-		CanvasNoteEvent *event = *i;
-		const boost::shared_ptr<Evoral::Note> copy(new Evoral::Note(*(event->note().get())));
-
-		if (relative) {
-			uint8_t new_velocity = copy->velocity() + velocity;
-			clamp_0_to_127(new_velocity);
-				
-			copy->set_velocity(new_velocity);
-		} else { // absolute
-			copy->set_velocity(velocity);			
-		}
-		
-		command_remove_note(event);
-		command_add_note(copy, true);
-		
+		change_note_velocity(*i, velocity, relative);
 		i = next;
 	}
 	
@@ -1557,13 +1561,13 @@ MidiRegionView::change_channel(uint8_t channel)
 		Selection::iterator next = i;
 		++next;
 
-		CanvasNoteEvent *event = *i;
+		CanvasNoteEvent* event = *i;
 		const boost::shared_ptr<Evoral::Note> copy(new Evoral::Note(*(event->note().get())));
 
 		copy->set_channel(channel);
 		
 		command_remove_note(event);
-		command_add_note(copy, true);
+		command_add_note(copy, event->selected());
 		
 		i = next;
 	}
diff --git a/gtk2_ardour/midi_region_view.h b/gtk2_ardour/midi_region_view.h
index d920a84ca0..6ee2210867 100644
--- a/gtk2_ardour/midi_region_view.h
+++ b/gtk2_ardour/midi_region_view.h
@@ -109,16 +109,13 @@ class MidiRegionView : public RegionView
 		}
 	};
 	
-	/**
-	 * Adds a new program change flag to the canvas
+	/** Add a new program change flag to the canvas.
 	 * @param program the MidiRegionView::ControlEvent to add
 	 * @param the text to display in the flag
 	 */
 	void add_pgm_change(ControlEvent& program, string displaytext);
 	
-	/**
-	 * Looks up in the automation list in the specified time and channel and sets keys
-	 * fields accordingly
+	/** Look up the given time and channel in the 'automation' and set keys accordingly.
 	 * @param time the time of the program change event
 	 * @param channel the MIDI channel of the event
 	 * @key a reference to an instance of MIDI::Name::PatchPrimaryKey whose fields will 
@@ -126,32 +123,28 @@ class MidiRegionView : public RegionView
 	 */
 	void get_patch_key_at(double time, uint8_t channel, MIDI::Name::PatchPrimaryKey& key);
 	
-	/**
-	 * changes the automation list data of old_program to the new values which correspond to new_patch
+	/** Change the 'automation' data of old_program to new values which correspond to new_patch.
 	 * @param old_program identifies the program change event which is to be altered
 	 * @param new_patch defines the new lsb, msb and program number which are to be set in the automation list data
 	 */
 	void alter_program_change(ControlEvent& old_program, const MIDI::Name::PatchPrimaryKey& new_patch);
 	
-	/**
-	 * alters a given program to the new given one (called on context menu select on CanvasProgramChange)
+	/** Alter a given program to the new given one.
+	 * (Called on context menu select on CanvasProgramChange)
 	 */
 	void program_selected(
 		ArdourCanvas::CanvasProgramChange& program, 
 		const MIDI::Name::PatchPrimaryKey& new_patch);
 	
-	/**
-	 * alters a given program to be its predecessor in the MIDNAM file
+	/** Alter a given program to be its predecessor in the MIDNAM file.
 	 */
 	void previous_program(ArdourCanvas::CanvasProgramChange& program);
 
-	/**
-	 * alters a given program to be its successor in the MIDNAM file
+	/** Alters a given program to be its successor in the MIDNAM file.
 	 */
 	void next_program(ArdourCanvas::CanvasProgramChange& program);
 	
-	/**
-	 * displays all program changed events in the region as flags on the canvas
+	/** Displays all program changed events in the region as flags on the canvas.
 	 */
 	void find_and_insert_program_change_flags();
 
@@ -180,21 +173,19 @@ class MidiRegionView : public RegionView
 	void move_selection(double dx, double dy);
 	void note_dropped(ArdourCanvas::CanvasNoteEvent* ev, double dt, uint8_t dnote);
 
-	/**
+	/** Get the region position in pixels.
 	 * This function is needed to subtract the region start in pixels
 	 * from world coordinates submitted by the mouse
 	 */
-	double get_position_pixels(void);
+	double get_position_pixels();
 
-	/**
-	 * This function is called by CanvasMidiNote when resizing starts,
-	 * i.e. when the user presses mouse-2 on the note
+	/** Begin resizing of some notes.
+	 * Called by CanvasMidiNote when resizing starts.
 	 * @param note_end which end of the note, NOTE_ON or NOTE_OFF
 	 */
 	void  begin_resizing(ArdourCanvas::CanvasNote::NoteEnd note_end);
 
-	/**
-	 * This function is called while the user moves the mouse when resizing notes
+	/** Update resizing notes while user drags.
 	 * @param note_end which end of the note, NOTE_ON or NOTE_OFF
 	 * @param x the difference in mouse motion, ie the motion difference if relative=true
 	 *           or the absolute mouse position (track-relative) if relative is false
@@ -202,24 +193,20 @@ class MidiRegionView : public RegionView
 	 */
 	void update_resizing(ArdourCanvas::CanvasNote::NoteEnd note_end, double x, bool relative);
 
-	/**
-	 * This function is called while the user releases the mouse button when resizing notes
+	/** Finish resizing notes when the user releases the mouse button.
 	 * @param note_end which end of the note, NOTE_ON or NOTE_OFF
 	 * @param event_x the absolute mouse position (track-relative)
 	 * @param relative true if relative resizing is taking place, false if absolute resizing
 	 */
 	void commit_resizing(ArdourCanvas::CanvasNote::NoteEnd note_end, double event_x, bool relative);
 
-	/**
-	 * This function is called while the user adjusts the velocity on a selection of notes
-	 * @param velocity the relative or absolute velocity, depending on the value of relative
-	 * @param relative true if the given velocity represents a delta to be applied to all notes, false
-	 *        if the absolute value of the note shoud be set
+	/** Adjust the velocity on a note, and the selection if applicable.
+	 * @param velocity the relative or absolute velocity
+	 * @param relative whether velocity is relative or absolute
 	 */
-	void change_velocity(uint8_t velocity, bool relative=false);
+	void change_velocity(ArdourCanvas::CanvasNoteEvent* ev, int8_t velocity, bool relative=false);
 	
-	/**
-	 * This function is called when the user adjusts the midi channel of a selection of notes
+	/** Change the channel of the selection.
 	 * @param channel - the channel number of the new channel, zero-based
 	 */
 	void change_channel(uint8_t channel);
@@ -233,24 +220,20 @@ class MidiRegionView : public RegionView
 		double                     current_x;
 	};
 	
-	/**
-	 * This function provides the snap function for region position relative coordinates
+	/** Snap a region relative pixel coordinate to pixel units.
 	 * for pixel units (double) instead of nframes64_t
 	 * @param x a pixel coordinate relative to region start
 	 * @return the snapped pixel coordinate relative to region start
 	 */
 	double snap_to_pixel(double x);
 
-	/**
-	 * This function provides the snap function for region position relative coordinates
-	 * for pixel units (double) instead of nframes64_t
+	/** Snap a region relative pixel coordinate to frame units.
 	 * @param x a pixel coordinate relative to region start
 	 * @return the snapped nframes64_t coordinate relative to region start
 	 */
 	nframes64_t snap_to_frame(double x);
 
-	/**
-	 * This function provides the snap function for region position relative coordinates
+	/** Snap a region relative frame coordinate to frame units.
 	 * @param x a pixel coordinate relative to region start
 	 * @return the snapped nframes64_t coordinate relative to region start
 	 */
@@ -258,11 +241,9 @@ class MidiRegionView : public RegionView
 	
   protected:
 
-    /**
-     * this constructor allows derived types
-     * to specify their visibility requirements
-     * to the TimeAxisViewItem parent class
-    */
+    /** Allows derived types to specify their visibility requirements
+     * to the TimeAxisViewItem parent class.
+     */
     MidiRegionView (ArdourCanvas::Group *,
 	                RouteTimeAxisView&,
 	                boost::shared_ptr<ARDOUR::MidiRegion>,
@@ -287,6 +268,8 @@ class MidiRegionView : public RegionView
 	
 	void midi_channel_mode_changed(ARDOUR::ChannelMode mode, uint16_t mask);
 	void midi_patch_settings_changed(std::string model, std::string custom_device_mode);
+	
+	void change_note_velocity(ArdourCanvas::CanvasNoteEvent* ev, int8_t velocity, bool relative=false);
 
 	void clear_selection_except(ArdourCanvas::CanvasNoteEvent* ev);
 	void clear_selection() { clear_selection_except(NULL); }