rationalize, consolidate and make work dbl-click editing for regions and markers, as part of the Drag model rather than discretely

4 files changed, 104 insertions, 115 deletions

diff --git a/gtk2_ardour/editor.h b/gtk2_ardour/editor.h
index 29e3ac2bef..47ce7d7828 100644
--- a/gtk2_ardour/editor.h
+++ b/gtk2_ardour/editor.h
@@ -1482,8 +1482,8 @@ class Editor : public PublicEditor, public PBD::ScopedConnectionList, public ARD
 
 	void edit_tempo_section (ARDOUR::TempoSection*);
 	void edit_meter_section (ARDOUR::MeterSection*);
-	void edit_tempo_marker (ArdourCanvas::Item*);
-	void edit_meter_marker (ArdourCanvas::Item*);
+	void edit_tempo_marker (TempoMarker&);
+	void edit_meter_marker (MeterMarker&);
 	void edit_control_point (ArdourCanvas::Item*);
         void edit_notes (TimeAxisViewItem&);
 
diff --git a/gtk2_ardour/editor_drag.cc b/gtk2_ardour/editor_drag.cc
index 1748c39736..d8209d4b51 100644
--- a/gtk2_ardour/editor_drag.cc
+++ b/gtk2_ardour/editor_drag.cc
@@ -872,7 +872,15 @@ RegionMoveDrag::finished (GdkEvent* ev, bool movement_occurred)
 	RegionMotionDrag::finished (ev, movement_occurred);
 	
 	if (!movement_occurred) {
+		
 		/* just a click */
+
+		if (was_double_click() && !_views.empty()) {
+			DraggingView dv = _views.front();
+			dv.view->show_region_editor ();
+			
+		}
+
 		return;
 	}
 
