== Long-Standing 2.5 Todo - Markers fully working again in all

animation editors (DopeSheet, Graph Editor, NLA, Sequencer) ==

=== Usage Notes ===
In animation editors, marker operators will only be considered while
the mouse is hovering near/over the horizontal scrollbar (i.e. where
the markers usually appear). That means, in order to do something to
the markers, just position your cursor in line with the row of
markers, and then use the same hotkeys you'd use in the TimeLine (so,
unlike in 2.4x, no more need to hold down extra modifier keys for this
case). In the TimeLine, nothing changes, so you don't need to worry
about mouse placement there :)

=== Technical Details ===
Since early 2.5 versions, this functionality has been disabled, as the
markers were always getting evaluated first, and hence "swallowing"
all the events before the editor's own keymaps could access them.

In order to get this working again, I've had to give every marker
operator a "wrapper" invoke callback which performs some checking to
ensure that the mouse is close to the markers (vertically) before the
operator will try to be run. This wrapper also makes sure that once
the operator has finished running, that if it didn't manage to do
anything, then the editor's own keymaps get to have a go.

The vertical tolerance used is currently 30 pixels (as was used for
the borderselect operator).

=== Other Assorted Changes ===
* Gave marker operators dependent on having selected markers to
operate on suitable poll() callbacks. These new poll callbacks ensure
that there are selected markers for the operator to operate on,
further cutting down the number of places where markers may override
standard hotkeys (and avoiding calls to the wrappers too)
* Simplified some of the selection code
* Made some formatting tweaks for consistency, and in one case so that
my text editor's function-list display doesn't get confused

1 files changed, 164 insertions, 51 deletions

diff --git a/source/blender/editors/animation/anim_markers.c b/source/blender/editors/animation/anim_markers.c
index 35b163ec5be..200eca263f5 100644
--- a/source/blender/editors/animation/anim_markers.c
+++ b/source/blender/editors/animation/anim_markers.c
@@ -82,6 +82,8 @@ static ListBase *context_get_markers(const bContext *C)
 	return &CTX_data_scene(C)->markers;
 }
 
+/* --------------------------------- */
+
 /* Get the marker that is closest to this point */
 /* XXX for select, the min_dist should be small */
 TimeMarker *ED_markers_find_nearest_marker (ListBase *markers, float x) 
@@ -175,6 +177,8 @@ void ED_markers_get_minmax (ListBase *markers, short sel, float *first, float *l
 	*last= max;
 }
 
+/* --------------------------------- */
+
 /* Adds a marker to list of cfra elems */
 void add_marker_to_cfra_elem(ListBase *lb, TimeMarker *marker, short only_sel)
 {
@@ -218,6 +222,23 @@ void ED_markers_make_cfra_list(ListBase *markers, ListBase *lb, short only_sel)
 		add_marker_to_cfra_elem(lb, marker, only_sel);
 }
 
+/* --------------------------------- */
+
+/* This function checks if there are any markers selected at all */
+short ED_markers_has_selected(ListBase *markers)
+{
+	TimeMarker *marker;
+	
+	if (markers) {
+		for (marker = markers->first; marker; marker = marker->next) {
+			if (marker->flag & SELECT)
+				return 1;
+		}
+	}
+	
+	return 0;
+}
+
 /* ************* Marker Drawing ************ */
 
 /* function to draw markers */
@@ -239,10 +260,11 @@ static void draw_marker(View2D *v2d, TimeMarker *marker, int cfra, int flag)
 	
 	/* vertical line - dotted */
 #ifdef DURIAN_CAMERA_SWITCH
