From 60a96b3786a2edb98a242d52fe4beeff5be4f9b4 Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 14 Apr 2023 18:57:41 +0300 Subject: [PATCH] Userland: Actually use the correct character map index from KeyEvent Instead of using a scan code, which for scan code set 2 will not represent the expected character mapping index, we could just use another variable in the KeyEvent structure that correctly points to the character index. This change is mostly relevant to the KeyboardMapper application, and also to the WindowServer code, as both handle KeyEvents and need to use the character mapping index in various situations. --- .../KeyboardMapper/KeyPositions.h | 2 +- .../KeyboardMapper/KeyboardMapperWidget.cpp | 4 +- .../LibGUI/ConnectionToWindowServer.cpp | 8 ++-- .../LibGUI/ConnectionToWindowServer.h | 4 +- Userland/Libraries/LibGUI/Event.h | 6 ++- Userland/Services/WindowServer/Event.h | 5 ++- Userland/Services/WindowServer/Screen.cpp | 2 +- Userland/Services/WindowServer/Window.cpp | 38 ++++++++++--------- .../Services/WindowServer/WindowClient.ipc | 4 +- 9 files changed, 41 insertions(+), 32 deletions(-) diff --git a/Userland/Applications/KeyboardMapper/KeyPositions.h b/Userland/Applications/KeyboardMapper/KeyPositions.h index b5ae6eef6d..c5e32bfc76 100644 --- a/Userland/Applications/KeyboardMapper/KeyPositions.h +++ b/Userland/Applications/KeyboardMapper/KeyPositions.h @@ -9,7 +9,7 @@ #include struct KeyPosition { - u32 scancode; + u32 kernel_map_entry_index; int x; int y; int width; diff --git a/Userland/Applications/KeyboardMapper/KeyboardMapperWidget.cpp b/Userland/Applications/KeyboardMapper/KeyboardMapperWidget.cpp index ba9756ef46..4777f4e4af 100644 --- a/Userland/Applications/KeyboardMapper/KeyboardMapperWidget.cpp +++ b/Userland/Applications/KeyboardMapper/KeyboardMapperWidget.cpp @@ -203,7 +203,7 @@ ErrorOr KeyboardMapperWidget::save_to_file(StringView filename) void KeyboardMapperWidget::keydown_event(GUI::KeyEvent& event) { for (int i = 0; i < KEY_COUNT; i++) { - if (keys[i].scancode != event.scancode()) + if (keys[i].kernel_map_entry_index != event.map_entry_index()) continue; auto& tmp_button = m_keys.at(i); tmp_button->set_pressed(true); @@ -221,7 +221,7 @@ void KeyboardMapperWidget::keydown_event(GUI::KeyEvent& event) void KeyboardMapperWidget::keyup_event(GUI::KeyEvent& event) { for (int i = 0; i < KEY_COUNT; i++) { - if (keys[i].scancode != event.scancode()) + if (keys[i].kernel_map_entry_index != event.map_entry_index()) continue; auto& tmp_button = m_keys.at(i); tmp_button->set_pressed(false); diff --git a/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp b/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp index fe1c6fc51e..d30324c7b4 100644 --- a/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp +++ b/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp @@ -180,13 +180,13 @@ static Action* action_for_shortcut(Window& window, Shortcut const& shortcut) return nullptr; } -void ConnectionToWindowServer::key_down(i32 window_id, u32 code_point, u32 key, u32 modifiers, u32 scancode) +void ConnectionToWindowServer::key_down(i32 window_id, u32 code_point, u32 key, u8 map_entry_index, u32 modifiers, u32 scancode) { auto* window = Window::from_window_id(window_id); if (!window) return; - auto key_event = make(Event::KeyDown, (KeyCode)key, modifiers, code_point, scancode); + auto key_event = make(Event::KeyDown, (KeyCode)key, map_entry_index, modifiers, code_point, scancode); bool focused_widget_accepts_emoji_input = window->focused_widget() && window->focused_widget()->on_emoji_input; if (!window->blocks_emoji_input() && focused_widget_accepts_emoji_input && (modifiers == (Mod_Ctrl | Mod_Alt)) && key == Key_Space) { @@ -201,13 +201,13 @@ void ConnectionToWindowServer::key_down(i32 window_id, u32 code_point, u32 key, Core::EventLoop::current().post_event(*window, move(key_event)); } -void ConnectionToWindowServer::key_up(i32 window_id, u32 code_point, u32 key, u32 modifiers, u32 scancode) +void ConnectionToWindowServer::key_up(i32 window_id, u32 code_point, u32 key, u8 map_entry_index, u32 modifiers, u32 scancode) { auto* window = Window::from_window_id(window_id); if (!window) return; - auto key_event = make(Event::KeyUp, (KeyCode)key, modifiers, code_point, scancode); + auto key_event = make(Event::KeyUp, (KeyCode)key, map_entry_index, modifiers, code_point, scancode); Core::EventLoop::current().post_event(*window, move(key_event)); } diff --git a/Userland/Libraries/LibGUI/ConnectionToWindowServer.h b/Userland/Libraries/LibGUI/ConnectionToWindowServer.h index f041f49209..6aee8540fd 100644 --- a/Userland/Libraries/LibGUI/ConnectionToWindowServer.h +++ b/Userland/Libraries/LibGUI/ConnectionToWindowServer.h @@ -33,8 +33,8 @@ private: virtual void mouse_wheel(i32, Gfx::IntPoint, u32, u32, u32, i32, i32, i32, i32) override; virtual void window_entered(i32) override; virtual void window_left(i32) override; - virtual void key_down(i32, u32, u32, u32, u32) override; - virtual void key_up(i32, u32, u32, u32, u32) override; + virtual void key_down(i32, u32, u32, u8, u32, u32) override; + virtual void key_up(i32, u32, u32, u8, u32, u32) override; virtual void window_activated(i32) override; virtual void window_deactivated(i32) override; virtual void window_input_preempted(i32) override; diff --git a/Userland/Libraries/LibGUI/Event.h b/Userland/Libraries/LibGUI/Event.h index 12cf319406..7b13555eb7 100644 --- a/Userland/Libraries/LibGUI/Event.h +++ b/Userland/Libraries/LibGUI/Event.h @@ -386,9 +386,10 @@ enum MouseButton : u8 { class KeyEvent final : public Event { public: - KeyEvent(Type type, KeyCode key, u8 modifiers, u32 code_point, u32 scancode) + KeyEvent(Type type, KeyCode key, u8 map_entry_index, u8 modifiers, u32 code_point, u32 scancode) : Event(type) , m_key(key) + , m_map_entry_index(map_entry_index) , m_modifiers(modifiers) , m_code_point(code_point) , m_scancode(scancode) @@ -411,6 +412,8 @@ public: } u32 scancode() const { return m_scancode; } + u8 map_entry_index() const { return m_map_entry_index; } + ByteString to_byte_string() const; bool is_arrow_key() const @@ -429,6 +432,7 @@ public: private: friend class ConnectionToWindowServer; KeyCode m_key { KeyCode::Key_Invalid }; + u8 m_map_entry_index { 0 }; u8 m_modifiers { 0 }; u32 m_code_point { 0 }; u32 m_scancode { 0 }; diff --git a/Userland/Services/WindowServer/Event.h b/Userland/Services/WindowServer/Event.h index 465a8123d0..b9aacb0a06 100644 --- a/Userland/Services/WindowServer/Event.h +++ b/Userland/Services/WindowServer/Event.h @@ -60,8 +60,9 @@ enum MouseButton : u8 { class KeyEvent final : public Event { public: - KeyEvent(Type type, int key, u32 code_point, u8 modifiers, u32 scancode) + KeyEvent(Type type, int key, u8 map_entry_index, u32 code_point, u8 modifiers, u32 scancode) : Event(type) + , m_map_entry_index(map_entry_index) , m_key(key) , m_code_point(code_point) , m_modifiers(modifiers) @@ -77,10 +78,12 @@ public: u8 modifiers() const { return m_modifiers; } u32 code_point() const { return m_code_point; } u32 scancode() const { return m_scancode; } + u8 map_entry_index() const { return m_map_entry_index; } private: friend class EventLoop; friend class Screen; + u8 m_map_entry_index { 0 }; int m_key { 0 }; u32 m_code_point { 0 }; u8 m_modifiers { 0 }; diff --git a/Userland/Services/WindowServer/Screen.cpp b/Userland/Services/WindowServer/Screen.cpp index 1e055074fc..f841d6f39b 100644 --- a/Userland/Services/WindowServer/Screen.cpp +++ b/Userland/Services/WindowServer/Screen.cpp @@ -459,7 +459,7 @@ void ScreenInput::on_receive_mouse_data(MousePacket const& packet) void ScreenInput::on_receive_keyboard_data(::KeyEvent kernel_event) { m_modifiers = kernel_event.modifiers(); - auto message = make(kernel_event.is_press() ? Event::KeyDown : Event::KeyUp, kernel_event.key, kernel_event.code_point, kernel_event.modifiers(), kernel_event.scancode); + auto message = make(kernel_event.is_press() ? Event::KeyDown : Event::KeyUp, kernel_event.key, kernel_event.map_entry_index, kernel_event.code_point, kernel_event.modifiers(), kernel_event.scancode); Core::EventLoop::current().post_event(WindowManager::the(), move(message)); } diff --git a/Userland/Services/WindowServer/Window.cpp b/Userland/Services/WindowServer/Window.cpp index 466a1741b8..c53af40420 100644 --- a/Userland/Services/WindowServer/Window.cpp +++ b/Userland/Services/WindowServer/Window.cpp @@ -477,6 +477,7 @@ void Window::event(Core::Event& event) m_client->async_key_up(m_window_id, (u32) static_cast(event).code_point(), (u32) static_cast(event).key(), + (u8) static_cast(event).map_entry_index(), static_cast(event).modifiers(), (u32) static_cast(event).scancode()); break; @@ -516,33 +517,34 @@ void Window::handle_keydown_event(KeyEvent const& event) if (event.modifiers() == Mod_Alt && event.code_point() && m_menubar.has_menus()) { // When handling alt shortcuts, we only care about the key that has been pressed in addition // to alt, not the code point that has been mapped to alt+[key], so we have to look up the - // scancode directly from the "unmodified" character map. + // kernel map_entry_index value directly from the "unmodified" character map. auto character_map_or_error = Keyboard::CharacterMap::fetch_system_map(); if (!character_map_or_error.is_error()) { auto& character_map = character_map_or_error.value(); - // The lowest byte serves as our index into the character table. - auto index = event.scancode() & 0xff; - u32 character = to_ascii_lowercase(character_map.character_map_data().map[index]); + auto index = event.map_entry_index(); + if (index != 0xff) { + u32 character = to_ascii_lowercase(character_map.character_map_data().map[index]); - Menu* menu_to_open = nullptr; - m_menubar.for_each_menu([&](Menu& menu) { - if (to_ascii_lowercase(menu.alt_shortcut_character()) == character) { - menu_to_open = &menu; - return IterationDecision::Break; + Menu* menu_to_open = nullptr; + m_menubar.for_each_menu([&](Menu& menu) { + if (to_ascii_lowercase(menu.alt_shortcut_character()) == character) { + menu_to_open = &menu; + return IterationDecision::Break; + } + return IterationDecision::Continue; + }); + + if (menu_to_open) { + frame().open_menubar_menu(*menu_to_open); + if (!menu_to_open->is_empty()) + menu_to_open->set_hovered_index(0); + return; } - return IterationDecision::Continue; - }); - - if (menu_to_open) { - frame().open_menubar_menu(*menu_to_open); - if (!menu_to_open->is_empty()) - menu_to_open->set_hovered_index(0); - return; } } } - m_client->async_key_down(m_window_id, (u32)event.code_point(), (u32)event.key(), event.modifiers(), (u32)event.scancode()); + m_client->async_key_down(m_window_id, (u32)event.code_point(), (u32)event.key(), (u8)event.map_entry_index(), event.modifiers(), (u32)event.scancode()); } void Window::set_visible(bool b) diff --git a/Userland/Services/WindowServer/WindowClient.ipc b/Userland/Services/WindowServer/WindowClient.ipc index 5da1570e42..237ff27840 100644 --- a/Userland/Services/WindowServer/WindowClient.ipc +++ b/Userland/Services/WindowServer/WindowClient.ipc @@ -15,8 +15,8 @@ endpoint WindowClient window_left(i32 window_id) =| window_input_preempted(i32 window_id) =| window_input_restored(i32 window_id) =| - key_down(i32 window_id, u32 code_point, u32 key, u32 modifiers, u32 scancode) =| - key_up(i32 window_id, u32 code_point, u32 key, u32 modifiers, u32 scancode) =| + key_down(i32 window_id, u32 code_point, u32 key, u8 map_entry_index, u32 modifiers, u32 scancode) =| + key_up(i32 window_id, u32 code_point, u32 key, u8 map_entry_index, u32 modifiers, u32 scancode) =| window_activated(i32 window_id) =| window_deactivated(i32 window_id) =| window_state_changed(i32 window_id, bool minimized, bool maximized, bool occluded) =|