View Issue Details

IDProjectCategoryView StatusLast Update
0001634OpenMPTUser Interfacepublic2022-11-03 06:38
Reporterc0d3h4x0r Assigned ToSaga Musix  
PriorityhighSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Platformx64OSWindowsOS Version10
Product VersionOpenMPT 1.30.07.00 / libopenmpt 0.6.6 (upgrade first) 
Target VersionOpenMPT 1.30.08.00 / libopenmpt 0.6.6 (upgrade first)Fixed in VersionOpenMPT 1.30.08.00 / libopenmpt 0.6.6 (upgrade first) 
Summary0001634: Keyboard input largely (completely?) broken over Remote Desktop
Description

I want to use Remote Desktop from my iPad Pro to my PC to do some tracking from bed or while away from home. Sadly, keyboard input (in the pattern editor, at least) appears to be totally broken over Remote Desktop. I'm guessing OpenMPT leverages keyboard events/APIs that are lower level than the key down/up events sent over the wire by Remote Desktop. Is there any way to enable usage of higher level keyboard events to support working over Remote Desktop? Even if that means the custom keyboard mappings don't apply the same way, it's still better to be able to track with goofy key mappings than not at all; OpenMPT could even put up a warning dialog saying "you're using Remote Desktop, so keyboard mappings won't be applied", or something, if that's a technical limitation that cannot be overcome.

Steps To Reproduce
  1. Connect to Win10 Pro x64 PC using Remote Desktop app on an iPad Pro with a Bluetooth keyboard attached.
  2. Within the RDP session, launch OpenMPT.
  3. Go to the pattern editor and try entering notes or other pattern data (volume column, effects column, etc) using the Bluetooth keyboard.

ACTUAL: doesn't work
EXPECT: works

TagsNo tags attached.
Has the bug occurred in previous versions?don't know
Tested code revision (in case you know it)

Relationships

related to 0001635 resolvedSaga Musix iOS RDP: typing into sample rate edit box double-registers each typed character 

Activities

Saga Musix

Saga Musix

2022-10-30 21:38

administrator   ~0005341

I'm pretty sure I was able to use OpenMPT over RDP before, but that was from another Windows client. Maybe your iOS RDP client indeed synthesizes higher-level keyboard events (WM_CHAR probably) than what is required (WM_KEYDOWN/WM_KEYUP). Unfortunately WM_CHAR wouldn't really be a suitable alternative for OpenMPT's keyboard handling so I'm not really sure what could be done here apart from trying a different RDP client.

c0d3h4x0r

c0d3h4x0r

2022-10-31 00:43

reporter   ~0005342

It's the official Microsoft Remote Desktop client for iOS, and I haven't encountered this issue with any other Windows apps, so I don't think the explanation can be quite that simple.

Is there any key event logging I can enable, or do you have a build with matching symbols I can use to set up some tracepoints with Visual Studio?

c0d3h4x0r

c0d3h4x0r

2022-10-31 01:04

reporter   ~0005343

I found this elsewhere online:

When I analyzed the incoming keystrokes on a remote Windows system, I saw the iOS RD client was sending
ASCII/Unicode values for the alphanumeric keys, whereas other Microsoft RD clients were sending the proper BIOS keyboard scan codes instead. Most Windows apps understood what was going on and adjusted accordingly, but VMware did not.

There doesn't appear to be a way for the user to correct this annoying behavior within iOS or in the client app.

c0d3h4x0r

c0d3h4x0r

2022-10-31 01:07

reporter   ~0005344

And elsewhere, I found this:

We switched from sending scan codes to unicodes with 8.1.x.

We did this because the many issues customers were having with mismatched keyboard layouts. This was especially bad on mobile devices where the input to our app is unicode and we needed to translate to scancodes based on the remote side’s keyboard layout. Often there would be a mismatch as the protocol does not allow detecting the remote side keyboard nor set it except for a new login.

We have talked about offering scan codes again in the future as an option users can set. In the meantime (not soon) they can use the remote side keyboard (osk).

c0d3h4x0r

c0d3h4x0r

2022-10-31 01:12

reporter   ~0005345

Most notably: arrow keys work for navigating around inside the pattern editor, none of the alphnumeric keys work, which seems to support the “scan codes versus unicode” explanation.

c0d3h4x0r

c0d3h4x0r

2022-10-31 02:18

