WindowServer: Fix menu over-drawing

We only need to re-draw the item being selected and the item being
deselected. We also don't care anymore if applets were added or
removed as we no longer have a global menu bar.

Userland/Services/WindowServer/AppletManager.cpp

@@ -94,8 +94,6 @@ void AppletManager::add_applet(Window& applet)
     });
 
     relayout();
-
-    MenuManager::the().refresh();
 }
 
 void AppletManager::relayout()

Userland/Services/WindowServer/Menu.cpp

@@ -118,6 +118,12 @@ void Menu::redraw()
     menu_window()->invalidate();
 }
 
+void Menu::redraw(MenuItem const& menu_item)
+{
+    draw(menu_item);
+    menu_window()->invalidate(menu_item.rect());
+}
+
 Window& Menu::ensure_menu_window(Gfx::IntPoint const& position)
 {
     auto& screen = Screen::closest_to_location(position);
@@ -151,8 +157,10 @@ Window& Menu::ensure_menu_window(Gfx::IntPoint const& position)
         // menu's rectangle as we have more or less screen available now
         auto new_rect = calculate_window_rect();
         if (new_rect != m_menu_window->rect()) {
+            auto size_changed = new_rect.size() != m_menu_window->rect().size();
             m_menu_window->set_rect(new_rect);
-            redraw();
+            if (size_changed)
+                draw();
         }
     } else {
         auto window = Window::construct(*this, WindowType::Menu);
@@ -173,6 +181,11 @@ size_t Menu::visible_item_count() const
     return m_menu_window->height() / item_height() - 2;
 }
 
+Gfx::IntRect Menu::stripe_rect()
+{
+    return { frame_thickness(), frame_thickness(), s_stripe_width, menu_window()->height() - frame_thickness() * 2 };
+}
+
 void Menu::draw()
 {
     auto palette = WindowManager::the().palette();
@@ -185,7 +198,6 @@ void Menu::draw()
     Gfx::IntRect rect { {}, menu_window()->size() };
     painter.draw_rect(rect, Color::Black);
     painter.fill_rect(rect.shrunken(2, 2), palette.menu_base());
-    int width = this->content_width();
 
     if (!s_checked_bitmap)
         s_checked_bitmap = &Gfx::CharacterBitmap::create_from_ascii(s_checked_bitmap_data, s_checked_bitmap_width, s_checked_bitmap_height).leak_ref();
@@ -197,10 +209,9 @@ void Menu::draw()
         has_items_with_icon = has_items_with_icon | !!item.icon();
     }
 
-    Gfx::IntRect stripe_rect { frame_thickness(), frame_thickness(), s_stripe_width, menu_window()->height() - frame_thickness() * 2 };
+    // Draw the stripe first, which may extend outside of individual items. We can
-    painter.fill_rect(stripe_rect, palette.menu_stripe());
+    // skip this step when painting an individual item since we're drawing all of them
-
+    painter.fill_rect(stripe_rect(), palette.menu_stripe());
-    int visible_item_count = this->visible_item_count();
 
     if (is_scrollable()) {
         bool can_go_up = m_scroll_offset > 0;
@@ -211,8 +222,26 @@ void Menu::draw()
         painter.draw_text(down_indicator_rect, "\xE2\xAC\x87", Gfx::TextAlignment::Center, can_go_down ? palette.menu_base_text() : palette.color(ColorRole::DisabledText));
     }
 
-    for (int i = 0; i < visible_item_count; ++i) {
+    int visible_item_count = this->visible_item_count();
-        auto& item = m_items.at(m_scroll_offset + i);
+    for (int i = 0; i < visible_item_count; ++i)
+        draw(m_items.at(m_scroll_offset + i), true);
+}
+
+void Menu::draw(MenuItem const& item, bool is_drawing_all)
+{
+    auto palette = WindowManager::the().palette();
+    int width = this->content_width();
+    Gfx::Painter painter(*menu_window()->backing_store());
+    painter.add_clip_rect(item.rect());
+
+    auto stripe_rect = this->stripe_rect();
+    if (!is_drawing_all) {
+        // If we're redrawing all of them then we already did this in draw()
+        painter.fill_rect(stripe_rect, palette.menu_stripe());
+        for (auto& rect : item.rect().shatter(stripe_rect))
+            painter.fill_rect(rect, palette.menu_base());
+    }
+
     if (item.type() == MenuItem::Text) {
         Color text_color = palette.menu_base_text();
         if (&item == hovered_item() && item.is_enabled()) {
@@ -280,7 +309,6 @@ void Menu::draw()
         painter.draw_line(p1.translated(0, 1), p2.translated(0, 1), palette.threed_highlight());
     }
 }
-}
 
 MenuItem* Menu::hovered_item() const
 {
@@ -302,7 +330,6 @@ void Menu::update_for_new_hovered_item(bool make_input)
             set_visible(true);
         }
     }
