Clean up note delta command code.

Use Variant to store the value and the same code path for all properties.

Factor out getting the value of whatever property instead of special casing the
handling.

Towards using this stuff for some fancy things...

3 files changed, 113 insertions, 89 deletions

diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h
index 1988c1a2d1..52bb5a6b27 100644
--- a/libs/ardour/ardour/midi_model.h
+++ b/libs/ardour/ardour/midi_model.h
@@ -21,17 +21,22 @@
 #ifndef __ardour_midi_model_h__
 #define __ardour_midi_model_h__
 
-#include <queue>
 #include <deque>
+#include <queue>
 #include <utility>
+
 #include <boost/utility.hpp>
 #include <glibmm/threads.h>
+
 #include "pbd/command.h"
-#include "ardour/libardour_visibility.h"
-#include "ardour/types.h"
+
 #include "ardour/automatable_sequence.h"
 #include "ardour/libardour_visibility.h"
+#include "ardour/libardour_visibility.h"
 #include "ardour/types.h"
+#include "ardour/types.h"
+#include "ardour/variant.h"
+
 #include "evoral/Note.hpp"
 #include "evoral/Sequence.hpp"
 
@@ -101,8 +106,15 @@ public:
 		void remove (const NotePtr note);
 		void side_effect_remove (const NotePtr note);
 