@@ -2146,6 +2154,10 @@ MeterMarkerDrag::setup_pointer_frame_offset ()
 void
 MeterMarkerDrag::motion (GdkEvent* event, bool first_move)
 {
+	if (!_marker->meter().movable()) {
+		return;
+	}
+
 	if (first_move) {
 
 		// create a dummy marker for visual representation of moving the
@@ -2191,6 +2203,13 @@ void
 MeterMarkerDrag::finished (GdkEvent* event, bool movement_occurred)
 {
 	if (!movement_occurred) {
+		if (was_double_click()) {
+			_editor->edit_meter_marker (*_marker);
+		}
+		return;
+	}
+
+	if (!_marker->meter().movable()) {
 		return;
 	}
 
@@ -2266,6 +2285,10 @@ TempoMarkerDrag::setup_pointer_frame_offset ()
 void
 TempoMarkerDrag::motion (GdkEvent* event, bool first_move)
 {
+	if (!_marker->tempo().movable()) {
+		return;
+	}
+
 	if (first_move) {
 
 		// create a dummy marker for visual representation of moving the
@@ -2310,6 +2333,13 @@ void
 TempoMarkerDrag::finished (GdkEvent* event, bool movement_occurred)
 {
 	if (!movement_occurred) {
+		if (was_double_click()) {
+			_editor->edit_tempo_marker (*_marker);
+		}
+		return;
+	}
+
+	if (!_marker->tempo().movable()) {
 		return;
 	}
 
@@ -3001,7 +3031,8 @@ MarkerDrag::finished (GdkEvent* event, bool movement_occurred)
 	if (!movement_occurred) {
 		
 		if (was_double_click()) {
-			cerr << "End of marker double click\n";
+			_editor->rename_marker (_marker);
+			return;
 		}
 
 		/* just a click, do nothing but finish
@@ -4128,10 +4159,24 @@ RangeMarkerBarDrag::finished (GdkEvent* event, bool movement_occurred)
 			_editor->new_transport_marker_context_menu (&event->button, _item);
 			break;
 		}
+
 	} else {
+
 		/* just a click, no pointer movement. remember that context menu stuff was handled elsewhere */
 
-		if (Keyboard::no_modifier_keys_pressed (&event->button) && _operation != CreateCDMarker) {
+		if (_operation == CreateTransportMarker) {
+
+			/* didn't drag, so just locate */
+
+			_editor->session()->request_locate (grab_frame(), _editor->session()->transport_rolling());
+
+		} else if (_operation == CreateCDMarker) {
+
+			/* didn't drag, but mark is already created so do
+			 * nothing */
+
+		} else { /* operation == CreateRangeMarker */
+			
 
 			framepos_t start;
 			framepos_t end;
diff --git a/gtk2_ardour/editor_mouse.cc b/gtk2_ardour/editor_mouse.cc
index e6772e42b2..131d0dd611 100644
--- a/gtk2_ardour/editor_mouse.cc
+++ b/gtk2_ardour/editor_mouse.cc
@@ -723,38 +723,30 @@ Editor::button_press_handler_1 (ArdourCanvas::Item* item, GdkEvent* event, ItemT
 	{
 		TempoMarker* m = reinterpret_cast<TempoMarker*> (item->get_data ("marker"));
 		assert (m);
-		if (m->tempo().movable ()) {
-			_drags->set (
-				new TempoMarkerDrag (
-					this,
-					item,
-					Keyboard::modifier_state_contains (event->button.state, Keyboard::CopyModifier)
-					),
-				event
-				);
-			return true;
-		} else {
-			return false;
-		}
+		_drags->set (
+			new TempoMarkerDrag (
+				this,
+				item,
+				Keyboard::modifier_state_contains (event->button.state, Keyboard::CopyModifier)
+				),
+			event
+			);
+		return true;
 	}
 
 	case MeterMarkerItem:
 	{
 		MeterMarker* m = reinterpret_cast<MeterMarker*> (item->get_data ("marker"));
 		assert (m);
-		if (m->meter().movable ()) {
-			_drags->set (
-				new MeterMarkerDrag (
-					this,
-					item,
-					Keyboard::modifier_state_contains (event->button.state, Keyboard::CopyModifier)
-					),
-				event
-				);
-			return true;
-		} else {
-			return false;
-		}
+		_drags->set (
+			new MeterMarkerDrag (
+				this,
+				item,
+				Keyboard::modifier_state_contains (event->button.state, Keyboard::CopyModifier)
+				),
+			event
+			);
+		return true;
 	}
 
 	case VideoBarItem:
@@ -1295,14 +1287,11 @@ Editor::button_press_handler (ArdourCanvas::Item* item, GdkEvent* event, ItemTyp
 {
 	if (event->type == GDK_2BUTTON_PRESS) {
 		_drags->mark_double_click ();
-		return false;
+		gdk_pointer_ungrab (GDK_CURRENT_TIME);
+		return true;
 	}
 
 	if (event->type != GDK_BUTTON_PRESS) {
-		if (event->type == GDK_2BUTTON_PRESS) {
-			gdk_pointer_ungrab (GDK_CURRENT_TIME);
-			return button_double_click_handler (item, event, item_type);
-		}
 		return false;
 	}
 
@@ -1427,52 +1416,6 @@ Editor::button_release_dispatch (GdkEventButton* ev)
 }
 
 bool
-Editor::button_double_click_handler (ArdourCanvas::Item* item, GdkEvent* event, ItemType item_type) {
-
-	if (event->button.button != 1) {
-		return false;
-	}
-
-	switch (item_type) {
-		case RegionItem:
-			RegionView *rv;
-			rv = clicked_regionview;
-			rv->show_region_editor ();
-			return true;
-		case NoteItem:
-		case PlayheadCursorItem:
-			break;
-		case MarkerItem:
-		case RangeMarkerBarItem:
-		case CdMarkerBarItem:
-			Marker* marker;
-			if ((marker = static_cast<Marker *> (item->get_data ("marker"))) == 0) {
-				break;
-			}
-			rename_marker (marker);
-			return true;
-		case TempoMarkerItem:
-			edit_tempo_marker (item);
-			return true;
-		case MeterMarkerItem:
-			edit_meter_marker (item);
-			return true;
-		case MarkerBarItem:
-		case TempoBarItem:
-		case MeterBarItem:
-		case TransportMarkerBarItem:
-		case StreamItem:
-			break;
-
-		default:
-			break;
-	}
-	return false;
-}
-
-
-
-bool
 Editor::button_release_handler (ArdourCanvas::Item* item, GdkEvent* event, ItemType item_type)
 {
 	framepos_t where = canvas_event_frame (event);
@@ -1512,13 +1455,40 @@ Editor::button_release_handler (ArdourCanvas::Item* item, GdkEvent* event, ItemT
 			show_region_properties ();
 			break;
 
-		case TempoMarkerItem:
-			edit_tempo_marker (item);
+		case TempoMarkerItem: {
+			Marker* marker;
+			TempoMarker* tempo_marker;
+			
+			if ((marker = reinterpret_cast<Marker *> (item->get_data ("marker"))) == 0) {
+				fatal << _("programming error: tempo marker canvas item has no marker object pointer!") << endmsg;
+				/*NOTREACHED*/
+			}
+			
+			if ((tempo_marker = dynamic_cast<TempoMarker*> (marker)) == 0) {
+				fatal << _("programming error: marker for tempo is not a tempo marker!") << endmsg;
+				/*NOTREACHED*/
+			}
+			
+			edit_tempo_marker (*tempo_marker);
 			break;
+		}
 
-		case MeterMarkerItem:
-			edit_meter_marker (item);
+		case MeterMarkerItem: {
+			Marker* marker;
+			MeterMarker* meter_marker;
+			
+			if ((marker = reinterpret_cast<Marker *> (item->get_data ("marker"))) == 0) {
+				fatal << _("programming error: tempo marker canvas item has no marker object pointer!") << endmsg;
+				/*NOTREACHED*/
+			}
+			
+			if ((meter_marker = dynamic_cast<MeterMarker*> (marker)) == 0) {
+				fatal << _("programming error: marker for meter is not a meter marker!") << endmsg;
+				/*NOTREACHED*/
+			}
+			edit_meter_marker (*meter_marker);
 			break;
+		}
 
 		case RegionViewName:
 			if (clicked_regionview->name_active()) {
diff --git a/gtk2_ardour/editor_tempodisplay.cc b/gtk2_ardour/editor_tempodisplay.cc
index b997d3bb5d..11c5513f68 100644
--- a/gtk2_ardour/editor_tempodisplay.cc
+++ b/gtk2_ardour/editor_tempodisplay.cc
@@ -343,41 +343,15 @@ Editor::edit_tempo_section (TempoSection* section)
 }
 
 void
-Editor::edit_tempo_marker (ArdourCanvas::Item *item)
+Editor::edit_tempo_marker (TempoMarker& tm)
 {
-	Marker* marker;
-	TempoMarker* tempo_marker;
-
-	if ((marker = reinterpret_cast<Marker *> (item->get_data ("marker"))) == 0) {
-		fatal << _("programming error: tempo marker canvas item has no marker object pointer!") << endmsg;
-		/*NOTREACHED*/
-	}
-
-	if ((tempo_marker = dynamic_cast<TempoMarker*> (marker)) == 0) {
-		fatal << _("programming error: marker for tempo is not a tempo marker!") << endmsg;
-		/*NOTREACHED*/
-	}
-
-	edit_tempo_section (&tempo_marker->tempo());
+	edit_tempo_section (&tm.tempo());
 }
 
 void
-Editor::edit_meter_marker (ArdourCanvas::Item *item)
+Editor::edit_meter_marker (MeterMarker& mm)
 {
-	Marker* marker;
-	MeterMarker* meter_marker;
-
-	if ((marker = reinterpret_cast<Marker *> (item->get_data ("marker"))) == 0) {
-		fatal << _("programming error: tempo marker canvas item has no marker object pointer!") << endmsg;
-		/*NOTREACHED*/
-	}
-
-	if ((meter_marker = dynamic_cast<MeterMarker*> (marker)) == 0) {
-		fatal << _("programming error: marker for meter is not a meter marker!") << endmsg;
-		/*NOTREACHED*/
-	}
-
-	edit_meter_section (&meter_marker->meter());
+	edit_meter_section (&mm.meter());
 }
 
 gint