Fix T67084: Modal keymaps could show the wrong shortcut
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Feb 4 2020, 12:30 PM.

Details

Reviewers

Campbell Barton (campbellbarton)
William Reynish (billreynish)
Julian Eisel (Severin)

Maniphest Tasks

T67084: Knife tool show wrong shortcut key with industry key map.

Commits

rB36d09d9029c9: Fix T67084: Modal keymaps could show the wrong shortcut

Summary

WM_modalkeymap_operator_items_to_string() and friends werent checking
WM_keymap_active(), so it was possible that e.g. when using the Industry
Compatible keymap, the shortcut from the Blender keymap was shown.

This also fixes the (now exposed) bug that the Industry Compatible keymap
would not have a ADD_CUT_CLOSED kmi defined for the Knife Tool [mandatory for the status bar].

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Feb 4 2020, 12:30 PM

Harbormaster completed remote builds in B6484: Diff 21380.Feb 4 2020, 12:30 PM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Feb 4 2020, 12:32 PM

Philipp Oeser (lichtwerk) added a project: User Interface.

Makes sense code wise (didn't test). Would just like to see one change before committing.

source/blender/windowmanager/intern/wm_keymap.c
1298–1300	I'd much prefer if we reduced the reliance on `bContext`, especially in such specific functions. So better to just pass `wmWindowManager` as parameter.

This revision is now accepted and ready to land.Feb 4 2020, 12:46 PM

William Reynish (billreynish) accepted this revision.Feb 4 2020, 1:08 PM

Agree with Julian the context argument should be avoided.
In this case I think it's fine to use G_MAIN, as some of the other keymap code does this & it cuts down passing around an extra arg.

diff --git a/release/scripts/presets/keyconfig/keymap_data/industry_compatible_data.py b/release/scripts/presets/keyconfig/keymap_data/industry_compatible_data.py
index ceca4687443..2af3bc4898c 100644
--- a/release/scripts/presets/keyconfig/keymap_data/industry_compatible_data.py
+++ b/release/scripts/presets/keyconfig/keymap_data/industry_compatible_data.py
@@ -3444,7 +3444,7 @@ def km_knife_tool_modal_map(_params):
         ("PANNING", {"type": 'LEFTMOUSE', "value": 'PRESS', "alt": True}, None),
         ("CONFIRM", {"type": 'RET', "value": 'PRESS', "any": True}, None),
         ("CONFIRM", {"type": 'NUMPAD_ENTER', "value": 'PRESS', "any": True}, None),
-        ("CONFIRM", {"type": 'LEFTMOUSE', "value": 'DOUBLE_CLICK', "any": True}, None),
+        ("ADD_CUT_CLOSED", {"type": 'LEFTMOUSE', "value": 'DOUBLE_CLICK', "any": True}, None),
         ("ADD_CUT", {"type": 'LEFTMOUSE', "value": 'ANY', "any": True}, None),
         ("NEW_CUT", {"type": 'E', "value": 'PRESS'}, None),
         ("SNAP_MIDPOINTS_ON", {"type": 'LEFT_CTRL', "value": 'PRESS', "any": False}, None),
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index f5970e8fb61..fdd424b5e31 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -5405,8 +5405,9 @@ void WM_window_cursor_keymap_status_refresh(bContext *C, wmWindow *win)
  *
  * \{ */
 
-bool WM_window_modal_keymap_status_draw(bContext *UNUSED(C), wmWindow *win, uiLayout *layout)
+bool WM_window_modal_keymap_status_draw(bContext *C, wmWindow *win, uiLayout *layout)
 {
+  wmWindowManager *wm = CTX_wm_manager(C);
   wmKeyMap *keymap = NULL;
   wmOperator *op = NULL;
   LISTBASE_FOREACH (wmEventHandler *, handler_base, &win->modalhandlers) {
@@ -5414,7 +5415,7 @@ bool WM_window_modal_keymap_status_draw(bContext *UNUSED(C), wmWindow *win, uiLa
       wmEventHandler_Op *handler = (wmEventHandler_Op *)handler_base;
       if (handler->op != NULL) {
         /* 'handler->keymap' could be checked too, seems not to be used. */
-        wmKeyMap *keymap_test = handler->op->type->modalkeymap;
+        wmKeyMap *keymap_test = WM_keymap_active(wm, handler->op->type->modalkeymap);
         if (keymap_test && keymap_test->modal_items) {
           keymap = keymap_test;
           op = handler->op;
diff --git a/source/blender/windowmanager/intern/wm_keymap.c b/source/blender/windowmanager/intern/wm_keymap.c
index 1809a233ce1..1587f7ab71d 100644
--- a/source/blender/windowmanager/intern/wm_keymap.c
+++ b/source/blender/windowmanager/intern/wm_keymap.c
@@ -1295,7 +1295,9 @@ int WM_modalkeymap_operator_items_to_string(wmOperatorType *ot,
                                             char *result,
                                             const int result_len)
 {
-  return WM_modalkeymap_items_to_string(ot->modalkeymap, propvalue, compact, result, result_len);
+  wmWindowManager *wm = G_MAIN->wm.first;
+  wmKeyMap *keymap = WM_keymap_active(wm, ot->modalkeymap);
+  return WM_modalkeymap_items_to_string(keymap, propvalue, compact, result, result_len);
 }
 
 char *WM_modalkeymap_operator_items_to_string_buf(wmOperatorType *ot,