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

Support VK_PACKET in vncviewer on Windows #1852

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions vncviewer/Viewport.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Copy link
Member

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.

Copy link
Author

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.

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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The 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__)
Expand Down
1 change: 1 addition & 0 deletions vncviewer/Viewport.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ class Viewport : public Fl_Widget, public EmulateMB {

#ifdef WIN32
bool altGrArmed;
uint32_t vkPacketHighSurrogate;
unsigned int altGrCtrlTime;
#endif

Expand Down
14 changes: 12 additions & 2 deletions vncviewer/keysym2ucs.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,18 @@ unsigned ucs2keysym(unsigned ucs)
if (keysym != NoSymbol)
return keysym;

/* us the directly encoded 24-bit UCS character */
if ((ucs & 0xff000000) == 0)
/* surrogates? */
if (0xd800 <= ucs && ucs <= 0xdfff)
return NoSymbol;

/* private use? */
if ((0xe000 <= ucs && ucs <= 0xf8ff) ||
(0x0f0000 <= ucs && ucs <= 0x0ffffd) ||
(0x100000 <= ucs && ucs <= 0x10fffd))
return NoSymbol;

/* ucs is a directly encoded 21-bit Unicode character */
if (ucs <= 0x10ffff)
return ucs | 0x01000000;

/* no matching keysym value found */
Expand Down