From 91d2388d123906e9f38839fbceb876f5f6941049 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Thu, 1 Oct 2026 14:36:33 +0100 Subject: [PATCH] Fix dialog placement and decorations and compact confirmation dialogs --- compositor/src/handlers/xdg_shell.rs | 21 ++---------- compositor/src/state.rs | 1 - compositor/src/state/floating.rs | 9 +++-- compositor/src/state/layout.rs | 10 +----- compositor/src/state/windows.rs | 22 +++++++----- compositor/src/window_rules.rs | 22 +++++++----- portal/src/picker.rs | 2 +- scripts/tests/fixtures/floating-size.c | 9 +++-- .../tests/test_floating_placement_isolated.py | 34 +++++++++++++++++++ shell/ferese-shell/src/system_modal.rs | 10 +++--- 10 files changed, 83 insertions(+), 57 deletions(-) diff --git a/compositor/src/handlers/xdg_shell.rs b/compositor/src/handlers/xdg_shell.rs index a5009d1..66d31ee 100644 --- a/compositor/src/handlers/xdg_shell.rs +++ b/compositor/src/handlers/xdg_shell.rs @@ -282,7 +282,7 @@ fn check_grab(seat: &Seat, surface: &WlSurface, serial: Serial) -> Optio impl XdgDecorationHandler for Ferese { fn new_decoration(&mut self, toplevel: ToplevelSurface) { - let mode = self.decoration_mode_for(&toplevel); + let mode = Mode::ServerSide; toplevel.with_pending_state(|state| state.decoration_mode = Some(mode)); // Wait for the initial surface commit so app-id and saved floating @@ -294,31 +294,16 @@ impl XdgDecorationHandler for Ferese { } } - fn request_mode(&mut self, toplevel: ToplevelSurface, _mode: Mode) { - let mode = self.decoration_mode_for(&toplevel); + fn request_mode(&mut self, toplevel: ToplevelSurface, mode: Mode) { set_decoration_mode(&toplevel, mode); } fn unset_mode(&mut self, toplevel: ToplevelSurface) { - let mode = self.decoration_mode_for(&toplevel); + let mode = Mode::ServerSide; set_decoration_mode(&toplevel, mode); } } -impl Ferese { - fn decoration_mode_for(&self, toplevel: &ToplevelSurface) -> Mode { - self.space - .elements() - .find(|window| { - window - .toplevel() - .is_some_and(|candidate| candidate.wl_surface() == toplevel.wl_surface()) - }) - .filter(|window| self.is_floating_window(window)) - .map_or(Mode::ServerSide, |_| Mode::ClientSide) - } -} - fn set_decoration_mode(toplevel: &ToplevelSurface, mode: Mode) { toplevel.with_pending_state(|state| state.decoration_mode = Some(mode)); if initial_configure_sent(toplevel) { diff --git a/compositor/src/state.rs b/compositor/src/state.rs index b52acf8..a9fed5d 100644 --- a/compositor/src/state.rs +++ b/compositor/src/state.rs @@ -37,7 +37,6 @@ use smithay::output::Output; use smithay::reexports::calloop::generic::Generic; use smithay::reexports::calloop::{EventLoop, Interest, LoopSignal, Mode, PostAction}; use smithay::reexports::drm::control::crtc; -use smithay::reexports::wayland_protocols::xdg::decoration::zv1::server::zxdg_toplevel_decoration_v1::Mode as DecorationMode; use smithay::reexports::wayland_protocols::xdg::shell::server::xdg_toplevel; use smithay::reexports::wayland_server::backend::{ClientData, ClientId, DisconnectReason, ObjectId}; use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface; diff --git a/compositor/src/state/floating.rs b/compositor/src/state/floating.rs index 8b0e379..98d03a0 100644 --- a/compositor/src/state/floating.rs +++ b/compositor/src/state/floating.rs @@ -264,6 +264,7 @@ impl Ferese { }) .and_then(|parent| self.presented_window_rect(parent)) .and_then(|rect| intersection(rect, work)); + let dialog = crate::window_rules::is_native_dialog(app.as_deref()); let remembered = id .and_then(|id| self.floating_window_memory.get(&id)) .or_else(|| { @@ -273,7 +274,7 @@ impl Ferese { None } }) - .filter(|entry| entry.output == output) + .filter(|entry| !dialog && entry.output == output) .and_then(|entry| entry.restore(&outputs)); let (width, height) = overrides.unwrap_or_default(); let constraints = window @@ -301,7 +302,9 @@ impl Ferese { constraints, ); let dimensions = (sized.width, sized.height); - let rect = if let Some(parent) = parent_rect { + let rect = if dialog { + centered(dimensions, work) + } else if let Some(parent) = parent_rect { centered(dimensions, parent) } else if let Some(mut rect) = remembered { rect.width = dimensions.0; @@ -341,7 +344,7 @@ impl Ferese { return; }; let (app, parent) = Self::floating_metadata(window); - if parent.is_some() { + if parent.is_some() || crate::window_rules::is_native_dialog(app.as_deref()) { return; } // A drag can finish on another output without changing workspace ownership. diff --git a/compositor/src/state/layout.rs b/compositor/src/state/layout.rs index 9da0c66..f094665 100644 --- a/compositor/src/state/layout.rs +++ b/compositor/src/state/layout.rs @@ -310,15 +310,7 @@ impl Ferese { | state.states.unset(xdg_toplevel::State::TiledBottom) }; - let decoration_mode = if is_floating && !is_fullscreen && !is_maximized { - DecorationMode::ClientSide - } else { - DecorationMode::ServerSide - }; - let decoration_changed = state.decoration_mode != Some(decoration_mode); - state.decoration_mode = Some(decoration_mode); - - fullscreen_changed || maximized_changed || tiled_changed || decoration_changed + fullscreen_changed || maximized_changed || tiled_changed }); if (natural_pending || requested_size.is_some() || state_changed) diff --git a/compositor/src/state/windows.rs b/compositor/src/state/windows.rs index c50b8ad..e0795d6 100644 --- a/compositor/src/state/windows.rs +++ b/compositor/src/state/windows.rs @@ -16,7 +16,8 @@ impl Ferese { || pending.states.contains(xdg_toplevel::State::Maximized) }); let rule = resolve_window_rules(&self.window_rules, app_id.as_deref(), title.as_deref(), transient); - if transient + if crate::window_rules::is_native_dialog(app_id.as_deref()) + || transient || special_mode || rule.fullscreen == Some(true) || !rule.floating.unwrap_or(rule.width.is_some() || rule.height.is_some()) @@ -71,14 +72,17 @@ impl Ferese { if let Some(pointer) = self.seat.get_pointer() { self.focus_output_at(pointer.current_location()); } - let remembered_work = app_id.and_then(|app| self.floating_memory.get(app)).and_then(|entry| { - let outputs = self.floating_outputs(); - entry.restore(&outputs)?; - outputs - .into_iter() - .find(|(name, _)| *name == entry.output) - .map(|(_, work)| work) - }); + let remembered_work = app_id + .filter(|app| !crate::window_rules::is_native_dialog(Some(app))) + .and_then(|app| self.floating_memory.get(app)) + .and_then(|entry| { + let outputs = self.floating_outputs(); + entry.restore(&outputs)?; + outputs + .into_iter() + .find(|(name, _)| *name == entry.output) + .map(|(_, work)| work) + }); if let Some(work) = remembered_work { self.focus_output_at((work.x + work.width / 2.0, work.y + work.height / 2.0).into()); } diff --git a/compositor/src/window_rules.rs b/compositor/src/window_rules.rs index fd45c58..805e73b 100644 --- a/compositor/src/window_rules.rs +++ b/compositor/src/window_rules.rs @@ -66,15 +66,7 @@ pub fn validate(configured: &[WindowRuleConfig]) -> Result, Stri pub fn resolve(rules: &[WindowRule], app_id: Option<&str>, title: Option<&str>, transient: bool) -> WindowRuleResult { let app_id = app_id.map(normalize_app_id); let mut result = WindowRuleResult::default(); - if matches!( - app_id.as_deref(), - Some( - "dev.ferese.authentication" - | "dev.ferese.screenshare" - | "dev.ferese.screenshot" - | "dev.ferese.portaldialog" - ) - ) { + if is_native_dialog(app_id.as_deref()) { result.floating = Some(true); } @@ -155,6 +147,18 @@ fn positive_dimension(value: Option, index: usize, field: &'static str) -> } } +pub(crate) fn is_native_dialog(app_id: Option<&str>) -> bool { + matches!( + app_id.map(normalize_app_id).as_deref(), + Some( + "dev.ferese.authentication" + | "dev.ferese.screenshare" + | "dev.ferese.screenshot" + | "dev.ferese.portaldialog" + ) + ) +} + fn normalize_app_id(value: &str) -> String { value .trim() diff --git a/portal/src/picker.rs b/portal/src/picker.rs index f737b42..52ffea5 100644 --- a/portal/src/picker.rs +++ b/portal/src/picker.rs @@ -252,7 +252,7 @@ impl cosmic::Application for Picker { } else { "Choose what to share" }) - .size(18), + .size(17), self.text(format!( "{} {app}", if self.prompt.window_capture { diff --git a/scripts/tests/fixtures/floating-size.c b/scripts/tests/fixtures/floating-size.c index 7f694ca..9378565 100644 --- a/scripts/tests/fixtures/floating-size.c +++ b/scripts/tests/fixtures/floating-size.c @@ -91,7 +91,9 @@ static void close_window(void *data, struct xdg_toplevel *top) { static const struct xdg_toplevel_listener top_listener = {.configure = size, .close = close_window}; #ifdef FERESE_TEST_DECORATION static void decoration_configure(void *data, struct zxdg_toplevel_decoration_v1 *decoration, uint32_t mode) { - (void)data; (void)decoration; (void)mode; + (void)data; (void)decoration; + printf("decoration %u\n", mode); + fflush(stdout); } static const struct zxdg_toplevel_decoration_v1_listener decoration_listener = {.configure = decoration_configure}; #endif @@ -115,7 +117,10 @@ int main(int argc, char **argv) { struct zxdg_toplevel_decoration_v1 *decoration = zxdg_decoration_manager_v1_get_toplevel_decoration(decoration_manager, top); zxdg_toplevel_decoration_v1_add_listener(decoration, &decoration_listener, NULL); - zxdg_toplevel_decoration_v1_set_mode(decoration, ZXDG_TOPLEVEL_DECORATION_V1_MODE_CLIENT_SIDE); + zxdg_toplevel_decoration_v1_set_mode(decoration, + argc >= 7 && !strcmp(argv[6], "server") + ? ZXDG_TOPLEVEL_DECORATION_V1_MODE_SERVER_SIDE + : ZXDG_TOPLEVEL_DECORATION_V1_MODE_CLIENT_SIDE); } #endif xdg_toplevel_set_app_id(top, argc >= 2 ? argv[1] : "ferese.test.floating-size"); diff --git a/scripts/tests/test_floating_placement_isolated.py b/scripts/tests/test_floating_placement_isolated.py index fd347b9..2b00e02 100644 --- a/scripts/tests/test_floating_placement_isolated.py +++ b/scripts/tests/test_floating_placement_isolated.py @@ -49,6 +49,9 @@ class FloatingPlacementTest(unittest.TestCase): ("reopen", [.55, .55, .25, .3]), ("rule-size", [.55, .55, .25, .3]), ("tiled", [.55, .55, .25, .3])]})) + saved = json.loads(memory.read_text()) + saved["dev.ferese.ScreenShare"] = {"output": "ferese-winit", "fractions": [.8, .1, .12, .1]} + memory.write_text(json.dumps(saved)) before = memory.read_bytes() display = Path(os.environ["WAYLAND_DISPLAY"]) if not display.is_absolute(): @@ -158,6 +161,37 @@ class FloatingPlacementTest(unittest.TestCase): if time.monotonic() > deadline: self.fail("closed client remained mapped") time.sleep(.01) + # Dialogs retain their natural size and center even with stale + # ordinary-window placement saved by an older compositor. + for attempt in range(2): + with (root / f"dialog-{attempt}.log").open("w+") as client_log: + client = launch([str(root / "client"), "dev.ferese.ScreenShare", + "400", "360", "configured", "decorated", "server"], + stdout=client_log, stderr=log) + deadline = time.monotonic() + 5 + while True: + dialog = next((w for w in command("get-windows") + if w["app_id"] == "dev.ferese.ScreenShare"), None) + if dialog and (dialog["width"], dialog["height"]) == (400, 360): + break + if time.monotonic() > deadline: + self.fail(f"dialog restored stale size: {dialog}") + time.sleep(.025) + output = next(o for o in command("get-outputs") if o["enabled"]) + self.assertAlmostEqual(dialog["x"] + 200, output["x"] + output["width"] / 2, delta=1) + self.assertAlmostEqual(dialog["y"] + 180, output["y"] + output["height"] / 2, delta=1) + time.sleep(.2) + client_log.seek(0) + modes = [line for line in client_log.read().splitlines() if line.startswith("decoration ")] + self.assertTrue(modes) + self.assertTrue(all(line == "decoration 2" for line in modes), modes) + client.terminate() + client.wait(timeout=3) + deadline = time.monotonic() + 5 + while any(w["app_id"] == "dev.ferese.ScreenShare" for w in command("get-windows")): + if time.monotonic() > deadline: + self.fail("closed dialog remained mapped") + time.sleep(.025) for mode in ["fullscreen", "maximized"]: app = "ferese.test.placement." + mode if mode == "maximized": diff --git a/shell/ferese-shell/src/system_modal.rs b/shell/ferese-shell/src/system_modal.rs index 48fffe6..b3aaa6e 100644 --- a/shell/ferese-shell/src/system_modal.rs +++ b/shell/ferese-shell/src/system_modal.rs @@ -439,12 +439,12 @@ impl FereseShell { let rows = match &modal.content { Content::Power(action) => { let mut rows = column![ - text(action.title()).size(23), + text(action.title()).size(17), text(action.description()) - .size(14) + .size(13) .class(theme::Text::Color(palette.muted)), ] - .spacing(12); + .spacing(10); if let Some(inhibitors) = &modal.inhibitors && !inhibitors.reasons.is_empty() { @@ -529,8 +529,8 @@ impl FereseShell { container(rows) .id("ferese-blur-card") .width(Length::Fill) - .max_width(if modal.is_guide() { 600 } else { 440 }) - .padding(24) + .max_width(if modal.is_guide() { 600 } else { 360 }) + .padding(if modal.is_guide() { 24 } else { 18 }) .class(theme::Container::custom(move |_| container::Style { background: (!material) .then_some(Background::Color(color_with_opacity(theme.surface_popover, 1.0))),