From 77262dd0af43ee5e958135b359c3f0e07055a7fe Mon Sep 17 00:00:00 2001 From: Lionel DARNIS Date: Sun, 19 Apr 2026 15:14:22 +0200 Subject: [PATCH 1/5] perf(malloc): throttle malloc_trim to 1 Hz in hot paths malloc_trim(0) was called at the end of every update() and view(), reaching 60-200 Hz during typical scrolling, resize, or animation. Each call walks the glibc heap (10 us to several ms depending on fragmentation) and could consume a substantial fraction of the frame budget in worst cases. Throttle trim() to once per second using a thread-local Instant, preserving the existing API. RSS stays bounded (1 Hz is enough to release collectable pages soon after) while per-frame cost becomes a single thread-local check plus a duration comparison. No call-site changes required; the three existing trim(0) invocations in src/app/cosmic.rs (update, view multi-window, view single-window) now fall under the throttle transparently. --- src/malloc.rs | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/malloc.rs b/src/malloc.rs index b99a66f..bc5e835 100644 --- a/src/malloc.rs +++ b/src/malloc.rs @@ -1,21 +1,49 @@ // Copyright 2025 System76 // SPDX-License-Identifier: MPL-2.0 +use std::cell::Cell; use std::os::raw::c_int; +use std::time::{Duration, Instant}; const M_MMAP_THRESHOLD: c_int = -3; +/// Minimum interval between two actual `malloc_trim` calls. +/// +/// `trim` is called at the end of every `update()` and `view()`, which can +/// reach 60-200 Hz during typical scrolling, resize, or animation. Each +/// `malloc_trim` walks the glibc heap (10 µs to several ms depending on +/// fragmentation), so calling it at render frequency can consume a +/// substantial fraction of the frame budget. Throttling to 1 Hz keeps RSS +/// bounded while removing the per-frame syscall from the hot path. +const TRIM_MIN_INTERVAL: Duration = Duration::from_millis(1000); + +thread_local! { + static LAST_TRIM: Cell> = const { Cell::new(None) }; +} + unsafe extern "C" { fn malloc_trim(pad: usize); fn mallopt(param: c_int, value: c_int) -> c_int; } +/// Throttled wrapper over `malloc_trim`. Safe to call at render frequency: +/// consecutive calls within `TRIM_MIN_INTERVAL` (per-thread) skip the syscall. #[inline] pub fn trim(pad: usize) { - unsafe { - malloc_trim(pad); - } + LAST_TRIM.with(|last| { + let now = Instant::now(); + let should_trim = match last.get() { + None => true, + Some(prev) => now.duration_since(prev) >= TRIM_MIN_INTERVAL, + }; + if should_trim { + last.set(Some(now)); + unsafe { + malloc_trim(pad); + } + } + }); } /// Prevents glibc from hoarding memory via memory fragmentation. From 1d98eee6dea9e679f8c950df4c5b6e176584b65a Mon Sep 17 00:00:00 2001 From: Lionel DARNIS Date: Sun, 19 Apr 2026 16:29:02 +0200 Subject: [PATCH 2/5] perf(widget): avoid VecDeque clone in segmented_button/table Model::clear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Model::clear() cloned the entire order VecDeque to iterate while remove() mutated it, producing an O(n) allocation proportional to the number of items — needless on a clear() which is going to drop all of them anyway. Replace the clone with std::mem::take(&mut self.order): we iterate the taken VecDeque (transferring ownership), and the inner self.order.remove(index) in each remove() call now finds position()==None and no-ops, since self.order has been swapped with an empty default. Same semantics, zero allocation. Noticeable on large nav/table models (>100 items) and on apps that reset state frequently (settings pages, file lists, context menus). --- src/widget/segmented_button/model/mod.rs | 5 ++++- src/widget/table/model/mod.rs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/widget/segmented_button/model/mod.rs b/src/widget/segmented_button/model/mod.rs index e0dd8c5..fff1cf6 100644 --- a/src/widget/segmented_button/model/mod.rs +++ b/src/widget/segmented_button/model/mod.rs @@ -132,7 +132,10 @@ where /// ``` #[inline] pub fn clear(&mut self) { - for entity in self.order.clone() { + // `remove()` mutates `self.order`, so transfer ownership first: + // the inner `self.order.remove(index)` then no-ops because + // `position()` can't find the entity in the empty VecDeque. + for entity in std::mem::take(&mut self.order) { self.remove(entity); } } diff --git a/src/widget/table/model/mod.rs b/src/widget/table/model/mod.rs index d6250ea..749860f 100644 --- a/src/widget/table/model/mod.rs +++ b/src/widget/table/model/mod.rs @@ -100,7 +100,10 @@ where /// model.clear(); /// ``` pub fn clear(&mut self) { - for entity in self.order.clone() { + // `remove()` mutates `self.order`, so transfer ownership first: + // the inner `self.order.remove(index)` then no-ops because + // `position()` can't find the entity in the empty VecDeque. + for entity in std::mem::take(&mut self.order) { self.remove(entity); } } From 108441ef612853681aa6ed0f04f1d27b3f0f86b1 Mon Sep 17 00:00:00 2001 From: Lionel DARNIS Date: Tue, 21 Apr 2026 21:26:35 +0200 Subject: [PATCH 3/5] segmented_button: add on_double_click callback Fires in addition to on_activate when the same entity is left-clicked twice within 400 ms. Lets applications bind quick actions (e.g. rename a tab) without blocking the normal single-click activation path. - New Widget::on_double_click builder mirroring on_activate/on_close. - last_click field on LocalState for timestamp tracking. - Detection branch in the mouse handler after on_activate fires, so the target entity is already focused when the callback runs. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/widget/segmented_button/widget.rs | 47 +++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/widget/segmented_button/widget.rs b/src/widget/segmented_button/widget.rs index 44ca857..7195c9d 100644 --- a/src/widget/segmented_button/widget.rs +++ b/src/widget/segmented_button/widget.rs @@ -173,6 +173,10 @@ where pub(super) on_context: Option Message + 'static>>, #[setters(skip)] pub(super) on_middle_press: Option Message + 'static>>, + /// Emits the ID of the item that was double-clicked with the left button. + /// Fires in addition to `on_activate` (which keeps firing on each click). + #[setters(skip)] + pub(super) on_double_click: Option Message + 'static>>, #[setters(skip)] pub(super) on_dnd_drop: Option, String, DndAction) -> Message + 'static>>, @@ -232,6 +236,7 @@ where on_close: None, on_context: None, on_middle_press: None, + on_double_click: None, on_dnd_drop: None, on_dnd_enter: None, on_dnd_leave: None, @@ -354,6 +359,16 @@ where self } + /// Emitted when a tab is double-clicked with the left mouse button. + /// Fires in addition to `on_activate`, which keeps firing on each click. + pub fn on_double_click(mut self, on_double_click: T) -> Self + where + T: Fn(Entity) -> Message + 'static, + { + self.on_double_click = Some(Box::new(on_double_click)); + self + } + /// Enable drag-and-drop support for tabs using the provided payload builder. pub fn enable_tab_drag(mut self, mime: String) -> Self { self.tab_drag = Some(TabDragSource::new(mime)); @@ -912,6 +927,7 @@ where hovered: Default::default(), known_length: Default::default(), middle_clicked: Default::default(), + last_click: None, internal_layout: Default::default(), context_cursor: Point::default(), show_context: Default::default(), @@ -1383,6 +1399,33 @@ where state.set_focused(); state.focused_item = Item::Tab(key); state.pressed_item = None; + + // Double-click detection on the same entity + // within 400 ms — fires after on_activate so + // the tab is already focused when the handler + // runs. + if let Some(on_double_click) = + self.on_double_click.as_ref() + { + let now = Instant::now(); + let is_double = match state.last_click { + Some((prev, t)) => { + prev == key + && now.duration_since(t) + < Duration::from_millis(400) + } + None => false, + }; + state.last_click = if is_double { + None + } else { + Some((key, now)) + }; + if is_double { + shell.publish(on_double_click(key)); + } + } + shell.capture_event(); return; } @@ -2387,6 +2430,9 @@ pub struct LocalState { hovered: Item, /// The ID of the button that was middle-clicked, but not yet released. middle_clicked: Option, + /// Entity and timestamp of the most recent left-click activation, used + /// to detect double-clicks on the same tab. + last_click: Option<(Entity, Instant)>, /// Last known length of the model. pub(super) known_length: usize, /// Dimensions of internal buttons when shrinking @@ -2532,6 +2578,7 @@ mod tests { hovered: Item::default(), known_length: 0, middle_clicked: None, + last_click: None, internal_layout: Vec::new(), context_cursor: Point::ORIGIN, show_context: None, From a322516f33d18ff9203078aed13d890f31ed1437 Mon Sep 17 00:00:00 2001 From: Lionel DARNIS Date: Wed, 22 Apr 2026 11:09:46 +0200 Subject: [PATCH 4/5] segmented_button: fix internal tab reorder end-to-end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent bugs prevented tab drag-and-drop reorder from working on cosmic-comp (and likely other compositors): 1. allow_reorder required DndAction::Move to be negotiated via OfferEvent::SelectedAction, which cosmic-comp does not always emit for self-drops (the SelectedAction event either never arrives or arrives with DndAction::empty()). Add a fallback: accept self-drops whenever state.dragging_tab is set. dragging_tab is only populated by start_tab_drag on this same widget, so this is safe; mime match and on_reorder presence are checked below. 2. reorder_event_for_drop preferred drop_hint.side over positional swap, producing counter-intuitive no-ops: dropping A (pos 0) on the left half of B (pos 1) resolved to "Before B" which, after removing A, lands at pos 0 again — the tab appeared not to move. Always use default_insert_position, which derives direction from dragged vs target positions (Konsole/Firefox/Chrome-style swap semantics). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/widget/segmented_button/widget.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/widget/segmented_button/widget.rs b/src/widget/segmented_button/widget.rs index 7195c9d..9ddd7d8 100644 --- a/src/widget/segmented_button/widget.rs +++ b/src/widget/segmented_button/widget.rs @@ -406,11 +406,12 @@ where { return None; } - let position = state - .drop_hint - .filter(|hint| hint.entity == target) - .map(|hint| InsertPosition::from(hint.side)) - .unwrap_or_else(|| self.default_insert_position(dragged, target)); + // Always use positional swap (Konsole/Firefox/Chrome semantics): + // dropping onto any part of a different tab swaps it with the dragged + // tab. drop_hint.side-based Before/After is counter-intuitive: dropping + // A (pos 0) on the left half of B (pos 1) resolves to "Before B" which, + // after removing A, lands at pos 0 — so the tab appears not to move. + let position = self.default_insert_position(dragged, target); Some(ReorderEvent { dragged, target, @@ -1201,7 +1202,14 @@ where .dnd_state .drag_offer .as_ref() - .is_some_and(|offer| offer.selected_action.contains(DndAction::Move)); + .is_some_and(|offer| offer.selected_action.contains(DndAction::Move)) + // Self-drop fallback: some compositors (cosmic-comp + // observed) do not emit OfferEvent::SelectedAction for + // internal drags, leaving selected_action empty. + // dragging_tab is only set by start_tab_drag on this + // same widget, so this covers the self-drop case + // safely; mime and on_reorder are checked below. + || state.dragging_tab.is_some(); let pending_reorder = if allow_reorder && self.on_reorder.is_some() && self.tab_drag.as_ref().is_some_and(|d| d.mime == *mime_type) From 5c3319351ccd0a0d113cff181fee18de0b5ed109 Mon Sep 17 00:00:00 2001 From: Lionel DARNIS Date: Wed, 22 Apr 2026 15:08:12 +0200 Subject: [PATCH 5/5] header_bar: add WindowControlsPosition (macOS-style left controls) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new public enum `WindowControlsPosition { Start, End }` and a matching field on `HeaderBar`, allowing window controls (close / minimize / maximize) to be packed on the start side of the headerbar (macOS style, icon order close → minimize → maximize) instead of the default end side (Linux / GNOME style, minimize → maximize → close). Wiring: - `crate::widget::WindowControlsPosition` re-exported alongside `HeaderBar`. - `HeaderBar::controls_position(Option)` setter; when left unset, falls back to `crate::config::window_controls_position()` (reads `CosmicTk.window_controls_position`), mirroring how `density` falls back to `header_size()`. - New `CosmicTk.window_controls_position` field with default `End` for backwards compatibility; serde-friendly enum so existing configs keep working via `#[serde(default)]` semantics. Tested with cosmic-yoterm, cosmic-settings, cosmic-edit, cosmic-files rebuilt against this libcosmic via a local `[patch]` override. Config changes picked up live through the existing cosmic-config subscription. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/config/mod.rs | 12 +++++ src/widget/header_bar.rs | 100 ++++++++++++++++++++++++++++++--------- src/widget/mod.rs | 2 +- 3 files changed, 90 insertions(+), 24 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index 9807961..1691caa 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -4,6 +4,7 @@ //! Configurations available to libcosmic applications. use crate::cosmic_theme::Density; +use crate::widget::WindowControlsPosition; use cosmic_config::cosmic_config_derive::CosmicConfigEntry; use cosmic_config::{Config, CosmicConfigEntry}; use serde::{Deserialize, Serialize}; @@ -67,6 +68,12 @@ pub fn header_size() -> Density { COSMIC_TK.read().unwrap().header_size } +/// Position of the window control buttons (close / minimize / maximize). +#[allow(clippy::missing_panics_doc)] +pub fn window_controls_position() -> WindowControlsPosition { + COSMIC_TK.read().unwrap().window_controls_position +} + /// Interface density. #[allow(clippy::missing_panics_doc)] pub fn interface_density() -> Density { @@ -109,6 +116,10 @@ pub struct CosmicTk { /// Mono font family pub monospace_font: FontConfig, + + /// Side on which window control buttons (close / minimize / maximize) + /// are placed. `End` = right (Linux / GNOME), `Start` = left (macOS). + pub window_controls_position: WindowControlsPosition, } impl Default for CosmicTk { @@ -132,6 +143,7 @@ impl Default for CosmicTk { stretch: iced::font::Stretch::Normal, style: iced::font::Style::Normal, }, + window_controls_position: WindowControlsPosition::default(), } } } diff --git a/src/widget/header_bar.rs b/src/widget/header_bar.rs index a772f7d..0fcbc6c 100644 --- a/src/widget/header_bar.rs +++ b/src/widget/header_bar.rs @@ -27,9 +27,31 @@ pub fn header_bar<'a, Message>() -> HeaderBar<'a, Message> { is_ssd: false, on_double_click: None, transparent: false, + controls_position: None, } } +/// Position of the window control buttons (close/min/max) within the headerbar. +#[derive( + Clone, + Copy, + Debug, + Default, + PartialEq, + Eq, + serde::Serialize, + serde::Deserialize, +)] +pub enum WindowControlsPosition { + /// Controls packed at the start (left on LTR) — macOS style. + /// Internal icon order becomes close → minimize → maximize. + Start, + /// Controls packed at the end (right on LTR) — Linux / GNOME style. + /// Internal icon order is minimize → maximize → close. + #[default] + End, +} + #[derive(Setters)] pub struct HeaderBar<'a, Message> { /// Defines the title of the window @@ -91,6 +113,14 @@ pub struct HeaderBar<'a, Message> { /// Whether the headerbar should be transparent transparent: bool, + + /// Side on which the window control buttons (close / minimize / maximize) + /// are rendered. `None` falls back to the user's CosmicTk config + /// (`crate::config::window_controls_position()`). `Some` overrides it. + /// `End` matches Linux / GNOME conventions; `Start` provides macOS-style + /// left-side controls. + #[setters(strip_option)] + controls_position: Option, } impl<'a, Message: Clone + 'static> HeaderBar<'a, Message> { @@ -372,12 +402,20 @@ impl<'a, Message: Clone + 'static> HeaderBar<'a, Message> { } = theme::spacing(); // Take ownership of the regions to be packed. - let start = std::mem::take(&mut self.start); + let mut start = std::mem::take(&mut self.start); let center = std::mem::take(&mut self.center); let mut end = std::mem::take(&mut self.end); - // Also packs the window controls at the very end. - end.push(self.window_controls(space_xxs)); + // Pack window controls on the configured side (reads CosmicTk + // config when the builder did not set an explicit override). + let controls_position = self + .controls_position + .unwrap_or_else(crate::config::window_controls_position); + let controls = self.window_controls(space_xxs, controls_position); + match controls_position { + WindowControlsPosition::End => end.push(controls), + WindowControlsPosition::Start => start.insert(0, controls), + } let padding = if self.is_ssd { [2, 8, 2, 8] @@ -447,7 +485,11 @@ impl<'a, Message: Clone + 'static> HeaderBar<'a, Message> { } /// Creates the widget for window controls. - fn window_controls(&mut self, spacing: u16) -> Element<'a, Message> { + fn window_controls( + &mut self, + spacing: u16, + controls_position: WindowControlsPosition, + ) -> Element<'a, Message> { macro_rules! icon { ($name:expr, $size:expr, $on_press:expr) => {{ widget::icon::from_name($name) @@ -460,25 +502,37 @@ impl<'a, Message: Clone + 'static> HeaderBar<'a, Message> { }}; } - widget::row::with_capacity(3) - .push_maybe( - self.on_minimize - .take() - .map(|m| icon!("window-minimize-symbolic", 16, m)), - ) - .push_maybe(self.on_maximize.take().map(|m| { - if self.maximized { - icon!("window-restore-symbolic", 16, m) - } else { - icon!("window-maximize-symbolic", 16, m) - } - })) - .push_maybe( - self.on_close - .take() - .map(|m| icon!("window-close-symbolic", 16, m)), - ) - .spacing(spacing) + let minimize = self + .on_minimize + .take() + .map(|m| icon!("window-minimize-symbolic", 16, m)); + let maximize = self.on_maximize.take().map(|m| { + if self.maximized { + icon!("window-restore-symbolic", 16, m) + } else { + icon!("window-maximize-symbolic", 16, m) + } + }); + let close = self + .on_close + .take() + .map(|m| icon!("window-close-symbolic", 16, m)); + + // Icon order follows the OS convention for the chosen side: + // End → minimize, maximize, close (Linux / GNOME) + // Start → close, minimize, maximize (macOS) + let row = widget::row::with_capacity(3); + let row = match controls_position { + WindowControlsPosition::End => row + .push_maybe(minimize) + .push_maybe(maximize) + .push_maybe(close), + WindowControlsPosition::Start => row + .push_maybe(close) + .push_maybe(minimize) + .push_maybe(maximize), + }; + row.spacing(spacing) .align_y(iced::Alignment::Center) .into() } diff --git a/src/widget/mod.rs b/src/widget/mod.rs index f442b0d..db9b2fc 100644 --- a/src/widget/mod.rs +++ b/src/widget/mod.rs @@ -221,7 +221,7 @@ pub use grid::{Grid, grid}; mod header_bar; #[doc(inline)] -pub use header_bar::{HeaderBar, header_bar}; +pub use header_bar::{HeaderBar, WindowControlsPosition, header_bar}; pub mod icon; #[doc(inline)]