reporter   ~0005346

It sounds like what iOS RDP sends are “virtual key codes” instead of the “scan codes” that would normally generate them. And sure enough, OpenMPT’s general key input handling relies entirely upon scancodes: https://github.com/OpenMPT/openmpt/blob/e0cdb885c09d888e14b03789762fd7743f580ee7/mptrack/InputHandler.cpp#L109

c0d3h4x0r

c0d3h4x0r

2022-10-31 02:24

reporter   ~0005347

It looks like that function could/should additionally check wParam for WM_KEYUP and/or WM_KEYDOWN, rather than trying to identify keyup/keydown events exclusively by examinng the code value.

c0d3h4x0r

c0d3h4x0r

2022-10-31 02:25

reporter   ~0005348

typo correction: by examining the scancode value

Saga Musix

Saga Musix

2022-10-31 18:21

administrator   ~0005350

I don't think that should be required; CViewPattern::PreTranslateMessage should be the code path handling virtual key codes. The keyboard hook (which calls CInputHandler::GeneralKeyEvent) is used for global shortcuts, not view-specific shortcuts.

Saga Musix

Saga Musix

2022-10-31 18:22

administrator   ~0005351

Is there any key event logging I can enable

No.

or do you have a build with matching symbols I can use to set up some tracepoints with Visual Studio?

Debug symbols for all official releases are available at https://download.openmpt.org/archive/.symbols/

c0d3h4x0r

c0d3h4x0r

2022-10-31 20:01

reporter   ~0005352

I downloaded and installed VS2022 and Win10SDK last night and successfully built and began debugging. The message being produced by the keypress from the iOS RDP app is VK_PACKET. That's about as far as I got before running out of steam for the night, but I'll keep digging.

c0d3h4x0r

c0d3h4x0r

2022-10-31 21:19

reporter   ~0005353

In this repro, the key I pressed on the iPad was z. Call stack of interest:

>   OpenMPT.exe!CInputHandler::GeneralKeyEvent(InputTargetContext context=kCtxAllContexts, int code=0x00000000, unsigned __int64 wParam=0x00000000000000e7, __int64 lParam=0x0000000000000001) Line 110 C++
    OpenMPT.exe!CMainFrame::KeyboardProc(int code=0x00000000, unsigned __int64 wParam=0x00000000000000e7, __int64 lParam=0x0000000000000001) Line 507   C++
    user32.dll!00007ffa002c0e62()   Unknown
    user32.dll!00007ffa002be3e9()   Unknown
    user32.dll!00007ffa002be2de()   Unknown
    ntdll.dll!00007ffa01430ef4()    Unknown
    win32u.dll!00007ff9ff181104()   Unknown
    user32.dll!00007ffa002c1b7e()   Unknown
    OpenMPT.exe!AfxInternalPumpMessage() Line 153   C++
    OpenMPT.exe!CWinThread::PumpMessage() Line 900  C++
    OpenMPT.exe!CWinThread::Run() Line 629  C++
    OpenMPT.exe!CWinApp::Run() Line 787 C++
    OpenMPT.exe!CTrackApp::Run() Line 1550  C++
    OpenMPT.exe!AfxWinMain(HINSTANCE__ * hInstance=0x00007ff76be80000, HINSTANCE__ * hPrevInstance=0x0000000000000000, wchar_t * lpCmdLine=0x0000027e8dc73064, int nCmdShow=0x0000000a) Line 47 C++
    OpenMPT.exe!wWinMain(HINSTANCE__ * hInstance=0x00007ff76be80000, HINSTANCE__ * hPrevInstance=0x0000000000000000, wchar_t * lpCmdLine=0x0000027e8dc73064, int nCmdShow=0x0000000a) Line 26   C++
    OpenMPT.exe!invoke_main() Line 123  C++
    OpenMPT.exe!__scrt_common_main_seh() Line 288   C++
    OpenMPT.exe!__scrt_common_main() Line 331   C++
    OpenMPT.exe!wWinMainCRTStartup(void * __formal=0x0000009681a57000) Line 17  C++
    kernel32.dll!00007ff9ff867034() Unknown
    ntdll.dll!00007ffa013e26a1()    Unknown

The message is: WM_KEYDOWN (wParam==0xe7==VK_PACKET, lParam==0x1).