-    redraw();
 }
 
 void Menu::open_hovered_item(bool leave_menu_open)
@@ -364,8 +391,11 @@ void Menu::event(Core::Event& event)
     if (event.type() == Event::MouseWheel && is_scrollable()) {
         VERIFY(menu_window());
         auto& mouse_event = static_cast<const MouseEvent&>(event);
+        auto previous_scroll_offset = m_scroll_offset;
         m_scroll_offset += mouse_event.wheel_delta();
         m_scroll_offset = clamp(m_scroll_offset, 0, m_max_scroll_offset);
+        if (m_scroll_offset != previous_scroll_offset)
+            redraw();
 
         int index = item_index_at(mouse_event.position());
         set_hovered_index(index);
@@ -657,7 +687,8 @@ void Menu::set_hovered_index(int index, bool make_input)
 {
     if (m_hovered_item_index == index)
         return;
-    if (auto* old_hovered_item = hovered_item()) {
+    auto* old_hovered_item = hovered_item();
+    if (old_hovered_item) {
         if (client() && old_hovered_item->type() != MenuItem::Type::Separator)
             client()->async_menu_item_left(m_menu_id, old_hovered_item->identifier());
     }
@@ -666,7 +697,10 @@ void Menu::set_hovered_index(int index, bool make_input)
     if (auto* new_hovered_item = hovered_item()) {
         if (client() && new_hovered_item->type() != MenuItem::Type::Separator)
             client()->async_menu_item_entered(m_menu_id, new_hovered_item->identifier());
+        redraw(*new_hovered_item);
     }
+    if (old_hovered_item)
+        redraw(*old_hovered_item);
 }
 
 bool Menu::is_open() const

Userland/Services/WindowServer/Menu.h

@@ -93,10 +93,12 @@ public:
     static constexpr int right_padding() { return 14; }
 
     void draw();
+    void draw(MenuItem const&, bool = false);
     const Gfx::Font& font() const;
 
     MenuItem* item_with_identifier(unsigned);
     void redraw();
+    void redraw(MenuItem const&);
 
     MenuItem* hovered_item() const;
 
@@ -130,6 +132,7 @@ private:
 
     void handle_mouse_move_event(const MouseEvent&);
     size_t visible_item_count() const;
+    Gfx::IntRect stripe_rect();
 
     int item_index_at(const Gfx::IntPoint&);
     static constexpr int padding_between_text_and_shortcut() { return 50; }

Userland/Services/WindowServer/MenuManager.cpp

@@ -43,6 +43,7 @@ void MenuManager::refresh()
 {
     ClientConnection::for_each_client([&](ClientConnection& client) {
         client.for_each_menu([&](Menu& menu) {
+            dbgln("MenuManager::refresh {}", &menu);
             menu.redraw();
             return IterationDecision::Continue;
         });
@@ -220,7 +221,6 @@ void MenuManager::close_everyone()
     }
     m_open_menu_stack.clear();
     clear_current_menu();
-    refresh();
 }
 
 void MenuManager::close_everyone_not_in_lineage(Menu& menu)
@@ -247,7 +247,6 @@ void MenuManager::close_menus(const Vector<Menu&>& menus)
             return entry == &menu;
         });
     }
-    refresh();
 }
 
 static void collect_menu_subtree(Menu& menu, Vector<Menu&>& menus)
@@ -313,8 +312,6 @@ void MenuManager::open_menu(Menu& menu, bool as_current_menu)
         // other menu is current
         set_current_menu(&menu);
     }
-
-    refresh();
 }
 
 void MenuManager::clear_current_menu()

Userland/Services/WindowServer/MenuManager.h

@@ -23,8 +23,6 @@ public:
     MenuManager();
     virtual ~MenuManager() override;
 
-    void refresh();
-
     bool is_open(const Menu&) const;
     bool has_open_menu() const { return !m_open_menu_stack.is_empty(); }
 
@@ -55,6 +53,8 @@ private:
     virtual void event(Core::Event&) override;
     void handle_mouse_event(MouseEvent&);
 
+    void refresh();
+
     WeakPtr<Menu> m_current_menu;
     WeakPtr<Window> m_previous_input_window;
     Vector<WeakPtr<Menu>> m_open_menu_stack;

Userland/Services/WindowServer/WindowManager.cpp

@@ -605,7 +605,6 @@ void WindowManager::notify_rect_changed(Window& window, Gfx::IntRect const& old_
     if (window.type() == WindowType::Applet)
         AppletManager::the().relayout();
 
-    MenuManager::the().refresh();
     reevaluate_hovered_window(&window);
 }