-		void change (const NotePtr note, Property prop, uint8_t new_value);
-		void change (const NotePtr note, Property prop, TimeType new_time);
+		void change (const NotePtr note, Property prop, uint8_t new_value) {
+			change(note, prop, Variant(new_value));
+		}
+
+		void change (const NotePtr note, Property prop, TimeType new_time) {
+			change(note, prop, Variant(new_time));
+		}
+
+		void change (const NotePtr note, Property prop, const Variant& new_value);
 
 		bool adds_or_removes() const {
 			return !_added_notes.empty() || !_removed_notes.empty();
@@ -110,15 +122,15 @@ public:
 
 		NoteDiffCommand& operator+= (const NoteDiffCommand& other);
 
+		static Variant get_value (const NotePtr note, Property prop);
+
 	private:
 		struct NoteChange {
 			NoteDiffCommand::Property property;
 			NotePtr note;
 			uint32_t note_id;
-			uint8_t  old_value;  // or...
-			TimeType old_time;   // this
-			uint8_t  new_value;  // or...
-			TimeType new_time;   // this
+			Variant old_value;
+			Variant new_value;
 		};
 
 		typedef std::list<NoteChange> ChangeList;
diff --git a/libs/ardour/ardour/variant.h b/libs/ardour/ardour/variant.h
index 8fd9c829f7..d99c0e4fd3 100644
--- a/libs/ardour/ardour/variant.h
+++ b/libs/ardour/ardour/variant.h
@@ -27,6 +27,7 @@
 #include <stdexcept>
 
 #include "ardour/libardour_visibility.h"
+#include "evoral/types.hpp"
 #include "pbd/compose.h"
 
 namespace ARDOUR {
@@ -37,6 +38,7 @@ class LIBARDOUR_API Variant
 public:
 	enum Type {
 		NOTHING, ///< Nothing (void)
+		BEATS,   ///< Beats+ticks
 		BOOL,    ///< Boolean
 		DOUBLE,  ///< C double (64-bit IEEE-754)
 		FLOAT,   ///< C float (32-bit IEEE-754)
@@ -54,6 +56,11 @@ public:
 	explicit Variant(int32_t value) : _type(INT)     { _int    = value; }
 	explicit Variant(int64_t value) : _type(LONG)    { _long   = value; }
 
+	explicit Variant(const Evoral::MusicalTime& beats)
+		: _type(BEATS)
+		, _beats(beats)
+	{}
+
 	/** Make a variant of a specific string type (string types only) */
 	Variant(Type type, const std::string& value)
 		: _type(type)
@@ -109,10 +116,57 @@ public:
 	int    get_int()    const { ensure_type(INT);    return _int;    }
 	long   get_long()   const { ensure_type(LONG);   return _long;   }
 
+	bool   operator==(bool v)   const { return _type == BOOL   && _bool == v; }
+	double operator==(double v) const { return _type == DOUBLE && _double == v; }
+	float  operator==(float v)  const { return _type == FLOAT  && _float == v; }
+	int    operator==(int v)    const { return _type == INT    && _int == v; }
+	long   operator==(long v)   const { return _type == LONG   && _long == v; }
+
+	Variant& operator=(bool v)   { _type = BOOL;   _bool = v;   return *this; }
+	Variant& operator=(double v) { _type = DOUBLE; _double = v; return *this; }
+	Variant& operator=(float v)  { _type = FLOAT;  _float = v;  return *this; }
+	Variant& operator=(int v)    { _type = INT;    _int = v;    return *this; }
+	Variant& operator=(long v)   { _type = LONG;   _long = v;   return *this; }
+
 	const std::string& get_path()   const { ensure_type(PATH);   return _string; }
 	const std::string& get_string() const { ensure_type(STRING); return _string; }
 	const std::string& get_uri()    const { ensure_type(URI);    return _string; }
 
+	bool operator==(const Variant& v) const {
+		if (_type != v._type) {
+			return false;
+		}
+
+		switch (_type) {
+		case NOTHING: return true;
+		case BEATS:   return _beats  == v._beats;
+		case BOOL:    return _bool   == v._bool;
+		case DOUBLE:  return _double == v._double;
+		case FLOAT:   return _float  == v._float;
+		case INT:     return _int    == v._int;
+		case LONG:    return _long   == v._long;
+		case PATH:
+		case STRING:
+		case URI:     return _string == v._string;
+		}
+
+		return false;
+	}
+
+	bool operator==(const Evoral::MusicalTime& v) const {
+		return _type == BEATS && _beats == v;
+	}
+
+	Variant& operator=(Evoral::MusicalTime v) {
+		_type  = BEATS;
+		_beats = v;
+		return *this;
+	}
+
+	const Evoral::MusicalTime& get_beats() const {
+		ensure_type(BEATS); return _beats;
+	}
+
 	Type type() const { return _type; }
 
 	static bool type_is_numeric(Type type) {
@@ -141,8 +195,9 @@ private:
 		}
 	}
 
-	Type        _type;    ///< Type tag
-	std::string _string;  ///< For all string types (PATH, STRING, URI)
+	Type                _type;    ///< Type tag
+	std::string         _string;  ///< PATH, STRING, URI
+	Evoral::MusicalTime _beats;   ///< BEATS
 
 	// Union of all primitive numeric types
 	union {
diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index f64a5f6d0c..e68068de2b 100644
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -164,85 +164,38 @@ MidiModel::NoteDiffCommand::side_effect_remove (const NotePtr note)
 	side_effect_removals.insert (note);
 }
 
-void
-MidiModel::NoteDiffCommand::change (const NotePtr note, Property prop,
-                                    uint8_t new_value)
+Variant
+MidiModel::NoteDiffCommand::get_value (const NotePtr note, Property prop)
 {
-	assert (note);
-
-	NoteChange change;
-
 	switch (prop) {
 	case NoteNumber:
-		if (new_value == note->note()) {
-			return;
-		}
-		change.old_value = note->note();
-		break;
+		return Variant(note->note());
 	case Velocity:
-		if (new_value == note->velocity()) {
-			return;
-		}
-		change.old_value = note->velocity();
-		break;
+		return Variant(note->velocity());
 	case Channel:
-		if (new_value == note->channel()) {
-			return;
-		}
-		change.old_value = note->channel();
-		break;
-
-
+		return Variant(note->channel());
 	case StartTime:
-		fatal << "MidiModel::DiffCommand::change() with integer argument called for start time" << endmsg;
-		abort(); /*NOTREACHED*/
-		break;
+		return Variant(note->time());
 	case Length:
-		fatal << "MidiModel::DiffCommand::change() with integer argument called for length" << endmsg;
-		abort(); /*NOTREACHED*/
-		break;
+		return Variant(note->length());
 	}
-
-	change.note = note;
-	change.property = prop;
-	change.new_value = new_value;
-
-	_changes.push_back (change);
 }
 
 void
-MidiModel::NoteDiffCommand::change (const NotePtr note, Property prop,
-                                    TimeType new_time)
+MidiModel::NoteDiffCommand::change (const NotePtr  note,
+                                    Property       prop,
+                                    const Variant& new_value)
 {
 	assert (note);
 
-	NoteChange change;
-
-	switch (prop) {
-	case NoteNumber:
-	case Channel:
-	case Velocity:
-		fatal << "MidiModel::NoteDiffCommand::change() with time argument called for note, channel or velocity" << endmsg;
-		break;
+	const NoteChange change = {
+		prop, note, 0, get_value(note, prop), new_value
+	};
 
-	case StartTime:
-		if (note->time() == new_time) {
-			return;
-		}
-		change.old_time = note->time();
-		break;
-	case Length:
-		if (note->length() == new_time) {
-			return;
-		}
-		change.old_time = note->length();
-		break;
+	if (change.old_value == new_value) {
+		return;
 	}
 
-	change.note = note;
-	change.property = prop;
-	change.new_time = new_time;
-
 	_changes.push_back (change);
 }
 
@@ -304,7 +257,7 @@ MidiModel::NoteDiffCommand::operator() ()
 					_model->remove_note_unlocked (i->note);
 					temporary_removals.insert (i->note);
 				}
-				i->note->set_note (i->new_value);
+				i->note->set_note (i->new_value.get_int());
 				break;
 
 			case StartTime:
@@ -312,7 +265,7 @@ MidiModel::NoteDiffCommand::operator() ()
 					_model->remove_note_unlocked (i->note);
 					temporary_removals.insert (i->note);
 				}
-				i->note->set_time (i->new_time);
+				i->note->set_time (i->new_value.get_beats());
 				break;
 
 			case Channel:
@@ -320,18 +273,18 @@ MidiModel::NoteDiffCommand::operator() ()
 					_model->remove_note_unlocked (i->note);
 					temporary_removals.insert (i->note);
 				}