According to the MSDN docs, when the stack unwinds back to AfxInternalPumpMessage and passes this message into TranslateMessage, that API should post a corresponding WM_CHAR to the message queue.

But before that can happen, another PreTranslateMessage handler gets a shot at it:

>   OpenMPT.exe!CInputHandler::KeyEvent(InputTargetContext context=kCtxViewPatternsNote, unsigned int & nChar=0x000000e7, unsigned int & __formal=0x00000001, unsigned int & nFlags=0x00000000, KeyEventType keyEventType=kKeyEventDown, CWnd * pSourceWnd=0x0000000000000000) Line 147 C++
    OpenMPT.exe!CViewPattern::PreTranslateMessage(tagMSG * pMsg=0x0000027e8dc7f1b8) Line 665    C++
    OpenMPT.exe!CWnd::WalkPreTranslateTree(HWND__ * hWndStop=0x0000000000080b3c, tagMSG * pMsg=0x0000027e8dc7f1b8) Line 3379    C++
    OpenMPT.exe!AfxInternalPreTranslateMessage(tagMSG * pMsg=0x0000027e8dc7f1b8) Line 233   C++
    OpenMPT.exe!CWinThread::PreTranslateMessage(tagMSG * pMsg=0x0000027e8dc7f1b8) Line 778  C++
    OpenMPT.exe!AfxPreTranslateMessage(tagMSG * pMsg=0x0000027e8dc7f1b8) Line 252   C++
    OpenMPT.exe!AfxInternalPumpMessage() Line 178   C++
    OpenMPT.exe!CWinThread::PumpMessage() Line 900  C++
    OpenMPT.exe!CWinThread::Run() Line 629  C++
    OpenMPT.exe!CWinApp::Run() Line 787 C++
    OpenMPT.exe!CTrackApp::Run() Line 1550  C++
    OpenMPT.exe!AfxWinMain(HINSTANCE__ * hInstance=0x00007ff76be80000, HINSTANCE__ * hPrevInstance=0x0000000000000000, wchar_t * lpCmdLine=0x0000027e8dc73064, int nCmdShow=0x0000000a) Line 47 C++
    OpenMPT.exe!wWinMain(HINSTANCE__ * hInstance=0x00007ff76be80000, HINSTANCE__ * hPrevInstance=0x0000000000000000, wchar_t * lpCmdLine=0x0000027e8dc73064, int nCmdShow=0x0000000a) Line 26   C++
    OpenMPT.exe!invoke_main() Line 123  C++
    OpenMPT.exe!__scrt_common_main_seh() Line 288   C++
    OpenMPT.exe!__scrt_common_main() Line 331   C++
    OpenMPT.exe!wWinMainCRTStartup(void * __formal=0x0000009681a57000) Line 17  C++
    kernel32.dll!00007ff9ff867034() Unknown
    ntdll.dll!00007ffa013e26a1()    Unknown

0xe7 (VK_PACKET) is getting passed into CInputHandler::KeyEvent as the value for nChar, which seems fishy at best, but nothing down this execution path chooses to handle the message, so the stack unwinds back to the pump, and TranslateMessage gets called.

As expected, a few message pumps later, I see GetMessage pull this out of the queue: message==0x102==WM_CHAR, (wParam==0x7a, lParam==0x1)

But neither CInputHandler::GeneralKeyEvent nor CInputHandler::KeyEvent ever get called for WM_CHAR, because it's not a keyboard event.

c0d3h4x0r

c0d3h4x0r

2022-10-31 22:12

reporter   ~0005354

Interesting remarks from https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-keybdinput:

INPUT_KEYBOARD supports nonkeyboard-input methods—such as handwriting recognition or voice recognition—as if it were text input by using the KEYEVENTF_UNICODE flag. If KEYEVENTF_UNICODE is specified, SendInput sends a WM_KEYDOWN or WM_KEYUP message to the foreground thread's message queue with wParam equal to VK_PACKET. Once GetMessage or PeekMessage obtains this message, passing the message to TranslateMessage posts a WM_CHAR message with the Unicode character originally specified by wScan. This Unicode character will automatically be converted to the appropriate ANSI value if it is posted to an ANSI window.

It sounds like this might be a viable approach:

