From 893cd3e547eefa93840088b5d298c4d85a8d1256 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 24 Jan 2022 23:11:36 -0800 Subject: [PATCH 1/4] Impl --- .../windows/keyboard_manager_win32.cc | 148 +++++++++--------- .../platform/windows/keyboard_manager_win32.h | 35 ++++- 2 files changed, 107 insertions(+), 76 deletions(-) diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index 365e75084e102..821f1169ea94d 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -93,7 +93,7 @@ static bool IsKeyDownShiftRight(int virtual_key, bool was_down) { } // Returns if a character sent by Win32 is a dead key. -static bool _IsDeadKey(uint32_t ch) { +static bool IsDeadKey(uint32_t ch) { return (ch & kDeadKeyCharMask) != 0; } @@ -261,7 +261,7 @@ void KeyboardManagerWin32::HandleOnKeyResult( // |SendInput|. const bool is_syskey = event->action == WM_SYSKEYDOWN || event->action == WM_SYSKEYUP; - const bool real_handled = handled || _IsDeadKey(event->character) || + const bool real_handled = handled || IsDeadKey(event->character) || is_syskey || IsKeyDownShiftRight(event->key, event->was_down); @@ -284,93 +284,94 @@ void KeyboardManagerWin32::HandleOnKeyResult( RedispatchEvent(std::move(event)); } -bool KeyboardManagerWin32::HandleMessage(UINT const message, +bool KeyboardManagerWin32::HandleMessage(UINT const action, WPARAM const wparam, LPARAM const lparam) { - switch (message) { + switch (action) { case WM_DEADCHAR: case WM_SYSDEADCHAR: case WM_CHAR: case WM_SYSCHAR: { - static wchar_t s_pending_high_surrogate = 0; + const Win32Message message = Win32Message{ + .action = action, .wparam = wparam, .lparam = lparam}; + current_session_.push_back(message); - wchar_t character = static_cast(wparam); std::u16string text; char32_t code_point; - if (IS_HIGH_SURROGATE(character)) { - // Save to send later with the trailing surrogate. - s_pending_high_surrogate = character; + if (message.IsHighSurrogate()) { + // A high surrogate is always followed by a low surrogate. Process the + // session later and consider this message as handled. return true; - } else if (IS_LOW_SURROGATE(character) && s_pending_high_surrogate != 0) { - text.push_back(s_pending_high_surrogate); - text.push_back(character); - // Merge the surrogate pairs for the key event. + } else if (message.IsLowSurrogate()) { + const Win32Message* last_message = + current_session_.size() <= 1 + ? nullptr + : ¤t_session_[current_session_.size() - 2]; + if (last_message == nullptr || !last_message->IsHighSurrogate()) { + return false; + } + // A low surrogate always follows a high surrogate, marking the end of + // a char session. Process the session after the if clause. + text.push_back(static_cast(last_message->wparam)); + text.push_back(static_cast(message.wparam)); code_point = - CodePointFromSurrogatePair(s_pending_high_surrogate, character); - s_pending_high_surrogate = 0; + CodePointFromSurrogatePair(last_message->wparam, message.wparam); } else { - text.push_back(character); - code_point = character; + // A non-surrogate character always appears alone. Process the session + // after the if clause. + text.push_back(static_cast(message.wparam)); + code_point = static_cast(message.wparam); } - const unsigned int scancode = (lparam >> 16) & 0xff; - - // All key presses that generate a character should be sent from - // WM_CHAR. In order to send the full key press information, the keycode - // is persisted in keycode_for_char_message_ obtained from WM_KEYDOWN. - // - // A high surrogate is always followed by a low surrogate, while a - // non-surrogate character always appears alone. Filter out high - // surrogates so that it's the low surrogate message that triggers - // the onKey, asks if the framework handles it (which can only be done - // once), and calls OnText during the redispatched messages. - if (keycode_for_char_message_ != 0 && !IS_HIGH_SURROGATE(character)) { + // If this char message is preceded by a key down message, then dispatch + // the key down message as a key down event first, and only dispatch the + // OnText if the key down event is not handled. + if (current_session_.front().IsGeneralKeyDown()) { + const Win32Message first_message = current_session_.front(); + current_session_.clear(); + const unsigned int scancode = (lparam >> 16) & 0xff; + const int keycode = first_message.wparam; const bool extended = ((lparam >> 24) & 0x01) == 0x01; const bool was_down = lparam & 0x40000000; // Certain key combinations yield control characters as WM_CHAR's // lParam. For example, 0x01 for Ctrl-A. Filter these characters. See // https://docs.microsoft.com/en-us/windows/win32/learnwin32/accelerator-tables - char32_t event_character; - if (message == WM_DEADCHAR || message == WM_SYSDEADCHAR) { + char32_t character; + if (action == WM_DEADCHAR || action == WM_SYSDEADCHAR) { // Mask the resulting char with kDeadKeyCharMask anyway, because in // rare cases the bit is *not* set (US INTL Shift-6 circumflex, see // https://github.com/flutter/flutter/issues/92654 .) - event_character = - window_delegate_->Win32MapVkToChar(keycode_for_char_message_) | + character = + window_delegate_->Win32MapVkToChar(keycode) | kDeadKeyCharMask; } else { - event_character = IsPrintable(code_point) ? code_point : 0; + character = IsPrintable(code_point) ? code_point : 0; } - bool is_new_event = - OnKey(keycode_for_char_message_, scancode, - message == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, - event_character, extended, was_down, - [this, message, text](std::unique_ptr event, - bool handled) { - HandleOnKeyResult(std::move(event), handled, message, text); + const bool is_unmet_event = + OnKey(keycode, scancode, + action == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, character, + extended, was_down, + [this, action, text](std::unique_ptr event, + bool handled) { + HandleOnKeyResult(std::move(event), handled, action, text); }); - if (!is_new_event) { - break; - } - keycode_for_char_message_ = 0; - + const bool is_syskey = action == WM_SYSCHAR; // For system characters, always pass them to the default WndProc so // that system keys like the ALT-TAB are processed correctly. - if (message == WM_SYSCHAR) { - break; - } - return true; + return is_unmet_event && !is_syskey; } - // Of the messages handled here, only WM_CHAR should be treated as - // characters. WM_SYS*CHAR are not part of text input, and WM_DEADCHAR - // will be incorporated into a later WM_CHAR with the full character. - // Also filter out: - // - Lead surrogates, which like dead keys will be send once combined. - // - ASCII control characters, which are sent as WM_CHAR events for all - // control key shortcuts. - if (message == WM_CHAR && s_pending_high_surrogate == 0 && - IsPrintable(character)) { + // If the charcter session is not preceded by a key down message, dispatch + // the OnText immediately. + + // Only WM_CHAR should be treated as characters. WM_SYS*CHAR are not part + // of text input, and WM_DEADCHAR will be incorporated into a later + // WM_CHAR with the full character. + // + // Also filter out ASCII control characters, which are sent as WM_CHAR + // events for all control key shortcuts. + current_session_.clear(); + if (action == WM_CHAR && IsPrintable(wparam)) { window_delegate_->OnText(text); } return true; @@ -380,8 +381,11 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, case WM_SYSKEYDOWN: case WM_KEYUP: case WM_SYSKEYUP: { + current_session_.clear(); + current_session_.push_back( + Win32Message{.action = action, .wparam = wparam, .lparam = lparam}); const bool is_keydown_message = - (message == WM_KEYDOWN || message == WM_SYSKEYDOWN); + (action == WM_KEYDOWN || action == WM_SYSKEYDOWN); // Check if this key produces a character. If so, the key press should // be sent with the character produced at WM_CHAR. Store the produced // keycode (it's not accessible from WM_CHAR) to be used in WM_CHAR. @@ -397,30 +401,28 @@ bool KeyboardManagerWin32::HandleMessage(UINT const message, next_key_message == WM_SYSDEADCHAR || next_key_message == WM_CHAR || next_key_message == WM_SYSCHAR); if (character > 0 && is_keydown_message && has_wm_char) { - keycode_for_char_message_ = wparam; + // This key down message has following char events. Process later, + // because the character for the OnKey should be decided by the char + // events. Consider this event as handled. return true; } - unsigned int keyCode(wparam); + + // Resolve session: A non-char key event. const uint8_t scancode = (lparam >> 16) & 0xff; const bool extended = ((lparam >> 24) & 0x01) == 0x01; // If the key is a modifier, get its side. - keyCode = ResolveKeyCode(keyCode, extended, scancode); + const uint16_t keyCode = ResolveKeyCode(wparam, extended, scancode); const bool was_down = lparam & 0x40000000; - bool is_syskey = message == WM_SYSKEYDOWN || message == WM_SYSKEYUP; - bool is_new_event = OnKey( - keyCode, scancode, message, 0, extended, was_down, + current_session_.clear(); + const bool is_unmet_event = OnKey( + keyCode, scancode, action, 0, extended, was_down, [this](std::unique_ptr event, bool handled) { HandleOnKeyResult(std::move(event), handled, 0, std::u16string()); }); - if (!is_new_event) { - break; - } + const bool is_syskey = action == WM_SYSKEYDOWN || action == WM_SYSKEYUP; // For system keys, always pass them to the default WndProc so that keys // like the ALT-TAB or Kanji switches are processed correctly. - if (is_syskey) { - break; - } - return true; + return is_unmet_event && !is_syskey; } default: assert(false); diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index a1c3ec1ad7688..31a709fbc2742 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -26,6 +26,18 @@ namespace flutter { // implements the window delegate. The |OnKey| and |OnText| results are // passed to those of |WindowWin32|'s, and consequently, those of // |FlutterWindowsView|'s. +// +// ## Terminology +// +// The keyboard system follows the following terminology instead of the +// inconsistent/incomplete one used by Win32: +// +// * Message: An invocation of |WndProc|, which consists of an +// action, an lparam, and a wparam. +// * Action: The type of a message. +// * Session: One to three messages that should be processed together, such +// as a key down message followed by char messages. +// * Event: A FlutterKeyEvent/ui.KeyData sent to the framework. class KeyboardManagerWin32 { public: // Define how the keyboard manager accesses Win32 system calls (to allow @@ -92,6 +104,24 @@ class KeyboardManagerWin32 { LPARAM const lparam); private: + struct Win32Message { + UINT const action; + WPARAM const wparam; + LPARAM const lparam; + + bool IsHighSurrogate() const { + return IS_HIGH_SURROGATE(wparam); + } + + bool IsLowSurrogate() const { + return IS_LOW_SURROGATE(wparam); + } + + bool IsGeneralKeyDown() const { + return action == WM_KEYDOWN || action == WM_SYSKEYDOWN; + } + }; + struct PendingEvent { uint32_t key; uint8_t scancode; @@ -141,9 +171,8 @@ class KeyboardManagerWin32 { WindowDelegate* window_delegate_; - // Keeps track of the last key code produced by a WM_KEYDOWN or WM_SYSKEYDOWN - // message. - int keycode_for_char_message_ = 0; + // Keeps track of all messages during the current session. + std::vector current_session_; std::map text_for_scancode_on_redispatch_; From ed405fec8e244c6e6a1fc0e29538bc2c40474d87 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 25 Jan 2022 01:04:44 -0800 Subject: [PATCH 2/4] Move session to pending event --- .../windows/keyboard_manager_win32.cc | 75 ++++++++++--------- .../platform/windows/keyboard_manager_win32.h | 16 ++-- 2 files changed, 45 insertions(+), 46 deletions(-) diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index 821f1169ea94d..a0d108d487645 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -181,7 +181,7 @@ bool KeyboardManagerWin32::RemoveRedispatchedEvent( const PendingEvent& incoming) { for (auto iter = pending_redispatches_.begin(); iter != pending_redispatches_.end(); ++iter) { - if ((*iter)->hash == incoming.hash) { + if ((*iter)->Hash() == incoming.Hash()) { pending_redispatches_.erase(iter); return true; } @@ -189,41 +189,25 @@ bool KeyboardManagerWin32::RemoveRedispatchedEvent( return false; } -bool KeyboardManagerWin32::OnKey(int key, - int scancode, - int action, - char32_t character, - bool extended, - bool was_down, +bool KeyboardManagerWin32::OnKey(std::unique_ptr event, OnKeyCallback callback) { - std::unique_ptr incoming = - std::make_unique(PendingEvent{ - .key = static_cast(key), - .scancode = static_cast(scancode), - .action = static_cast(action), - .character = character, - .extended = extended, - .was_down = was_down, - }); - incoming->hash = ComputeEventHash(*incoming); - - if (RemoveRedispatchedEvent(*incoming)) { + if (RemoveRedispatchedEvent(*event)) { return false; } - if (IsKeyDownAltRight(action, key, extended)) { + if (IsKeyDownAltRight(event->action, event->key, event->extended)) { if (last_key_is_ctrl_left_down) { should_synthesize_ctrl_left_up = true; } } - if (IsKeyDownCtrlLeft(action, key)) { + if (IsKeyDownCtrlLeft(event->action, event->key)) { last_key_is_ctrl_left_down = true; - ctrl_left_scancode = scancode; + ctrl_left_scancode = event->scancode; should_synthesize_ctrl_left_up = false; } else { last_key_is_ctrl_left_down = false; } - if (IsKeyUpAltRight(action, key, extended)) { + if (IsKeyUpAltRight(event->action, event->key, event->extended)) { if (should_synthesize_ctrl_left_up) { should_synthesize_ctrl_left_up = false; PendingEvent ctrl_left_up{ @@ -236,8 +220,10 @@ bool KeyboardManagerWin32::OnKey(int key, } } - window_delegate_->OnKey(key, scancode, action, character, extended, was_down, - [this, event = incoming.release(), + const PendingEvent clone = *event; + window_delegate_->OnKey(clone.key, clone.scancode, clone.action, + clone.character, clone.extended, clone.was_down, + [this, event = event.release(), callback = std::move(callback)](bool handled) { callback(std::unique_ptr(event), handled); @@ -329,8 +315,8 @@ bool KeyboardManagerWin32::HandleMessage(UINT const action, if (current_session_.front().IsGeneralKeyDown()) { const Win32Message first_message = current_session_.front(); current_session_.clear(); - const unsigned int scancode = (lparam >> 16) & 0xff; - const int keycode = first_message.wparam; + const uint8_t scancode = (lparam >> 16) & 0xff; + const uint16_t key_code = first_message.wparam; const bool extended = ((lparam >> 24) & 0x01) == 0x01; const bool was_down = lparam & 0x40000000; // Certain key combinations yield control characters as WM_CHAR's @@ -342,18 +328,25 @@ bool KeyboardManagerWin32::HandleMessage(UINT const action, // rare cases the bit is *not* set (US INTL Shift-6 circumflex, see // https://github.com/flutter/flutter/issues/92654 .) character = - window_delegate_->Win32MapVkToChar(keycode) | + window_delegate_->Win32MapVkToChar(key_code) | kDeadKeyCharMask; } else { character = IsPrintable(code_point) ? code_point : 0; } + auto event = std::make_unique(PendingEvent{ + .key = key_code, + .scancode = scancode, + .action = static_cast(action == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN), + .character = character, + .extended = extended, + .was_down = was_down, + .session = std::move(current_session_), + }); const bool is_unmet_event = - OnKey(keycode, scancode, - action == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, character, - extended, was_down, - [this, action, text](std::unique_ptr event, - bool handled) { - HandleOnKeyResult(std::move(event), handled, action, text); + OnKey(std::move(event), + [this, char_action = action, text]( + std::unique_ptr event, bool handled) { + HandleOnKeyResult(std::move(event), handled, char_action, text); }); const bool is_syskey = action == WM_SYSCHAR; // For system characters, always pass them to the default WndProc so @@ -411,11 +404,19 @@ bool KeyboardManagerWin32::HandleMessage(UINT const action, const uint8_t scancode = (lparam >> 16) & 0xff; const bool extended = ((lparam >> 24) & 0x01) == 0x01; // If the key is a modifier, get its side. - const uint16_t keyCode = ResolveKeyCode(wparam, extended, scancode); + const uint16_t key_code = ResolveKeyCode(wparam, extended, scancode); const bool was_down = lparam & 0x40000000; - current_session_.clear(); + auto event = std::make_unique(PendingEvent{ + .key = key_code, + .scancode = scancode, + .action = action, + .character = 0, + .extended = extended, + .was_down = was_down, + .session = std::move(current_session_), + }); const bool is_unmet_event = OnKey( - keyCode, scancode, action, 0, extended, was_down, + std::move(event), [this](std::unique_ptr event, bool handled) { HandleOnKeyResult(std::move(event), handled, 0, std::u16string()); }); diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index 31a709fbc2742..a1dbf5a253ec3 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -123,29 +123,27 @@ class KeyboardManagerWin32 { }; struct PendingEvent { - uint32_t key; + uint16_t key; uint8_t scancode; uint32_t action; char32_t character; bool extended; bool was_down; + std::vector session; + // A value calculated out of critical event information that can be used // to identify redispatched events. - uint64_t hash; + uint64_t Hash() const { + return ComputeEventHash(*this); + } }; using OnKeyCallback = std::function, bool)>; // Returns true if it's a new event, or false if it's a redispatched event. - bool OnKey(int key, - int scancode, - int action, - char32_t character, - bool extended, - bool was_down, - OnKeyCallback callback); + bool OnKey(std::unique_ptr event, OnKeyCallback callback); void HandleOnKeyResult(std::unique_ptr event, bool handled, From 380ac2dbb46446f71f39d73ebd28cd4f1e73b179 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 25 Jan 2022 01:17:59 -0800 Subject: [PATCH 3/4] Format --- .../windows/keyboard_manager_win32.cc | 22 +++++++++---------- .../platform/windows/keyboard_manager_win32.h | 12 +++------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index a0d108d487645..28b9419b0bc7f 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -278,8 +278,8 @@ bool KeyboardManagerWin32::HandleMessage(UINT const action, case WM_SYSDEADCHAR: case WM_CHAR: case WM_SYSCHAR: { - const Win32Message message = Win32Message{ - .action = action, .wparam = wparam, .lparam = lparam}; + const Win32Message message = + Win32Message{.action = action, .wparam = wparam, .lparam = lparam}; current_session_.push_back(message); std::u16string text; @@ -328,26 +328,26 @@ bool KeyboardManagerWin32::HandleMessage(UINT const action, // rare cases the bit is *not* set (US INTL Shift-6 circumflex, see // https://github.com/flutter/flutter/issues/92654 .) character = - window_delegate_->Win32MapVkToChar(key_code) | - kDeadKeyCharMask; + window_delegate_->Win32MapVkToChar(key_code) | kDeadKeyCharMask; } else { character = IsPrintable(code_point) ? code_point : 0; } auto event = std::make_unique(PendingEvent{ .key = key_code, .scancode = scancode, - .action = static_cast(action == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN), + .action = static_cast(action == WM_SYSCHAR ? WM_SYSKEYDOWN + : WM_KEYDOWN), .character = character, .extended = extended, .was_down = was_down, .session = std::move(current_session_), }); - const bool is_unmet_event = - OnKey(std::move(event), - [this, char_action = action, text]( - std::unique_ptr event, bool handled) { - HandleOnKeyResult(std::move(event), handled, char_action, text); - }); + const bool is_unmet_event = OnKey( + std::move(event), + [this, char_action = action, text]( + std::unique_ptr event, bool handled) { + HandleOnKeyResult(std::move(event), handled, char_action, text); + }); const bool is_syskey = action == WM_SYSCHAR; // For system characters, always pass them to the default WndProc so // that system keys like the ALT-TAB are processed correctly. diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index a1dbf5a253ec3..50626f8121f07 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -109,13 +109,9 @@ class KeyboardManagerWin32 { WPARAM const wparam; LPARAM const lparam; - bool IsHighSurrogate() const { - return IS_HIGH_SURROGATE(wparam); - } + bool IsHighSurrogate() const { return IS_HIGH_SURROGATE(wparam); } - bool IsLowSurrogate() const { - return IS_LOW_SURROGATE(wparam); - } + bool IsLowSurrogate() const { return IS_LOW_SURROGATE(wparam); } bool IsGeneralKeyDown() const { return action == WM_KEYDOWN || action == WM_SYSKEYDOWN; @@ -134,9 +130,7 @@ class KeyboardManagerWin32 { // A value calculated out of critical event information that can be used // to identify redispatched events. - uint64_t Hash() const { - return ComputeEventHash(*this); - } + uint64_t Hash() const { return ComputeEventHash(*this); } }; using OnKeyCallback = From b4550a041bc6bbe52d4325b91aec5dad161c85c6 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 25 Jan 2022 15:56:46 -0800 Subject: [PATCH 4/4] better type --- shell/platform/windows/keyboard_manager_win32.cc | 4 ++-- shell/platform/windows/keyboard_manager_win32.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/windows/keyboard_manager_win32.cc b/shell/platform/windows/keyboard_manager_win32.cc index 28b9419b0bc7f..ba9014ca78728 100644 --- a/shell/platform/windows/keyboard_manager_win32.cc +++ b/shell/platform/windows/keyboard_manager_win32.cc @@ -335,8 +335,8 @@ bool KeyboardManagerWin32::HandleMessage(UINT const action, auto event = std::make_unique(PendingEvent{ .key = key_code, .scancode = scancode, - .action = static_cast(action == WM_SYSCHAR ? WM_SYSKEYDOWN - : WM_KEYDOWN), + .action = static_cast(action == WM_SYSCHAR ? WM_SYSKEYDOWN + : WM_KEYDOWN), .character = character, .extended = extended, .was_down = was_down, diff --git a/shell/platform/windows/keyboard_manager_win32.h b/shell/platform/windows/keyboard_manager_win32.h index 50626f8121f07..8cd1dd49a81d1 100644 --- a/shell/platform/windows/keyboard_manager_win32.h +++ b/shell/platform/windows/keyboard_manager_win32.h @@ -119,9 +119,9 @@ class KeyboardManagerWin32 { }; struct PendingEvent { - uint16_t key; + WPARAM key; uint8_t scancode; - uint32_t action; + UINT action; char32_t character; bool extended; bool was_down;