== Long-Standing 2.5 Todo - Markers fully working again in all
authorJoshua Leung <aligorith@gmail.com>
Thu, 6 Jan 2011 02:35:12 +0000 (02:35 +0000)
committerJoshua Leung <aligorith@gmail.com>
Thu, 6 Jan 2011 02:35:12 +0000 (02:35 +0000)
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

source/blender/editors/animation/anim_markers.c
source/blender/editors/include/ED_markers.h
source/blender/editors/space_action/space_action.c
source/blender/editors/space_graph/space_graph.c
source/blender/editors/space_nla/space_nla.c
source/blender/editors/space_sequencer/space_sequencer.c

index 35b163ec5be5edea776c39cf57a422890de57dd4..200eca263f51cf20170be588289063ffa666f678 100644 (file)
@@ -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;
index e5e1f3cef109e36550f1a47fbada731ab76d98ad..54c1b473bd89b817010daad973645eb00ebc0bac 100644 (file)
@@ -51,6 +51,8 @@ void ED_markers_get_minmax(ListBase *markers, short sel, float *first, float *la
 
 void ED_markers_make_cfra_list(ListBase *markers, ListBase *lb, short sel);
 
+short ED_markers_has_selected(ListBase *markers);
+
 /* Operators ------------------------------ */
 
 /* called in screen_ops.c:ED_operatortypes_screen() */
index 678558e64500fe806caba112c5099abfb1fd2256..a2d116797e35cb728948fe2907ee23b83550bda8 100644 (file)
@@ -456,7 +456,7 @@ void ED_spacetype_action(void)
        art->init= action_main_area_init;
        art->draw= action_main_area_draw;
        art->listener= action_main_area_listener;
-       art->keymapflag= ED_KEYMAP_VIEW2D/*|ED_KEYMAP_MARKERS*/|ED_KEYMAP_ANIMATION|ED_KEYMAP_FRAMES;
+       art->keymapflag= ED_KEYMAP_VIEW2D|ED_KEYMAP_MARKERS|ED_KEYMAP_ANIMATION|ED_KEYMAP_FRAMES;
 
        BLI_addhead(&st->regiontypes, art);
        
index b3636f6ccf2032f155282190be50090963e7e930..d9d1f0ae3e4b81ecdeec1e4dbb8180c131a605b5 100644 (file)
@@ -599,7 +599,7 @@ void ED_spacetype_ipo(void)
        art->init= graph_main_area_init;
        art->draw= graph_main_area_draw;
        art->listener= graph_region_listener;
-       art->keymapflag= ED_KEYMAP_VIEW2D/*|ED_KEYMAP_MARKERS*/|ED_KEYMAP_ANIMATION|ED_KEYMAP_FRAMES;
+       art->keymapflag= ED_KEYMAP_VIEW2D|ED_KEYMAP_MARKERS|ED_KEYMAP_ANIMATION|ED_KEYMAP_FRAMES;
 
        BLI_addhead(&st->regiontypes, art);
        
index 3bff1d4159f95c6d254d27683e7a738331e625d2..814b7c0c2c307362267b74ab5589115999e06eb7 100644 (file)
@@ -514,7 +514,7 @@ void ED_spacetype_nla(void)
        art->init= nla_main_area_init;
        art->draw= nla_main_area_draw;
        art->listener= nla_main_area_listener;
-       art->keymapflag= ED_KEYMAP_VIEW2D/*|ED_KEYMAP_MARKERS*/|ED_KEYMAP_ANIMATION|ED_KEYMAP_FRAMES;
+       art->keymapflag= ED_KEYMAP_VIEW2D|ED_KEYMAP_MARKERS|ED_KEYMAP_ANIMATION|ED_KEYMAP_FRAMES;
 
        BLI_addhead(&st->regiontypes, art);
        
index 179db47ae126e0c349c25151244138d98d76366d..3dd8a29eac388978038cf0ec0a9d6c667dc534dd 100644 (file)
@@ -518,7 +518,7 @@ void ED_spacetype_sequencer(void)
        art->init= sequencer_main_area_init;
        art->draw= sequencer_main_area_draw;
        art->listener= sequencer_main_area_listener;
-       art->keymapflag= ED_KEYMAP_VIEW2D|ED_KEYMAP_FRAMES|ED_KEYMAP_ANIMATION;
+       art->keymapflag= ED_KEYMAP_VIEW2D|ED_KEYMAP_MARKERS|ED_KEYMAP_FRAMES|ED_KEYMAP_ANIMATION;
 
        BLI_addhead(&st->regiontypes, art);