From 50531c8e9ec275763120be8809974b2bab26d79d Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 00:04:25 +0200 Subject: [PATCH] fix(host/display): a screen picker that cannot stream a screen now says so instead of saving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows console's "Streamed screen" picker SAVED and then did nothing. `capture_monitor` is platform-neutral, so the PUT persisted; every consumer of it is Linux-gated, so a virtual display was still created on connect. The operator got a control that acknowledged the click and changed nothing — the worst of the three possible behaviours. The root cause is not a missing gate, it is a missing BACKEND. Per-monitor capture is Linux/portal only: `vdisplay::open`'s mirror arm is `#[cfg(target_os = "linux")]` because `pf-capture` has no Windows entry point that can capture an arbitrary head. Its sole Windows entry point is `open_idd_push`, a frame channel pushed by our OWN IddCx virtual display; DXGI Desktop Duplication was deliberately REMOVED (`windows/dxgi.rs` keeps the GPU-preference hook only to stop DXGI reparenting the virtual display off the pinned adapter, and says so). So there is nothing for a pin to aim at, and exposing the picker got ahead of that. Decision of record (user, this session): mark it unsupported now rather than build the Windows backend here. A Windows mirror backend is a real feature gap and a project of its own — it needs a duplication capturer plus everything the IDD-push path carries today (cursor sidechannel, 444, 10-bit, PyroWave, HDR) re-plumbed through it — not a follow-on to an enumeration commit. Three places, so the answer is consistent wherever it is asked: * `MonitorsResponse.pin_supported` — a CAPABILITY, reported by the build that would have to honor the pin rather than sniffed from the OS client-side. When a Windows mirror backend lands this flips and the console needs no change. `pinned` stays `None` off-Linux deliberately, and now says why: it is what the console highlights as "sessions stream this", and highlighting a head nothing will capture is the same lie in a different place. * `enforced` drops `capture_monitor` off Linux. That list is exactly the "which controls are live vs. coming soon" contract, and claiming this one unconditionally is what let the picker ship enabled. * The PUT drops a non-Linux `capture_monitor` instead of storing it, and logs that it did. COERCED, not rejected with a 400: this PUT is WHOLE-OBJECT, so a host that already stored a pin would have every later settings save rejected over a field the operator cannot see — taking the other axes down with it. This way such a policy self-heals on the next write. Console: the picker renders read-only with an explanation, reusing the `envLocked` shape rather than inventing a second one (`locked = envLocked || !pinSupported`). Managed heads were ALREADY excluded from selection here (`mon.enabled && !mon.managed`), which is the "grey out, never filter blindly" rule working as intended — the heads stay listed and explicable. `pin_supported` defaults to TRUE when absent so an older host, which only ever shipped this picker where it worked, is not retroactively locked out. ⚠️ `api/openapi.json` is hand-patched — punktfunk-host does not build on macOS, so the spec could not be regenerated from the binary. The shape was verified by running the real generator over it: orval emits `pin_supported: boolean` (required, no `?`) on `MonitorsResponse`. Verified: `bun run codegen` (orval + paraglide + i18n parity: 434 messages, en + de), `bun run build`, `bun run lint` (tsc --noEmit) all clean. Rust verified separately on .173 — see the next commit's note if that lands, since neither xcheck target covers punktfunk-host. --- api/openapi.json | 7 ++- crates/punktfunk-host/src/mgmt/display.rs | 76 ++++++++++++++++++----- web/messages/de.json | 1 + web/messages/en.json | 1 + web/src/sections/Displays/MonitorCard.tsx | 21 +++++-- 5 files changed, 86 insertions(+), 20 deletions(-) diff --git a/api/openapi.json b/api/openapi.json index 84a38a49..5bc4acd4 100644 --- a/api/openapi.json +++ b/api/openapi.json @@ -5831,7 +5831,8 @@ "type": "object", "description": "The host's physical monitors + which one capture is pinned to.", "required": [ - "monitors" + "monitors", + "pin_supported" ], "properties": { "compositor": { @@ -5855,6 +5856,10 @@ }, "description": "The heads, ordered left-to-right by desktop position." }, + "pin_supported": { + "type": "boolean", + "description": "Whether this build can actually STREAM one of these monitors.\n\nEnumeration and capture are separate capabilities, and on Windows only the first exists: the\nheads below are real and worth showing (they explain the topology, and `/display/state`\ncross-references them), but `pf-capture`'s sole Windows entry point is `open_idd_push` — a\nframe channel pushed by our OWN IddCx virtual display. There is no desktop-duplication\ncapturer to point at a chosen head (DXGI Desktop Duplication was deliberately removed), so\n`vdisplay::open` has no mirror arm outside Linux and a pin could not be honored.\n\nThe console renders the picker read-only on `false`. Reported as a capability rather than\nsniffed client-side from the OS so the answer comes from the build that would have to honor\nit — when a Windows mirror backend lands, this flips and the UI needs no change." + }, "pinned": { "type": [ "string", diff --git a/crates/punktfunk-host/src/mgmt/display.rs b/crates/punktfunk-host/src/mgmt/display.rs index 2d6e2414..b3e30900 100644 --- a/crates/punktfunk-host/src/mgmt/display.rs +++ b/crates/punktfunk-host/src/mgmt/display.rs @@ -71,27 +71,34 @@ pub(crate) fn display_settings_state() -> DisplaySettingsState { }) }) .collect(); + let mut enforced: Vec = vec![ + "keep_alive".into(), + "topology".into(), + "mode_conflict".into(), + "identity".into(), + "layout".into(), + "game_session".into(), + // EXPERIMENTAL, Windows-only in effect: acted on at the `exclusive` isolate + // (`vdisplay/windows/manager.rs`); stored-but-inert elsewhere. + "ddc_power_off".into(), + "pnp_disable_monitors".into(), + ]; + // `capture_monitor` routes every session to the MIRROR backend, and that backend exists only on + // Linux — `vdisplay::open`'s mirror arm is `#[cfg(target_os = "linux")]`, because `pf-capture` + // has no Windows entry point that can capture an arbitrary head. This list is precisely the + // "which controls are live vs. coming soon" contract, so claiming it unconditionally is what let + // the Windows console offer a picker that saved and then did nothing + // (`design/per-monitor-portal-capture.md` §5.3). + if cfg!(target_os = "linux") { + enforced.push("capture_monitor".into()); + } DisplaySettingsState { effective: settings.effective(), settings, configured, presets, custom_presets: policy::load_custom_presets(), - enforced: vec![ - "keep_alive".into(), - "topology".into(), - "mode_conflict".into(), - "identity".into(), - "layout".into(), - "game_session".into(), - // EXPERIMENTAL, Windows-only in effect: acted on at the `exclusive` isolate - // (`vdisplay/windows/manager.rs`); stored-but-inert elsewhere. - "ddc_power_off".into(), - "pnp_disable_monitors".into(), - // Linux-only in effect: routes every session to the mirror backend - // (design/per-monitor-portal-capture.md). - "capture_monitor".into(), - ], + enforced, } } @@ -135,6 +142,24 @@ pub(crate) async fn get_display_settings() -> Json { pub(crate) async fn set_display_settings( ApiJson(policy): ApiJson, ) -> Response { + #[cfg_attr(target_os = "linux", allow(unused_mut))] + let mut policy = policy; + // A pin nothing can honor must not be STORED as though it were. Off Linux there is no mirror + // backend (`MonitorsResponse::pin_supported`), so drop the field rather than persisting a + // setting whose only effect would be to mislead: the console re-reads this state after the PUT + // and would otherwise render a screen as "streamed" while every session kept creating a virtual + // display. Coerced rather than rejected with a 400 on purpose — this PUT is WHOLE-OBJECT, so a + // host that already stored a pin (before this build) would have every later settings save + // rejected over a field the operator cannot even see, taking the other axes down with it. This + // way such a policy self-heals on the next write, and the response body shows the truth + // immediately. + #[cfg(not(target_os = "linux"))] + if let Some(dropped) = policy.capture_monitor.take() { + tracing::warn!( + "management API: ignoring capture_monitor={dropped:?} — streaming a chosen physical \ + monitor is Linux-only (no Windows mirror backend); the pin was NOT stored" + ); + } // `keep_alive: forever` (the gaming-rig preset) is now honored: the display is Pinned (Linux // registry + Windows `MgrState::Pinned`) and freed via `POST /display/release` (the escape hatch). if let Err(e) = crate::vdisplay::policy::prefs().set(policy) { @@ -222,6 +247,19 @@ pub(crate) struct MonitorsResponse { /// The configured `PUNKTFUNK_CAPTURE_MONITOR`, if any — reported even when it matches nothing, /// so the console can show "pinned to DP-2, which this host doesn't have". pinned: Option, + /// Whether this build can actually STREAM one of these monitors. + /// + /// Enumeration and capture are separate capabilities, and on Windows only the first exists: the + /// heads below are real and worth showing (they explain the topology, and `/display/state` + /// cross-references them), but `pf-capture`'s sole Windows entry point is `open_idd_push` — a + /// frame channel pushed by our OWN IddCx virtual display. There is no desktop-duplication + /// capturer to point at a chosen head (DXGI Desktop Duplication was deliberately removed), so + /// `vdisplay::open` has no mirror arm outside Linux and a pin could not be honored. + /// + /// The console renders the picker read-only on `false`. Reported as a capability rather than + /// sniffed client-side from the OS so the answer comes from the build that would have to honor + /// it — when a Windows mirror backend lands, this flips and the UI needs no change. + pin_supported: bool, /// Why the list is empty, when enumeration failed (compositor unreachable, unsupported /// platform). `None` with an empty list means "asked, and there are none". error: Option, @@ -248,8 +286,15 @@ pub(crate) async fn get_display_monitors() -> Json { // sessions will actually mirror, not just what the console last wrote. #[cfg(target_os = "linux")] let pinned = crate::vdisplay::capture_monitor(); + // Off Linux there is no mirror backend, so there is nothing a pin could aim (see + // `MonitorsResponse::pin_supported`). Kept `None` DELIBERATELY rather than reporting the stored + // value: `pinned` is what the console highlights as "this is the screen sessions stream", and a + // highlight on a head nothing will ever capture is precisely the lie this endpoint is here to + // stop telling. `pin_supported: false` is how the unsupportedness is reported instead. #[cfg(not(target_os = "linux"))] let pinned: Option = None; + // Enumeration works on Windows; capture of an enumerated head does not. + let pin_supported = cfg!(target_os = "linux"); // Enumeration shells out / round-trips D-Bus + Wayland (and on Windows walks the CCD // database, which can serialize on the display-config lock), so keep it off the async worker. let (compositor, listed) = tokio::task::spawn_blocking(|| { @@ -297,6 +342,7 @@ pub(crate) async fn get_display_monitors() -> Json { compositor, monitors, pinned, + pin_supported, error, }) } diff --git a/web/messages/de.json b/web/messages/de.json index a0e4ce11..c5ed2748 100644 --- a/web/messages/de.json +++ b/web/messages/de.json @@ -430,6 +430,7 @@ "display_monitor_none": "Dieser Host meldet keine Monitore.", "display_monitor_unavailable": "Monitore konnten auf diesem Host nicht ermittelt werden.", "display_monitor_env_locked": "Auf diesem Host über PUNKTFUNK_CAPTURE_MONITOR festgelegt — dort entfernen, um hier zu wählen.", + "display_monitor_unsupported": "Das Übertragen eines dieser Bildschirme wird auf diesem Host noch nicht unterstützt — das gibt es bisher nur auf Linux-Hosts. Die Bildschirme sind hier aufgeführt, damit du siehst, was dieser Computer hat; Clients bekommen weiterhin ihren eigenen virtuellen Bildschirm.", "display_monitor_primary": "primär", "display_monitor_disabled": "aus", "display_monitor_saved": "Übertragener Bildschirm gespeichert" diff --git a/web/messages/en.json b/web/messages/en.json index f747a2e5..22a9857b 100644 --- a/web/messages/en.json +++ b/web/messages/en.json @@ -430,6 +430,7 @@ "display_monitor_none": "This host reports no monitors.", "display_monitor_unavailable": "Monitors could not be listed on this host.", "display_monitor_env_locked": "Pinned by PUNKTFUNK_CAPTURE_MONITOR on this host — unset it to choose here.", + "display_monitor_unsupported": "Streaming one of these screens isn't supported on this host yet — it's available on Linux hosts only. The screens are listed so you can see what this computer has; clients keep getting their own virtual screen.", "display_monitor_primary": "primary", "display_monitor_disabled": "off", "display_monitor_saved": "Streamed screen saved" diff --git a/web/src/sections/Displays/MonitorCard.tsx b/web/src/sections/Displays/MonitorCard.tsx index 87d8c862..47e1fb19 100644 --- a/web/src/sections/Displays/MonitorCard.tsx +++ b/web/src/sections/Displays/MonitorCard.tsx @@ -42,9 +42,17 @@ export const MonitorCard: FC = () => { // environment is read-only here: offering controls that silently lose to the env would be worse // than saying so. const envLocked = !!pinned && policy?.capture_monitor !== pinned; + // The host says whether it can honor a pin at all. Windows enumerates its heads but has no + // backend that can capture one (see `MonitorsResponse.pin_supported`), and this card used to + // offer the choice anyway: the PUT persisted, nothing consumed it, and a virtual display was + // still created on connect. Defaults to TRUE when the field is absent so an older host — which + // only ever shipped this picker where it worked — is not retroactively locked out. + const pinSupported = monitors.data?.pin_supported ?? true; + // Both reasons produce the same read-only card; only the explanation above it differs. + const locked = envLocked || !pinSupported; const choose = (connector: string | null) => { - if (!policy || envLocked) return; + if (!policy || locked) return; save.mutate( { data: { ...policy, capture_monitor: connector } }, { @@ -71,13 +79,13 @@ export const MonitorCard: FC = () => {