From 9592fd7acb5b68f3fae139dfac22b728c35b6198 Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Tue, 28 Oct 2025 20:09:00 +0200 Subject: Fix a fake monitor's layer-shell surfaces misrouting onto the real primary Live incident, root-caused jointly with the AGS peer session: creating a fake monitor visibly shrank the real primary output's usable area (full_y stayed 0 throughout - its true position never moved) each time, tracking almost exactly one bar height per fake monitor created. create_virtual_head registered its new Output in udev.virtual_heads but never in CompState::outputs, the list output_for_wl searches to resolve a client-named wl_output back to anything. new_layer_surface's own fallback for an output it can't resolve is landing on the primary output - so AGS's own per-monitor bar, aimed at the fake monitor it reasonably believed was a new real one, silently landed on the real primary output instead, stacking its own exclusive-zone reservation on top of the real bar already there. Two fake monitors, two misrouted bars, two zone increments, matching the observed climb exactly. Fixed by registering (and, on removal, deregistering) a virtual head's Output in CompState::outputs the same way bring_up_head already does for a real one. Also adds Monitor::is_virtual / MonitorInfo's "virtual" JSON field, requested directly by the AGS peer session as the real discriminator their own temporary FAKE- name-pattern match was standing in for. The X-position half of this same incident was AGS's own remembered- layout restore treating a fake monitor's wl_output as a real hotplug -- already fixed on their side (readArrangeable() now filters split/virtual outputs). --- crates/core/src/monitor.rs | 32 +++++++++++++++++++++++++++++++- crates/platform/src/ipc/tests.rs | 24 ++++++++++++++++++++++++ crates/platform/src/ipc/types.rs | 20 ++++++++++++++++++++ crates/wayland/src/udev/platform.rs | 1 + crates/wayland/src/udev/virtual_heads.rs | 21 +++++++++++++++++++++ docs/TODO.md | 10 ++++++++++ 6 files changed, 107 insertions(+), 1 deletion(-) diff --git a/crates/core/src/monitor.rs b/crates/core/src/monitor.rs index 6ea053f..f18edea 100644 --- a/crates/core/src/monitor.rs +++ b/crates/core/src/monitor.rs @@ -65,11 +65,41 @@ pub struct Monitor { /// other than `1.0`) traced back to exactly this missing piece of /// information. pub scale: f64, + /// `true` for a fully virtual/headless output created by `srd dispatch + /// create fake-monitor` (`crates/wayland/src/udev/virtual_heads.rs`) -- + /// a real, independent `wl_output` global with no DRM connector behind + /// it. `false` for every ordinary connected output, split part + /// included (`split` and `is_virtual` are independent: a split part is + /// still a real output's own rectangle, not a second `wl_output`). + /// + /// Requested directly by the AGS peer session after a fake monitor's + /// `wl_output` caused a real live incident: a fake output looks like an + /// ordinary new monitor to any client watching the core Wayland + /// registry (not just `wlr-output-management-v1`, which already + /// deliberately excludes it - see `virtual_heads.rs`'s own module doc + /// comment), so AGS's own remembered-layout restore treated it as a + /// real hotplug and repositioned the *real* monitor to make room for + /// it, twice, once per fake monitor created. AGS's own fix was a + /// name-pattern match (`/^FAKE-/i`) since nothing else in `srd + /// monitors`' output let it tell a fake output apart from a real one -- + /// this field is the real discriminator that match was standing in for. + pub is_virtual: bool, } impl Monitor { pub fn new(id: MonitorId, name: impl Into, geometry: Rect) -> Self { - Self { id, name: name.into(), geometry, full_geometry: geometry, maximize_geometry: geometry, refresh_rate_mhz: 60_000, primary: false, split: false, scale: 1.0 } + Self { + id, + name: name.into(), + geometry, + full_geometry: geometry, + maximize_geometry: geometry, + refresh_rate_mhz: 60_000, + primary: false, + split: false, + scale: 1.0, + is_virtual: false, + } } } diff --git a/crates/platform/src/ipc/tests.rs b/crates/platform/src/ipc/tests.rs index aa86e95..591be56 100644 --- a/crates/platform/src/ipc/tests.rs +++ b/crates/platform/src/ipc/tests.rs @@ -496,6 +496,30 @@ fn set_monitor_split_with_neither_name_nor_a_resolvable_id_errors() { assert!(line.contains(r#""error""#)); } +#[test] +fn monitors_query_marks_a_virtual_output_so_a_client_does_not_treat_it_as_a_real_hotplug() { + let dir = tempfile::tempdir().unwrap(); + let mut server = IpcServer::bind_in(dir.path(), "test").unwrap(); + let wm = Rc::new(RefCell::new(WindowManager::new())); + let real = srdwm_core::Monitor::new(0, "eDP-1", srdwm_core::Rect::new(0, 0, 1920, 1080)); + let mut fake = srdwm_core::Monitor::new(1, "FAKE-1", srdwm_core::Rect::new(1920, 0, 1920, 1080)); + fake.is_virtual = true; + wm.borrow_mut().set_monitors(vec![real, fake]); + + let mut client = UnixStream::connect(&server.path).unwrap(); + let mut reader = std::io::BufReader::new(client.try_clone().unwrap()); + client.write_all(b"{\"cmd\":\"monitors\"}\n").unwrap(); + server.poll(&wm); + let line = read_line(&mut reader); + + let parsed: serde_json::Value = serde_json::from_str(&line).unwrap(); + let monitors = parsed["monitors"].as_array().unwrap(); + let real = monitors.iter().find(|m| m["name"] == "eDP-1").unwrap(); + let fake = monitors.iter().find(|m| m["name"] == "FAKE-1").unwrap(); + assert_eq!(real["virtual"], false, "a real output must not be marked virtual"); + assert_eq!(fake["virtual"], true, "a fake monitor must be marked virtual so a client can tell it apart from a real hotplug"); +} + #[test] fn monitors_query_lists_a_disabled_output_alongside_live_ones() { // What the AGS peer session asked for directly: a disabled output diff --git a/crates/platform/src/ipc/types.rs b/crates/platform/src/ipc/types.rs index 1dc95f6..df11592 100644 --- a/crates/platform/src/ipc/types.rs +++ b/crates/platform/src/ipc/types.rs @@ -288,6 +288,21 @@ pub(crate) struct MonitorInfo { // actually displaying. See `WorkspaceInfo::monitor` for the same fact // indexed from the other direction. pub(crate) active_workspace: usize, + // `true` for a fully virtual/headless output (`srd dispatch create + // fake-monitor`) - a real `wl_output` global with no DRM connector + // behind it. Requested directly by the AGS peer session after a fake + // monitor's `wl_output` caused a real live incident: it looks like an + // ordinary new physical monitor to any client watching the core + // Wayland registry (unlike `wlr-output-management-v1`, which already + // excludes it), so AGS's own remembered-layout restore treated one + // appearing as a real hotplug and repositioned the *real* monitor to + // make room for it. Before this field existed, AGS's only option was + // matching the name against `^FAKE-` - this is the real + // discriminator that pattern was standing in for. `#[serde(rename)]` + // rather than a field literally named `virtual` because that word is + // a reserved identifier in Rust. + #[serde(rename = "virtual")] + pub(crate) is_virtual: bool, } /// Pushed to every subscriber (and used as `subscribe`'s own initial @@ -446,6 +461,7 @@ pub(crate) fn monitor_snapshot(wm: &std::rc::Rc(&self.dh); + // Also registered in `self.outputs` (mirroring `bring_up_head`'s own + // `entry` for a real head), not just `udev.virtual_heads` - without + // this, `output_for_wl` (which only ever searches `self.outputs`) + // can never resolve *this* output's own `wl_output` back to + // anything, so any client naming it in a `zwlr_layer_shell_v1:: + // get_layer_surface` request (a per-monitor bar/panel, concretely) + // silently falls through `new_layer_surface`'s "or land on the + // primary output" fallback instead - landing a bar meant for this + // fake monitor on the real primary one, stacking its exclusive + // zone on top of whatever real bar is already reserving space + // there. Confirmed live: a second fake monitor's own bar attempt + // was exactly what pushed the real monitor's reserved top strip up + // by another zone's worth, on top of the real bar's own - reported + // as the real monitor's position drifting after a fake monitor + // appeared, which was actually its *usable* (bar-shrunk) area + // shrinking further, not its true position moving at all. + self.outputs.push(crate::state::OutputEntry { output: output.clone(), location }); self.udev.as_mut().unwrap().virtual_heads.push(VirtualHead { name, output, global, size: (width, height), location }); // Payload discarded unread - `main.rs`'s own handler for this // event just re-queries the whole monitor list, same as a real @@ -111,6 +128,10 @@ impl CompState { }; let head = udev.virtual_heads.remove(index); self.dh.remove_global::(head.global); + // Mirrors `create_virtual_head`'s own registration - see that + // function's doc comment for why this output was in `self.outputs` + // at all. + self.outputs.retain(|e| e.output != head.output); self.pending.borrow_mut().push(CoreEvent::MonitorRemoved(0)); Ok(()) } diff --git a/docs/TODO.md b/docs/TODO.md index 0db3a23..f58f1df 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -13,6 +13,16 @@ that has the full story. Keep this list current as items close or open; update the source doc's own entry too, don't let this drift into a second stale copy the way `PANEL_SUPPORT_TODO.md` did. +## Fake-monitor incident, root-caused and fixed jointly with the AGS peer session (2026-08-27) + +Follow-up to the incident entry directly below. `dotfiles-1a` (AGS) confirmed from AGS's own session log (`~/.local/state/wm-session-*.log` - not `~/.cache/ags/ags.log`, which was stale) that the X-position jump was AGS's own remembered "extend-left" layout restore firing twice, once per fake monitor appearing (`arrange()` places the primary at 0,0 and walks the rest `left -= width`, then normalizes - correct behavior for a *real* second monitor; the only fault was a fake one being in the arrangeable set at all). Fixed on AGS's side: `readArrangeable()` now filters `!m.split && !m.virtual`. Their match was keyed on connector *name*, not index, ruling out the index-shift half of my own original theory. + +The Y drift (34 -> 66 -> 97, `full_y` staying `0` throughout every snapshot - confirmed from my own captured `srd monitors` output, so this was never the real monitor's true position moving, only its *usable*/bar-shrunk area shrinking further) was left unexplained by AGS's fix and flagged back to me to root-cause srdwm-side. Found it: `create_virtual_head` (`virtual_heads.rs`) created a real `wl_output` global and pushed the new head into `udev.virtual_heads`, but never registered it in `CompState::outputs` - the list `output_for_wl` (`state/mod.rs`) searches to resolve a client-named `wl_output` back to anything. `protocols/layer_shell.rs::new_layer_surface`'s own fallback for an output it can't resolve is "land on the primary output" (its own comment says so directly) - so AGS's own per-monitor bar, spawned for what it (reasonably, before its own fix above) believed was a new real monitor and aimed at that fake output, silently landed on the real primary output instead, each one stacking its own exclusive-zone reservation on top of the real bar already there. Two fake monitors, two misrouted bars, two zone increments (~32px, ~31px) - exactly the observed climb, and exactly reproducible from reading the code, not guessed. + +Fixed by registering (and, on removal, deregistering) a virtual head's `Output` in `CompState::outputs` the same way `bring_up_head` already does for a real one, so `output_for_wl` can actually resolve it and a client's layer-shell surface lands where it was actually aimed. Also added the discriminator `dotfiles-1a` asked for directly: `Monitor::is_virtual` / `MonitorInfo`'s `"virtual"` JSON field (`#[serde(rename = "virtual")]`, `is_virtual` Rust-side since `virtual` is a reserved identifier), `true` only for a fake monitor - AGS's own name-pattern match (`/^FAKE-/i`) was standing in for exactly this and can now be retired in favor of a real field. Full workspace build/test/clippy clean (34 platform tests, up from 33). + +Fake monitors are safe to create again. Not yet live-verified against a real repro (needs a restart); `dotfiles-1a` asked for a heads-up before the next live test so AGS's log can be watched at the same time. + ## Live incident: creating a fake/virtual monitor corrupted the real monitor's position, repeatedly, with no further input (2026-08-27) Asked to demo fake monitors live. `srd dispatch create fake-monitor FAKE-1 1920x1080` was harmless (`eDP-1` stayed at `full_x=0`), but creating a *second* one (`FAKE-2`) immediately moved `eDP-1` to `full_x=1920`, and it kept drifting on its own for at least one more tick afterward (`full_x=3840, y` climbing ~31px per tick) with zero further commands issued. Removing both fake monitors stopped the drift but did not self-restore `eDP-1`'s position; fixed by hand via `srd dispatch set output position eDP-1 0 0`, confirmed restored. The user's actual laptop panel visibly shifted during this. -- cgit v1.2.3