VSE in Preview mode, assigned hotkeys does not show in tooltip of transform tools in toolshelf #118190

New Issue

Closed

opened 2024-02-13 09:47:48 +01:00 by Arunderan · 8 comments

Arunderan commented

2024-02-13 09:47:48 +01:00

System Information
Operating system: Windows-10-10.0.19045-SP0 64 Bits
Graphics card: NVIDIA GeForce RTX 3060 Ti/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 536.23

Blender Version
Broken: version: 4.2.0 Alpha, branch: main, commit date: 2024-02-11 06:19, hash: 835b9a506d44
Worked: 4.0
Likely caused by fa6384eb39

Short description of error
Adding a shortcut to one of the transform tools in the vse toolshelf in preview mode does not show the shortcut in the tooltip, and is also not removeable. In the 3d view for example it is.

Exact steps for others to reproduce the error

In 3D View, add a shortcut to let's say the rotate tool in the tool shelf. Watch the tooltip. It shows the new shortcut. And it can be changed or removed in the right click menu.
In Video Sequencer in Preview mode, add a shortcut to let's say the rotate tool in the tool shelf. Watch the tooltip. It does not show the new shortcut. And it cannot be changed or removed in the right click menu.

Might be related: #92140

**System Information** Operating system: Windows-10-10.0.19045-SP0 64 Bits Graphics card: NVIDIA GeForce RTX 3060 Ti/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 536.23 **Blender Version** Broken: version: 4.2.0 Alpha, branch: main, commit date: 2024-02-11 06:19, hash: `835b9a506d44` Worked: 4.0 Likely caused by fa6384eb39f6f703bbbde06524c9246979876d3f **Short description of error** Adding a shortcut to one of the transform tools in the vse toolshelf in preview mode does not show the shortcut in the tooltip, and is also not removeable. In the 3d view for example it is. **Exact steps for others to reproduce the error** - In 3D View, add a shortcut to let's say the rotate tool in the tool shelf. Watch the tooltip. It shows the new shortcut. And it can be changed or removed in the right click menu. - In Video Sequencer in Preview mode, add a shortcut to let's say the rotate tool in the tool shelf. Watch the tooltip. It does not show the new shortcut. And it cannot be changed or removed in the right click menu. Might be related: https://projects.blender.org/blender/blender/issues/92140

shortcut1.jpg

74 KiB

shortcut2.jpg

75 KiB

2024-02-13 09-35-47.mp4

9.4 MiB

Arunderan added the

labels 2024-02-13 09:47:48 +01:00

Pratik Borhade commented

2024-02-13 11:54:32 +01:00

Member

Hi, thanks for the report. Can confirm
cc @iss

Hi, thanks for the report. Can confirm cc @iss

Pratik Borhade added

Video Sequencer

and removed

labels 2024-02-13 11:54:53 +01:00

Richard Antalik commented

2024-02-13 17:52:13 +01:00

Member

Will check

Will check

Harley Acheson referenced this issue

2024-02-14 18:36:29 +01:00

VSE Preview - assigngin hotkeys to the toolshelf tools adds them to the Window category #92140

Richard Antalik referenced this issue

2024-02-14 18:38:37 +01:00

VSE Preview - assigngin hotkeys to the toolshelf tools adds them to the Window category #92140

Richard Antalik commented

2024-02-14 18:42:50 +01:00

Member

@Harley If you assign unused shortcut, VSE will not show it in tooltip, but also does not allow you to clear the shortcut.
In any case will check this more in detail now, and see if I can make sense of why this happens

@Harley If you assign unused shortcut, VSE will not show it in tooltip, but also does not allow you to clear the shortcut. In any case will check this more in detail now, and see if I can make sense of why this happens

Richard Antalik commented

2024-02-14 18:50:46 +01:00

Member

I think, that priority could be lowered, since this is not exacly caused by anything, as assigning shortcuts to VSE tools did not work before.

Will post findings here in edits in order to not spam this thread:
So far it seems, that but->opcontext is set to incorrect value in ui_popup_context_menu_for_button()
Ok, so attempt to set but->opcontext from ED_region_panels_ex() is completely wrong. Even if this is fixed, it doesn't seem to work.

