diff options
| author | srdusr <[email protected]> | 2025-12-01 19:28:00 +0200 |
|---|---|---|
| committer | srdusr <[email protected]> | 2025-12-01 19:28:00 +0200 |
| commit | 9e88d3e3f86a1248b72fdb9bab59d9936a3323a6 (patch) | |
| tree | 4e96d180377b7b450fc60d236ceb5090d19efa0e /crates | |
| parent | d48dfec46a6502d23648053916c2af91ee2a6f2e (diff) | |
| download | srdwm-9e88d3e3f86a1248b72fdb9bab59d9936a3323a6.tar.gz srdwm-9e88d3e3f86a1248b72fdb9bab59d9936a3323a6.zip | |
Let a new window pick its own size instead of forcing a guessed placeholder
Root-caused "windows spawn small and square, not remembering placement or
size": new_managed_window hardcoded a fresh toplevel's geometry to 800x632
before the client had said anything about its own preferred size, and
sync_geometry forced that guess onto the client's very first
xdg_toplevel::configure unconditionally. Per xdg-shell, size: None on that
first configure is how every mainstream compositor lets a client pick its
own natural size instead; this one never did, so every app converged on
the same placeholder rectangle regardless of what it would have chosen.
Window::size_is_provisional marks a size that really was just the guess
(not a remembered geometry, a rule's explicit geometry action, or a
maximize/phone-mode fill, none of which are guesses). sync_geometry sends
size: None for such a window's first configure; a new adopt_provisional_size,
called from the commit handler, adopts the client's own real first size
into Window::geometry the moment it commits one, clamping only position so
a bigger-than-guessed window can't hang off its monitor's edge.
Live-verified in a nested compositor: a zenity dialog now renders at its
own compact natural size instead of being stretched to the old guess.
Diffstat (limited to 'crates')
| -rw-r--r-- | crates/core/src/manager/mod.rs | 2 | ||||
| -rw-r--r-- | crates/core/src/manager/tests.rs | 45 | ||||
| -rw-r--r-- | crates/core/src/manager/windows.rs | 12 | ||||
| -rw-r--r-- | crates/core/src/window.rs | 20 | ||||
| -rw-r--r-- | crates/wayland/src/protocols/compositor.rs | 7 | ||||
| -rw-r--r-- | crates/wayland/src/state/geometry.rs | 58 | ||||
| -rw-r--r-- | crates/wayland/src/state/lifecycle.rs | 9 | ||||
| -rw-r--r-- | crates/wayland/src/state/mod.rs | 8 | ||||
| -rw-r--r-- | crates/wayland/src/udev/platform.rs | 1 | ||||
| -rw-r--r-- | crates/wayland/src/winit/connect.rs | 1 |
10 files changed, 162 insertions, 1 deletions
diff --git a/crates/core/src/manager/mod.rs b/crates/core/src/manager/mod.rs index e4ac1c2..d449486 100644 --- a/crates/core/src/manager/mod.rs +++ b/crates/core/src/manager/mod.rs @@ -3,6 +3,8 @@ use crate::layout::{Layout, MasterStackLayout, NoOpLayout, TilingConfig}; use crate::monitor::{DisabledMonitor, Monitor, MonitorId, MonitorSplit}; use crate::placement::{PlacementConfig, SmartPlacement, SnapZoneKind, MIN_WINDOW_HEIGHT, MIN_WINDOW_WIDTH}; use crate::rules::WindowRule; +#[cfg(test)] +use crate::rules::{WindowMatch, WindowRuleActions}; use crate::lock_config::LockConfig; use crate::theme::ThemeConfig; use crate::window::{likely_draws_own_titlebar, ResizeEdge, TitlebarHit, Window, WindowId, RESIZE_MARGIN}; diff --git a/crates/core/src/manager/tests.rs b/crates/core/src/manager/tests.rs index 860af89..f703f25 100644 --- a/crates/core/src/manager/tests.rs +++ b/crates/core/src/manager/tests.rs @@ -26,6 +26,51 @@ } #[test] + fn a_smart_placed_window_is_marked_size_provisional() { + let mut wm = wm_with_monitor(); + let id = wm.alloc_window_id(); + let w = Window::new(id, "first"); + wm.add_window(w); + // No remembered geometry, no rule, not maximized - the size that + // just got smart-placed is nothing but `Window::new`'s own default + // guess, so a backend should be free to let the client override it. + assert!(wm.window(id).unwrap().size_is_provisional); + } + + #[test] + fn a_remembered_geometry_is_never_provisional() { + let mut wm = wm_with_monitor(); + wm.set_remembered_geometry("some-app".to_string(), (100, 100, 900, 700)); + let id = wm.alloc_window_id(); + let mut w = Window::new(id, "second"); + w.app_id = "some-app".to_string(); + wm.add_window(w); + let win = wm.window(id).unwrap(); + assert_eq!((win.geometry.width, win.geometry.height), (900, 700)); + assert!(!win.size_is_provisional, "a deliberately remembered size must never be second-guessed by the client's own default"); + } + + #[test] + fn a_rules_explicit_geometry_is_never_provisional() { + let mut wm = wm_with_monitor(); + wm.rules.push(WindowRule { matcher: WindowMatch { class: Some("ruled-app".to_string()), ..Default::default() }, actions: WindowRuleActions { geometry: Some(Rect::new(10, 10, 500, 400)), ..Default::default() } }); + let id = wm.alloc_window_id(); + let mut w = Window::new(id, "third"); + w.app_id = "ruled-app".to_string(); + wm.add_window(w); + assert!(!wm.window(id).unwrap().size_is_provisional, "a rule's own explicit geometry is a deliberate choice, not a guess"); + } + + #[test] + fn phone_mode_maximize_is_never_provisional() { + let mut wm = wm_with_monitor(); + wm.phone_mode = true; + let id = wm.alloc_window_id(); + wm.add_window(Window::new(id, "fourth")); + assert!(!wm.window(id).unwrap().size_is_provisional, "a deliberate full-monitor fill is not a guess needing a client override"); + } + + #[test] fn add_window_picks_up_the_configured_default_decoration_mode() { let mut wm = wm_with_monitor(); wm.theme.default_decorated = false; diff --git a/crates/core/src/manager/windows.rs b/crates/core/src/manager/windows.rs index 40b4c17..f29d19a 100644 --- a/crates/core/src/manager/windows.rs +++ b/crates/core/src/manager/windows.rs @@ -151,10 +151,17 @@ impl WindowManager { let step = self.next_cascade_step.get(); self.next_cascade_step.set(step.wrapping_add(1)); window.geometry = SmartPlacement::place(monitor, &existing, size, &self.placement, step); + // See `Window::size_is_provisional`'s own doc comment: + // `size` above is whatever a backend hardcoded before this + // window's real content was known, not a genuine + // preference - reset below if a rule or maximize goes on + // to give this window a real, deliberate size instead. + window.size_is_provisional = true; } } if let Some(geometry) = actions.as_ref().and_then(|a| a.geometry) { window.geometry = geometry; + window.size_is_provisional = false; } // `general.phone_mode`'s own real default (see its doc comment on // `WindowManager` for the full "optional phone mode" reasoning): @@ -176,6 +183,11 @@ impl WindowManager { self.restack_pinned(); if maximize { self.toggle_maximize(id); + // Deliberately full-monitor, not a guess - see `Window:: + // size_is_provisional`'s own doc comment. + if let Some(w) = self.windows.get_mut(&id) { + w.size_is_provisional = false; + } } id } diff --git a/crates/core/src/window.rs b/crates/core/src/window.rs index 21faeff..cc841b1 100644 --- a/crates/core/src/window.rs +++ b/crates/core/src/window.rs @@ -244,6 +244,25 @@ pub struct Window { /// This window's global-menu D-Bus address, if the client has exported /// one. See [`GlobalMenu`]'s own doc comment. pub global_menu: Option<GlobalMenu>, + /// Set by `WindowManager::add_window` when `geometry`'s size just came + /// from `SmartPlacement`'s own guessed default (`Window::new`'s + /// `640x480`, or whatever a backend hardcodes before a client has said + /// anything about its own preferred size) rather than a deliberate + /// decision - a remembered size, a rule's explicit `geometry` action, + /// or a maximize/phone-mode fill. `false` for all three of those, since + /// there is nothing provisional about a size someone actually chose. + /// + /// A backend reads this once, right after `add_window` returns, to + /// decide whether the *client's own* first real committed size should + /// be allowed to win once it arrives (see `crates/wayland/src/state/ + /// geometry.rs`'s own use of this) - reported live as "windows always + /// spawn small and square, not remembering placement or size": every + /// new toplevel was forced, via its very first `xdg_toplevel.configure`, + /// into this guessed placeholder size regardless of what the + /// application itself would have preferred, which is why every app + /// converged on the same generic footprint instead of its own natural + /// one. + pub size_is_provisional: bool, } impl Window { @@ -281,6 +300,7 @@ impl Window { rules_applied: false, anim_from: None, global_menu: None, + size_is_provisional: false, } } } diff --git a/crates/wayland/src/protocols/compositor.rs b/crates/wayland/src/protocols/compositor.rs index f7e7b74..e2868e4 100644 --- a/crates/wayland/src/protocols/compositor.rs +++ b/crates/wayland/src/protocols/compositor.rs @@ -83,6 +83,13 @@ impl CompositorHandler for CompState { if let Some(w) = self.id_to_window.get(&id) { w.on_commit(); } + // See `Window::size_is_provisional`'s own doc comment: before + // anything below reads `Window::geometry` (`redraw_decoration_ + // buffer`, `sync_geometry`), give a window still waiting on its + // own first real size a chance to adopt it from what `on_commit` + // just recomputed, rather than keep rendering/configuring + // against the guessed placeholder for one more round-trip. + self.adopt_provisional_size(id); // See `content_epoch`'s doc comment: this is the only per-commit // signal the udev backend's rounded-corner mask cache has to // invalidate itself, since content can change every frame, diff --git a/crates/wayland/src/state/geometry.rs b/crates/wayland/src/state/geometry.rs index 3d201dd..b8629bc 100644 --- a/crates/wayland/src/state/geometry.rs +++ b/crates/wayland/src/state/geometry.rs @@ -392,10 +392,19 @@ impl CompState { !caught_up && sent_at.elapsed() < CONFIGURE_THROTTLE_TIMEOUT }); if size_changed && !throttled { + // See `Window::size_is_provisional`'s own doc comment: + // the very first configure for a window whose size was + // never a real decision - just `Window::new`'s/a + // backend's own hardcoded guess - tells the client to + // pick its own size (`state.size = None`) instead of + // forcing this one on it. `last_synced_size` having no + // entry yet is exactly "this is that first configure"; + // checked before the `insert` just below overwrites it. + let let_client_choose = self.provisional_size.contains(&id) && !self.last_synced_size.contains_key(&id); self.last_synced_size.insert(id, size); self.pending_size_configure.insert(id, (size, Instant::now())); top.with_pending_state(|state| { - state.size = Some(size.into()); + state.size = if let_client_choose { None } else { Some(size.into()) }; // No configure from this compositor, ever, set any // `xdg_toplevel` state bit at all before this -- // confirmed by grepping the whole crate for @@ -507,4 +516,51 @@ impl CompState { self.resync_stacking_order(); } } + + /// Called from `CompositorHandler::commit`, right after `w.on_commit()` + /// - the first time a window still in `provisional_size` commits a + /// real, non-empty buffer, adopts the client's own chosen content size + /// into `Window::geometry` instead of leaving `add_window`'s guessed + /// placeholder in place. A no-op once `provisional_size` no longer + /// names this window (the ordinary case, checked first, so every other + /// commit pays only one `HashSet` lookup). + /// + /// Position is left exactly where `SmartPlacement` put it - only + /// clamped so a client that picked a bigger size than the guess can't + /// end up hanging off its monitor's right/bottom edge - since the + /// guessed size was only ever wrong about *size*; the cascade/grid + /// position it computed is still a perfectly good place for a window + /// of any size to open. + pub(crate) fn adopt_provisional_size(&mut self, id: WindowId) { + if !self.provisional_size.contains(&id) { + return; + } + let Some(dwindow) = self.id_to_window.get(&id) else { return }; + // Logical points, same as `xdg_toplevel::configure`'s own `size` + // (see `sync_geometry`'s doc comment on that) - converted to this + // compositor's physical-pixel `Rect` space below via the window's + // own monitor scale, the same conversion `sync_geometry` does in + // reverse. + let content = dwindow.geometry(); + if content.size.w <= 0 || content.size.h <= 0 { + // Compositor/role-only commit, no real buffer attached yet -- + // wait for the commit that actually has one. + return; + } + self.provisional_size.remove(&id); + let mut wm = self.wm.borrow_mut(); + let Some(w) = wm.window(id) else { return }; + let scale = wm.monitors().iter().find(|m| m.id == w.monitor).map(|m| m.scale).unwrap_or(1.0); + let monitor_geometry = wm.monitors().iter().find(|m| m.id == w.monitor).map(|m| m.geometry); + let band = if w.decorated { TITLEBAR_HEIGHT } else { 0 }; + let width = ((content.size.w as f64 * scale).round() as u32).max(srdwm_core::placement::MIN_WINDOW_WIDTH); + let height = ((content.size.h as f64 * scale).round() as u32).max(srdwm_core::placement::MIN_WINDOW_HEIGHT) + band; + let Some(w) = wm.window_mut(id) else { return }; + w.geometry.width = width; + w.geometry.height = height; + if let Some(monitor) = monitor_geometry { + w.geometry.x = w.geometry.x.min(monitor.right() - width as i32).max(monitor.x); + w.geometry.y = w.geometry.y.min(monitor.bottom() - height as i32).max(monitor.y); + } + } } diff --git a/crates/wayland/src/state/lifecycle.rs b/crates/wayland/src/state/lifecycle.rs index 1749a3c..1afa738 100644 --- a/crates/wayland/src/state/lifecycle.rs +++ b/crates/wayland/src/state/lifecycle.rs @@ -10,6 +10,15 @@ impl CompState { w.app_id = with_toplevel_app_id(toplevel.wl_surface()).unwrap_or_default(); w.geometry = srdwm_core::Rect::new(0, 0, 800, 600 + TITLEBAR_HEIGHT as i32 as u32); wm.add_window(w); + // See `Window::size_is_provisional`'s own doc comment: only + // when `add_window` actually used the guessed `800x600` above + // (not a remembered size, a rule's own `geometry` action, or a + // maximize/phone-mode fill) does the client get to pick its own + // size instead - `sync_geometry`/`adopt_provisional_size` are + // what actually act on membership here. + if wm.window(id).is_some_and(|w| w.size_is_provisional) { + self.provisional_size.insert(id); + } // Starts the open-slide tween (see `WindowAnim`'s doc comment): // the window's first `sync_geometry` call below will see this, // register the tween, and place it here - a few pixels below diff --git a/crates/wayland/src/state/mod.rs b/crates/wayland/src/state/mod.rs index 629b58c..f39edcc 100644 --- a/crates/wayland/src/state/mod.rs +++ b/crates/wayland/src/state/mod.rs @@ -586,6 +586,14 @@ pub(crate) struct CompState { /// motion event of every drag, which is what made moving a window /// stutter. Only a real size change now does either. pub(crate) last_synced_size: HashMap<WindowId, (i32, i32)>, + /// Windows whose `Window::size_is_provisional` was `true` at creation + /// and whose client hasn't sent a real, non-empty content commit yet -- + /// see that field's own doc comment. `sync_geometry` sends `size: None` + /// (let the client pick) instead of forcing this guessed size for the + /// one configure sent while a window is in this set; `commit()` removes + /// it and adopts the client's own real first size into `Window:: + /// geometry` the moment one arrives. + pub(crate) provisional_size: HashSet<WindowId>, /// A size-changing `xdg_toplevel.configure` that's been sent but not /// yet reflected in the client's own real committed content size -- /// `(size requested, when it was sent)`. `sync_geometry` won't send diff --git a/crates/wayland/src/udev/platform.rs b/crates/wayland/src/udev/platform.rs index d7132ee..732fbc2 100644 --- a/crates/wayland/src/udev/platform.rs +++ b/crates/wayland/src/udev/platform.rs @@ -269,6 +269,7 @@ impl UdevPlatform { border_side_buffers: HashMap::new(), color_filter_buffers: HashMap::new(), last_synced_size: HashMap::new(), + provisional_size: HashSet::new(), pending_size_configure: HashMap::new(), pending: pending.clone(), bound_keys: Rc::new(bound_keys.iter().cloned().collect::<HashSet<_>>()), diff --git a/crates/wayland/src/winit/connect.rs b/crates/wayland/src/winit/connect.rs index 39eda5a..4c6954b 100644 --- a/crates/wayland/src/winit/connect.rs +++ b/crates/wayland/src/winit/connect.rs @@ -181,6 +181,7 @@ impl WaylandPlatform { border_side_buffers: HashMap::new(), color_filter_buffers: HashMap::new(), last_synced_size: HashMap::new(), + provisional_size: HashSet::new(), pending_size_configure: HashMap::new(), pending: pending.clone(), bound_keys: Rc::new(bound_keys.iter().cloned().collect()), |