-				i->note->set_channel (i->new_value);
+				i->note->set_channel (i->new_value.get_int());
 				break;
 
 				/* no remove-then-add required for these properties, since we do not index them
 				 */
 
 			case Velocity:
-				i->note->set_velocity (i->new_value);
+				i->note->set_velocity (i->new_value.get_int());
 				break;
 
 			case Length:
-				i->note->set_length (i->new_time);
+				i->note->set_length (i->new_value.get_beats());
 				break;
 
 			}
@@ -411,7 +364,7 @@ MidiModel::NoteDiffCommand::undo ()
 					_model->remove_note_unlocked (i->note);
 					temporary_removals.insert (i->note);
 				}
-				i->note->set_note (i->old_value);
+				i->note->set_note (i->old_value.get_int());
 				break;
 
 			case StartTime:
@@ -423,7 +376,7 @@ MidiModel::NoteDiffCommand::undo ()
 					_model->remove_note_unlocked (i->note);
 					temporary_removals.insert (i->note);
 				}
-				i->note->set_time (i->old_time);
+				i->note->set_time (i->old_value.get_beats());
 				break;
 
 			case Channel:
@@ -435,18 +388,18 @@ MidiModel::NoteDiffCommand::undo ()
 					_model->remove_note_unlocked (i->note);
 					temporary_removals.insert (i->note);
 				}
-				i->note->set_channel (i->old_value);
+				i->note->set_channel (i->old_value.get_int());
 				break;
 
 				/* no remove-then-add required for these properties, since we do not index them
 				 */
 
 			case Velocity:
-				i->note->set_velocity (i->old_value);
+				i->note->set_velocity (i->old_value.get_int());
 				break;
 
 			case Length:
-				i->note->set_length (i->old_time);
+				i->note->set_length (i->old_value.get_beats());
 				break;
 			}
 		}
@@ -593,9 +546,9 @@ MidiModel::NoteDiffCommand::marshal_change (const NoteChange& change)
 	{
 		ostringstream old_value_str (ios::ate);
 		if (change.property == StartTime || change.property == Length) {
-			old_value_str << change.old_time;
+			old_value_str << change.old_value.get_beats();
 		} else {
-			old_value_str << (unsigned int) change.old_value;
+			old_value_str << change.old_value.get_int();
 		}
 		xml_change->add_property ("old", old_value_str.str());
 	}
@@ -603,9 +556,9 @@ MidiModel::NoteDiffCommand::marshal_change (const NoteChange& change)
 	{
 		ostringstream new_value_str (ios::ate);
 		if (change.property == StartTime || change.property == Length) {
-			new_value_str << change.new_time;
+			new_value_str << change.new_value.get_beats();
 		} else {
-			new_value_str << (unsigned int) change.new_value;
+			new_value_str << change.new_value.get_int();
 		}
 		xml_change->add_property ("new", new_value_str.str());
 	}
@@ -640,7 +593,9 @@ MidiModel::NoteDiffCommand::unmarshal_change (XMLNode *xml_change)
 	if ((prop = xml_change->property ("old")) != 0) {
 		istringstream old_str (prop->value());
 		if (change.property == StartTime || change.property == Length) {
-			old_str >> change.old_time;
+			Evoral::MusicalTime old_time;
+			old_str >> old_time;
+			change.old_value = old_time;
 		} else {
 			int integer_value_so_that_istream_does_the_right_thing;
 			old_str >> integer_value_so_that_istream_does_the_right_thing;
@@ -654,7 +609,9 @@ MidiModel::NoteDiffCommand::unmarshal_change (XMLNode *xml_change)
 	if ((prop = xml_change->property ("new")) != 0) {
 		istringstream new_str (prop->value());
 		if (change.property == StartTime || change.property == Length) {
-			new_str >> change.new_time;
+			Evoral::MusicalTime new_time;
+			new_str >> new_time;
+			change.new_value = Variant(new_time);
 		} else {
 			int integer_value_so_that_istream_does_the_right_thing;
 			new_str >> integer_value_so_that_istream_does_the_right_thing;