-	if ((marker->camera) || (flag & DRAW_MARKERS_LINES)) {
+	if ((marker->camera) || (flag & DRAW_MARKERS_LINES))
 #else
-	if (flag & DRAW_MARKERS_LINES) {
+	if (flag & DRAW_MARKERS_LINES)
 #endif
+	{
 		setlinestyle(3);
 		
 		if (marker->flag & SELECT)
@@ -333,6 +355,84 @@ void draw_markers_time(const bContext *C, int flag)
 	}
 }
 
+/* ************************ Marker Wrappers API ********************* */
+/* These wrappers allow marker operators to function within the confines 
+ * of standard animation editors, such that they can coexist with the 
+ * primary operations of those editors.
+ */
+
+/* maximum y-axis value (in region screen-space) that marker events should still be accepted for  */
+#define ANIMEDIT_MARKER_YAXIS_MAX 	30
+
+/* ------------------------ */
+
+/* special poll() which checks if there are selected markers first */
+static int ed_markers_poll_selected_markers(bContext *C)
+{
+	ListBase *markers = context_get_markers(C);
+	
+	/* first things first: markers can only exist in timeline views */
+	if (ED_operator_animview_active(C) == 0)
+		return 0;
+		
+	/* check if some marker is selected */
+	return ED_markers_has_selected(markers);
+}
+ 
+/* ------------------------ */ 
+
+/* Second-tier invoke() callback that performs context validation before running the  
+ * "custom"/third-tier invoke() callback supplied as the last arg (which would normally
+ * be the operator's invoke() callback elsewhere)
+ *
+ * < invoke_func: (fn(bContext*, wmOperator*, wmEvent*)=int) "standard" invoke function 
+ *			that operator would otherwise have used. If NULL, the operator's standard
+ *			exec() callback will be called instead in the appropriate places.
+ */
+static int ed_markers_opwrap_invoke_custom(bContext *C, wmOperator *op, wmEvent *evt, 
+		int (*invoke_func)(bContext*,wmOperator*,wmEvent*))
+{
+	ScrArea *sa = CTX_wm_area(C);
+	int retval = OPERATOR_PASS_THROUGH;
+	
+	/* only timeline view doesn't need calling-location validation as it's the only dedicated view */
+	if (sa->spacetype != SPACE_TIME) {
+		/* restrict y-values to within ANIMEDIT_MARKER_YAXIS_MAX of the view's vertical extents, including scrollbars */
+		if (evt->mval[1] > ANIMEDIT_MARKER_YAXIS_MAX) {
+			/* not ok... "pass-through" to let normal editor's operators have a chance at tackling this event... */
+			//printf("MARKER-WRAPPER-DEBUG: event mval[1] = %d, so over accepted tolerance\n", evt->mval[1]);
+			return OPERATOR_CANCELLED|OPERATOR_PASS_THROUGH;
+		}
+	}
+	
+	/* allow operator to run now */
+	if (invoke_func)
+		retval = invoke_func(C, op, evt);
+	else if (op->type->exec)
+		retval = op->type->exec(C, op);
+	else
+		BKE_report(op->reports, RPT_ERROR, "Programming error: operator doesn't actually have code to do anything!");
+		
+	/* return status modifications - for now, make this spacetype dependent as above */
+	if (sa->spacetype != SPACE_TIME) {
+		/* unless successful, must add "pass-through" to let normal operator's have a chance at tackling this event */
+		if (retval != OPERATOR_FINISHED)
+			retval |= OPERATOR_PASS_THROUGH;
+	}
+	
+	return retval;
+}
+
+/* standard wrapper - first-tier invoke() callback to be directly assigned to operator typedata
+ * for operators which don't need any special invoke calls. Any operators with special invoke calls
+ * though will need to implement their own wrapper which calls the second-tier callback themselves
+ * (passing through the custom invoke function they use)
+ */
+static int ed_markers_opwrap_invoke(bContext *C, wmOperator *op, wmEvent *evt)
+{
+	return ed_markers_opwrap_invoke_custom(C, op, evt, NULL);
+}
+
 /* ************************** add markers *************************** */
 
 /* add TimeMarker at curent frame */
@@ -376,6 +476,7 @@ static void MARKER_OT_add(wmOperatorType *ot)
 	
 	/* api callbacks */
 	ot->exec= ed_marker_add;
+	ot->invoke = ed_markers_opwrap_invoke;
 	ot->poll= ED_operator_animview_active;
 	
 	/* flags */
@@ -396,7 +497,7 @@ functions:
 
 	exit()	cleanup, send notifier
 
-	cancel() to escpae from modal
+	cancel() to escape from modal
 
 callbacks:
 
@@ -488,6 +589,11 @@ static int ed_marker_move_invoke(bContext *C, wmOperator *op, wmEvent *evt)
 	return OPERATOR_CANCELLED;
 }
 