I think, that priority could be lowered, since this is not exacly caused by anything, as assigning shortcuts to VSE tools did not work before. Will post findings here in edits in order to not spam this thread: So far it seems, that `but->opcontext` is set to incorrect value in `ui_popup_context_menu_for_button()` Ok, so attempt to set ` but->opcontext` from `ED_region_panels_ex()` is completely wrong. Even if this is fixed, it doesn't seem to work.

Harley Acheson commented

2024-02-14 19:23:09 +01:00

Member

I am a little over my skiis here since I don't know these areas very well. So the following might only give clues to what the problem might be, not necessarily the fix.

But with the following changes...

diff --git a/source/blender/editors/interface/interface_context_menu.cc b/source/blender/editors/interface/interface_context_menu.cc
index 6d28b25730c..97755234ed3 100644
--- a/source/blender/editors/interface/interface_context_menu.cc
+++ b/source/blender/editors/interface/interface_context_menu.cc
@@ -293,14 +293,10 @@ static void remove_shortcut_func(bContext *C, uiBut *but)
   IDProperty *prop;
   const char *idname = shortcut_get_operator_property(C, but, &prop);
 
-  wmKeyMap *km;
-  wmKeyMapItem *kmi = WM_key_event_operator(C,
-                                            idname,
-                                            but->opcontext,
-                                            prop,
-                                            EVT_TYPE_MASK_HOTKEY_INCLUDE,
-                                            EVT_TYPE_MASK_HOTKEY_EXCLUDE,
-                                            &km);
+  wmKeyMap *km = WM_keymap_guess_opname(C, idname);
+  wmKeyMapItem *kmi = WM_key_event_operator_from_keymap(
+      km, idname, prop, EVT_TYPE_MASK_HOTKEY_INCLUDE, EVT_TYPE_MASK_HOTKEY_EXCLUDE);
+
   BLI_assert(kmi != nullptr);
 
   WM_keymap_remove_item(km, kmi);
