From 0adee378fd0486134bc059cd197ddf2d408328f4 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Fri, 21 Jan 2022 15:22:36 +0200 Subject: [PATCH] Kernel: Stop using LibKeyboard's CharacterMap in HIDManagement This was easily done, as the Kernel and Userland don't actually share any of the APIs exposed by it, so instead the Kernel APIs were moved to the Kernel, and the Userland APIs stayed in LibKeyboard. This has multiple advantages: * The non OOM-fallible String is not longer used for storing the character map name in the Kernel * The kernel no longer has to link to the userland LibKeyboard code * A lot of #ifdef KERNEL cruft can be removed from LibKeyboard --- Kernel/CMakeLists.txt | 5 -- Kernel/Devices/HID/HIDManagement.cpp | 48 ++++++++++++-- Kernel/Devices/HID/HIDManagement.h | 14 +++-- Kernel/Devices/HID/KeyboardDevice.cpp | 2 +- Kernel/Syscalls/keymap.cpp | 4 +- .../Libraries/LibKeyboard/CharacterMap.cpp | 63 +------------------ Userland/Libraries/LibKeyboard/CharacterMap.h | 11 +--- 7 files changed, 58 insertions(+), 89 deletions(-) diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt index 1f1c529018..8b27969e0d 100644 --- a/Kernel/CMakeLists.txt +++ b/Kernel/CMakeLists.txt @@ -354,10 +354,6 @@ set(VT_SOURCES ../Userland/Libraries/LibVT/EscapeSequenceParser.cpp ) -set(KEYBOARD_SOURCES - ../Userland/Libraries/LibKeyboard/CharacterMap.cpp -) - set(CRYPTO_SOURCES ../Userland/Libraries/LibCrypto/Cipher/AES.cpp ../Userland/Libraries/LibCrypto/Hash/SHA2.cpp @@ -372,7 +368,6 @@ if (NOT "${SERENITY_ARCH}" STREQUAL "aarch64") ${SOURCES} ${ELF_SOURCES} ${VT_SOURCES} - ${KEYBOARD_SOURCES} ${CRYPTO_SOURCES} ) else() diff --git a/Kernel/Devices/HID/HIDManagement.cpp b/Kernel/Devices/HID/HIDManagement.cpp index c7211fefbb..adfd30ecb4 100644 --- a/Kernel/Devices/HID/HIDManagement.cpp +++ b/Kernel/Devices/HID/HIDManagement.cpp @@ -87,15 +87,16 @@ size_t HIDManagement::generate_minor_device_number_for_keyboard() } UNMAP_AFTER_INIT HIDManagement::HIDManagement() - : m_character_map("en-us", DEFAULT_CHARACTER_MAP) + : m_character_map_name(KString::must_create("en-us"sv)) + , m_character_map(DEFAULT_CHARACTER_MAP) { } -void HIDManagement::set_maps(const Keyboard::CharacterMapData& character_map_data, const String& character_map_name) +void HIDManagement::set_maps(NonnullOwnPtr character_map_name, Keyboard::CharacterMapData const& character_map_data) { - m_character_map.set_character_map_data(character_map_data); - m_character_map.set_character_map_name(character_map_name); - dbgln("New Character map '{}' passed in by client.", character_map_name); + m_character_map_name = move(character_map_name); + m_character_map = character_map_data; + dbgln("New Character map '{}' passed in by client.", m_character_map_name); } UNMAP_AFTER_INIT void HIDManagement::enumerate() @@ -129,4 +130,41 @@ HIDManagement& HIDManagement::the() return *s_the; } +u32 HIDManagement::get_char_from_character_map(KeyEvent event) const +{ + auto modifiers = event.modifiers(); + auto index = event.scancode & 0xFF; // Index is last byte of scan code. + auto caps_lock_on = event.caps_lock_on; + + u32 code_point; + if (modifiers & Mod_Alt) + code_point = m_character_map.alt_map[index]; + else if ((modifiers & Mod_Shift) && (modifiers & Mod_AltGr)) + code_point = m_character_map.shift_altgr_map[index]; + else if (modifiers & Mod_Shift) + code_point = m_character_map.shift_map[index]; + else if (modifiers & Mod_AltGr) + code_point = m_character_map.altgr_map[index]; + else + code_point = m_character_map.map[index]; + + if (caps_lock_on && (modifiers == 0 || modifiers == Mod_Shift)) { + if (code_point >= 'a' && code_point <= 'z') + code_point &= ~0x20; + else if (code_point >= 'A' && code_point <= 'Z') + code_point |= 0x20; + } + + if (event.e0_prefix && event.key == Key_Slash) { + // If Key_Slash (scancode = 0x35) mapped to other form "/", we fix num pad key of "/" with this case. + code_point = '/'; + } else if (event.e0_prefix && event.key != Key_Return) { + // Except for `keypad-/` and 'keypad-return', all e0 scan codes are not actually characters. i.e., `keypad-0` and + // `Insert` have the same scancode except for the prefix, but insert should not have a code_point. + code_point = 0; + } + + return code_point; +} + } diff --git a/Kernel/Devices/HID/HIDManagement.h b/Kernel/Devices/HID/HIDManagement.h index e5a48442a8..d4caf53782 100644 --- a/Kernel/Devices/HID/HIDManagement.h +++ b/Kernel/Devices/HID/HIDManagement.h @@ -17,7 +17,7 @@ #include #include #include -#include +#include namespace Kernel { @@ -39,11 +39,12 @@ public: void enumerate(); - const String& keymap_name() const { return m_character_map.character_map_name(); } - const Keyboard::CharacterMapData& character_maps() const { return m_character_map.character_map_data(); } - const Keyboard::CharacterMap& character_map() const { return m_character_map; } + StringView keymap_name() const { return m_character_map_name->view(); } + Keyboard::CharacterMapData const& character_map() const { return m_character_map; } + u32 get_char_from_character_map(KeyEvent) const; + void set_client(KeyboardClient* client) { m_client = client; } - void set_maps(const Keyboard::CharacterMapData& character_map, const String& character_map_name); + void set_maps(NonnullOwnPtr character_map_name, Keyboard::CharacterMapData const& character_map); private: size_t generate_minor_device_number_for_mouse(); @@ -51,7 +52,8 @@ private: size_t m_mouse_minor_number { 0 }; size_t m_keyboard_minor_number { 0 }; - Keyboard::CharacterMap m_character_map; + NonnullOwnPtr m_character_map_name; + Keyboard::CharacterMapData m_character_map; KeyboardClient* m_client { nullptr }; RefPtr m_i8042_controller; NonnullRefPtrVector m_hid_devices; diff --git a/Kernel/Devices/HID/KeyboardDevice.cpp b/Kernel/Devices/HID/KeyboardDevice.cpp index 9c6ecbf137..f606271246 100644 --- a/Kernel/Devices/HID/KeyboardDevice.cpp +++ b/Kernel/Devices/HID/KeyboardDevice.cpp @@ -234,7 +234,7 @@ void KeyboardDevice::key_state_changed(u8 scan_code, bool pressed) event.flags = m_modifiers; event.e0_prefix = m_has_e0_prefix; event.caps_lock_on = m_caps_lock_on; - event.code_point = HIDManagement::the().character_map().get_char(event); + event.code_point = HIDManagement::the().get_char_from_character_map(event); // If using a non-QWERTY layout, event.key needs to be updated to be the same as event.code_point KeyCode mapped_key = code_point_to_key_code(event.code_point); diff --git a/Kernel/Syscalls/keymap.cpp b/Kernel/Syscalls/keymap.cpp index fe656e4391..6a8645351c 100644 --- a/Kernel/Syscalls/keymap.cpp +++ b/Kernel/Syscalls/keymap.cpp @@ -33,7 +33,7 @@ ErrorOr Process::sys$setkeymap(Userspacelength() > map_name_max_size) return ENAMETOOLONG; - HIDManagement::the().set_maps(character_map_data, map_name->view()); + HIDManagement::the().set_maps(move(map_name), character_map_data); return 0; } @@ -44,7 +44,7 @@ ErrorOr Process::sys$getkeymap(Userspace - -#ifndef KERNEL -# include -# include -#endif +#include +#include +#include namespace Keyboard { -#ifndef KERNEL -// The Kernel explicitly and exclusively links only this file into it. -// Thus, we cannot even include a reference to the symbol `CharacterMapFile::load_from_file`. ErrorOr CharacterMap::load_from_file(const String& map_name) { auto result = TRY(CharacterMapFile::load_from_file(map_name)); return CharacterMap(map_name, result); } -#endif CharacterMap::CharacterMap(const String& map_name, const CharacterMapData& map_data) : m_character_map_data(map_data) @@ -31,8 +25,6 @@ CharacterMap::CharacterMap(const String& map_name, const CharacterMapData& map_d { } -#ifndef KERNEL - int CharacterMap::set_system_map() { return setkeymap(m_character_map_name.characters(), m_character_map_data.map, m_character_map_data.shift_map, m_character_map_data.alt_map, m_character_map_data.altgr_map, m_character_map_data.shift_altgr_map); @@ -49,55 +41,6 @@ ErrorOr CharacterMap::fetch_system_map() return CharacterMap { keymap_name, map_data }; } -#endif - -u32 CharacterMap::get_char(KeyEvent event) const -{ - auto modifiers = event.modifiers(); - auto index = event.scancode & 0xFF; // Index is last byte of scan code. - auto caps_lock_on = event.caps_lock_on; - - u32 code_point; - if (modifiers & Mod_Alt) - code_point = m_character_map_data.alt_map[index]; - else if ((modifiers & Mod_Shift) && (modifiers & Mod_AltGr)) - code_point = m_character_map_data.shift_altgr_map[index]; - else if (modifiers & Mod_Shift) - code_point = m_character_map_data.shift_map[index]; - else if (modifiers & Mod_AltGr) - code_point = m_character_map_data.altgr_map[index]; - else - code_point = m_character_map_data.map[index]; - - if (caps_lock_on && (modifiers == 0 || modifiers == Mod_Shift)) { - if (code_point >= 'a' && code_point <= 'z') - code_point &= ~0x20; - else if (code_point >= 'A' && code_point <= 'Z') - code_point |= 0x20; - } - - if (event.e0_prefix && event.key == Key_Slash) { - // If Key_Slash (scancode = 0x35) mapped to other form "/", we fix num pad key of "/" with this case. - code_point = '/'; - } else if (event.e0_prefix && event.key != Key_Return) { - // Except for `keypad-/` and 'keypad-return', all e0 scan codes are not actually characters. i.e., `keypad-0` and - // `Insert` have the same scancode except for the prefix, but insert should not have a code_point. - code_point = 0; - } - - return code_point; -} - -void CharacterMap::set_character_map_data(CharacterMapData character_map_data) -{ - m_character_map_data = character_map_data; -} - -void CharacterMap::set_character_map_name(const String& character_map_name) -{ - m_character_map_name = character_map_name; -} - const String& CharacterMap::character_map_name() const { return m_character_map_name; diff --git a/Userland/Libraries/LibKeyboard/CharacterMap.h b/Userland/Libraries/LibKeyboard/CharacterMap.h index b4d00477db..7d3e96f176 100644 --- a/Userland/Libraries/LibKeyboard/CharacterMap.h +++ b/Userland/Libraries/LibKeyboard/CharacterMap.h @@ -6,11 +6,8 @@ #pragma once -#ifndef KERNEL -# include -#endif +#include #include -#include #include namespace Keyboard { @@ -21,14 +18,8 @@ public: CharacterMap(const String& map_name, const CharacterMapData& map_data); static ErrorOr load_from_file(const String& filename); -#ifndef KERNEL int set_system_map(); static ErrorOr fetch_system_map(); -#endif - - u32 get_char(KeyEvent) const; - void set_character_map_data(CharacterMapData character_map_data); - void set_character_map_name(const String& character_map_name); const CharacterMapData& character_map_data() const { return m_character_map_data; }; const String& character_map_name() const;