-
Notifications
You must be signed in to change notification settings - Fork 964
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
Support VK_PACKET in vncviewer on Windows #1852
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,6 +120,7 @@ Viewport::Viewport(int w, int h, const rfb::PixelFormat& /*serverPF*/, CConn* cc | |
lastPointerPos(0, 0), lastButtonMask(0), | ||
#ifdef WIN32 | ||
altGrArmed(false), | ||
vkPacketHighSurrogate(0), | ||
#endif | ||
firstLEDState(true), pendingClientClipboard(false), | ||
menuCtrlKey(false), menuAltKey(false), cursor(nullptr) | ||
|
@@ -971,6 +972,12 @@ int Viewport::handleSystemEvent(void *event, void *data) | |
keyCode = 0x38; | ||
} | ||
|
||
// VK_PACKET handling: Translate to WM_*CHAR message, handled below. | ||
if (vKey == VK_PACKET) { | ||
TranslateMessage(msg); | ||
return 1; | ||
} | ||
|
||
// Windows doesn't have a proper AltGr, but handles it using fake | ||
// Ctrl+Alt. However the remote end might not be Windows, so we need | ||
// to merge those in to a single AltGr event. We detect this case | ||
|
@@ -1086,6 +1093,12 @@ int Viewport::handleSystemEvent(void *event, void *data) | |
keyCode = 0x38; | ||
} | ||
|
||
// VK_PACKET handling: Translate to WM_*CHAR message, handled below. | ||
if (vKey == VK_PACKET) { | ||
TranslateMessage(msg); | ||
return 1; | ||
} | ||
|
||
// We can't get a release in the middle of an AltGr sequence, so | ||
// abort that detection | ||
if (self->altGrArmed) | ||
|
@@ -1118,6 +1131,55 @@ int Viewport::handleSystemEvent(void *event, void *data) | |
self->handleKeyRelease(0x36); | ||
} | ||
|
||
return 1; | ||
} else if ((msg->message == WM_CHAR) || (msg->message == WM_DEADCHAR) || | ||
(msg->message == WM_SYSCHAR) || (msg->message == WM_SYSDEADCHAR)) { | ||
// Windows will send VK_PACKET key events if it doesn't have a physical key | ||
// associated with the event (e.g. from a mobile device virtual keyboard). | ||
// After translating a keydown/keyup event pair with VK_PACKET as vKey, a | ||
// WM_*CHAR message will be issued containing a UTF-16 code unit. Unicode | ||
// characters outside of the range U+0000 - U+ffff (Basic Multilingual | ||
// Plane, BMP), are encoded as a pair of UTF-16 code units that will need to | ||
// be synthesized into one Unicode code point. | ||
uint32_t ucsCode = msg->wParam; | ||
if ((ucsCode & 0xfc00) == 0xd800) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the surrogate handling be a separate commit? Makes it easier to see which parts are connected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could, but only by making a first commit that exhibits wrong behavior w.r.t. surrogate pairs. Would you like that? |
||
// We have received a high surrogate code unit. Remember it and wait for | ||
// the low surrogate which should come immediately after. | ||
if (self->vkPacketHighSurrogate) { | ||
vlog.error(_("Unmatched UTF-16 surrogate pair through VK_PACKET, " | ||
"codes: 0x%04x, 0x%04x"), | ||
self->vkPacketHighSurrogate, ucsCode); | ||
} | ||
self->vkPacketHighSurrogate = ucsCode; | ||
return 1; | ||
} | ||
uint32_t codePoint = 0; | ||
if ((ucsCode & 0xfc00) == 0xdc00) { | ||
// We have received a low surrogate code unit. We should have a high | ||
// surrogate saved that we can use to calculate the code point. | ||
if (!self->vkPacketHighSurrogate) { | ||
vlog.error(_("Unmatched UTF-16 surrogate pair through VK_PACKET, " | ||
"code: 0x%04x"), | ||
ucsCode); | ||
return 1; | ||
} | ||
codePoint = (((self->vkPacketHighSurrogate & 0x03ff) << 10) | | ||
(ucsCode & 0x03ff)) + 0x010000; | ||
} else { | ||
// BMP character. Unicode characters in this range are encoded as a | ||
// single UTF-16 code unit. | ||
if (self->vkPacketHighSurrogate) { | ||
vlog.error(_("Unmatched UTF-16 surrogate pair through VK_PACKET, " | ||
"codes: 0x%04x, U+%04x"), | ||
self->vkPacketHighSurrogate, ucsCode); | ||
self->vkPacketHighSurrogate = 0; | ||
} | ||
codePoint = ucsCode; | ||
} | ||
uint32_t keySym = ucs2keysym(codePoint); | ||
uint32_t keyCode = 0x01000000 | codePoint; // Fake key code | ||
self->handleKeyPress(keyCode, keySym); | ||
self->handleKeyRelease(keyCode); | ||
return 1; | ||
} | ||
#elif defined(__APPLE__) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather avoid the risk of intercepting another message type. Was there not enough information in the
WM_KEYDOWN
event?TranslateMessage()
is able to make sense of it somehow, so there should be something there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I analyzed a hex dump of the entire
WM_KEYDOWN
message, and there is nothing there.TranslateMessage()
is a Windows API call and need not be limited to the information contained in the parameter. Perhaps there is something in adjacent memory, but even if so it'd be riskier to access that than to use the provided API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played around with it here and I could not find anything in the event either.
But I was getting proper symbols back from
win32_vkey_to_keysym()
. The complexity fromTranslateMessage()
might not be needed.Did you test that path?