Browse Source

WindowServer: Cache internal Alt shortcuts on the Menu object

This way we don't have to recompute the set of shortcut keys every
time we're handling an in-menu keydown event.
Andreas Kling 4 years ago
parent
commit
2bac9eb79d

+ 17 - 0
Userland/Services/WindowServer/Menu.cpp

@@ -40,6 +40,7 @@
 #include <LibGfx/Triangle.h>
 #include <LibGfx/Triangle.h>
 #include <WindowServer/ClientConnection.h>
 #include <WindowServer/ClientConnection.h>
 #include <WindowServer/WindowClientEndpoint.h>
 #include <WindowServer/WindowClientEndpoint.h>
+#include <ctype.h>
 
 
 namespace WindowServer {
 namespace WindowServer {
 
 
@@ -638,4 +639,20 @@ void Menu::set_visible(bool visible)
         m_client->post_message(Messages::WindowClient::MenuVisibilityDidChange(m_menu_id, visible));
         m_client->post_message(Messages::WindowClient::MenuVisibilityDidChange(m_menu_id, visible));
 }
 }
 
 
+void Menu::add_item(NonnullOwnPtr<MenuItem> item)
+{
+    if (auto alt_shortcut = find_ampersand_shortcut_character(item->text())) {
+        m_alt_shortcut_character_to_item_indexes.ensure(tolower(alt_shortcut)).append(m_items.size());
+    }
+    m_items.append(move(item));
+}
+
+const Vector<size_t>* Menu::items_with_alt_shortcut(u32 alt_shortcut) const
+{
+    auto it = m_alt_shortcut_character_to_item_indexes.find(tolower(alt_shortcut));
+    if (it == m_alt_shortcut_character_to_item_indexes.end())
+        return nullptr;
+    return &it->value;
+}
+
 }
 }

+ 5 - 1
Userland/Services/WindowServer/Menu.h

@@ -61,7 +61,7 @@ public:
     const MenuItem& item(int index) const { return m_items.at(index); }
     const MenuItem& item(int index) const { return m_items.at(index); }
     MenuItem& item(int index) { return m_items.at(index); }
     MenuItem& item(int index) { return m_items.at(index); }
 
 
-    void add_item(NonnullOwnPtr<MenuItem>&& item) { m_items.append(move(item)); }
+    void add_item(NonnullOwnPtr<MenuItem>);
 
 
     String name() const { return m_name; }
     String name() const { return m_name; }
 
 
@@ -128,6 +128,8 @@ public:
     void descend_into_submenu_at_hovered_item();
     void descend_into_submenu_at_hovered_item();
     void open_hovered_item(bool leave_menu_open);
     void open_hovered_item(bool leave_menu_open);
 
 
+    const Vector<size_t>* items_with_alt_shortcut(u32 alt_shortcut) const;
+
 private:
 private:
     virtual void event(Core::Event&) override;
     virtual void event(Core::Event&) override;
 
 
@@ -158,6 +160,8 @@ private:
     bool m_scrollable { false };
     bool m_scrollable { false };
     int m_scroll_offset { 0 };
     int m_scroll_offset { 0 };
     int m_max_scroll_offset { 0 };
     int m_max_scroll_offset { 0 };
+
+    HashMap<u32, Vector<size_t>> m_alt_shortcut_character_to_item_indexes;
 };
 };
 
 
 u32 find_ampersand_shortcut_character(const StringView&);
 u32 find_ampersand_shortcut_character(const StringView&);

+ 7 - 18
Userland/Services/WindowServer/MenuManager.cpp

@@ -30,7 +30,6 @@
 #include <WindowServer/MenuManager.h>
 #include <WindowServer/MenuManager.h>
 #include <WindowServer/Screen.h>
 #include <WindowServer/Screen.h>
 #include <WindowServer/WindowManager.h>
 #include <WindowServer/WindowManager.h>
-#include <ctype.h>
 
 
 namespace WindowServer {
 namespace WindowServer {
 
 
@@ -91,23 +90,13 @@ void MenuManager::event(Core::Event& event)
             && ((key_event.key() >= Key_A && key_event.key() <= Key_Z)
             && ((key_event.key() >= Key_A && key_event.key() <= Key_Z)
                 || (key_event.key() >= Key_0 && key_event.key() <= Key_9))) {
                 || (key_event.key() >= Key_0 && key_event.key() <= Key_9))) {
 
 
-            // FIXME: Maybe cache this on the menu instead of recomputing it on every alphanumeric menu keystroke?
-            HashMap<u32, size_t> alt_shortcut_to_item_index;
-            for (int i = 0; i < m_current_menu->item_count(); ++i) {
-                auto& item = m_current_menu->item(i);
-                if (!item.is_enabled())
-                    continue;
-                auto alt_shortcut = find_ampersand_shortcut_character(item.text());
-                if (!alt_shortcut)
-                    continue;
-                alt_shortcut_to_item_index.set(tolower(alt_shortcut), i);
-            }
-
-            auto it = alt_shortcut_to_item_index.find(tolower(key_event.code_point()));
-
-            if (it != alt_shortcut_to_item_index.end()) {
-                auto& item = m_current_menu->item(it->value);
-                m_current_menu->set_hovered_item(it->value);
+            if (auto* shortcut_item_indexes = m_current_menu->items_with_alt_shortcut(key_event.code_point())) {
+                VERIFY(!shortcut_item_indexes->is_empty());
+                // FIXME: If there are multiple items with the same Alt shortcut, we should cycle through them
+                //        with each keypress instead of activating immediately.
+                auto index = shortcut_item_indexes->at(0);
+                auto& item = m_current_menu->item(index);
+                m_current_menu->set_hovered_item(index);
                 if (item.is_submenu())
                 if (item.is_submenu())
                     m_current_menu->descend_into_submenu_at_hovered_item();
                     m_current_menu->descend_into_submenu_at_hovered_item();
                 else
                 else