@@ -1156,9 +1152,8 @@ bool ui_popup_context_menu_for_button(bContext *C, uiBut *but, const wmEvent *ev
     const int w = uiLayoutGetWidth(layout);
 
     /* We want to know if this op has a shortcut, be it hotkey or not. */
-    wmKeyMap *km;
-    wmKeyMapItem *kmi = WM_key_event_operator(
-        C, idname, but->opcontext, prop, EVT_TYPE_MASK_ALL, 0, &km);
+    wmKeyMap *km = WM_keymap_guess_opname(C, idname);
+    wmKeyMapItem *kmi = WM_key_event_operator_from_keymap(km, idname, prop, EVT_TYPE_MASK_ALL, 0);
 
     /* We do have a shortcut, but only keyboard ones are editable that way... */
     if (kmi) {
diff --git a/source/blender/windowmanager/intern/wm_keymap.cc b/source/blender/windowmanager/intern/wm_keymap.cc
index 200dd8953a0..5a04b0a9db2 100644
--- a/source/blender/windowmanager/intern/wm_keymap.cc
+++ b/source/blender/windowmanager/intern/wm_keymap.cc
@@ -1634,8 +1634,10 @@ std::optional<std::string> WM_key_event_operator_string(const bContext *C,
   wmKeyMapItemFind_Params params{};
   params.filter_fn = kmi_filter_is_visible;
   params.user_data = nullptr;
-  wmKeyMapItem *kmi = wm_keymap_item_find(
-      C, opname, opcontext, properties, is_strict, &params, nullptr);
+  wmKeyMap *km = WM_keymap_guess_opname(C, opname);
+  wmKeyMapItem *kmi = WM_key_event_operator_from_keymap(
+      km, opname, properties, EVT_TYPE_MASK_HOTKEY_INCLUDE, EVT_TYPE_MASK_HOTKEY_EXCLUDE);
+
   if (kmi) {
     return WM_keymap_item_to_string(kmi, false);
   }

The change to remove_shortcut_func allows the shortcut removal to work without error. Changes to ui_popup_context_menu_for_button allows the "remove shortcut" to appear in the context menu. And changes to WM_key_event_operator_string then show the shortcut on the tooltip.

But I really have no idea why this way of getting the wmKeyMap and wmKeyMapItem (using WM_keymap_guess_opname and WM_key_event_operator_from_keymap) work in these cases while wm_keymap_item_find and WM_key_event_operator do not. So I don't know if this points to errors in the latter, or if the above changes would have some bad side effects.

I am assuming you know this stuff better than me. If not, I could always post this as a fix for this issue and assign Campbell as reviewer?

@iss I am a little over my skiis here since I don't know these areas very well. So the following might only give clues to what the problem might be, not necessarily the fix. But with the following changes... ``` diff --git a/source/blender/editors/interface/interface_context_menu.cc b/source/blender/editors/interface/interface_context_menu.cc index 6d28b25730c..97755234ed3 100644 --- a/source/blender/editors/interface/interface_context_menu.cc +++ b/source/blender/editors/interface/interface_context_menu.cc @@ -293,14 +293,10 @@ static void remove_shortcut_func(bContext *C, uiBut *but) IDProperty *prop; const char *idname = shortcut_get_operator_property(C, but, &prop); - wmKeyMap *km; - wmKeyMapItem *kmi = WM_key_event_operator(C, - idname, - but->opcontext, - prop, - EVT_TYPE_MASK_HOTKEY_INCLUDE, - EVT_TYPE_MASK_HOTKEY_EXCLUDE, - &km); + wmKeyMap *km = WM_keymap_guess_opname(C, idname); + wmKeyMapItem *kmi = WM_key_event_operator_from_keymap( + km, idname, prop, EVT_TYPE_MASK_HOTKEY_INCLUDE, EVT_TYPE_MASK_HOTKEY_EXCLUDE); + BLI_assert(kmi != nullptr); WM_keymap_remove_item(km, kmi); @@ -1156,9 +1152,8 @@ bool ui_popup_context_menu_for_button(bContext *C, uiBut *but, const wmEvent *ev const int w = uiLayoutGetWidth(layout); /* We want to know if this op has a shortcut, be it hotkey or not. */ - wmKeyMap *km; - wmKeyMapItem *kmi = WM_key_event_operator( - C, idname, but->opcontext, prop, EVT_TYPE_MASK_ALL, 0, &km); + wmKeyMap *km = WM_keymap_guess_opname(C, idname); + wmKeyMapItem *kmi = WM_key_event_operator_from_keymap(km, idname, prop, EVT_TYPE_MASK_ALL, 0); /* We do have a shortcut, but only keyboard ones are editable that way... */ if (kmi) { diff --git a/source/blender/windowmanager/intern/wm_keymap.cc b/source/blender/windowmanager/intern/wm_keymap.cc index 200dd8953a0..5a04b0a9db2 100644 --- a/source/blender/windowmanager/intern/wm_keymap.cc +++ b/source/blender/windowmanager/intern/wm_keymap.cc @@ -1634,8 +1634,10 @@ std::optional<std::string> WM_key_event_operator_string(const bContext *C, wmKeyMapItemFind_Params params{}; params.filter_fn = kmi_filter_is_visible; params.user_data = nullptr; - wmKeyMapItem *kmi = wm_keymap_item_find( - C, opname, opcontext, properties, is_strict, &params, nullptr); + wmKeyMap *km = WM_keymap_guess_opname(C, opname); + wmKeyMapItem *kmi = WM_key_event_operator_from_keymap( + km, opname, properties, EVT_TYPE_MASK_HOTKEY_INCLUDE, EVT_TYPE_MASK_HOTKEY_EXCLUDE); + if (kmi) { return WM_keymap_item_to_string(kmi, false); } ``` The change to `remove_shortcut_func` allows the shortcut removal to work without error. Changes to `ui_popup_context_menu_for_button` allows the "remove shortcut" to appear in the context menu. And changes to `WM_key_event_operator_string` then show the shortcut on the tooltip. But I really have no idea why this way of getting the wmKeyMap and wmKeyMapItem (using WM_keymap_guess_opname and WM_key_event_operator_from_keymap) work in these cases while `wm_keymap_item_find` and `WM_key_event_operator` do not. So I don't know if this points to errors in the latter, or if the above changes would have some bad side effects. I am assuming you know this stuff better than me. If not, I could always post this as a fix for this issue and assign Campbell as reviewer?

Harley Acheson referenced this issue

2024-02-14 19:35:20 +01:00

VSE Preview - assigngin hotkeys to the toolshelf tools adds them to the Window category #92140

Richard Antalik commented

2024-02-14 19:48:23 +01:00

Member

Thanks for your pointers. Preview area should set but->opcontext for toolbar, but for some reason it is not picked up. Hardcoding opcontext for WM_key_event_operator() in ui_popup_context_menu_for_button() also causes VSE toolbar to work. So I would say, that code is OK and issue is somewhere on VSE UI side. I am also struggling a bit to follow data structures here and figuring out where to put debug prints :) Your code snippet is pretty much black magic :P

Thanks for your pointers. Preview area should set `but->opcontext` for toolbar, but for some reason it is not picked up. Hardcoding opcontext for `WM_key_event_operator() ` in `ui_popup_context_menu_for_button()` also causes VSE toolbar to work. So I would say, that code is OK and issue is somewhere on VSE UI side. I am also struggling a bit to follow data structures here and figuring out where to put debug prints :) Your code snippet is pretty much black magic :P

Harley Acheson commented

2024-02-14 20:46:25 +01:00

Member

So I would say, that code is OK and issue is somewhere on VSE UI side

Yes, that sounds right. My change probably just ignores that error and might cause problems elsewhere.

Preview area should set but->opcontext for toolbar, but for some reason it is not picked up. Hardcoding opcontext...

Oh, I see. That part works if you force it to WM_OP_INVOKE_REGION_PREVIEW. I see it set to WM_OP_INVOKE_REGION_WIN at that point.

A weird thing I am seeing is that sequencer_tools_region_draw seems to always set WM_OP_INVOKE_REGION_WIN. It has a switch in there that I never see hitting any of the cases. I only see this function called with a region->regiontype of RGN_TYPE_TOOLS, which makes sense to me because that is the type of the region as it is defined at about line 1047.

However, if I change sequencer_tools_region_draw to always sets an op_context of WM_OP_INVOKE_REGION_PREVIEW it doesn't seem to do anything. As in the op_context when it is evaluated in ui_popup_context_menu_for_button is still WM_OP_INVOKE_REGION_WIN.

> So I would say, that code is OK and issue is somewhere on VSE UI side Yes, that sounds right. My change probably just ignores that error and might cause problems elsewhere. > Preview area should set but->opcontext for toolbar, but for some reason it is not picked up. Hardcoding opcontext... Oh, I see. That part works if you force it to WM_OP_INVOKE_REGION_PREVIEW. I see it set to WM_OP_INVOKE_REGION_WIN at that point. A weird thing I am seeing is that `sequencer_tools_region_draw` seems to always set WM_OP_INVOKE_REGION_WIN. It has a switch in there that I never see hitting any of the cases. I only see this function called with a region->regiontype of RGN_TYPE_TOOLS, which makes sense to me because that is the type of the region as it is defined at about line 1047. However, if I change `sequencer_tools_region_draw` to always sets an op_context of WM_OP_INVOKE_REGION_PREVIEW it doesn't seem to do anything. As in the op_context when it is evaluated in ui_popup_context_menu_for_button is still WM_OP_INVOKE_REGION_WIN.

Richard Antalik referenced a pull request that will close this issue

2024-02-14 21:05:04 +01:00

Fix #118190: Tool shortcut does not show in tooltip #118292

Richard Antalik commented

2024-02-14 21:11:07 +01:00

Member

Well turns out, that solution looks about like #118292. That somehow works for toolbar in timeline, which it should not...

Well turns out, that solution looks about like #118292. That somehow works for toolbar in timeline, which it should not...

Richard Antalik referenced this issue from a commit

2024-02-16 18:05:33 +01:00

Fix #118190: Tool shortcut does not show in tooltip

Richard Antalik closed this issue

2024-02-16 18:05:37 +01:00

Blender Bot added

and removed

labels 2024-02-16 18:06:45 +01:00

Sign in to join this conversation.

No Label

Animation & Rigging

Asset Browser Project (Legacy)

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

4 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#118190

No description provided.