+static int ed_marker_move_invoke_wrapper(bContext *C, wmOperator *op, wmEvent *evt)
+{
+	return ed_markers_opwrap_invoke_custom(C, op, evt, ed_marker_move_invoke);
+}
+
 /* note, init has to be called succesfully */
 static void ed_marker_move_apply(wmOperator *op)
 {
@@ -665,9 +771,9 @@ static void MARKER_OT_move(wmOperatorType *ot)
 	
 	/* api callbacks */
 	ot->exec= ed_marker_move_exec;
-	ot->invoke= ed_marker_move_invoke;
+	ot->invoke= ed_marker_move_invoke_wrapper;
 	ot->modal= ed_marker_move_modal;
-	ot->poll= ED_operator_animview_active;
+	ot->poll= ed_markers_poll_selected_markers;
 	
 	/* flags */
 	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO|OPTYPE_BLOCKING|OPTYPE_GRAB_POINTER;
@@ -744,6 +850,11 @@ static int ed_marker_duplicate_invoke(bContext *C, wmOperator *op, wmEvent *evt)
 	return ed_marker_move_invoke(C, op, evt);
 }
 
+static int ed_marker_duplicate_invoke_wrapper(bContext *C, wmOperator *op, wmEvent *evt)
+{
+	return ed_markers_opwrap_invoke_custom(C, op, evt, ed_marker_duplicate_invoke);
+}
+
 static void MARKER_OT_duplicate(wmOperatorType *ot)
 {
 	/* identifiers */
@@ -753,9 +864,9 @@ static void MARKER_OT_duplicate(wmOperatorType *ot)
 	
 	/* api callbacks */
 	ot->exec= ed_marker_duplicate_exec;
-	ot->invoke= ed_marker_duplicate_invoke;
+	ot->invoke= ed_marker_duplicate_invoke_wrapper;
 	ot->modal= ed_marker_move_modal;
-	ot->poll= ED_operator_animview_active;
+	ot->poll= ed_markers_poll_selected_markers;
 	
 	/* flags */
 	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO;
@@ -794,7 +905,7 @@ static int ed_marker_select(bContext *C, wmEvent *evt, int extend, int camera)
 	float viewx;
 	int x, y, cfra;
 	
-	if(markers == NULL)
+	if (markers == NULL)
 		return OPERATOR_PASS_THROUGH;
 
 	x= evt->x - CTX_wm_region(C)->winrct.xmin;
@@ -811,27 +922,27 @@ static int ed_marker_select(bContext *C, wmEvent *evt, int extend, int camera)
 	
 #ifdef DURIAN_CAMERA_SWITCH
 
-	if(camera) {
+	if (camera) {
 		Scene *scene= CTX_data_scene(C);
 		Base *base;
 		TimeMarker *marker;
 		int sel= 0;
-
+		
 		if (!extend)
 			scene_deselect_all(scene);
-
+		
 		for (marker= markers->first; marker; marker= marker->next) {
 			if(marker->frame==cfra) {
 				sel= (marker->flag & SELECT);
 				break;
 			}
 		}
-
+		
 		for (marker= markers->first; marker; marker= marker->next) {
-			if(marker->camera) {
-				if(marker->frame==cfra) {
+			if (marker->camera) {
+				if (marker->frame==cfra) {
 					base= object_in_scene(marker->camera, scene);
-					if(base) {
+					if (base) {
 						ED_base_object_select(base, sel);
 						if(sel)
 							ED_base_object_activate(C, base);
@@ -839,7 +950,7 @@ static int ed_marker_select(bContext *C, wmEvent *evt, int extend, int camera)
 				}
 			}
 		}
-
+		
 		WM_event_add_notifier(C, NC_SCENE|ND_OB_SELECT, scene);
 	}
 #endif
@@ -861,6 +972,11 @@ static int ed_marker_select_invoke(bContext *C, wmOperator *op, wmEvent *evt)
 	return ed_marker_select(C, evt, extend, camera);
 }
 
+static int ed_marker_select_invoke_wrapper(bContext *C, wmOperator *op, wmEvent *evt)
+{
+	return ed_markers_opwrap_invoke_custom(C, op, evt, ed_marker_select_invoke);
+}
+
 static void MARKER_OT_select(wmOperatorType *ot)
 {
 	/* identifiers */
@@ -869,7 +985,7 @@ static void MARKER_OT_select(wmOperatorType *ot)
 	ot->idname= "MARKER_OT_select";
 	
 	/* api callbacks */
-	ot->invoke= ed_marker_select_invoke;
+	ot->invoke= ed_marker_select_invoke_wrapper;
 	ot->poll= ED_operator_animview_active;
 	
 	/* flags */
@@ -917,24 +1033,18 @@ static int ed_marker_border_select_exec(bContext *C, wmOperator *op)
 	UI_view2d_region_to_view(v2d, xmin, ymin, &xminf, &yminf);	
 	UI_view2d_region_to_view(v2d, xmax, ymax, &xmaxf, &ymaxf);	
 	
-	/* XXX disputable */
-	if(yminf > 30.0f || ymaxf < 0.0f)
-		return 0;
-	
-	if(markers == NULL)
+	if (markers == NULL)
 		return 0;
 	
 	/* XXX marker context */
-	for(marker= markers->first; marker; marker= marker->next) {
+	for (marker= markers->first; marker; marker= marker->next) {
 		if ((marker->frame > xminf) && (marker->frame <= xmaxf)) {
 			switch (gesture_mode) {
 				case GESTURE_MODAL_SELECT:
-					if ((marker->flag & SELECT) == 0) 
-						marker->flag |= SELECT;
+					marker->flag |= SELECT;
 					break;
 				case GESTURE_MODAL_DESELECT:
-					if (marker->flag & SELECT) 
-						marker->flag &= ~SELECT;
+					marker->flag &= ~SELECT;
 					break;
 			}
 		}
@@ -946,6 +1056,11 @@ static int ed_marker_border_select_exec(bContext *C, wmOperator *op)
 	return 1;
 }
 
+static int ed_marker_select_border_invoke_wrapper(bContext *C, wmOperator *op, wmEvent *evt)
+{
+	return ed_markers_opwrap_invoke_custom(C, op, evt, WM_border_select_invoke);
+}
+
 static void MARKER_OT_select_border(wmOperatorType *ot)
 {
 	/* identifiers */
@@ -955,7 +1070,7 @@ static void MARKER_OT_select_border(wmOperatorType *ot)
 	
 	/* api callbacks */
 	ot->exec= ed_marker_border_select_exec;
-	ot->invoke= WM_border_select_invoke;
+	ot->invoke= ed_marker_select_border_invoke_wrapper;
 	ot->modal= WM_border_select_modal;
 	
 	ot->poll= ED_operator_animview_active;
@@ -975,17 +1090,11 @@ static int ed_marker_select_all_exec(bContext *C, wmOperator *op)
 	TimeMarker *marker;
 	int action = RNA_enum_get(op->ptr, "action");
 
-	if(markers == NULL)
+	if (markers == NULL)
 		return OPERATOR_CANCELLED;
 
 	if (action == SEL_TOGGLE) {
-		action = SEL_SELECT;
-		for(marker= markers->first; marker; marker= marker->next) {
-			if(marker->flag & SELECT) {
-				action = SEL_DESELECT;
-				break;
-			}
-		}
+		action = (ED_markers_has_selected(markers)) ? SEL_DESELECT : SEL_SELECT;
 	}
 	
 	for(marker= markers->first; marker; marker= marker->next) {
@@ -997,11 +1106,7 @@ static int ed_marker_select_all_exec(bContext *C, wmOperator *op)
 			marker->flag &= ~SELECT;
 			break;
 		case SEL_INVERT:
-			if (marker->flag & SELECT) {
-				marker->flag &= ~SELECT;
-			} else {
-				marker->flag |= SELECT;
-			}
+			marker->flag ^= SELECT;
 			break;
 		}
 	}
@@ -1021,6 +1126,7 @@ static void MARKER_OT_select_all(wmOperatorType *ot)
 	
 	/* api callbacks */
 	ot->exec= ed_marker_select_all_exec;
+	ot->invoke = ed_markers_opwrap_invoke;
 	ot->poll= ED_operator_animview_active;
 	
 	/* flags */
@@ -1039,12 +1145,12 @@ static int ed_marker_delete_exec(bContext *C, wmOperator *UNUSED(op))
 	TimeMarker *marker, *nmarker;
 	short changed= 0;
 	
-	if(markers == NULL)
+	if (markers == NULL)
 		return OPERATOR_CANCELLED;
 	
-	for(marker= markers->first; marker; marker= nmarker) {
+	for (marker= markers->first; marker; marker= nmarker) {
 		nmarker= marker->next;
-		if(marker->flag & SELECT) {
+		if (marker->flag & SELECT) {
 			BLI_freelinkN(markers, marker);
 			changed= 1;
 		}
@@ -1058,6 +1164,11 @@ static int ed_marker_delete_exec(bContext *C, wmOperator *UNUSED(op))
 	return OPERATOR_FINISHED;
 }
 
+static int ed_marker_delete_invoke_wrapper(bContext *C, wmOperator *op, wmEvent *evt)
+{
+	// XXX: must we keep these confirmations?
+	return ed_markers_opwrap_invoke_custom(C, op, evt, WM_operator_confirm);
+}
 
 static void MARKER_OT_delete(wmOperatorType *ot)
 {
@@ -1067,9 +1178,9 @@ static void MARKER_OT_delete(wmOperatorType *ot)
 	ot->idname= "MARKER_OT_delete";
 	
 	/* api callbacks */
-	ot->invoke= WM_operator_confirm;
+	ot->invoke= ed_marker_delete_invoke_wrapper;
 	ot->exec= ed_marker_delete_exec;
-	ot->poll= ED_operator_animview_active;
+	ot->poll= ed_markers_poll_selected_markers;
 	
 	/* flags */
 	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO;
@@ -1084,19 +1195,19 @@ static int ed_marker_make_links_scene_exec(bContext *C, wmOperator *op)
 	Scene *scene_to= BLI_findlink(&CTX_data_main(C)->scene, RNA_enum_get(op->ptr, "scene"));
 	TimeMarker *marker, *marker_new;
 
-	if(scene_to==NULL) {
+	if (scene_to==NULL) {
 		BKE_report(op->reports, RPT_ERROR, "Scene not found");
 		return OPERATOR_CANCELLED;
 	}
 
-	if(scene_to == CTX_data_scene(C)) {
+	if (scene_to == CTX_data_scene(C)) {
 		BKE_report(op->reports, RPT_ERROR, "Can't re-link markers into the same scene");
 		return OPERATOR_CANCELLED;
 	}
 
 	/* copy markers */
 	for (marker= markers->first; marker; marker= marker->next) {
-		if(marker->flag & SELECT) {
+		if (marker->flag & SELECT) {
 			marker_new= MEM_dupallocN(marker);
 			marker_new->prev= marker_new->next = NULL;
 			
@@ -1118,7 +1229,8 @@ static void MARKER_OT_make_links_scene(wmOperatorType *ot)
 
 	/* api callbacks */
 	ot->exec= ed_marker_make_links_scene_exec;
-	ot->poll= ED_operator_animview_active;
+	ot->invoke = ed_markers_opwrap_invoke;
+	ot->poll= ed_markers_poll_selected_markers;
 
 	/* flags */
 	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO;
@@ -1167,7 +1279,8 @@ static void MARKER_OT_camera_bind(wmOperatorType *ot)
 
 	/* api callbacks */
 	ot->exec= ed_marker_camera_bind_exec;
-	ot->poll= ED_operator_animview_active;
+	ot->invoke = ed_markers_opwrap_invoke;
+	ot->poll= ed_markers_poll_selected_markers;
 
 	/* flags */
 	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO;