Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Alt+letter hot keys in non-latin layout in turbo under far2l #1562

Closed
unxed opened this issue Mar 19, 2023 · 18 comments · Fixed by #1569
Closed

fix Alt+letter hot keys in non-latin layout in turbo under far2l #1562

unxed opened this issue Mar 19, 2023 · 18 comments · Fixed by #1569

Comments

@unxed
Copy link
Contributor

unxed commented Mar 19, 2023

There is a problem using Alt+letter hot keys then running turbo under far2l wx: they do not work in non-latin keyboard layout. This is a complex problem and need solving on both far2l and turbo sides.

First of all, wx do not send key codes in such cases at all (event.GetKeyCode() is zero for Alt+non_latin_letter). So we should detect corresponding latin letter on far2l side:

wx2INPUT_RECORD::wx2INPUT_RECORD(BOOL KeyDown, const wxKeyEvent& event, const KeyTracker &key_tracker)
{
	auto key_code = event.GetKeyCode();

#ifdef __WXGTK__
	if (key_code == 0) {
	    Display *display;
	    display = XOpenDisplay(NULL);
	    // GetRawKeyFlags() gives us raw x11 hardware key code under wxGTK
	    char* keysym = XKeysymToString(XKeycodeToKeysym(display, event.GetRawKeyFlags(), 0));
	    if (strlen(keysym) == 1) {
	    	// char key
	    	key_code = toupper(*keysym);
	    }
	    XCloseDisplay(display);
	}
#endif

On the turbo side, we should use wVirtualKeyCode value to detect such hot keys instead of uChar.UnicodeChar value. @magiblot can we implement this?

@unxed unxed changed the title fix Alt+letter hot keys in non-latin layout in turbo under far2l fix Alt+letter hot keys in non-latin layout in turbo under far2l wx Mar 19, 2023
@unxed
Copy link
Contributor Author

unxed commented Mar 19, 2023

Code snippet above is just proof of concept. Better approach would be to use XGetKeyboardMapping instead of deprecated XKeycodeToKeysym and go through all returned keysyms searching for one that is Latin letter. This would work no matter what keyboard layout is set as default.

@magiblot
Copy link

magiblot commented Mar 22, 2023

Hi @unxed! Turbo Vision checks both wVirtualScanCode and uChar.UnicodeChar. Pressing Alt+A in the Windows Console with the Russian keyboard layout produces an event like the following:

 {wVirtualKeyCode: 0x41, wVirtualScanCode: 0x1E, uChar.UnicodeChar: 1092, dwControlKeyState: 0x0002}

Even though uChar.UnicodeChar contains a Russian character, Turbo Vision looks at wVirtualScanCode and dwControlKeyState and realizes that this is actually Alt+A.

Therefore, Turbo Vision will recognize these hot keys properly as long as far2l mimics the behaviour of Windows.

@magiblot
Copy link

Although what I said in my previous comment is true, I forgot that in the case of far2l Turbo Vision also checks wVirtualKeyCode because it is known that far2l does not exactly match the behaviour of Windows. Although it could be in far2l's interest to do so, it is true that a workaround can be implemented in Turbo Vision as @unxed suggested.

@unxed
Copy link
Contributor Author

unxed commented Mar 23, 2023

Turbo Vision also checks wVirtualKeyCode

That's not actually working. With the patch above applied, far2l enqueues events with correct wVirtualKeyCode set. For example, for Alt+Russian_а which is same as Alt+F (first is unicode char, second is unicode char as hex, next goes wVirtualKeyCode as hex and the last is dwControlKeyState as hex):

ConsoleInput::Enqueue: а 430 46 2 DOWN

You see, 0x46 is a correct wVirtualKeyCode for "F" key. Still turbo running under patched far2l recognizes this event as pressing Russian "а", not Alt+LatinF. Even if I run turbo under putty4far2l there wVirtualKeyCode and wVirtualScanCode are taken exactly from Windows key events, Alt+Russian_а still works as just Russian "а", not as Alt+LatinF.

@magiblot
Copy link

I know, I know. Turbo Vision is already checking wVirtualKeyCode, but not for the case of the Alt+letter key combinations. I could have explained it better.

@unxed
Copy link
Contributor Author

unxed commented Mar 23, 2023

Ok! So I possibly should wait for fix on the Turbo Vision side before making PR to far2l?

Although it could be in far2l's interest to do so

Not an easy task. Needs importing MapVirtualKey implementation from Wine, and looks like it does not use any static translation table, but is keyboard layout dependent.

@unxed
Copy link
Contributor Author

unxed commented Mar 23, 2023

Better approach would be to use XGetKeyboardMapping

wx2INPUT_RECORD::wx2INPUT_RECORD(BOOL KeyDown, const wxKeyEvent& event, const KeyTracker &key_tracker)
{
	auto key_code = event.GetKeyCode();

#ifdef __WXGTK__
	if (key_code == 0) {
		Display *display;
		display = XOpenDisplay(NULL);
		KeySym *keymap;
		int keysyms_per_keycode;
		char* keysym;
		// GetRawKeyFlags() gives us raw x11 hardware key code under wxGTK
		keymap = XGetKeyboardMapping(display, event.GetRawKeyFlags(), 1, &keysyms_per_keycode);
		for (int i = 0; i < keysyms_per_keycode; i++) {
			if (keymap[i] && (keymap[i] != NoSymbol)) {
				keysym = XKeysymToString(keymap[i]);
				if (strlen(keysym) == 1) {
					// char key
					key_code = toupper(*keysym);
					break;
				}
			}
		}

		XFree(keymap);
		XCloseDisplay(display);
	}
#endif

magiblot added a commit to magiblot/tvision that referenced this issue Mar 25, 2023
@magiblot
Copy link

Hi @unxed! Can you confirm whether magiblot/turbo@3db219e recognizes these keys?

@unxed
Copy link
Contributor Author

unxed commented Mar 26, 2023

@magiblot now working, thanks!

@unxed
Copy link
Contributor Author

unxed commented Mar 29, 2023

Although it could be in far2l's interest to do so

Not an easy task. Needs importing MapVirtualKey implementation from Wine, and looks like it does not use any static translation table, but is keyboard layout dependent.

Actually translation appeared to be quite static. Implemented. wVirtualScanCode should contain meaningful values now.

@elfmz
Copy link
Owner

elfmz commented Mar 29, 2023

Still better to make it configure-able, via optional config, like palette.ini

@unxed
Copy link
Contributor Author

unxed commented Mar 29, 2023

Rewritten detection code keeping in mind some corner cases, see #1573

@unxed unxed changed the title fix Alt+letter hot keys in non-latin layout in turbo under far2l wx fix Alt+letter hot keys in non-latin layout in turbo under far2l Mar 30, 2023
@unxed
Copy link
Contributor Author

unxed commented Mar 30, 2023

Also as wVirtualScanCode is not zero now, far2l's tty extensions "compact input" mode possibly not working now. Enabled it for non-zero virtual scan codes also in #1574, with virtual key code to virtual scan code translation on tty backend side.

elfmz added a commit that referenced this issue Mar 30, 2023
kb layout independent keystroke info for tty backend also. see #1562
@unxed
Copy link
Contributor Author

unxed commented Mar 31, 2023

First of all, wx do not send key codes in such cases at all (event.GetKeyCode() is zero for Alt+non_latin_letter)

Trying to fix that on wx side: wxWidgets/wxWidgets#23379

I use a more universal solution on the basis of the xkbcommon library there, because It will work regardless of whether the English keyboard layout is installed or not, and will also work for both X11 and Wayland.

@unxed
Copy link
Contributor Author

unxed commented Apr 1, 2023

Finally wxWidgets fix is merged. Now OnChar/KeyUp/KeyDown events behavior is consistent between Windows ans GTK backends and OnChar event fire as it should on GTK platform even if modifiers are pressed.

Good news: in further wx versions we would be able to get Latin equivalents for alt/ctrl+non_latin_keys from wx, no need for detection on our side any more. Also no need to use accelerators hack.

Bad news: wx events behavior has changed (to maintain behavior consistency between wx on Windows and on GTK). Maybe some adoption will be needed for further wx versions. The most noticeable thing is that we should be using unicodekey field of onchar events, not onkeyup/down events for getting unicode input.

wxWidgets/wxWidgets@2c0f6a2

@unxed
Copy link
Contributor Author

unxed commented Apr 4, 2023

Still better to make it configure-able, via optional config, like palette.ini

Looked through some resources documenting scan code values, such as
https://docs.vmware.com/en/VMware-Workstation-Player-for-Windows/17.0/com.vmware.player.win.using.doc/GUID-D2C43B86-32EF-44EA-A2ED-D890483D70BD.html

Looks like Windows Virtual Scan Codes mapping to logical key values (such as Virtual Key Code) is static. But there is another problem converting from VKC to VSC: VKC does not have enough information to convert properly!

For example. VK_SHIFT is 16. Corresponding scan codes may be 42 (left Shift) or 54 (Right shift).

There are also cases then two keys have same VKC and VSC. Such two keys can be distinguished only via ENHANCED_KEY flag of dwControlKeyState event record field. Such key for example is Control: it has VKC of 17 and VSC of 29 for both left and right keys, but right one has ENHANCED_KEY flag set, and left one not.

More of this, the problem is wx does not distinguish left or right modifiers in key_code field just as Windows does not distinguish them in VKCs. And even more, wx does not have separate key code constants for numpad keys in digit mode as do Window.

So direct translation from wx key_code to combination of VKC, VSC and ENHANCED_KEY flag state is not possible without fidelity loose.

The only way to do accurate translation is to always use X11KeyCodeLookup method expanding it from character keys only to full keyboard support so any English layout X11 KeySym could be translated to corresponding combination of VKC, VSC and ENHANCED_KEY (using the same trick with virtual keyboard having English layout set).

UPD: Another option is to use constants from GDK for keys that have same wx key codes and needs to be distinguished (like it is done with RAW_ALTGR and RAW_RCTRL now. Those are (as I wrote above) modifier keys and numpad keys. Also IsEnhancedKey should be updated as, for example, right Control is enhanced key, but it can't be detected using wx key code only.

@unxed
Copy link
Contributor Author

unxed commented Apr 4, 2023

After a few more experiments, I found out that the scan codes seem to be really tied to the keyboard layout. We will use Latin for now, so that in applications like turbo, hotkeys are more likely to work correctly. Over time, you can really make it customizable.

I also found out that for almost all keys it is still possible to get an exact scan code from the virtual key code. The difference is only for the right and left Shift keys. Also, the extended key flag carries important information, which allows us to distinguish, for example, the left and right Control and Alt keys.

As a result, I prepared a small PR. First, it adds support for correctly setting the scan code for the right Shift key. Second, it adds an extended key flag setting for some keys for which it should be set. And also it adds to the table of scan codes some codes of the numeric keypad, which are received when NumLock is on. Due to my mistake, they did not get into the table earlier.

See #1588

@unxed
Copy link
Contributor Author

unxed commented Apr 5, 2023

As all that stuff is probably directly unrelated to turbo right now, let's move better keyboard input processing discussion to another issue, #1589

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants