diff --git a/crates/punktfunk-host/src/vdisplay/linux/kwin.rs b/crates/punktfunk-host/src/vdisplay/linux/kwin.rs index 6922d9b..c2e0892 100644 --- a/crates/punktfunk-host/src/vdisplay/linux/kwin.rs +++ b/crates/punktfunk-host/src/vdisplay/linux/kwin.rs @@ -223,19 +223,35 @@ impl VirtualDisplay for KwinDisplay { /// Re-enable the outputs an `exclusive` topology disabled (bootstrap / physical), so KWin re-homes onto /// them. Called by the registry when the display group's last member is torn down (design §6.1), BEFORE /// that member's output is reclaimed — so KWin is never momentarily left with zero enabled outputs. -fn reenable_outputs(outputs: &[String]) { +fn reenable_outputs(outputs: &[(String, String)]) { if outputs.is_empty() { return; } - let args: Vec = outputs + // Enable FIRST, as a standalone apply — a bare `output.X.enable` always succeeds, so a physical + // can never be left DARK. (Batching a possibly-stale `mode` arg into the same invocation risks + // kscreen-doctor rejecting the whole config and leaving the output disabled.) + let enable_args: Vec = outputs .iter() - .map(|o| format!("output.{o}.enable")) + .map(|(name, _)| format!("output.{name}.enable")) .collect(); let _ = std::process::Command::new("kscreen-doctor") - .args(&args) + .args(&enable_args) .status(); + // THEN re-assert each captured mode, best-effort — a bare re-enable lets KWin fall back to the + // EDID-preferred mode (a 120 Hz panel returns at ~60 Hz); this restores the exact refresh. The + // output is enabled now, so the mode set is valid; a rejected mode just leaves KWin's default. + let mode_args: Vec = outputs + .iter() + .filter(|(_, mode)| !mode.is_empty()) + .map(|(name, mode)| format!("output.{name}.mode.{mode}")) + .collect(); + if !mode_args.is_empty() { + let _ = std::process::Command::new("kscreen-doctor") + .args(&mode_args) + .status(); + } std::thread::sleep(Duration::from_millis(200)); - tracing::info!(reenabled = ?outputs, "KWin: restored the physical/bootstrap outputs (group empty)"); + tracing::info!(reenabled = ?outputs, "KWin: restored the physical/bootstrap outputs at their captured modes (group empty)"); } /// Best-effort: raise the just-created virtual output's refresh above KWin's default 60 Hz by @@ -327,12 +343,39 @@ fn read_active_refresh(output: &str) -> Option { /// recognised by this prefix, so we never have to thread the live set through the backend. const MANAGED_PREFIX: &str = "Virtual-punktfunk"; -/// Names of currently-ENABLED outputs that are **not managed by us** — the headless session's -/// bootstrap output(s) + any physical monitor, i.e. exactly what `exclusive` must disable. +/// The current mode of an output as a kscreen-doctor mode setter, from its `-j` entry — preferring +/// the human `WxH@Hz` form (survives a mode-id re-enumeration across disable→enable) and falling back +/// to the raw `currentModeId`. `None` if the current mode can't be resolved. +fn output_current_mode_spec(o: &serde_json::Value) -> Option { + let as_id = |v: &serde_json::Value| -> Option { + v.as_str() + .map(|s| s.to_string()) + .or_else(|| v.as_u64().map(|n| n.to_string())) + }; + let current = o.get("currentModeId").and_then(&as_id)?; + let mode = o + .get("modes")? + .as_array()? + .iter() + .find(|m| m.get("id").and_then(&as_id).as_deref() == Some(current.as_str()))?; + let human = (|| { + let size = mode.get("size")?; + let w = size.get("width").and_then(|v| v.as_u64())?; + let h = size.get("height").and_then(|v| v.as_u64())?; + let hz = mode.get("refreshRate").and_then(|r| r.as_f64())?.round() as u64; + Some(format!("{w}x{h}@{hz}")) + })(); + Some(human.unwrap_or(current)) +} + +/// Currently-ENABLED outputs that are **not managed by us** — the headless session's bootstrap +/// output(s) + any physical monitor, i.e. exactly what `exclusive` must disable — EACH PAIRED WITH ITS +/// CURRENT MODE (`WxH@Hz`, empty if unresolved) so teardown can put it back at that exact refresh (a +/// bare re-enable drops a 120 Hz panel to KWin's default ~60 Hz). /// **Group-aware (§6.1):** excludes the WHOLE managed family (the [`MANAGED_PREFIX`]), not just this /// session's own output — so a 2nd `exclusive` session (with a distinct per-slot name) never disables /// the 1st session's live output. Parsed from `kscreen-doctor -j` (same source as [`read_active_refresh`]). -fn other_enabled_outputs() -> Vec { +fn other_enabled_outputs() -> Vec<(String, String)> { let out = match std::process::Command::new("kscreen-doctor") .arg("-j") .output() @@ -349,9 +392,15 @@ fn other_enabled_outputs() -> Vec { .map(|outs| { outs.iter() .filter(|o| o.get("enabled").and_then(|e| e.as_bool()).unwrap_or(false)) - .filter_map(|o| o.get("name").and_then(|n| n.as_str())) - .filter(|n| !n.starts_with(MANAGED_PREFIX)) - .map(String::from) + .filter_map(|o| { + let name = o.get("name").and_then(|n| n.as_str())?; + (!name.starts_with(MANAGED_PREFIX)).then(|| { + ( + name.to_string(), + output_current_mode_spec(o).unwrap_or_default(), + ) + }) + }) .collect() }) .unwrap_or_default() @@ -392,7 +441,7 @@ fn a_managed_output_is_primary() -> bool { /// the sole desktop (KWin re-homes plasmashell + windows onto it). Returns the disabled outputs for /// the keepalive to re-enable on teardown. Best-effort: on failure, streaming continues (just possibly /// showing only the wallpaper) rather than failing the session. -fn apply_virtual_primary(name: &str) -> Vec { +fn apply_virtual_primary(name: &str) -> Vec<(String, String)> { let ours = format!("Virtual-{name}"); let kscreen = |args: &[String]| { std::process::Command::new("kscreen-doctor") @@ -415,11 +464,12 @@ fn apply_virtual_primary(name: &str) -> Vec { } // Disable everything still enabled that ISN'T a managed group member (bootstrap / physical), so // the group is unambiguously the desktop — never a sibling session's output (group-aware filter). + // Each is captured WITH its current mode so teardown restores its real refresh, not KWin's default. let others = other_enabled_outputs(); if !others.is_empty() { let args: Vec = others .iter() - .map(|o| format!("output.{o}.disable")) + .map(|(o, _mode)| format!("output.{o}.disable")) .collect(); let _ = kscreen(&args); } diff --git a/crates/punktfunk-host/src/vdisplay/linux/mutter.rs b/crates/punktfunk-host/src/vdisplay/linux/mutter.rs index fccaa83..15861c2 100644 --- a/crates/punktfunk-host/src/vdisplay/linux/mutter.rs +++ b/crates/punktfunk-host/src/vdisplay/linux/mutter.rs @@ -412,8 +412,8 @@ fn mode_flag(md: &DbusMode, key: &str) -> bool { matches!(md.6.get(key).map(|v| &**v), Some(&Value::Bool(true))) } -/// The current (else preferred, else first) mode of `connector` → (mode_id, width, height). -fn current_mode(state: &CurrentState, connector: &str) -> Option<(String, i32, i32)> { +/// The current (else preferred, else first) mode of `connector` → `(mode_id, width, height, refresh)`. +fn current_mode_full(state: &CurrentState, connector: &str) -> Option<(String, i32, i32, f64)> { let mon = state.1.iter().find(|m| m.0 .0 == connector)?; let pick = mon .1 @@ -421,7 +421,83 @@ fn current_mode(state: &CurrentState, connector: &str) -> Option<(String, i32, i .find(|md| mode_flag(md, "is-current")) .or_else(|| mon.1.iter().find(|md| mode_flag(md, "is-preferred"))) .or_else(|| mon.1.first())?; - Some((pick.0.clone(), pick.1, pick.2)) + Some((pick.0.clone(), pick.1, pick.2, pick.3)) +} + +/// As [`current_mode_full`] but dropping the refresh (callers that only place by width). +fn current_mode(state: &CurrentState, connector: &str) -> Option<(String, i32, i32)> { + current_mode_full(state, connector).map(|(id, w, h, _)| (id, w, h)) +} + +/// Pure mode-pick for a KEPT physical (unit-tested). Given the physical's PRE-connect mode +/// (`pre_mode = (id, w, h, refresh)`; `None` when the connector is new since the snapshot) and the +/// mode list Mutter reports for it in the POST-virtual state +/// (`(id, w, h, refresh, is_current, is_preferred)`), return the `(mode_id, width)` to re-apply. +/// +/// Mutter re-derives its layout when the `RecordVirtual` output appears and can silently drop a +/// 120 Hz panel to its EDID-preferred 60 Hz — so the post-virtual `is-current` is *already* 60 Hz. +/// We therefore prefer the PRE mode (its real refresh), resolved to a mode id valid at apply time; +/// only when the physical genuinely no longer offers that mode do we fall back to the post-virtual +/// current (never inventing a mode id `ApplyMonitorsConfig` would reject). +fn pick_keep_mode( + pre_mode: Option<(String, i32, i32, f64)>, + state_modes: &[(String, i32, i32, f64, bool, bool)], +) -> Option<(String, i32)> { + let state_current = || { + state_modes + .iter() + .find(|m| m.4) + .or_else(|| state_modes.iter().find(|m| m.5)) + .or_else(|| state_modes.first()) + .map(|m| (m.0.clone(), m.1)) + }; + let Some((pre_id, w, h, hz)) = pre_mode else { + return state_current(); + }; + // The exact pre mode id, if the connector still offers it (same session ⇒ usually true). + if state_modes.iter().any(|m| m.0 == pre_id) { + return Some((pre_id, w)); + } + // Else a re-keyed id with the same geometry + refresh (still the real 120 Hz). + if let Some(m) = state_modes + .iter() + .find(|m| m.1 == w && m.2 == h && (m.3 - hz).abs() < 0.5) + { + return Some((m.0.clone(), m.1)); + } + // The physical genuinely no longer offers that mode — use whatever is valid now. + state_current() +} + +/// The `(mode_id, width)` a kept physical should be RE-APPLIED at — its PRE-connect mode preserved +/// across Mutter's virtual-output layout re-derive. See [`pick_keep_mode`]. +fn physical_keep_mode( + pre: &CurrentState, + state: &CurrentState, + conn: &str, +) -> Option<(String, i32)> { + let pre_mode = current_mode_full(pre, conn); + let state_modes: Vec<(String, i32, i32, f64, bool, bool)> = state + .1 + .iter() + .find(|m| m.0 .0 == conn) + .map(|mon| { + mon.1 + .iter() + .map(|md| { + ( + md.0.clone(), + md.1, + md.2, + md.3, + mode_flag(md, "is-current"), + mode_flag(md, "is-preferred"), + ) + }) + .collect() + }) + .unwrap_or_default(); + pick_keep_mode(pre_mode, &state_modes) } /// Wait for the virtual output to appear in DisplayConfig (its size follows PipeWire negotiation, @@ -465,7 +541,7 @@ async fn make_virtual_primary( let config = if exclusive { build_exclusive_config(&vconn, &vmode) } else { - build_primary_keeping_physicals(&state, &vconn, &vmode, mode.width as i32) + build_primary_keeping_physicals(pre, &state, &vconn, &vmode, mode.width as i32) }; let _: () = dc .call( @@ -505,13 +581,20 @@ fn build_exclusive_config(vconn: &str, vmode: &str) -> Vec { } /// **Primary** — the virtual output primary at `(0, 0)`, with every currently-active physical -/// monitor KEPT as a secondary (laid left-to-right past the virtual, each at its current mode). So -/// the shell + new windows land on the streamed surface, but the operator's physical screen stays -/// on. On a headless host (no physicals) this is identical to [`build_exclusive_config`]. +/// monitor KEPT as a secondary (laid left-to-right past the virtual, each at its **pre-connect** +/// mode). So the shell + new windows land on the streamed surface, but the operator's physical +/// screen stays on **at its real refresh**. On a headless host (no physicals) this is identical to +/// [`build_exclusive_config`]. +/// +/// `pre` is the snapshot taken *before* the virtual output existed (physical still at its true +/// refresh); `state` is the post-virtual state. We read each physical's mode from `pre` because +/// Mutter can knock a 120 Hz panel down to 60 Hz when it re-derives the layout for the virtual +/// monitor — reading `state` would cement that 60 Hz (`physical_keep_mode`). /// /// *Physical-keep is unvalidated on-glass* — the lab boxes are headless (no attached display to keep /// on); the layout math is conservative (append to the right) but wants a display-attached box. fn build_primary_keeping_physicals( + pre: &CurrentState, state: &CurrentState, vconn: &str, vmode: &str, @@ -525,15 +608,15 @@ fn build_primary_keeping_physicals( true, vec![(vconn.to_string(), vmode.to_string(), HashMap::new())], )]; - // Append each physical (non-virtual) connector that has a usable current mode, to the right of - // the virtual output, as a non-primary secondary. + // Append each physical (non-virtual) connector that has a usable mode, to the right of the + // virtual output, as a non-primary secondary — at its PRE-connect mode (real refresh preserved). let mut x = virt_width.max(0); for mon in &state.1 { let conn = &mon.0 .0; if conn == vconn { continue; } - if let Some((mode_id, w, _h)) = current_mode(state, conn) { + if let Some((mode_id, w)) = physical_keep_mode(pre, state, conn) { logicals.push(( x, 0, @@ -547,3 +630,84 @@ fn build_primary_keeping_physicals( } logicals } + +#[cfg(test)] +mod tests { + use super::pick_keep_mode; + + // (id, w, h, refresh, is_current, is_preferred) + fn m( + id: &str, + w: i32, + h: i32, + hz: f64, + cur: bool, + pref: bool, + ) -> (String, i32, i32, f64, bool, bool) { + (id.to_string(), w, h, hz, cur, pref) + } + + #[test] + fn keep_mode_prefers_pre_refresh_over_downgraded_state() { + // Physical was 2560x1440@120 pre-connect; after the virtual appeared Mutter marked 60 Hz + // current (the reported bug). We must re-apply the 120 Hz mode, not the state's 60 Hz. + let pre = Some(("M120".to_string(), 2560, 1440, 120.0)); + let state = vec![ + m("M120", 2560, 1440, 120.0, false, false), + m("M60", 2560, 1440, 60.0, true, true), + ]; + assert_eq!( + pick_keep_mode(pre, &state), + Some(("M120".to_string(), 2560)) + ); + } + + #[test] + fn keep_mode_rekeyed_id_matches_by_geometry_and_refresh() { + // The pre id is no longer offered (Mutter re-keyed the mode list), but a 120 Hz mode of the + // same geometry exists — match it so the real refresh survives. + let pre = Some(("old-120".to_string(), 2560, 1440, 120.0)); + let state = vec![ + m("new-120", 2560, 1440, 119.998, false, false), + m("new-60", 2560, 1440, 60.0, true, true), + ]; + assert_eq!( + pick_keep_mode(pre, &state), + Some(("new-120".to_string(), 2560)) + ); + } + + #[test] + fn keep_mode_falls_back_to_state_current_when_pre_mode_gone() { + // The physical genuinely no longer offers its pre mode (e.g. cable renegotiated to a lower + // max) — never invent an id; use the post-virtual current. + let pre = Some(("gone-165".to_string(), 3440, 1440, 165.0)); + let state = vec![ + m("s-100", 3440, 1440, 100.0, true, false), + m("s-60", 3440, 1440, 60.0, false, true), + ]; + assert_eq!( + pick_keep_mode(pre, &state), + Some(("s-100".to_string(), 3440)) + ); + } + + #[test] + fn keep_mode_no_pre_uses_state_current_then_preferred() { + // A connector new since the pre-snapshot (no pre mode): is-current wins, else is-preferred. + let state = vec![ + m("A", 1920, 1080, 60.0, true, false), + m("B", 1920, 1080, 144.0, false, true), + ]; + assert_eq!(pick_keep_mode(None, &state), Some(("A".to_string(), 1920))); + + let no_current = vec![ + m("A", 1920, 1080, 60.0, false, false), + m("B", 1920, 1080, 144.0, false, true), + ]; + assert_eq!( + pick_keep_mode(None, &no_current), + Some(("B".to_string(), 1920)) + ); + } +} diff --git a/crates/punktfunk-host/src/vdisplay/windows/manager.rs b/crates/punktfunk-host/src/vdisplay/windows/manager.rs index e5fd9c5..d97d26a 100644 --- a/crates/punktfunk-host/src/vdisplay/windows/manager.rs +++ b/crates/punktfunk-host/src/vdisplay/windows/manager.rs @@ -33,8 +33,8 @@ use windows::Win32::System::Threading::{ use super::{DisplayOwnership, Mode, VirtualOutput}; use crate::win_display::{ - force_extend_topology, isolate_displays_ccd, resolve_gdi_name, restore_displays_ccd, - set_active_mode, set_virtual_primary_ccd, SavedConfig, + count_other_active, force_extend_topology, isolate_displays_ccd, resolve_gdi_name, + restore_displays_ccd, set_active_mode, set_virtual_primary_ccd, SavedConfig, }; /// The per-backend REMOVE key the driver stamps on ADD and consumes on REMOVE. SudoVDA keys monitors by @@ -673,16 +673,32 @@ impl VirtualDisplayManager { ccd_saved = unsafe { isolate_displays_ccd(added.target_id) }; } Topology::Primary => { - // The IDD auto-activates as the SOLE display on a headless box, so the - // physical (if present) is deactivated and QueryDisplayConfig sees only the - // virtual. Force EXTEND first to (re)activate every CONNECTED display - // alongside the virtual, THEN reposition to make the virtual primary — so the - // physical stays active. (The bring-up above only force-EXTENDs when the - // virtual FAILS to auto-resolve; here it resolved, so we do it explicitly.) - // SAFETY: `force_extend_topology` drives the CCD topology FFI (no args, no borrowed - // memory), under the `state` lock — the sole topology mutator. - unsafe { force_extend_topology() }; - thread::sleep(Duration::from_millis(300)); + // On a headless box the IDD auto-activates as the SOLE display, so a physical + // (if present) is deactivated and QueryDisplayConfig sees only the virtual — + // force EXTEND to (re)activate every connected display alongside the virtual, + // THEN reposition to make the virtual primary. BUT on a box whose physical is + // ALREADY active (the IDD came up extended beside it — the common desktop case), + // that physical is already lit at its real mode; re-applying the bare + // `SDC_TOPOLOGY_EXTEND` preset would only re-pull each display's mode from the + // persistence DB, RESETTING a 120 Hz panel to 60 Hz. So force-EXTEND only when the + // virtual is currently sole; otherwise skip straight to the reposition, which + // re-supplies each physical's QUERIED mode verbatim (preserving its refresh). + // SAFETY: `count_other_active` runs the CCD QueryDisplayConfig FFI (Copy target id + // by value, owned result), under the `state` lock. + let already_extended = + unsafe { count_other_active(added.target_id) }.unwrap_or(0) > 0; + if already_extended { + tracing::info!( + "display topology=primary — a physical display is already active; \ + skipping force-EXTEND (preserves its refresh) before making the \ + virtual primary" + ); + } else { + // SAFETY: `force_extend_topology` drives the CCD topology FFI (no args, no + // borrowed memory), under the `state` lock — the sole topology mutator. + unsafe { force_extend_topology() }; + thread::sleep(Duration::from_millis(300)); + } // SAFETY: `set_virtual_primary_ccd` takes the `Copy` target id by value and returns // an owned `SavedConfig` (no borrowed memory crosses), under the `state` lock. ccd_saved = unsafe { set_virtual_primary_ccd(added.target_id) }; diff --git a/crates/punktfunk-host/src/windows/win_display.rs b/crates/punktfunk-host/src/windows/win_display.rs index dff5108..731d96e 100644 --- a/crates/punktfunk-host/src/windows/win_display.rs +++ b/crates/punktfunk-host/src/windows/win_display.rs @@ -384,8 +384,10 @@ unsafe fn query_active_config() -> Option { } /// Count currently-ACTIVE display paths whose target id != `keep_target_id` — i.e. displays that would -/// still be lit besides the virtual one. `None` on query failure. Used to VERIFY isolation actually took. -unsafe fn count_other_active(keep_target_id: u32) -> Option { +/// still be lit besides the virtual one. `None` on query failure. Used to VERIFY isolation actually +/// took, and (in the `primary` topology) to detect a physical that is ALREADY active so we can skip a +/// force-EXTEND that would reset its refresh. +pub(crate) unsafe fn count_other_active(keep_target_id: u32) -> Option { let (paths, _) = query_active_config()?; Some( paths