diff options
| -rw-r--r-- | crates/core/src/monitor.rs | 32 | ||||
| -rw-r--r-- | crates/platform/src/ipc/tests.rs | 24 | ||||
| -rw-r--r-- | crates/platform/src/ipc/types.rs | 20 | ||||
| -rw-r--r-- | crates/wayland/src/udev/platform.rs | 1 | ||||
| -rw-r--r-- | crates/wayland/src/udev/virtual_heads.rs | 21 | ||||
| -rw-r--r-- | docs/TODO.md | 10 |
6 files changed, 107 insertions, 1 deletions
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<String>, 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 @@ -497,6 +497,30 @@ fn set_monitor_split_with_neither_name_nor_a_resolvable_id_errors() { } #[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 // must not just vanish from `srd monitors` - it needs a row 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<std::cell::RefCell<WindowManager split: m.split, scale: m.scale, active_workspace: wm.workspace_for_monitor(m.id), + is_virtual: m.is_virtual, }); // Disabled-but-still-connected outputs, appended rather than merged in // by name - see `MonitorInfo::enabled`'s own doc comment for why @@ -482,6 +498,10 @@ pub(crate) fn monitor_snapshot(wm: &std::rc::Rc<std::cell::RefCell<WindowManager // "shows nothing, not tracked" the same way `id: u32::MAX` above // is a deliberate not-a-real-value sentinel for this same entry. active_workspace: 0, + // A fake monitor is never administratively disabled/re-enabled -- + // see `virtual_heads.rs`'s own module doc comment - so this + // branch (disabled-but-still-connected outputs) can never be one. + is_virtual: false, }); live.chain(disabled).collect() } diff --git a/crates/wayland/src/udev/platform.rs b/crates/wayland/src/udev/platform.rs index bf82b32..4012e3c 100644 --- a/crates/wayland/src/udev/platform.rs +++ b/crates/wayland/src/udev/platform.rs @@ -864,6 +864,7 @@ impl Platform for UdevPlatform { m.full_geometry = full; m.maximize_geometry = full; m.primary = false; + m.is_virtual = true; out.push(m); next_id += 1; } diff --git a/crates/wayland/src/udev/virtual_heads.rs b/crates/wayland/src/udev/virtual_heads.rs index 38444e3..a1e3569 100644 --- a/crates/wayland/src/udev/virtual_heads.rs +++ b/crates/wayland/src/udev/virtual_heads.rs @@ -90,6 +90,23 @@ impl CompState { output.set_preferred(mode); let global = output.create_global::<CompState>(&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::<CompState>(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. |