Fix #129630: Inconsistent marker selection

- Modifications for marker selection to use the same behavior as
  key selection in the dopesheet.
- Animation editors now mask out events for the timeline (when there are markers).
- Resolve logical conflicts with the Dope sheet keymap.
- Add Shift Drag & Ctrl Drag for box select & de-select.

Pull Request: https://projects.blender.org/blender/blender/pulls/129841
This commit is contained in:
Campbell Barton
2024-11-12 10:37:51 +01:00
committed by Sybren A. Stüvel
parent f28bbfc702
commit 4306e746a5
16 changed files with 215 additions and 86 deletions
@@ -1127,6 +1127,10 @@ def km_markers(params):
{"properties": [("extend", True), ("camera", True)]}),
("marker.select_box", {"type": params.select_mouse, "value": 'CLICK_DRAG'},
{"properties": [("tweak", True)]}),
("marker.select_box", {"type": params.select_mouse, "value": 'CLICK_DRAG', "shift": True},
{"properties": [("tweak", True), ("mode", 'ADD')]}),
("marker.select_box", {"type": params.select_mouse, "value": 'CLICK_DRAG', "ctrl": True},
{"properties": [("tweak", True), ("mode", 'SUB')]}),
("marker.select_box", {"type": 'B', "value": 'PRESS'}, None),
*_template_items_select_actions(params, "marker.select_all"),
("marker.delete", {"type": 'X', "value": 'PRESS'}, None),
@@ -60,8 +60,7 @@
/** \name Marker API
* \{ */
/* helper function for getting the list of markers to work on */
static ListBase *context_get_markers(Scene *scene, ScrArea *area)
ListBase *ED_scene_markers_get(Scene *scene, ScrArea *area)
{
/* local marker sets... */
if (area) {
@@ -87,13 +86,13 @@ static ListBase *context_get_markers(Scene *scene, ScrArea *area)
ListBase *ED_context_get_markers(const bContext *C)
{
return context_get_markers(CTX_data_scene(C), CTX_wm_area(C));
return ED_scene_markers_get(CTX_data_scene(C), CTX_wm_area(C));
}
ListBase *ED_animcontext_get_markers(const bAnimContext *ac)
{
if (ac) {
return context_get_markers(ac->scene, ac->area);
return ED_scene_markers_get(ac->scene, ac->area);
}
return nullptr;
}
@@ -240,10 +239,12 @@ static bool ED_operator_markers_region_active(bContext *C)
return false;
}
static bool region_position_is_over_marker(View2D *v2d, ListBase *markers, float region_x)
static TimeMarker *region_position_is_over_marker(const View2D *v2d,
ListBase *markers,
float region_x)
{
if (markers == nullptr || BLI_listbase_is_empty(markers)) {
return false;
return nullptr;
}
float frame_at_position = UI_view2d_region_to_view_x(v2d, region_x);
@@ -251,7 +252,10 @@ static bool region_position_is_over_marker(View2D *v2d, ListBase *markers, float
float pixel_distance = UI_view2d_scale_get_x(v2d) *
fabsf(nearest_marker->frame - frame_at_position);
return pixel_distance <= UI_ICON_SIZE;
if (pixel_distance <= UI_ICON_SIZE) {
return nearest_marker;
}
return nullptr;
}
/* --------------------------------- */
@@ -1256,51 +1260,6 @@ static void deselect_markers(ListBase *markers)
}
}
static int select_timeline_marker_frame(ListBase *markers,
int frame,
bool extend,
bool wait_to_deselect_others)
{
TimeMarker *marker, *marker_cycle_selected = nullptr;
int ret_val = OPERATOR_FINISHED;
if (extend) {
wait_to_deselect_others = false;
}
/* support for selection cycling */
LISTBASE_FOREACH (TimeMarker *, marker, markers) {
if (marker->frame == frame) {
if (marker->flag & SELECT) {
marker_cycle_selected = static_cast<TimeMarker *>(marker->next ? marker->next :
markers->first);
break;
}
}
}
if (wait_to_deselect_others && marker_cycle_selected) {
ret_val = OPERATOR_RUNNING_MODAL;
}
/* if extend is not set, then deselect markers */
else {
if (extend == false) {
deselect_markers(markers);
}
LISTBASE_CIRCULAR_FORWARD_BEGIN (TimeMarker *, markers, marker, marker_cycle_selected) {
/* this way a not-extend select will always give 1 selected marker */
if (marker->frame == frame) {
marker->flag ^= SELECT;
break;
}
}
LISTBASE_CIRCULAR_FORWARD_END(TimeMarker *, markers, marker, marker_cycle_selected);
}
return ret_val;
}
static void select_marker_camera_switch(
bContext *C, bool camera, bool extend, ListBase *markers, int cfra)
{
@@ -1347,29 +1306,88 @@ static void select_marker_camera_switch(
#endif
}
static int ed_marker_select(
bContext *C, const int mval[2], bool extend, bool camera, bool wait_to_deselect_others)
static int ed_marker_select(bContext *C,
const int mval[2],
bool extend,
bool deselect_all,
bool camera,
bool wait_to_deselect_others)
{
/* NOTE: keep this functionality in sync with #ACTION_OT_clickselect.
* The logic here closely matches its internals.
* From a user perspective the functions should also behave in much the same way.
* The main difference with marker selection is support for selecting the camera.
*
* The variables (`sel_op` & `deselect_all`) have been included so marker
* selection can use identical checks to dope-sheet selection. */
ListBase *markers = ED_context_get_markers(C);
View2D *v2d = UI_view2d_fromcontext(C);
const View2D *v2d = UI_view2d_fromcontext(C);
int ret_val = OPERATOR_FINISHED;
TimeMarker *nearest_marker = region_position_is_over_marker(v2d, markers, mval[0]);
const float frame_at_mouse_position = UI_view2d_region_to_view_x(v2d, mval[0]);
const int cfra = ED_markers_find_nearest_marker_time(markers, frame_at_mouse_position);
const bool found = (nearest_marker != nullptr);
const bool is_selected = (nearest_marker && nearest_marker->flag & SELECT);
if (region_position_is_over_marker(v2d, markers, mval[0])) {
float frame_at_mouse_position = UI_view2d_region_to_view_x(v2d, mval[0]);
int cfra = ED_markers_find_nearest_marker_time(markers, frame_at_mouse_position);
ret_val = select_timeline_marker_frame(markers, cfra, extend, wait_to_deselect_others);
eSelectOp sel_op = (extend) ? (is_selected ? SEL_OP_SUB : SEL_OP_ADD) : SEL_OP_SET;
select_marker_camera_switch(C, camera, extend, markers, cfra);
if ((sel_op == SEL_OP_SET && found) || (!found && deselect_all)) {
sel_op = SEL_OP_ADD;
/* Rather than deselecting others, users may want to drag to box-select (drag from empty space)
* or tweak-translate an already selected item. If these cases may apply, delay deselection. */
if (wait_to_deselect_others && (!found || is_selected)) {
ret_val = OPERATOR_RUNNING_MODAL;
}
else {
/* Deselect all markers. */
deselect_markers(markers);
select_marker_camera_switch(C, camera, extend, markers, cfra);
}
}
else {
deselect_markers(markers);
if (found) {
TimeMarker *marker, *marker_cycle_selected = nullptr;
TimeMarker *marker_found = nullptr;
/* support for selection cycling */
LISTBASE_FOREACH (TimeMarker *, marker, markers) {
if (marker->frame == cfra) {
if (marker->flag & SELECT) {
marker_cycle_selected = static_cast<TimeMarker *>(marker->next ? marker->next :
markers->first);
break;
}
}
}
/* if extend is not set, then deselect markers */
LISTBASE_CIRCULAR_FORWARD_BEGIN (TimeMarker *, markers, marker, marker_cycle_selected) {
/* this way a not-extend select will always give 1 selected marker */
if (marker->frame == cfra) {
marker_found = marker;
break;
}
}
LISTBASE_CIRCULAR_FORWARD_END(TimeMarker *, markers, marker, marker_cycle_selected);
if (marker_found) {
if (sel_op == SEL_OP_SUB) {
marker_found->flag &= ~SELECT;
}
else {
marker_found->flag |= SELECT;
}
}
}
WM_event_add_notifier(C, NC_SCENE | ND_MARKERS, nullptr);
WM_event_add_notifier(C, NC_ANIMATION | ND_MARKERS, nullptr);
/* allowing tweaks, but needs OPERATOR_FINISHED, otherwise renaming fails, see #25987. */
return ret_val | OPERATOR_PASS_THROUGH;
return ret_val;
}
static int ed_marker_select_exec(bContext *C, wmOperator *op)
@@ -1392,8 +1410,11 @@ static int ed_marker_select_exec(bContext *C, wmOperator *op)
int mval[2];
mval[0] = RNA_int_get(op->ptr, "mouse_x");
mval[1] = RNA_int_get(op->ptr, "mouse_y");
bool deselect_all = true;
return ed_marker_select(C, mval, extend, camera, wait_to_deselect_others);
int ret_value = ed_marker_select(C, mval, extend, deselect_all, camera, wait_to_deselect_others);
return ret_value | OPERATOR_PASS_THROUGH;
}
static void MARKER_OT_select(wmOperatorType *ot)
@@ -1455,7 +1476,7 @@ static int ed_marker_box_select_invoke(bContext *C, wmOperator *op, const wmEven
ListBase *markers = ED_context_get_markers(C);
bool over_marker = region_position_is_over_marker(
v2d, markers, event->xy[0] - region->winrct.xmin);
v2d, markers, event->xy[0] - region->winrct.xmin) != nullptr;
bool tweak = RNA_boolean_get(op->ptr, "tweak");
if (tweak && over_marker) {
@@ -197,6 +197,14 @@ bool ED_time_scrub_event_in_region(const ARegion *region, const wmEvent *event)
return BLI_rcti_isect_pt_v(&rect, event->xy);
}
bool ED_time_scrub_event_in_region_poll(const wmWindow * /*win*/,
const ScrArea * /*area*/,
const ARegion *region,
const wmEvent *event)
{
return ED_time_scrub_event_in_region(region, event);
}
void ED_time_scrub_channel_search_draw(const bContext *C, ARegion *region, bDopeSheet *dopesheet)
{
GPU_matrix_push_projection();
@@ -9,6 +9,7 @@
#pragma once
struct Scene;
struct ScrArea;
struct TimeMarker;
struct bAnimContext;
struct bContext;
@@ -34,12 +35,23 @@ void ED_markers_draw(const bContext *C, int flag);
/** \name Backend API
* \{ */
/**
* Public API for getting markers from the scene & area.
*
* \return A #TimeMarker list.
*/
ListBase *ED_scene_markers_get(Scene *scene, ScrArea *area);
/**
* Public API for getting markers from context.
*
* \return A #TimeMarker list.
*/
ListBase *ED_context_get_markers(const bContext *C);
/**
* Public API for getting markers from "animation" context.
*
* \return A #TimeMarker list.
*/
ListBase *ED_animcontext_get_markers(const bAnimContext *ac);
@@ -12,6 +12,7 @@ struct bContext;
struct bDopeSheet;
struct wmEvent;
struct rcti;
struct wmWindow;
void ED_time_scrub_draw_current_frame(const ARegion *region,
const Scene *scene,
@@ -31,6 +32,11 @@ void ED_time_scrub_draw(const ARegion *region,
rcti ED_time_scrub_clamp_scroller_mask(const rcti &scroller_mask);
bool ED_time_scrub_event_in_region(const ARegion *region, const wmEvent *event);
/** Follow the #EventHandlerPoll function signature. */
bool ED_time_scrub_event_in_region_poll(const wmWindow *win,
const ScrArea *area,
const ARegion *region,
const wmEvent *event);
void ED_time_scrub_channel_search_draw(const bContext *C, ARegion *region, bDopeSheet *dopesheet);
void ED_time_scrub_region_rect_get(const ARegion *region, rcti *r_rect);
@@ -343,7 +343,8 @@ static void menu_types_add_from_keymap_items(bContext *C,
continue;
}
if (handler_base->poll == nullptr || handler_base->poll(region, win->eventstate)) {
if (handler_base->poll == nullptr || handler_base->poll(win, area, region, win->eventstate))
{
wmEventHandler_Keymap *handler = (wmEventHandler_Keymap *)handler_base;
wmEventHandler_KeymapResult km_result;
WM_event_get_keymaps_from_handler(wm, win, handler, &km_result);
+2 -9
View File
@@ -1842,13 +1842,6 @@ static void region_evaulate_visibility(ARegion *region)
region->visible = !hidden;
}
static bool event_in_markers_region(const ARegion *region, const wmEvent *event)
{
rcti rect = region->winrct;
rect.ymax = rect.ymin + UI_MARKER_MARGIN_Y;
return BLI_rcti_isect_pt_v(&rect, event->xy);
}
/**
* \param region: Region, may be nullptr when adding handlers for \a area.
*/
@@ -1895,11 +1888,11 @@ static void ed_default_handlers(
/* time-markers */
keymap = WM_keymap_ensure(wm->defaultconf, "Markers", SPACE_EMPTY, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_poll(handlers, keymap, event_in_markers_region);
WM_event_add_keymap_handler_poll(handlers, keymap, WM_event_handler_region_marker_poll);
/* time-scrub */
keymap = WM_keymap_ensure(wm->defaultconf, "Time Scrub", SPACE_EMPTY, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_poll(handlers, keymap, ED_time_scrub_event_in_region);
WM_event_add_keymap_handler_poll(handlers, keymap, ED_time_scrub_event_in_region_poll);
/* frame changing and timeline operators (for time spaces) */
keymap = WM_keymap_ensure(wm->defaultconf, "Animation", SPACE_EMPTY, RGN_TYPE_WINDOW);
@@ -1941,6 +1941,10 @@ static int mouse_action_keys(bAnimContext *ac,
const bool same_channel,
bool wait_to_deselect_others)
{
/* NOTE: keep this functionality in sync with #MARKER_OT_select.
* The logic here closely matches its internals.
* From a user perspective the functions should also behave in much the same way. */
eAnimFilter_Flags filter = ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE |
ANIMFILTER_LIST_CHANNELS;
@@ -158,7 +158,9 @@ static void action_main_region_init(wmWindowManager *wm, ARegion *region)
/* own keymap */
keymap = WM_keymap_ensure(wm->defaultconf, "Dopesheet", SPACE_ACTION, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_v2d_mask(&region->handlers, keymap);
WM_event_add_keymap_handler_poll(
&region->handlers, keymap, WM_event_handler_region_v2d_mask_no_marker_poll);
keymap = WM_keymap_ensure(wm->defaultconf, "Dopesheet Generic", SPACE_ACTION, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler(&region->handlers, keymap);
}
@@ -829,7 +829,7 @@ static void clip_preview_region_init(wmWindowManager *wm, ARegion *region)
WM_event_add_keymap_handler_v2d_mask(&region->handlers, keymap);
keymap = WM_keymap_ensure(wm->defaultconf, "Clip Time Scrub", SPACE_CLIP, RGN_TYPE_PREVIEW);
WM_event_add_keymap_handler_poll(&region->handlers, keymap, ED_time_scrub_event_in_region);
WM_event_add_keymap_handler_poll(&region->handlers, keymap, ED_time_scrub_event_in_region_poll);
keymap = WM_keymap_ensure(wm->defaultconf, "Clip Graph Editor", SPACE_CLIP, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_v2d_mask(&region->handlers, keymap);
@@ -176,7 +176,8 @@ static void graph_main_region_init(wmWindowManager *wm, ARegion *region)
/* own keymap */
keymap = WM_keymap_ensure(wm->defaultconf, "Graph Editor", SPACE_GRAPH, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_v2d_mask(&region->handlers, keymap);
WM_event_add_keymap_handler_poll(
&region->handlers, keymap, WM_event_handler_region_v2d_mask_no_marker_poll);
keymap = WM_keymap_ensure(wm->defaultconf, "Graph Editor Generic", SPACE_GRAPH, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler(&region->handlers, keymap);
}
@@ -164,7 +164,8 @@ static void nla_track_region_init(wmWindowManager *wm, ARegion *region)
/* own keymap */
/* own tracks map first to override some track keymaps */
keymap = WM_keymap_ensure(wm->defaultconf, "NLA Tracks", SPACE_NLA, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_v2d_mask(&region->handlers, keymap);
WM_event_add_keymap_handler_poll(
&region->handlers, keymap, WM_event_handler_region_v2d_mask_no_marker_poll);
/* now generic channels map for everything else that can apply */
keymap = WM_keymap_ensure(wm->defaultconf, "Animation Channels", SPACE_EMPTY, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_v2d_mask(&region->handlers, keymap);
@@ -442,7 +442,8 @@ static void sequencer_main_region_init(wmWindowManager *wm, ARegion *region)
/* Own keymap. */
keymap = WM_keymap_ensure(wm->defaultconf, "Sequencer", SPACE_SEQ, RGN_TYPE_WINDOW);
WM_event_add_keymap_handler_v2d_mask(&region->handlers, keymap);
WM_event_add_keymap_handler_poll(
&region->handlers, keymap, WM_event_handler_region_v2d_mask_no_marker_poll);
/* Add drop boxes. */
lb = WM_dropboxmap_find("Sequencer", SPACE_SEQ, RGN_TYPE_WINDOW);
+35 -1
View File
@@ -471,11 +471,45 @@ enum eWM_EventHandlerFlag {
};
ENUM_OPERATORS(eWM_EventHandlerFlag, WM_HANDLER_DO_FREE)
using EventHandlerPoll = bool (*)(const ARegion *region, const wmEvent *event);
using EventHandlerPoll = bool (*)(const wmWindow *win,
const ScrArea *area,
const ARegion *region,
const wmEvent *event);
wmEventHandler_Keymap *WM_event_add_keymap_handler(ListBase *handlers, wmKeyMap *keymap);
wmEventHandler_Keymap *WM_event_add_keymap_handler_poll(ListBase *handlers,
wmKeyMap *keymap,
EventHandlerPoll poll);
/**
* \return true when the `event` should be handled by the 2D views masked region.
*
* \note uses the #EventHandlerPoll signature.
*/
bool WM_event_handler_region_v2d_mask_poll(const wmWindow *win,
const ScrArea *area,
const ARegion *region,
const wmEvent *event);
/**
* \return true when the `event` is inside the marker region.
*
* \note There are no checks that markers are displayed.
*/
bool WM_event_handler_region_marker_poll(const wmWindow *win,
const ScrArea *area,
const ARegion *region,
const wmEvent *event);
/**
* A version of #WM_event_handler_region_v2d_mask_poll which excludes events
* (returning false) in the marker region.
*
* \note uses the #EventHandlerPoll signature.
*/
bool WM_event_handler_region_v2d_mask_no_marker_poll(const wmWindow *win,
const ScrArea *area,
const ARegion *region,
const wmEvent *event);
wmEventHandler_Keymap *WM_event_add_keymap_handler_v2d_mask(ListBase *handlers, wmKeyMap *keymap);
/**
* \note Priorities not implemented yet, for time being just insert in begin of list.
@@ -53,6 +53,7 @@
#include "ED_asset.hh"
#include "ED_fileselect.hh"
#include "ED_info.hh"
#include "ED_markers.hh"
#include "ED_render.hh"
#include "ED_screen.hh"
#include "ED_undo.hh"
@@ -64,6 +65,7 @@
#include "RNA_access.hh"
#include "UI_interface.hh"
#include "UI_view2d.hh"
#include "WM_api.hh"
#include "WM_message.hh"
@@ -3381,7 +3383,9 @@ static eHandlerActionFlag wm_handlers_do_intern(bContext *C,
if (handler_base->flag & WM_HANDLER_DO_FREE) {
/* Pass. */
}
else if (handler_base->poll == nullptr || handler_base->poll(CTX_wm_region(C), event)) {
else if (handler_base->poll == nullptr ||
handler_base->poll(win, CTX_wm_area(C), CTX_wm_region(C), event))
{
/* In advance to avoid access to freed event on window close. */
const bool always_pass = wm_event_always_pass(event);
@@ -4832,13 +4836,45 @@ static bool event_or_prev_in_rect(const wmEvent *event, const rcti *rect)
return false;
}
static bool handler_region_v2d_mask_test(const ARegion *region, const wmEvent *event)
bool WM_event_handler_region_v2d_mask_poll(const wmWindow * /*win*/,
const ScrArea * /*area*/,
const ARegion *region,
const wmEvent *event)
{
rcti rect = region->v2d.mask;
BLI_rcti_translate(&rect, region->winrct.xmin, region->winrct.ymin);
return event_or_prev_in_rect(event, &rect);
}
bool WM_event_handler_region_marker_poll(const wmWindow * /*win*/,
const ScrArea * /*area*/,
const ARegion *region,
const wmEvent *event)
{
rcti rect = region->winrct;
rect.ymax = rect.ymin + UI_MARKER_MARGIN_Y;
/* TODO: investigate returning `event_or_prev_in_rect(event, &rect)` here.
* The difference is subtle but correct so dragging away from the region works. */
return BLI_rcti_isect_pt_v(&rect, event->xy);
}
bool WM_event_handler_region_v2d_mask_no_marker_poll(const wmWindow *win,
const ScrArea *area,
const ARegion *region,
const wmEvent *event)
{
if (!WM_event_handler_region_v2d_mask_poll(win, area, region, event)) {
return false;
}
/* Casting away `const` is only needed for a non-constant return value. */
const ListBase *markers = ED_scene_markers_get(WM_window_get_active_scene(win),
const_cast<ScrArea *>(area));
if (markers && !BLI_listbase_is_empty(markers)) {
return !WM_event_handler_region_marker_poll(win, area, region, event);
}
return true;
}
wmEventHandler_Keymap *WM_event_add_keymap_handler_poll(ListBase *handlers,
wmKeyMap *keymap,
EventHandlerPoll poll)
@@ -4854,7 +4890,7 @@ wmEventHandler_Keymap *WM_event_add_keymap_handler_poll(ListBase *handlers,
wmEventHandler_Keymap *WM_event_add_keymap_handler_v2d_mask(ListBase *handlers, wmKeyMap *keymap)
{
return WM_event_add_keymap_handler_poll(handlers, keymap, handler_region_v2d_mask_test);
return WM_event_add_keymap_handler_poll(handlers, keymap, WM_event_handler_region_v2d_mask_poll);
}
void WM_event_remove_keymap_handler(ListBase *handlers, wmKeyMap *keymap)
@@ -6334,7 +6370,9 @@ wmKeyMapItem *WM_event_match_keymap_item_from_handlers(
if (handler_base->flag & WM_HANDLER_DO_FREE) {
/* Pass. */
}
else if (handler_base->poll == nullptr || handler_base->poll(CTX_wm_region(C), event)) {
else if (handler_base->poll == nullptr ||
handler_base->poll(win, CTX_wm_area(C), CTX_wm_region(C), event))
{
if (handler_base->type == WM_HANDLER_TYPE_KEYMAP) {
wmEventHandler_Keymap *handler = (wmEventHandler_Keymap *)handler_base;
wmEventHandler_KeymapResult km_result;
@@ -34,7 +34,10 @@ enum eWM_EventHandlerType {
WM_HANDLER_TYPE_KEYMAP,
};
using EventHandlerPoll = bool (*)(const ARegion *region, const wmEvent *event);
using EventHandlerPoll = bool (*)(const wmWindow *win,
const ScrArea *area,
const ARegion *region,
const wmEvent *event);
struct wmEventHandler {
wmEventHandler *next, *prev;