Modify CViewPattern::PreTranslateMessage to handle VK_PACKET by explicitly calling TranslateMessage on it to post the corresponding WM_CHAR message to the queue, then immediately call PeekMessage to remove the most recently-posted WM_CHAR message from the queue and extract its character code, repost that message back to the queue using PostMessage, then call VkKeyScanEx to convert the character code to a virtual key code, and then finally pass that value (rather than VK_PACKET) for wParam down into the rest of the usual WM_KEYDOWN/WM_SYSKEYDOWN/WM_KEYUP/WM_SYSKEYUP handling.

Thoughts?

c0d3h4x0r

c0d3h4x0r

2022-10-31 23:54

reporter   ~0005355

The alternative would be to rewrite all the existing keyboard handling to use "Raw Input": https://learn.microsoft.com/en-us/windows/win32/inputdev/about-raw-input

c0d3h4x0r

c0d3h4x0r

2022-11-01 00:22

reporter   ~0005356

Handling VK_PACKET by using TranslateMessage and PeekMessage(PM_NOREMOVE) works. Not sure what else it might regress, but it definitely resolves my repro. I'll push a MR to you in GitHub.

c0d3h4x0r

c0d3h4x0r

2022-11-01 00:33

reporter   ~0005357

PR created: https://github.com/OpenMPT/openmpt/pull/12

Saga Musix

Saga Musix

2022-11-01 15:29

administrator   ~0005358

Last edited: 2022-11-01 15:30

Thanks, I can see that the same issue exists with the Windows on-screen keyboard, so I can round off the patch (e.g. by extending it to cover the other views such as sample and instrument editor) without RDP.

Edit: No, I was wrong, the on-screen keyboard is fine. I was just testing the wrong pattern column...

Saga Musix

Saga Musix

2022-11-01 16:17

administrator   ~0005359

This should be fixed as of r18129 in all views, not just pattern view.

c0d3h4x0r

c0d3h4x0r

2022-11-01 16:35

reporter   ~0005360

Awesome, thanks!

Issue History

Date Modified Username Field Change
2022-10-30 19:51 c0d3h4x0r New Issue
2022-10-30 19:53 manx Relationship added related to 0000713
2022-10-30 21:38 Saga Musix Note Added: 0005341
2022-10-31 00:43 c0d3h4x0r Note Added: 0005342
2022-10-31 01:04 c0d3h4x0r Note Added: 0005343
2022-10-31 01:07 c0d3h4x0r Note Added: 0005344
2022-10-31 01:12 c0d3h4x0r Note Added: 0005345
2022-10-31 02:18 c0d3h4x0r Note Added: 0005346
2022-10-31 02:24 c0d3h4x0r Note Added: 0005347
2022-10-31 02:25 c0d3h4x0r Note Added: 0005348
2022-10-31 18:21 Saga Musix Note Added: 0005350
2022-10-31 18:22 Saga Musix Note Added: 0005351
2022-10-31 20:01 c0d3h4x0r Note Added: 0005352
2022-10-31 21:19 c0d3h4x0r Note Added: 0005353
2022-10-31 22:12 c0d3h4x0r Note Added: 0005354
2022-10-31 23:54 c0d3h4x0r Note Added: 0005355
2022-11-01 00:22 c0d3h4x0r Note Added: 0005356
2022-11-01 00:33 c0d3h4x0r Note Added: 0005357
2022-11-01 15:29 Saga Musix Note Added: 0005358
2022-11-01 15:29 Saga Musix Assigned To => Saga Musix
2022-11-01 15:29 Saga Musix Status new => assigned
2022-11-01 15:30 Saga Musix Note Edited: 0005358
2022-11-01 16:17 Saga Musix Status assigned => feedback
2022-11-01 16:17 Saga Musix Note Added: 0005359
2022-11-01 16:17 Saga Musix Target Version => OpenMPT 1.30.08.00 / libopenmpt 0.6.6 (upgrade first)
2022-11-01 16:35 c0d3h4x0r Note Added: 0005360
2022-11-01 16:35 c0d3h4x0r Status feedback => assigned
2022-11-01 16:36 Saga Musix Status assigned => resolved
2022-11-01 16:36 Saga Musix Resolution open => fixed
2022-11-01 16:36 Saga Musix Fixed in Version => OpenMPT 1.30.08.00 / libopenmpt 0.6.6 (upgrade first)
2022-11-01 16:59 Saga Musix Relationship deleted related to 0000713
2022-11-03 06:38 c0d3h4x0r Relationship added related to 0001635