Hey Ian, If I remember correctly, the windows platform grabs the wxEVT_CHAR_HOOK events to implement menu accelerators which get priority over the translated wxEVT_CHAR events and breaks our hotkey implementation when there is a conflict. JP can probably shed some more light on this. I suspect we could find a better solution where we use the "standard" wxWidgets menu accelerator paradigm and factor out our hotkey implementation but it would most likely be a very painful transition.
Cheers, Wayne On 7/31/19 5:50 AM, Ian McInerney wrote: > I was just doing some digging into the processing of the shifted key > codes because there was another bug filed about them > (https://bugs.launchpad.net/kicad/+bug/1838420), and there seems to be > another problem with their processing. > > Inside RunHotkey there are 2 searches performed for hotkeys, the first > for the actual key combination given and the second (if nothing is found > in the first) with the shift modifier removed. This is posing a problem > with the untranslated shifted key codes in the wxEVT_CHAR_HOOK, since if > there is any hotkey on the non-shifted key then it will be run and the > event is stopped processing and the corresponding wxEVT_CHAR is never > generated. > > For instance, when I set the hotkey for switch track posture to be * > (which on my keyboad is Shift+8) and also have tune differential pair > length on hotkey 8 then the wxEVT_CHAR_HOOK event runs the tune action > and stops the event processing, see the below trace: > > 11:32:28 AM: Trace: (KICAD_KEY_EVENTS) EDA_BASE_FRAME::OnCharHook > Hook SHIFT 306 --S- 0 (U+0000) 65505 0x00000032 > ( 298, 171) > 11:32:28 AM: Trace: (KICAD_KEY_EVENTS) TOOL_DISPATCHER::DispatchWxEvent > Hook '8' 56 --S- 56 (U+0038) 42 > 0x00000011 ( 298, 171) > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_DISPATCHER::DispatchWxEvent > category: keyboard action: key-pressed key: 56 mods: shift > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::processEvent > category: keyboard action: key-pressed key: 56 mods: shift > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) ACTION_MANAGER::RunHotKey Key: > Shift+8 > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) ACTION_MANAGER::RunHotKey No > actions found, searching with key: 8 > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) ACTION_MANAGER::RunHotKey Running > action: pcbnew.LengthTuner.TuneDiffPair for hotkey Shift+8 > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::processEvent > category: command action: activate cmd-str: pcbnew.LengthTuner.TuneDiffPair > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > category: command action: activate cmd-str: pcbnew.LengthTuner.TuneDiffPair > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > Waking tool pcbnew.InteractiveRouter for event: category: command > action: activate cmd-str: pcbnew.LengthTuner.TuneDiffPair > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > Waking tool pcbnew.InteractiveSelection for event: category: command > action: activate cmd-str: pcbnew.LengthTuner.TuneDiffPair > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > Running tool pcbnew.LengthTuner for event: category: command action: > activate cmd-str: pcbnew.LengthTuner.TuneDiffPair > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::processEvent > category: command action: action cmd-str: pcbnew.InteractiveSelection.Clear > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > category: command action: action cmd-str: pcbnew.InteractiveSelection.Clear > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > Waking tool pcbnew.InteractiveSelection for event: category: command > action: action cmd-str: pcbnew.InteractiveSelection.Clear > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > Running tool pcbnew.InteractiveSelection for event: category: command > action: action cmd-str: pcbnew.InteractiveSelection.Clear > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > handled: true category: command action: action cmd-str: > pcbnew.InteractiveSelection.Clear > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchActivation > category: command action: action cmd-str: pcbnew.InteractiveSelection.Clear > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::processEvent > handled: true category: command action: action cmd-str: > pcbnew.InteractiveSelection.Clear > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::processEvent > category: command action: activate cmd-str: pcbnew.LengthTuner > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > category: command action: activate cmd-str: pcbnew.LengthTuner > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > Waking tool pcbnew.InteractiveSelection for event: category: command > action: activate cmd-str: pcbnew.LengthTuner > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > handled: false category: command action: activate cmd-str: > pcbnew.LengthTuner > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchActivation > category: command action: activate cmd-str: pcbnew.LengthTuner > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchActivation > Running tool pcbnew.LengthTuner for event: category: command action: > activate cmd-str: pcbnew.LengthTuner > 11:32:28 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::processEvent > handled: true category: command action: activate cmd-str: > pcbnew.LengthTuner > 11:32:29 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchInternal > handled: true category: command action: activate cmd-str: > pcbnew.LengthTuner.TuneDiffPair > 11:32:29 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::dispatchActivation > category: command action: activate cmd-str: pcbnew.LengthTuner.TuneDiffPair > 11:32:29 AM: Trace: (KICAD_TOOL_STACK) TOOL_MANAGER::processEvent > handled: true category: command action: activate cmd-str: > pcbnew.LengthTuner.TuneDiffPair > > > From the comment about the second search step (action_manager.cpp:101), > it sounds like this was added because of the shifted key problem. This > doesn't seem like a good solution to the problem though, because it > causes more issues with other keys as well (for instance, if nothing is > assigned to shift+A, then it just runs the hotkey for A instead). > > Is there a reason not to just skip all keyboard events with the shift > modifier that happen in a wxEVT_CHAR_HOOK event? That way they will > continue processing and generate the wxEVT_CHAR, and we don't have to > have every tool skipping the keyboard events (which I am also seeing > some issues with in the routing tool). I think that would also mean the > second hotkey search step could be removed, fixing the issues it is > creating. > > I have been doing all of this testing on GTK, so I am not familar with > any of the Windows/Mac processing that it does, so is any of this key > translation also platform specific? > > -Ian > > On Sun, Jul 28, 2019 at 2:27 PM jp charras <[email protected] > <mailto:[email protected]>> wrote: > > Le 28/07/2019 à 02:22, Jeff Young a écrit : > > I think JP ran into this earlier, so he might know more about it. > > In fact issue I h=worked on was the fact the Char events where no > correctly skipped. > > If I correctly understand the issue related by this mail, this is a > dispatch issue. > > I am thinking I already saw this issue. > The key event dispatch incorrectly try to dispatch the key events: > For me it try to find an action that matches the corresponding hotkey in > its lists, regardless the fact the action is attached to the active > frame (the frame having the focus) or not. > If this is true, this dispatch behavior is really not correct: > Only actions depending on the active frame must be taken in account. > This is what the wxWidgets menu accelerators work. > > > Now, about wxEVT_CHAR_HOOK and wxEVT_CHAR events (and related hotkeys): > Among differences between these events, there is one major diff: > wxEVT_CHAR_HOOK identify the keyboard key (i.e. the not shifted key > code). > wxEVT_CHAR identify the key code (depending on the fact the shift key is > pressed or not). > They are very different for keys that have 2 very different key codes. > > I explain: > > On a French keyboard, all "digit" keys have 2 key code (like in many > other keyboards), but digit keys are shifted. > For instance the '3' key is also the '"' key. > In other words '3' and '3' use the same key ( I will call it 3") > '3' needs to type SHIFT + 3" key > '"' needs to type the 3" key > > Now what about wxEVT_CHAR_HOOK: > wxEVT_CHAR_HOOK returns the key '"' with the modifier Shift. > wxEVT_CHAR returns the key '3' with the modifier Shift. > > As a result: to show the 3D viewer in PCBNEW: > - on an English keyboard you use Alt+3 > - on an French keyboard you use Alt+" > > And I am not sure this is a bug in wxWidgets: > some other applications have this annoying behavior, for instance > Firefox and its zoom commands (but I used also a few other apps showing > this behavior). > > > > > Another issue might be that at the end of the tool loop the event > will get skipped which will send it to wxWidgets for processing (at > which point it will probably come back as a hotkey again). We might > already have logic to deal with that, but it’s something to look out > for. > > > > At the end of the day, though, we need to design around what is > “right”. If what’s right breaks other stuff then we need to fix the > other stuff. > > > > Cheers, > > Jeff. > > > >> On 27 Jul 2019, at 17:48, Ian McInerney <[email protected] > <mailto:[email protected]>> wrote: > >> > >> In the tool dispatcher currently, if any action is associated > with a key combination (even if the action is not handled by any > active tools) then a key event with that combination will not > progress further than the dispatchHotKey function in the dispatcher. > For instance, this means that the letter 'B' will not make it into > any of the dispatchInternal calls inside any program (e.g. eeschema, > pleditor, cvpcb, etc.) because it is assigned to a pcbnew action to > refill the zones, even when those other programs do not use that > action at all so it goes unhandled in the dispatchHotkey function. > That event therefore is inaccessible in any tool loops that may be > running. > >> > >> Is there a reason the dispatchHotkey logic looks only at the fact > an action with that hotkey exists rather than if any tool has > handled the associated action? For my work in cvpcb it would be > better if it were the latter, so that any key events not handled by > the tools continue processing (e.g. for down/up/left/right keys, > single letter keys, etc.). It should be possible to know if the > action is handled by the hotkey handler, since they actions are > spawned immediately and the handled return value is then available. > >> > >> Would a change to this system break any of the existing tool > loops? e.g., are any unable to cope with receiving key pressed > events (I don't think any would be problematic, since some keys such > as 'C' don't have an associated action and would therefore generate > key pressed events in them)? > >> > >> -Ian > >> _______________________________________________ > >> Mailing list: https://launchpad.net/~kicad-developers > >> Post to : [email protected] > <mailto:[email protected]> > >> Unsubscribe : https://launchpad.net/~kicad-developers > >> More help : https://help.launchpad.net/ListHelp > > > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~kicad-developers > > Post to : [email protected] > <mailto:[email protected]> > > Unsubscribe : https://launchpad.net/~kicad-developers > > More help : https://help.launchpad.net/ListHelp > > > > > -- > Jean-Pierre CHARRAS > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : [email protected] > <mailto:[email protected]> > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : [email protected] > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
