srdusr
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsrdusr <[email protected]>2025-12-01 19:28:00 +0200
committersrdusr <[email protected]>2025-12-01 19:28:00 +0200
commit9e88d3e3f86a1248b72fdb9bab59d9936a3323a6 (patch)
tree4e96d180377b7b450fc60d236ceb5090d19efa0e
parentd48dfec46a6502d23648053916c2af91ee2a6f2e (diff)
downloadsrdwm-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.
-rw-r--r--crates/core/src/manager/mod.rs2
-rw-r--r--crates/core/src/manager/tests.rs45
-rw-r--r--crates/core/src/manager/windows.rs12
-rw-r--r--crates/core/src/window.rs20
-rw-r--r--crates/wayland/src/protocols/compositor.rs7
-rw-r--r--crates/wayland/src/state/geometry.rs58
-rw-r--r--crates/wayland/src/state/lifecycle.rs9
-rw-r--r--crates/wayland/src/state/mod.rs8
-rw-r--r--crates/wayland/src/udev/platform.rs1
-rw-r--r--crates/wayland/src/winit/connect.rs1
-rw-r--r--docs/TODO.md8
11 files changed, 170 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()),
diff --git a/docs/TODO.md b/docs/TODO.md
index eba054c..c212993 100644
--- a/docs/TODO.md
+++ b/docs/TODO.md
@@ -1,5 +1,13 @@
# TODO / planned features - master checklist
+## Root cause found and fixed: every new window forced to the same guessed size, never its own (2026-08-28)
+
+Asked directly why windows "spawn small and as a square" and not centred, on top of the already-fixed "don't remember placement" bug. Read the actual code path rather than guessing, and found the real cause: `new_managed_window` hardcodes a brand-new toplevel's `Window::geometry` to `800x632` (800x600 plus the titlebar band) *before* the client has said anything about its own size, `WindowManager::add_window` feeds that same guessed number into `SmartPlacement` as if it were real, and - the actual bug - `sync_geometry` then forces that guessed size onto the client's very first `xdg_toplevel::configure` via `state.size = Some(size.into())`, unconditionally, on every single new window. Per the xdg-shell protocol, `size: None` on that first configure is the standard way every mainstream compositor (Mutter, KWin, Hyprland, sway, niri) lets a client pick its own natural size; this compositor never did, so every app - a tiny dialog and a browser alike - was flattened onto the exact same placeholder rectangle regardless of what it would have chosen for itself. That is why windows read as "the same size, small, square" rather than each app looking like itself.
+
+Fixed with a new `Window::size_is_provisional` flag, set by `add_window` only when the size it just used really was nothing but the placeholder guess - not when a remembered geometry, a rule's own explicit `geometry` action, or a phone-mode/maximize fill decided the size instead, since none of those are guesses and must never be second-guessed by whatever the client defaults to. A backend (currently just the Wayland one; XWayland/X11 already share `add_window` and could get the same treatment later) tracks membership in a new `CompState::provisional_size` set: `sync_geometry` sends `size: None` instead of the guess for that one window's first configure, and a new `adopt_provisional_size` - called from `CompositorHandler::commit` right after `on_commit()` recomputes the client's real content geometry - adopts whatever real size the client picked for itself into `Window::geometry` the moment its first non-empty buffer commit arrives, clamping only the *position* so a client that picked something bigger than the old guess can't hang off its monitor's edge. Cascade/grid placement's own *position* choice is left alone throughout - only the size was ever wrong.
+
+Live-verified in a nested compositor (`WAYLAND_DISPLAY=wayland-1`, winit backend, never the live session): a plain `zenity --info` dialog previously would have been stretched to the old guessed box; with this fix it renders at its own real, compact natural size (screenshotted via `grim`), titlebar sized to match. Full workspace build/test/clippy clean (237 core tests, +4 for `size_is_provisional`'s own remembered/rule/maximize-are-never-provisional invariants; 152 wayland, unchanged in count but exercising the new path via the nested test above).
+
## Lock screen: real content, not a bare box, plus a working on-screen keyboard (2026-08-28)
Asked directly: the native lock UI "shouldn't show a square, looks ugly/AI like," should have "other features like a normal lock," and needs a virtual keyboard. Read `render_ui_box` cold and the complaint was accurate - a flat, bordered rectangle with three left-aligned text lines (username, password dots, status), no clock, no avatar, no shadow. Every mainstream lock screen (GNOME, macOS, Windows) shows a clock/date and some identity marker; this one showed neither.