diff --git a/crates/pf-capture/src/linux/pipewire.rs b/crates/pf-capture/src/linux/pipewire.rs index 0f80219d..0dcbf2e5 100644 --- a/crates/pf-capture/src/linux/pipewire.rs +++ b/crates/pf-capture/src/linux/pipewire.rs @@ -1450,16 +1450,21 @@ pub fn pipewire_thread( RGB CSC; PUNKTFUNK_PIPEWIRE_NV12=0 restores the packed-RGB negotiation)" ); } - // Modifiers our import stack handles for BGRx: the EGL-importable (tiled) set, plus LINEAR - // (0) — NVIDIA's EGL won't list it, but LINEAR dmabufs (gamescope's only offer) import via - // CUDA external memory instead. For the VAAPI passthrough path we advertise LINEAR only: - // radeonsi/iHD import it and any compositor can allocate it. - let mut modifiers = importer - .as_mut() - .map(|i| i.supported_modifiers(pf_frame::drm_fourcc(PixelFormat::Bgrx).unwrap())) - .unwrap_or_default(); - if (importer.is_some() || vaapi_passthrough) && !modifiers.contains(&0) { - modifiers.push(0); // DRM_FORMAT_MOD_LINEAR + // Modifiers our import stack handles, enumerated PER FOURCC. `XR24` (BGRx) and `AR24` (BGRA) + // are asked separately on purpose: EGL/libva answer per format, and nothing entitles us to + // assume a driver that imports one imports the other. Keeping them apart is also what makes + // the BGRA pod below correct on AMD and Intel rather than an NVIDIA-shaped guess — each list + // is whatever THIS GPU's stack actually said. + // + // To each list we add LINEAR (0) — NVIDIA's EGL won't list it, but LINEAR dmabufs (gamescope's + // only offer) import via CUDA external memory instead. For the VAAPI passthrough path there is + // no importer at all, so the lists start empty and LINEAR is all we advertise: radeonsi/iHD + // import it and any compositor can allocate it. + let mut modifiers = Vec::new(); + let mut modifiers_bgra = Vec::new(); + if let Some(i) = importer.as_mut() { + modifiers = i.supported_modifiers(pf_frame::drm_fourcc(PixelFormat::Bgrx).unwrap()); + modifiers_bgra = i.supported_modifiers(pf_frame::drm_fourcc(PixelFormat::Bgra).unwrap()); } // PyroWave passthrough: the encoder imports through Vulkan, not libva — extend the // advertisement with every modifier its device samples from, so compositors that @@ -1468,12 +1473,20 @@ pub fn pipewire_thread( // the host's `pyrowave` feature is on AND the session (or the global encoder pref) is // PyroWave — so capture never calls back into `encode` and needs no feature gate of its // own (the emptiness check gates it). - if vaapi_passthrough && !policy.pyrowave_modifiers.is_empty() { - for &m in &policy.pyrowave_modifiers { - if !modifiers.contains(&m) { - modifiers.push(m); + let extend_pyrowave = vaapi_passthrough && !policy.pyrowave_modifiers.is_empty(); + for list in [&mut modifiers, &mut modifiers_bgra] { + if (importer.is_some() || vaapi_passthrough) && !list.contains(&0) { + list.push(0); // DRM_FORMAT_MOD_LINEAR + } + if extend_pyrowave { + for &m in &policy.pyrowave_modifiers { + if !list.contains(&m) { + list.push(m); + } } } + } + if extend_pyrowave { tracing::info!( count = modifiers.len(), "zero-copy: advertising the PyroWave device's Vulkan-importable dmabuf modifiers" @@ -1540,9 +1553,14 @@ pub fn pipewire_thread( ); } else if want_dmabuf { tracing::info!( - count = modifiers.len(), + bgrx_count = modifiers.len(), + bgra_count = modifiers_bgra.len(), + // `sample` is TRUNCATED to 6, and LINEAR is pushed last — so reading the sample as the + // whole list makes a perfectly good offer look tiled-only. That misreading cost a full + // debugging session on 2026-08-14, hence stating the one bit that was actually wanted. + linear_offered = modifiers.contains(&0), sample = ?&modifiers[..modifiers.len().min(6)], - "zero-copy: advertising EGL-importable dmabuf modifiers" + "zero-copy: advertising EGL-importable dmabuf modifiers (BGRx + BGRA pods)" ); } else if consumer.cpu_is_downgrade() { // Reached only when no dmabuf is advertised at all (every arm above rules out a @@ -2094,17 +2112,39 @@ pub fn pipewire_thread( .map(|fmt| build_hdr_dmabuf_format(*fmt, preferred)) .collect::>>()? } else if want_dmabuf { - let mut pods = Vec::with_capacity(if prefer_native_nv12 { 2 } else { 1 }); + let mut pods = Vec::with_capacity(if prefer_native_nv12 { 3 } else { 2 }); if prefer_native_nv12 { // First compatible consumer pod wins. Gamescope advertises NV12 and BGRx; pinning // BT.709 limited here selects its RGB→NV12 shader with our bitstream colorimetry. pods.push(build_dmabuf_format(VideoFormat::NV12, &[0], preferred)?); } - pods.push(build_dmabuf_format( - VideoFormat::BGRx, - &modifiers, - preferred, - )?); + if !modifiers.is_empty() { + pods.push(build_dmabuf_format( + VideoFormat::BGRx, + &modifiers, + preferred, + )?); + } + // xdph (Hyprland/sway) offers ONLY **BGRA** on its dmabuf EnumFormat — it lists BGRA *and* + // BGRx on the SHM pod, so a BGRx-only dmabuf offer intersects with nothing and PipeWire + // fails the link outright: + // pw.link: negotiating -> error no more input formats (-22) + // Measured 2026-08-14 on Hyprland 0.55.4 + xdph 1.3.12: the 12 tiled modifiers matched on + // both sides perfectly — only the fourcc never did, which is why the failure reads like a + // GPU/modifier problem and is not one. + // + // BGRA and BGRx are the same 32-bit layout; the alpha byte is ignored the whole way to the + // encoder (`vk_util` maps both to `B8G8R8A8_UNORM`, VAAPI both to `Pixel::BGRA`), and the + // dmabuf import is driven by the NEGOTIATED format's fourcc, so an AR24 frame imports as + // AR24. Listed AFTER BGRx so a producer offering both still lands on the pre-existing path + // — first compatible consumer pod wins, so this is purely additive. + if !modifiers_bgra.is_empty() { + pods.push(build_dmabuf_format( + VideoFormat::BGRA, + &modifiers_bgra, + preferred, + )?); + } pods } else { vec![serialize_pod(obj)?] diff --git a/crates/pf-inject/src/inject/linux/stream_output.rs b/crates/pf-inject/src/inject/linux/stream_output.rs new file mode 100644 index 00000000..e46d4113 --- /dev/null +++ b/crates/pf-inject/src/inject/linux/stream_output.rs @@ -0,0 +1,79 @@ +//! The compositor output absolute coordinates belong to, by NAME — the Linux counterpart of the +//! Windows `stream_target` slot, and what the wlroots virtual-pointer backend aims at. +//! +//! `MouseMoveAbs` carries its own reference extent (`w`/`h` — the client's letterboxed video rect +//! in ITS window, not the streamed mode), and the wlr protocol normalizes `x`/`y` against it and +//! maps the result onto whichever `wl_output` the virtual pointer was **created with**. So the +//! extent takes care of itself and the OUTPUT is the whole question. The injector used to pass the +//! first `wl_output` the registry advertised, which is the oldest global — on any multi-head box +//! the operator's physical head, never the per-session headless output the client is looking at. +//! On the EXTEND backends (Hyprland, wlroots/sway) the streamed head sits *beside* the operator's, +//! so absolute samples landed on a screen no session was streaming. Reported from the field as +//! "no cursor was visible in the session", and later as a cursor pinned near the left edge that +//! vanished part-way across. +//! +//! The host publishes the streamed output's compositor name at capture bring-up +//! ([`set_stream_output`]) — Hyprland's `PF--`, sway's `HEADLESS-N`, or a mirrored head's +//! connector — and the wlr backend re-creates its virtual pointer bound to the matching `wl_output` +//! (`wl_output.name`, protocol v4; the name is explicitly "the same for all clients", so the name +//! `hyprctl`/`swaymsg` minted is the name we can match here). +//! +//! **One slot per process**, exactly like the Windows original: the injector is host-lifetime and +//! every concurrent session's input flows through it, so with parallel sessions the LAST capture +//! bring-up wins for every session's absolute input. Per-session routing needs source-tagged input +//! events (the injector has to become session-aware first — see [`crate::set_absolute_anchor`]'s +//! note), and the single slot is never worse than what it replaces: today EVERY session's absolute +//! input lands on a head that no session is streaming. +//! +//! With nothing published — before the first bring-up, or on a compositor whose `wl_output` is +//! older than v4 and therefore nameless — the pointer is bound to NO output, which maps absolute +//! coordinates over the whole layout. On a single-output compositor that is identical to binding +//! that output; on a multi-head one it is at least *reachable*, unlike a pin to the wrong head. + +use std::sync::RwLock; + +/// The streamed output's compositor name, or `None` when nothing has been published yet. +static STREAM_OUTPUT: RwLock> = RwLock::new(None); + +/// Publish the compositor output (by name) that absolute input maps into. The host calls this at +/// capture bring-up, and ONLY there: nothing clears it at teardown, because an output that goes +/// away simply stops resolving (the backend falls back to whole-layout mapping, and between +/// sessions nothing injects anyway). A later bring-up is what rewrites it — including to `None`, +/// which a backend that needs no named binding passes so a stale name cannot outlive its +/// compositor. See the module doc for the one-slot-per-process trade with parallel sessions. +pub fn set_stream_output(name: Option) { + let mut cur = STREAM_OUTPUT.write().unwrap_or_else(|e| e.into_inner()); + if *cur != name { + tracing::info!(output = ?name, "absolute-input stream output set"); + *cur = name; + } +} + +/// The streamed output's compositor name, if one has been published. +pub fn stream_output() -> Option { + STREAM_OUTPUT + .read() + .unwrap_or_else(|e| e.into_inner()) + .clone() +} + +#[cfg(test)] +mod tests { + use super::*; + + /// ONE test on purpose, like the libei anchor's: the slot is process-wide and cargo runs + /// tests on threads in one process, so splitting this into several would let them race. + #[test] + fn publishes_clears_and_round_trips() { + set_stream_output(Some("PF-1643-1".into())); + assert_eq!(stream_output().as_deref(), Some("PF-1643-1")); + // Re-publishing the same name is a no-op, not a second "set" (the backend keys its + // pointer re-creation off the resolved name, but the log line should not repeat). + set_stream_output(Some("PF-1643-1".into())); + assert_eq!(stream_output().as_deref(), Some("PF-1643-1")); + set_stream_output(Some("HEADLESS-2".into())); + assert_eq!(stream_output().as_deref(), Some("HEADLESS-2")); + set_stream_output(None); + assert_eq!(stream_output(), None); + } +} diff --git a/crates/pf-inject/src/inject/linux/wlr.rs b/crates/pf-inject/src/inject/linux/wlr.rs index 34bf2c76..bd685bee 100644 --- a/crates/pf-inject/src/inject/linux/wlr.rs +++ b/crates/pf-inject/src/inject/linux/wlr.rs @@ -5,6 +5,11 @@ //! virtual keyboard (the host's layout via the standard `XKB_DEFAULT_LAYOUT` et al., defaulting //! to evdev/US), and translate events into virtual pointer/keyboard requests, tracking modifier //! state so the compositor resolves shifted keysyms correctly. +//! +//! **Absolute** motion is mapped by the compositor onto the `wl_output` the virtual pointer was +//! CREATED with, so which output that is decides where every absolute sample lands. We aim it at +//! the head the session is actually streaming — published by name in [`crate::stream_output`] and +//! re-resolved (re-creating the pointer) whenever it changes; see [`WlrootsInjector::retarget`]. use super::{gs_button_to_evdev, vk_to_evdev, InputEvent, InputInjector}; use anyhow::{bail, Context, Result}; @@ -12,7 +17,12 @@ use punktfunk_core::input::InputKind; use std::io::Write; use std::os::fd::{AsFd, FromRawFd}; use std::time::Instant; -use wayland_client::protocol::{wl_output::WlOutput, wl_pointer, wl_registry, wl_seat::WlSeat}; +use wayland_client::backend::WaylandError; +use wayland_client::protocol::{ + wl_output::{self, WlOutput}, + wl_pointer, wl_registry, + wl_seat::WlSeat, +}; use wayland_client::{Connection, Dispatch, EventQueue, Proxy, QueueHandle}; use wayland_protocols_misc::zwp_virtual_keyboard_v1::client::{ zwp_virtual_keyboard_manager_v1::ZwpVirtualKeyboardManagerV1, @@ -27,13 +37,65 @@ use xkbcommon::xkb; /// `code` value marking a horizontal scroll event (mirrors `gamestream::input`). const SCROLL_HORIZONTAL: u32 = 1; +/// `wl_output.name` — the connector name we match the streamed head on — arrived in v4. Nothing +/// else we ask of an output needs more than v1, so a lower advert only costs us the names (and +/// with them the ability to aim absolute input; see [`index_named`]). Same constant, same reason, +/// as `pf_vdisplay`'s `kwin_dpms`. +const WL_OUTPUT_MAX: u32 = 4; + +/// One `wl_output` the compositor has advertised. +struct Output { + /// The registry global name — the key `wl_registry.global_remove` reports, and the user data + /// each `wl_output` event carries back so we know which head it describes. + global: u32, + proxy: WlOutput, + /// `wl_output.name` (protocol v4): the compositor's own name for the head — `HDMI-A-1`, + /// Hyprland's `PF--`, sway's `HEADLESS-N`. The protocol guarantees this is "the same + /// output name for all clients", which is what lets us match the name `hyprctl`/`swaymsg` + /// minted on the vdisplay side. `None` on a compositor stuck at v3, which has no name event at + /// all — then there is nothing to match on and the pointer stays unbound. + name: Option, +} + /// Globals bound from the registry (the Wayland dispatch state). #[derive(Default)] struct Globals { pointer_mgr: Option, keyboard_mgr: Option, seat: Option, - output: Option, + /// EVERY advertised output, in advertisement order — not just the first. The streamed head is + /// created per session, so it is never the first one advertised (that is the operator's + /// oldest physical head), and binding only the first is what aimed absolute input at the + /// wrong screen on every EXTEND box. + outputs: Vec, +} + +/// Which advertised output — by position in `names`, which is advertisement order — the virtual +/// pointer should bind to for the published target `want`. +/// +/// The rule has **no fallback on purpose**, and that absence is the fix: what this replaced was a +/// fallback ("bind whatever `wl_output` came first"), and the first-advertised output is the oldest +/// global, i.e. the operator's physical head — never the per-session headless one the client is +/// looking at. A target that matches nothing therefore yields `None`, which binds the pointer to no +/// output and maps absolute coordinates over the whole layout: wrong-ish, but reachable, where a +/// pin to the wrong head is unreachable. +/// +/// Split out of [`Globals::output_named`] so the rule is testable — a `WlOutput` proxy cannot be +/// constructed without a live Wayland connection, but the decision it feeds can. +fn index_named<'a>( + names: impl IntoIterator>, + want: Option<&str>, +) -> Option { + let want = want?; + names.into_iter().position(|n| n == Some(want)) +} + +impl Globals { + /// The `wl_output` whose compositor name is `want`, if it is currently advertised. + fn output_named(&self, want: &str) -> Option { + index_named(self.outputs.iter().map(|o| o.name.as_deref()), Some(want)) + .map(|i| self.outputs[i].proxy.clone()) + } } impl Dispatch for Globals { @@ -45,13 +107,12 @@ impl Dispatch for Globals { _: &Connection, qh: &QueueHandle, ) { - if let wl_registry::Event::Global { - name, - interface, - version, - } = event - { - match interface.as_str() { + match event { + wl_registry::Event::Global { + name, + interface, + version, + } => match interface.as_str() { "zwlr_virtual_pointer_manager_v1" => { state.pointer_mgr = Some(registry.bind(name, version.min(2), qh, ())); } @@ -61,16 +122,52 @@ impl Dispatch for Globals { "wl_seat" => { state.seat = Some(registry.bind(name, version.min(7), qh, ())); } - "wl_output" if state.output.is_none() => { - state.output = Some(registry.bind(name, version.min(3), qh, ())); + "wl_output" => { + // The `name` event is the only thing that tells the streamed head from the + // operator's. Older compositors bind lower and stay nameless (harmless: + // `output_named` then matches nothing and the pointer maps over the layout). + // The registry global name rides along as user data so the events that follow + // land on the right entry. + let proxy = registry.bind(name, version.min(WL_OUTPUT_MAX), qh, name); + state.outputs.push(Output { + global: name, + proxy, + name: None, + }); } _ => {} + }, + // A head went away — a session's headless output being torn down is the common case, + // and the pointer must stop being aimed at a dead object (`retarget` re-resolves and + // falls back to the whole layout on the next absolute sample). + wl_registry::Event::GlobalRemove { name } => { + state.outputs.retain(|o| o.global != name); + } + _ => {} + } + } +} + +impl Dispatch for Globals { + fn event( + state: &mut Self, + _: &WlOutput, + event: wl_output::Event, + global: &u32, + _: &Connection, + _: &QueueHandle, + ) { + // Only the name matters here: geometry/mode/scale are the compositor's problem, because + // binding the pointer to an output makes IT do the mapping (see `retarget`). + if let wl_output::Event::Name { name } = event { + if let Some(o) = state.outputs.iter_mut().find(|o| o.global == *global) { + o.name = Some(name); } } } } -// The managers, the two virtual devices, the seat and the output emit no events we use. +// The managers, the two virtual devices and the seat emit no events we use. macro_rules! ignore_events { ($($t:ty),* $(,)?) => {$( impl Dispatch<$t, ()> for Globals { @@ -80,7 +177,6 @@ macro_rules! ignore_events { } ignore_events!( WlSeat, - WlOutput, ZwlrVirtualPointerManagerV1, ZwlrVirtualPointerV1, ZwpVirtualKeyboardManagerV1, @@ -92,6 +188,14 @@ pub struct WlrootsInjector { queue: EventQueue, globals: Globals, pointer: ZwlrVirtualPointerV1, + /// The compositor name of the output `pointer` is bound to, or `None` when it is bound to no + /// output (absolute coordinates then span the whole layout). Compared against + /// [`crate::stream_output`] on every absolute sample; a difference re-creates the pointer. + bound_output: Option, + /// evdev codes of the mouse buttons currently held on `pointer`, so re-creating the device + /// can release them first — the compositor has no reason to, and a virtual pointer destroyed + /// mid-press leaves the host with a stuck mouse button. + pressed: Vec, keyboard: ZwpVirtualKeyboardV1, xkb_state: xkb::State, _keymap_file: std::fs::File, // keep the memfd alive for the compositor's mmap @@ -100,6 +204,25 @@ pub struct WlrootsInjector { start: Instant, } +/// Resolve the published stream output ([`crate::stream_output`]) against the outputs this +/// connection has bound: `(proxy, name)` when the target is live, `(None, None)` otherwise. +/// +/// `(None, None)` covers three cases that all want the same answer — nothing published yet (before +/// the first capture bring-up), the target's `wl_output` global not advertised yet (the injector +/// opens on the first input event, which can beat the session's display), and the target torn down +/// (session end). A pointer bound to no output maps absolute coordinates over the whole layout, +/// which on a single-output compositor is exactly that output and on a multi-head one at least +/// keeps the streamed head reachable — unlike a pin to a head nobody is streaming. +fn resolve_target(globals: &Globals) -> (Option, Option) { + let Some(want) = crate::stream_output() else { + return (None, None); + }; + match globals.output_named(&want) { + Some(proxy) => (Some(proxy), Some(want)), + None => (None, None), + } +} + /// Cap on distinct characters the dynamic text keymap holds before it restarts from scratch /// (keycodes grow upward from 9; xkb tops out at 255, so stay well under). const TEXT_KEYMAP_MAX: usize = 200; @@ -140,12 +263,16 @@ impl WlrootsInjector { .clone() .context("compositor advertised no wl_seat")?; - let pointer = pointer_mgr.create_virtual_pointer_with_output( - Some(&seat), - globals.output.as_ref(), - &qh, - (), - ); + // A second roundtrip: the first only said WHICH globals exist. The `wl_output.name` events + // that identify each head are emitted on the objects we bound *during* that roundtrip, so + // they only land now — and the pointer's output has to be resolved before we create it. + queue + .roundtrip(&mut globals) + .context("Wayland output-name roundtrip")?; + + let (target, bound_output) = resolve_target(&globals); + let pointer = + pointer_mgr.create_virtual_pointer_with_output(Some(&seat), target.as_ref(), &qh, ()); let keyboard = keyboard_mgr.create_virtual_keyboard(&seat, &qh, ()); // The keymap the compositor resolves our raw evdev keycodes with. Empty names defer to @@ -174,7 +301,9 @@ impl WlrootsInjector { conn.flush().ok(); tracing::info!( - output = globals.output.is_some(), + outputs = globals.outputs.len(), + want = ?crate::stream_output(), + bound = ?bound_output, "wlroots virtual input ready (pointer + keyboard)" ); Ok(Self { @@ -182,6 +311,8 @@ impl WlrootsInjector { queue, globals, pointer, + bound_output, + pressed: Vec::new(), keyboard, xkb_state, _keymap_file: file, @@ -190,6 +321,90 @@ impl WlrootsInjector { }) } + /// Aim the virtual pointer at the output the session is streaming, re-creating it when that + /// changes — the fix for absolute input landing on the operator's screen. + /// + /// The wlr protocol maps `motion_absolute` onto the output the pointer was **created with** + /// and offers no way to re-aim one, so a change means destroy + create. Cheap and rare: the + /// host publishes the target once per capture bring-up, so a re-create fires at most a couple + /// of times per session. The no-change path — every other absolute sample — costs one `RwLock` + /// read and a scan of the output list, which has one entry per head. + /// + /// Called from the `MouseMoveAbs` arm immediately BEFORE the motion is sent, so a re-created + /// pointer gets its first position in the same batch rather than sitting wherever the + /// compositor puts a brand-new device. + /// + /// Resolution is by NAME, never by size: `MouseMoveAbs`'s extent is the client's letterboxed + /// content rect in ITS window, not the streamed mode, so no size ladder could identify the + /// head. Falling back to no output at all (whole-layout mapping) when the target is unknown is + /// deliberate — see [`crate::stream_output`]'s module doc. + fn retarget(&mut self) { + let (target, want) = resolve_target(&self.globals); + if want == self.bound_output { + return; + } + let (Some(mgr), Some(seat)) = (self.globals.pointer_mgr.clone(), self.globals.seat.clone()) + else { + return; // cannot re-create without the manager/seat; keep the pointer we have + }; + // Never destroy a device with a button held: nothing else will release it. + if !self.pressed.is_empty() { + let t = self.now_ms(); + for btn in std::mem::take(&mut self.pressed) { + self.pointer + .button(t, btn, wl_pointer::ButtonState::Released); + } + self.pointer.frame(); + } + self.pointer.destroy(); + self.pointer = mgr.create_virtual_pointer_with_output( + Some(&seat), + target.as_ref(), + &self.queue.handle(), + (), + ); + tracing::info!( + from = ?self.bound_output, + to = ?want, + "wlroots virtual pointer re-aimed (absolute input now maps into this output)" + ); + self.bound_output = want; + } + + /// Drain the compositor's half of the connection, then push our batch to it — run after every + /// injected event. + /// + /// The **read** is the load-bearing half, and it used to be missing: `dispatch_pending`'s own + /// documentation says it "will not perform reads on the Wayland socket", so the queue only + /// ever held what [`Self::open`]'s roundtrips put there. Two consequences, both real. The + /// injector could never learn about a `wl_output` created AFTER it opened — which is exactly + /// the ordering the field report was captured in, and would have left [`Self::retarget`] with + /// nothing to resolve. And everything the compositor sent us piled up unread in the socket + /// buffer for the host's lifetime, including the protocol errors the code here claimed to be + /// surfacing but structurally could not. + /// + /// Non-blocking by construction: `read()` is documented to answer `WouldBlock` when the socket + /// has nothing for us, which is the common case at input rates and is not an error. + fn pump(&mut self) -> Result<()> { + // `prepare_read` will not hand out a guard while events are still queued, so dispatch first. + self.queue + .dispatch_pending(&mut self.globals) + .context("wayland dispatch")?; + if let Some(guard) = self.conn.prepare_read() { + match guard.read() { + Ok(_) => { + self.queue + .dispatch_pending(&mut self.globals) + .context("wayland dispatch (post-read)")?; + } + Err(WaylandError::Io(e)) if e.kind() == std::io::ErrorKind::WouldBlock => {} + Err(e) => return Err(e).context("wayland read"), + } + } + self.conn.flush().context("wayland flush")?; + Ok(()) + } + fn now_ms(&self) -> u32 { self.start.elapsed().as_millis() as u32 } @@ -271,6 +486,12 @@ impl InputInjector for WlrootsInjector { let w = (event.flags >> 16) & 0xffff; let h = event.flags & 0xffff; if w > 0 && h > 0 { + // The compositor maps these onto the pointer's bound output, so make sure that + // is the head this session streams before sending any. Checked here rather + // than per inject: only absolute motion depends on the binding, and a pointer + // swapped mid-drag is the one thing `retarget` has to work to be safe about. + self.retarget(); + let t = self.now_ms(); // `retarget` may have consumed time releasing buttons let x = event.x.clamp(0, w as i32) as u32; let y = event.y.clamp(0, h as i32) as u32; self.pointer.motion_absolute(t, x, y, w, h); @@ -280,8 +501,12 @@ impl InputInjector for WlrootsInjector { InputKind::MouseButtonDown | InputKind::MouseButtonUp => { if let Some(btn) = gs_button_to_evdev(event.code) { let st = if event.kind == InputKind::MouseButtonDown { + if !self.pressed.contains(&btn) { + self.pressed.push(btn); + } wl_pointer::ButtonState::Pressed } else { + self.pressed.retain(|&b| b != btn); wl_pointer::ButtonState::Released }; self.pointer.button(t, btn, st); @@ -328,12 +553,7 @@ impl InputInjector for WlrootsInjector { // wlroots has no virtual-touch protocol wired here; touch is the libei path only. InputKind::TouchDown | InputKind::TouchMove | InputKind::TouchUp => {} } - // Surface protocol errors / disconnects, then push the batch to the compositor. - self.queue - .dispatch_pending(&mut self.globals) - .context("wayland dispatch")?; - self.conn.flush().context("wayland flush")?; - Ok(()) + self.pump() } } @@ -383,3 +603,40 @@ fn memfd_with(s: &str) -> Result { f.write_all(&[0]).context("write keymap NUL")?; Ok(f) } + +#[cfg(test)] +mod tests { + use super::*; + + /// The live-box layout the field report came from: the operator's `HDMI-A-1` is advertised + /// FIRST (it exists from compositor start), and the session's headless head is added later — + /// so "first advertised" is always the wrong answer, whichever order the injector and the + /// display happen to come up in. + const HYPRLAND_BOX: [Option<&str>; 2] = [Some("HDMI-A-1"), Some("PF-87756-3")]; + + #[test] + fn binds_the_streamed_head_not_the_first_advertised_one() { + assert_eq!(index_named(HYPRLAND_BOX, Some("PF-87756-3")), Some(1)); + assert_eq!(index_named(HYPRLAND_BOX, Some("HDMI-A-1")), Some(0)); + // sway's own naming, and a mirrored physical head, resolve the same way. + let sway = [Some("HEADLESS-1"), Some("DP-2"), Some("HEADLESS-2")]; + assert_eq!(index_named(sway, Some("HEADLESS-2")), Some(2)); + assert_eq!(index_named(sway, Some("DP-2")), Some(1)); + } + + /// Every "we don't know" must land on NO output (whole-layout mapping), never on a guess — + /// the regression this whole change exists to prevent. + #[test] + fn an_unknown_target_binds_nothing_rather_than_falling_back() { + // Published but not advertised (yet, or any more — the injector opens on the first input + // event, which can beat the display, and the head goes away at session end). + assert_eq!(index_named(HYPRLAND_BOX, Some("PF-87756-9")), None); + // Nothing published at all — before the first capture bring-up. + assert_eq!(index_named(HYPRLAND_BOX, None), None); + // A compositor older than wl_output v4 emits no `name` event, so nothing is matchable. + assert_eq!(index_named([None, None], Some("PF-87756-3")), None); + // …and a compositor advertising no outputs at all cannot resolve anything either. + let headless: [Option<&str>; 0] = []; + assert_eq!(index_named(headless, Some("PF-87756-3")), None); + } +} diff --git a/crates/pf-inject/src/lib.rs b/crates/pf-inject/src/lib.rs index 71bf4957..61dc3c77 100644 --- a/crates/pf-inject/src/lib.rs +++ b/crates/pf-inject/src/lib.rs @@ -149,6 +149,15 @@ static ABSOLUTE_ANCHOR: std::sync::RwLock> = std::sync::R /// record in `design/per-monitor-portal-capture.md` §5.3) and wrong for anything per-client. A /// per-session anchor needs the injector to become session-aware first; don't call this from a /// session path until it is. +/// +/// The wlroots backend does **not** consult this — it aims at a named output via +/// `stream_output::set_stream_output` (Linux), which the host DOES publish per session and which +/// therefore takes exactly the last-bring-up-wins trade this warning describes: on purpose, and +/// stated in the open in that module's doc, matching the Windows `stream_target` slot that already +/// made the same call. The two are separate slots because they answer different questions and are +/// written by different owners: this anchor is the operator's host-wide capture pin, recomputed +/// from policy whenever the console writes it — which would wipe a per-session value written here — +/// while the stream output is whatever head the session's capture actually attached to. pub fn set_absolute_anchor(anchor: Option) { let anchor = anchor.filter(|a| !a.is_empty()); tracing::debug!(?anchor, "input: absolute-coordinate anchor set"); @@ -529,6 +538,14 @@ pub mod pen; pub mod stream_target; #[cfg(target_os = "windows")] pub use stream_target::set_stream_target; +/// Linux: the streamed compositor output (by name) that absolute coordinates map into — the +/// counterpart of the Windows `stream_target` module, published by the host at capture bring-up and +/// consumed by the wlroots virtual-pointer backend, which binds its pointer to that `wl_output`. +#[cfg(target_os = "linux")] +#[path = "inject/linux/stream_output.rs"] +pub mod stream_output; +#[cfg(target_os = "linux")] +pub use stream_output::{set_stream_output, stream_output}; /// Stub — pen injection needs the Linux uinput tablet or Windows synthetic pointers; /// `pen_supported()` is false here, so no host advertises the cap and no batches arrive. #[cfg(not(any(target_os = "linux", target_os = "windows")))] diff --git a/crates/pf-vdisplay/src/lib.rs b/crates/pf-vdisplay/src/lib.rs index f51bc2d7..04c71401 100644 --- a/crates/pf-vdisplay/src/lib.rs +++ b/crates/pf-vdisplay/src/lib.rs @@ -79,6 +79,10 @@ pub(crate) fn emit_display_event(ev: DisplayEvent) { #[path = "vdisplay/backend.rs"] pub(crate) mod backend; pub use backend::{DisplayOwnership, VirtualDisplay, VirtualOutput}; +/// The NEGOTIATED ScreenCast cursor mode of a portal-backed output, reported per session by +/// [`VirtualDisplay::last_portal_cursor_mode`]. (The module itself stays private — the ladder that +/// picks the mode is this crate's business; the verdict is the caller's.) +pub use portal_cursor::Mode as PortalCursorMode; /// Time-bounded child-process helpers — every compositor query shells out, and an unbounded one /// can wedge the calling (session) thread forever. @@ -833,6 +837,21 @@ mod portal_config; #[path = "vdisplay/linux/portal_cursor.rs"] mod portal_cursor; +/// The line fed to xdph's custom picker to select an output headlessly. +/// +/// Declared unconditionally for the same reason again: it is a wire format with no schema and no +/// error report, so the transcribed-parser tests are the only place a malformed line is visible +/// without a compositor. That is not hypothetical — a missing separator shipped, and the one +/// assertion that existed for it passed throughout. +#[path = "vdisplay/linux/portal_picker.rs"] +mod portal_picker; + +/// The single, never-dropped tokio runtime the portal handshakes run on. Linux-only: it exists to +/// outlive ashpd's process-global cached D-Bus connection, and only the Linux backends speak to it. +#[cfg(target_os = "linux")] +#[path = "vdisplay/linux/portal_rt.rs"] +mod portal_rt; + #[cfg(target_os = "linux")] #[path = "vdisplay/linux/hyprland.rs"] mod hyprland; diff --git a/crates/pf-vdisplay/src/vdisplay/backend.rs b/crates/pf-vdisplay/src/vdisplay/backend.rs index cb09cfad..3ed2b880 100644 --- a/crates/pf-vdisplay/src/vdisplay/backend.rs +++ b/crates/pf-vdisplay/src/vdisplay/backend.rs @@ -76,6 +76,20 @@ pub struct VirtualOutput { /// capturer must hold frames until that renegotiation lands. Linux-only. #[cfg(target_os = "linux")] pub expect_exact_dims: bool, + /// The compositor's own name for this output (Hyprland's `PF--`, sway's `HEADLESS-N`, + /// a mirrored head's connector) — the Linux answer to what `win_capture` carries on Windows: + /// the identity the host needs to aim **absolute input** at the head it is streaming + /// (`pf_inject::set_stream_output`, called from `capture::capture_virtual_output`). + /// + /// It is the `wl_output.name` of that head, which the protocol guarantees is the same string + /// for every client — so the injector can match it on its own Wayland connection. `None` on + /// the backends whose absolute mapping does not need it (KWin/Mutter inject through libei, + /// which selects by region; gamescope owns its whole seat). + /// + /// This crate must not depend on pf-inject (see the crate doc), so the name is only CARRIED + /// here — the host publishes it. + #[cfg(target_os = "linux")] + pub output_name: Option, } impl VirtualOutput { @@ -101,6 +115,8 @@ impl VirtualOutput { pool_gen: None, #[cfg(target_os = "linux")] expect_exact_dims: false, + #[cfg(target_os = "linux")] + output_name: None, } } } @@ -184,6 +200,33 @@ pub trait VirtualDisplay: Send { fn hw_cursor(&self) -> bool { false } + /// The ScreenCast cursor mode the backend's portal actually NEGOTIATED for the most recent + /// [`create`](Self::create) — the answer to [`set_hw_cursor`](Self::set_hw_cursor), which is + /// only ever a *request*. + /// + /// This is the difference between the two that matters downstream: on the whole wlr family + /// (xdph, xdpw) `AvailableCursorModes` is `Hidden|Embedded`, so a session that asked for + /// metadata is served **`Embedded`** — the compositor paints the pointer into the frames and + /// sends no `SPA_META_Cursor`, ever, wherever the pointer is. A consumer that reads "no cursor + /// overlay" as a symptom (the host's park schedule reads it as "the seat pointer has not + /// reached the streamed output" — true on Mutter, which suppresses metadata while the pointer + /// is off the recorded view) is then acting on noise; see + /// [`PortalCursorMode::delivers_metadata`](crate::PortalCursorMode::delivers_metadata). + /// + /// `None` — the default, and what every non-portal backend reports — means "nothing was + /// negotiated through the xdg ScreenCast portal here, so this says nothing at all": KWin + /// (`zkde_screencast` `pointer` mode), Mutter (`RecordVirtual` `cursor-mode`), gamescope (no + /// pointer either way) and Windows (IddCx) all get exactly what they ask for through their own + /// protocols, and their consumers must keep behaving as they always did. It is also `None` + /// before the first `create`. + /// + /// Reported by the wlr-family backends (`hyprland`, `wlroots`) and by the monitor + /// [`mirror`](crate::open_mirror) when it delegates to one. Those outputs are never registry- + /// pooled (`remote_fd.is_some()` — the portal fd cannot be re-opened per attach), so a reused + /// kept display can never hand back a *stale* answer here. + fn last_portal_cursor_mode(&self) -> Option { + None + } /// The stable identity slot the backend resolved for the most recent [`create`](Self::create) — /// the per-client id the identity policy assigned (`Some`), or `None` for shared/anonymous. The /// registry reads it right after `create` to key the display's group **arrangement** (manual diff --git a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs index 51c83b39..4a6a5eb2 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs @@ -525,6 +525,9 @@ impl VirtualDisplay for GamescopeDisplay { reused_gen: None, pool_gen: None, expect_exact_dims: false, + // gamescope owns its own seat and injects through its EIS socket, not the wlr + // virtual pointer (`point_injector_at_eis`) — nothing here to aim by name. + output_name: None, }); } check_gamescope_version(); // diagnostic only — warns on known-deadlock-prone versions @@ -718,6 +721,9 @@ fn create_managed_session(client: &str, mode: Mode, hdr: bool) -> Result VirtualOutput { reused_gen: None, pool_gen: None, expect_exact_dims: false, + // gamescope owns its own seat and injects through its EIS socket, not the wlr + // virtual pointer (`point_injector_at_eis`) — nothing here to aim by name. + output_name: None, } } @@ -3595,6 +3604,9 @@ pub(crate) fn stream_existing_output( Ok(crate::mirror::MirrorStream { node_id, remote_fd: None, + // No xdg portal in this path at all (gamescope publishes the node itself), and no pointer + // in the node either way — nothing to report. + cursor_mode: None, keepalive: Box::new(()), }) } diff --git a/crates/pf-vdisplay/src/vdisplay/linux/hyprland.rs b/crates/pf-vdisplay/src/vdisplay/linux/hyprland.rs index 5f3d70bf..dc985356 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/hyprland.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/hyprland.rs @@ -16,10 +16,12 @@ //! 3. The xdg ScreenCast portal (served by **xdph**) yields the output's PipeWire node. There is //! no GUI to pick an output headlessly, so xdph is steered through its **custom picker**: a //! managed config (`~/.config/hypr/xdph.conf`) points `screencopy:custom_picker_binary` at a tiny -//! installed shim that cats a per-session selection file we write (`[SELECTION]screen:`) -//! right before the handshake — byte-for-byte the xdpw pattern, xdph's picker wire format. -//! 4. Teardown is RAII: drop stops the portal thread (its zbus connection ends the cast) and runs -//! `hyprctl output remove NAME`. +//! installed shim that cats a per-session selection file we write right before the handshake — +//! `[SELECTION]/screen:`, whose leading `/` is xdph's mandatory empty-flags separator (see +//! [`crate::portal_picker`], which owns the format and its tests). +//! 4. Teardown is RAII **and ordered**: drop closes the ScreenCast session and WAITS for the portal +//! to confirm it, and only then runs `hyprctl output remove NAME`. Removing the output first is +//! what made every stream after the first one fail on Hyprland — see [`StopGuard`]. //! //! Requirements: the host runs inside (or can reach) the Hyprland session — either //! `HYPRLAND_INSTANCE_SIGNATURE` is inherited, or [`is_available`] discovers it from @@ -27,7 +29,8 @@ //! the ScreenCast interface routed to xdph (`scripts/headless/portals.conf`). //! //! Contracts verified on **Hyprland 0.55.4 + xdph 1.3.x** (`design/hyprland-support.md` Phase 0): -//! `hyprctl` subcommands / JSON shapes, the `[SELECTION]screen:` picker format, the +//! `hyprctl` subcommands / JSON shapes, the `[SELECTION]/screen:` picker format (re-derived +//! from xdph 1.3.12's own parser on 2026-08-14, which is when the missing `/` turned up), the //! `~/.config/hypr/xdph.conf` path + `screencopy:custom_picker_binary` key, and that `eval` needs //! the Lua config manager. Not yet exercised end-to-end on real DRM hardware: a headless output's //! GBM/dmabuf allocation (fails on a nested/NVIDIA test box — Sunshine#4197); `set_monitor_rule` @@ -44,7 +47,7 @@ use std::thread; use std::time::{Duration, Instant}; /// Per-session file the xdph custom picker reads the selected output from. We write -/// `screen:\n` here right before the portal handshake selects sources. Lives under +/// [`picker_selection_line`] here right before the portal handshake selects sources. Lives under /// `$XDG_RUNTIME_DIR` (per-user, 0700) — NOT a world-writable /tmp path another local user could /// pre-create or rewrite between our write and xdph's read (steer capture elsewhere). Mirrors the /// wlroots chooser file. @@ -61,13 +64,11 @@ fn picker_shim_path() -> String { format!("{dir}/punktfunk-xdph-picker.sh") } -/// The picker line for output `name`. Verified against xdph 1.3.x / hyprland-share-picker on -/// Hyprland 0.55.4: xdph reads the custom picker's stdout and requires the `[SELECTION]` marker -/// followed by `screen:` (or `window:` / `region:…`); anything else is rejected as -/// "strange output" and falls back to the interactive picker. So a monitor selection is -/// `[SELECTION]screen:`. +/// The picker line for output `name` — `[SELECTION]/screen:`, whose every byte is load-bearing. +/// Lives in [`crate::portal_picker`] with a transcription of xdph's parser, because it is a wire +/// format with no error report and this file only compiles on Linux. fn picker_selection_line(name: &str) -> String { - format!("[SELECTION]screen:{name}\n") + crate::portal_picker::selection_line(name) } /// Monotonic per-process counter for headless output names (`PF--1`, `PF--2`, …). Named @@ -131,11 +132,18 @@ pub struct HyprlandDisplay { /// only. Every session on this backend therefore resolves to `Embedded` today; KWin/Mutter /// remain the legs where the metadata channel is actually exercised. hw_cursor: bool, + /// What the portal actually gave us on the most recent [`create`](VirtualDisplay::create) — see + /// [`VirtualDisplay::last_portal_cursor_mode`], which is how the host learns that a cursor + /// overlay is never coming instead of inferring it from an absence. + last_cursor_mode: Option, } impl HyprlandDisplay { pub fn new() -> Result { - Ok(HyprlandDisplay { hw_cursor: false }) + Ok(HyprlandDisplay { + hw_cursor: false, + last_cursor_mode: None, + }) } } @@ -201,6 +209,10 @@ impl VirtualDisplay for HyprlandDisplay { self.hw_cursor } + fn last_portal_cursor_mode(&self) -> Option { + self.last_cursor_mode + } + fn create(&mut self, mode: Mode) -> Result { // Log the permission-system caveat once per process (silent black frames otherwise). preflight_once(); @@ -224,16 +236,21 @@ impl VirtualDisplay for HyprlandDisplay { // thread (it parks to keep the cast alive, like the other backends). Serialized: the // selection is one per-user file, so a concurrent session's write between ours and xdph's // read would silently capture the wrong output (see `SELECTION_LOCK`). - let (fd, node_id, stop) = { + let (fd, node_id, cursor_mode, stop) = { let _sel = SELECTION_LOCK.lock().unwrap_or_else(|e| e.into_inner()); select_and_cast(&name, self.hw_cursor)? }; + // Latched for `last_portal_cursor_mode`: on today's xdph this is `embedded` whatever we + // asked for, and the session's whole cursor behaviour follows from that fact rather than + // from `hw_cursor`. + self.last_cursor_mode = Some(cursor_mode); tracing::info!( node_id, output = %name, w = mode.width, h = mode.height, hz = mode.refresh_hz, + cursor = cursor_mode.name(), "hyprland headless output ready" ); Ok(VirtualOutput { @@ -251,24 +268,100 @@ impl VirtualDisplay for HyprlandDisplay { reused_gen: None, pool_gen: None, expect_exact_dims: false, + // Hyprland is an EXTEND topology: this head sits BESIDE the operator's, so absolute + // input has to be aimed at it by name or it lands on their screen. `hyprctl`'s monitor + // name is the head's `wl_output.name`, which is what the injector matches. + output_name: Some(name), }) } } -/// Drop order matters: stop the portal thread first (zbus connection drop ends the cast), then -/// remove the output (fields drop in declaration order). +/// Drop order matters, and it is the whole fix: [`StopGuard`] **blocks until the ScreenCast session +/// is actually closed**, and only then does [`OutputGuard`] remove the compositor output (fields drop +/// in declaration order). +/// +/// 🛑 THIS ORDERING USED TO BE A LIE. `StopGuard::drop` only set an atomic and returned, while the +/// portal thread noticed it 200 ms later — so `OutputGuard::drop` ran `hyprctl output remove` on an +/// output xdph was still actively capturing, every single teardown. See [`StopGuard`] for what that +/// did to xdph. struct Keepalive { _stop: StopGuard, _output: OutputGuard, } -/// Dropping this ends the portal keepalive thread, closing its zbus connection — the portal then -/// tears the screencast session down. -struct StopGuard(Arc); +/// How long teardown waits for the portal to confirm the ScreenCast session is closed before giving +/// up and removing the output anyway. One D-Bus round trip through xdg-desktop-portal to xdph; three +/// seconds is generous. Bounded on purpose: a portal that has already wedged must not be able to +/// wedge the host's teardown with it — every other blocking helper on this path is bounded the same +/// way (see [`HYPRCTL_BUDGET`]). +const CAST_CLOSE_BUDGET: Duration = Duration::from_secs(3); + +/// Ends the cast: signals the portal thread, then **waits for it to have closed the ScreenCast +/// session**, so the caller may safely remove the output afterwards. +/// +/// 🛑 THE WAIT IS THE POINT — "only the first stream after a portal start works" on Hyprland was +/// this, root-caused 2026-08-14 against Hyprland 0.55.4 + xdph 1.3.12 + xdg-desktop-portal 1.20.4. +/// +/// This used to be a bare `AtomicBool` that `drop` merely SET. The portal thread polled it every +/// 200 ms and then just dropped its zbus connection, and xdph destroys a session on exactly one +/// event — an explicit `org.freedesktop.impl.portal.Session.Close` (`Session.cpp:37`, +/// `onCloseSession`); it has no peer-vanished watcher of its own. The frontend does have one +/// (`xdg-desktop-portal.c:230` `peer_died_cb` → `close_sessions_for_sender`), but it only fires once +/// our unique bus name goes away, which is *after* the 200 ms poll, and it runs asynchronously on a +/// GTask thread. Meanwhile `OutputGuard::drop` had already removed the output — synchronously, +/// microseconds after the flag was set. +/// +/// So every teardown destroyed the `wl_output` out from under a live screencopy session. xdph's next +/// `Start` then built a PipeWire stream against that wreckage and fell into +/// +/// ```text +/// while (pSession->sharingData.nodeID == SPA_ID_INVALID) { // Screencopy.cpp:307-313 +/// int ret = pw_loop_iterate(g_pPortalManager->m_sPipewire.loop, 0); // timeout 0 = NON-blocking +/// ``` +/// +/// — an unbounded hot spin on xdph's ONLY event-loop thread, inside the `Start` handler, holding its +/// `m_mEventLock`. From that moment xdph answers no D-Bus, no Wayland and no PipeWire, ever again, and +/// every later `select_and_cast` dies on our 20 s timeout. MEASURED on the box: the wedged instance's +/// unit reported `Consumed 3min 51.971s CPU time over 23min 41.092s wall clock`, and there were +/// exactly 232.7 s of wall clock between its last log flush and its restart — 231.971 s of CPU +/// against 232.7 s of wall, i.e. one core pinned solid for precisely the wedged interval. +/// +/// Waiting here closes that window: `Session.Close` is answered synchronously by the frontend +/// (`xdp-session.c:217` `handle_close` → `xdp_session_close` → +/// `xdp_dbus_impl_session_call_close_sync`), so by the time `close()` returns, xdph has already run +/// `destroyStream` and logged `Session destroyed`. The output we remove next is one nobody is +/// capturing. +struct StopGuard { + stop: Arc, + /// Signalled by the portal thread once it has closed the ScreenCast session. + /// + /// `None` on every path where no cast was ever established (a rejected or timed-out handshake): + /// there is nothing to close, and a portal that just failed to answer for 20 s is precisely the + /// one that would burn the whole budget here for nothing. + closed: Option>, +} impl Drop for StopGuard { fn drop(&mut self) { - self.0.store(true, Ordering::Relaxed); + self.stop.store(true, Ordering::Relaxed); + let Some(closed) = self.closed.take() else { + return; + }; + match closed.recv_timeout(CAST_CLOSE_BUDGET) { + // Closed — xdph has torn the capture down, the output is safe to remove. + Ok(()) => {} + // The thread is gone without confirming (it panicked, or the runtime died). Nothing is + // holding the cast either way, so there is nothing left to wait for. + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {} + // Still going after the budget. Fall through and remove the output anyway — a leaked + // output is worse than a racy one — but say so, because this is the state that wedges + // xdph and the next session will be the one that pays for it. + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => tracing::warn!( + budget_s = CAST_CLOSE_BUDGET.as_secs(), + "the ScreenCast session did not close in time — removing the output underneath it, \ + which is what wedges xdph's frame loop; the next cast may find the portal busy" + ), + } } } @@ -349,6 +442,12 @@ impl Drop for OutputGuard { /// stream thread, whose only way to end a session is to return, so one hung query used to wedge the /// session for good. Generous next to a healthy call (single-digit milliseconds), and every call /// site already has a failed-query path. +/// Ceiling on the whole ScreenCast handshake (`create_session` → `select_sources` → `start` → +/// `open_pipe_wire_remote`). Deliberately under [`select_and_cast`]'s 20 s wait so a stuck portal is +/// reported by the thread that owns it, with a reason, instead of the caller timing out on it — and, +/// far more importantly, so that thread EXITS. See the note at the handshake itself. +const HANDSHAKE_BUDGET: Duration = Duration::from_secs(15); + const HYPRCTL_BUDGET: Duration = Duration::from_secs(5); /// Budget for the one-shot xdph restart. `systemctl --user try-restart` waits for the user manager's @@ -401,19 +500,31 @@ impl Drop for SelectionFile { /// Point xdph's custom picker at `output` and run the ScreenCast handshake, returning the portal fd /// + node id and the guard that stops the cast. The caller must hold [`SELECTION_LOCK`]. -fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, StopGuard)> { +fn select_and_cast( + output: &str, + hw_cursor: bool, +) -> Result<(OwnedFd, u32, crate::portal_cursor::Mode, StopGuard)> { ensure_xdph_config()?; let sel = selection_file(); std::fs::write(&sel, picker_selection_line(output)).with_context(|| format!("write {sel}"))?; // Owned from the write on: every arm below (and every `?`) leaves the handshake, which is the // only thing that reads it. let _sel_file = SelectionFile(sel); - let (setup_tx, setup_rx) = std::sync::mpsc::channel::>(); + // The NEGOTIATED cursor mode rides back with the fd and node id: it is decided inside the + // portal thread (only there is the proxy to ask), and nothing downstream can re-derive it — + // `hw_cursor` is the request, not the answer. + let (setup_tx, setup_rx) = + std::sync::mpsc::channel::>(); + // The teardown handshake: the thread signals this once it has closed the ScreenCast session, and + // `StopGuard::drop` waits on it before the output is removed (see `StopGuard`). Kept a SEPARATE + // channel from the setup one above — it fires at the other end of the cast's life, long after + // `setup_rx` has been consumed. + let (closed_tx, closed_rx) = std::sync::mpsc::channel::<()>(); let stop = Arc::new(AtomicBool::new(false)); let stop_thread = stop.clone(); thread::Builder::new() .name("punktfunk-hypr-cast".into()) - .spawn(move || portal_thread(setup_tx, stop_thread, hw_cursor)) + .spawn(move || portal_thread(setup_tx, closed_tx, stop_thread, hw_cursor)) .context("spawn hyprland portal thread")?; // Built BEFORE the wait so EVERY error arm below sets the flag on its way out — as Mutter's // `create` does. Returning the bare `Arc` and letting the CALLER wrap it left the two failure @@ -422,9 +533,14 @@ fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, StopG // parks forever on `while !stop`, holding a live ScreenCast session, its zbus connection, an // `OwnedFd` and a 2-worker tokio runtime — one more set per slow-portal connect, for the host's // lifetime, against an output that no longer exists. - let guard = StopGuard(stop); + let mut guard = StopGuard { stop, closed: None }; match setup_rx.recv_timeout(Duration::from_secs(20)) { - Ok(Ok((fd, node_id))) => Ok((fd, node_id, guard)), + Ok(Ok((fd, node_id, cursor_mode))) => { + // A cast exists now, so teardown has something to close and must wait for it. Only this + // arm arms the wait: see the field note on `StopGuard::closed`. + guard.closed = Some(closed_rx); + Ok((fd, node_id, cursor_mode, guard)) + } Ok(Err(e)) => bail!("ScreenCast portal on {output} failed: {e}"), Err(_) => bail!("timed out waiting for the ScreenCast portal on {output}"), } @@ -440,10 +556,11 @@ pub(crate) fn stream_existing_output( hw_cursor: bool, ) -> Result { let _sel = SELECTION_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - let (fd, node_id, stop) = select_and_cast(connector, hw_cursor)?; + let (fd, node_id, cursor_mode, stop) = select_and_cast(connector, hw_cursor)?; Ok(crate::mirror::MirrorStream { node_id, remote_fd: Some(fd), + cursor_mode: Some(cursor_mode), keepalive: Box::new(stop), }) } @@ -793,7 +910,8 @@ fn ensure_xdph_config() -> Result<()> { /// custom picker, no dialog. (Kept separate from wlroots' copy so each wlr-family backend stays /// self-owned per D1; unify if they ever diverge no further.) fn portal_thread( - setup_tx: Sender>, + setup_tx: Sender>, + closed_tx: Sender<()>, stop: Arc, hw_cursor: bool, ) { @@ -801,16 +919,15 @@ fn portal_thread( use ashpd::desktop::PersistMode; use ashpd::enumflags2::BitFlags; - // Multi-thread runtime: the zbus background reader must be pumped across the - // create_session → select_sources → start handshake (see capture/linux.rs). - let rt = match tokio::runtime::Builder::new_multi_thread() - .worker_threads(2) - .enable_all() - .build() - { + // 🛑 The SHARED, never-dropped runtime — NOT a per-cast one. ashpd caches its D-Bus connection + // process-globally, and a per-cast runtime takes that connection's background reader down with + // it when the cast ends, leaving every later handshake in this process awaiting a reply nothing + // is alive to read. That is the whole "the first stream works, the rest are black" bug. See + // [`crate::portal_rt`] for the measurement. + let rt = match crate::portal_rt::portal_runtime() { Ok(rt) => rt, Err(e) => { - let _ = setup_tx.send(Err(format!("build tokio runtime: {e}"))); + let _ = setup_tx.send(Err(e)); return; } }; @@ -818,9 +935,21 @@ fn portal_thread( rt.block_on(async move { let result: Result<()> = async { - let proxy = Screencast::new().await.context( - "connect ScreenCast portal (is xdg-desktop-portal running with the hyprland backend/xdph?)", - )?; + // Inside the bound below, deliberately: when the cached connection was orphaned this is + // where the thread hung — `Screencast::new()` itself, before a single handshake call — + // and a bound that started after it reported the caller's generic timeout instead. + let connect = async { + Screencast::new().await.context( + "connect ScreenCast portal (is xdg-desktop-portal running with the hyprland backend/xdph?)", + ) + }; + let proxy = match tokio::time::timeout(HANDSHAKE_BUDGET, connect).await { + Ok(v) => v?, + Err(_) => bail!( + "connecting to the ScreenCast portal did not return within {}s", + HANDSHAKE_BUDGET.as_secs() + ), + }; // NEGOTIATED against what xdph advertises, never asserted from `hw_cursor` alone: a // cursor mode the backend does not offer does not degrade — xdg-desktop-portal's // FRONTEND fails the call ("Unavailable cursor mode %x") before xdph sees it. @@ -829,51 +958,99 @@ fn portal_thread( // hardcode killed EVERY cursor-forward session here, on today's packages, not just on // old installs: `unavailable cursor mode 4`, "pipeline build failed", black client. let cursor_mode = crate::portal_cursor::negotiate(&proxy, hw_cursor, "xdph").await; - let session = proxy - .create_session(Default::default()) - .await - .context("create_session")?; - proxy - .select_sources( - &session, - SelectSourcesOptions::default() - .set_cursor_mode(cursor_mode) - // xdph offers MONITOR; the custom picker selects our output. - .set_sources(BitFlags::from_flag(SourceType::Monitor)) - .set_multiple(false) - .set_persist_mode(PersistMode::DoNot), - ) - .await - .context("select_sources")? - .response() - .context("select_sources rejected")?; - let streams = proxy - .start(&session, None, Default::default()) - .await - .context("start cast")? - .response() - .context("start response (custom picker declined? check the xdph config/shim/selection file)")?; - let stream = streams - .streams() - .first() - .context("portal returned no streams")? - .clone(); - let node_id = stream.pipe_wire_node_id(); - let fd = proxy - .open_pipe_wire_remote(&session, Default::default()) - .await - .context("open_pipe_wire_remote")?; + // 🛑 BOUNDED, and that bound is load-bearing. `select_sources`/`start` await a D-Bus + // reply a wedged portal never sends, and an await that never returns CANNOT be cancelled + // by the `stop` flag — the thread never reaches the park loop that reads it. That is how + // one host accumulated NINE live cast threads (28 tokio workers) on 2026-08-14: each + // timed-out attempt left one behind holding a half-created portal session on this + // process's shared D-Bus connection, and from the first hang onwards EVERY later request + // from this process hung too — while a freshly-spawned process talking to the very same + // portal completed the identical handshake fine. Shorter than the caller's 20 s wait, so + // the failure is reported HERE with a reason instead of surfacing as a bare timeout. + let handshake = async { + let session = proxy + .create_session(Default::default()) + .await + .context("create_session")?; + proxy + .select_sources( + &session, + SelectSourcesOptions::default() + .set_cursor_mode(cursor_mode.to_ashpd()) + // xdph offers MONITOR; the custom picker selects our output. + .set_sources(BitFlags::from_flag(SourceType::Monitor)) + .set_multiple(false) + .set_persist_mode(PersistMode::DoNot), + ) + .await + .context("select_sources")? + .response() + .context("select_sources rejected")?; + let streams = proxy + .start(&session, None, Default::default()) + .await + .context("start cast")? + .response() + .context("start response (custom picker declined? check the xdph config/shim/selection file)")?; + let stream = streams + .streams() + .first() + .context("portal returned no streams")? + .clone(); + let node_id = stream.pipe_wire_node_id(); + let fd = proxy + .open_pipe_wire_remote(&session, Default::default()) + .await + .context("open_pipe_wire_remote")?; + Ok::<_, anyhow::Error>((session, fd, node_id)) + }; + let (session, fd, node_id) = + match tokio::time::timeout(HANDSHAKE_BUDGET, handshake).await { + Ok(v) => v?, + Err(_) => bail!( + "the ScreenCast portal did not complete the handshake within {}s — \ + abandoning it instead of parking this thread on it forever (a hung \ + request poisons every later one from this process)", + HANDSHAKE_BUDGET.as_secs() + ), + }; setup_tx - .send(Ok((fd, node_id))) + .send(Ok((fd, node_id, cursor_mode))) .map_err(|_| anyhow!("virtual-output opener went away"))?; - // Park, keeping `proxy` + `session` (the zbus connection) alive until stopped — the cast - // is torn down when the connection drops. + // Park, keeping `proxy` + `session` alive until stopped. Polled at 20 ms rather than the + // 200 ms this used to use, because the teardown now WAITS on what follows — every + // millisecond here is a millisecond of stream teardown. let _keep_alive = (&proxy, &session); while !stop.load(Ordering::Relaxed) { - tokio::time::sleep(Duration::from_millis(200)).await; + tokio::time::sleep(Duration::from_millis(20)).await; } + + // 🛑 CLOSE THE SESSION, AND CLOSE IT *BEFORE* THE OUTPUT GOES AWAY. Dropping the + // connection and trusting the peer to notice is what this used to do, and it is not the + // contract: xdph destroys a session only on an explicit + // `org.freedesktop.impl.portal.Session.Close` (`Session.cpp:37`). The caller is blocked + // in `StopGuard::drop` waiting for the signal below, and only removes the compositor + // output afterwards — that ordering is the whole fix; see `StopGuard`. + // + // Bounded: `close()` goes through xdg-desktop-portal to xdph, and an already-wedged xdph + // never answers. Timing out here still signals, so teardown pays the budget once and + // moves on rather than hanging on a portal that is already gone. + match tokio::time::timeout(CAST_CLOSE_BUDGET, session.close()).await { + Ok(Ok(())) => {} + Ok(Err(e)) => tracing::warn!( + error = %e, + "closing the ScreenCast session failed — the next cast may find xdph busy" + ), + Err(_) => tracing::warn!( + budget_s = CAST_CLOSE_BUDGET.as_secs(), + "the ScreenCast portal did not answer Session.Close in time — it is probably \ + already wedged" + ), + } + // Release the teardown. Best-effort: the receiver is gone if the caller already gave up. + let _ = closed_tx.send(()); Ok(()) } .await; @@ -929,9 +1106,10 @@ mod tests { } } + /// The backend hands the picker exactly what [`crate::portal_picker`] says — that module owns the + /// format and its xdph-parser tests, which run on every platform rather than only this leg. #[test] - fn picker_line_carries_the_selection_marker() { - // xdph requires the `[SELECTION]` prefix; a bare `screen:NAME` is rejected as strange output. - assert_eq!(picker_selection_line("PF-1"), "[SELECTION]screen:PF-1\n"); + fn picker_line_is_the_shared_selection_format() { + assert_eq!(picker_selection_line("PF-1"), "[SELECTION]/screen:PF-1\n"); } } diff --git a/crates/pf-vdisplay/src/vdisplay/linux/kwin.rs b/crates/pf-vdisplay/src/vdisplay/linux/kwin.rs index 51f9aaa6..3ea211a8 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/kwin.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/kwin.rs @@ -1467,6 +1467,9 @@ pub(crate) fn stream_existing_output( node_id, // KWin publishes on the user's own PipeWire daemon — no portal remote to carry. remote_fd: None, + // Not an xdg-portal session either: the `zkde_screencast` pointer mode was asked of KWin + // directly and KWin honours it, so the request IS the answer. + cursor_mode: None, keepalive: Box::new(StopOnDrop(stop)), }) } diff --git a/crates/pf-vdisplay/src/vdisplay/linux/mutter.rs b/crates/pf-vdisplay/src/vdisplay/linux/mutter.rs index 171697c8..069c9579 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/mutter.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/mutter.rs @@ -602,6 +602,9 @@ pub(crate) fn stream_existing_output( node_id, // Mutter's RecordMonitor node lives on the user's PipeWire daemon (like RecordVirtual). remote_fd: None, + // Not an xdg-portal session: `cursor-mode` was set directly on `RecordMonitor` and Mutter + // honours it, so the request IS the answer and there is nothing to report back. + cursor_mode: None, keepalive: Box::new(guard), }) } diff --git a/crates/pf-vdisplay/src/vdisplay/linux/portal_cursor.rs b/crates/pf-vdisplay/src/vdisplay/linux/portal_cursor.rs index 65dde2b6..bc9031a8 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/portal_cursor.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/portal_cursor.rs @@ -33,8 +33,15 @@ /// A ScreenCast cursor mode, valued as the portal's own wire bits — which is what a backend prints /// when it rejects one, so `Metadata`'s `4` is literally the number in the field report. +/// +/// Public because the NEGOTIATED mode is a per-session fact the consumer needs: the host's stream +/// loop reads it back off the backend ([`VirtualDisplay::last_portal_cursor_mode`]) to know whether +/// `SPA_META_Cursor` can ever arrive on this output. Re-exported as +/// [`crate::PortalCursorMode`](crate::PortalCursorMode). +/// +/// [`VirtualDisplay::last_portal_cursor_mode`]: crate::VirtualDisplay::last_portal_cursor_mode #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum Mode { +pub enum Mode { /// No pointer in the cast at all. Hidden = 1, /// The compositor paints the pointer into the frames it hands us. @@ -52,7 +59,7 @@ impl Mode { } /// The spelling used in logs and in `PUNKTFUNK_PORTAL_CURSOR_MODE`. - pub(crate) const fn name(self) -> &'static str { + pub const fn name(self) -> &'static str { match self { Mode::Hidden => "hidden", Mode::Embedded => "embedded", @@ -60,6 +67,20 @@ impl Mode { } } + /// Can `SPA_META_Cursor` EVER arrive under this mode? Only under [`Metadata`](Mode::Metadata) — + /// and this is the whole point of surfacing the negotiated mode. + /// + /// Under `Embedded` the compositor paints the pointer into the frames and sends no cursor + /// metadata **regardless of where the pointer is**, so on such a session the absence of a cursor + /// overlay carries NO information: not about the pointer's position, not about whether the + /// capture is healthy. Consumers that treat "no overlay" as a symptom (the host's seat-pointer + /// park schedule, which reads it as "the pointer has not reached the streamed output" — true on + /// Mutter, which suppresses metadata while the pointer is off the recorded view) must ask this + /// first. Under `Hidden` there is no pointer at all, so the same holds. + pub const fn delivers_metadata(self) -> bool { + matches!(self, Mode::Metadata) + } + /// What to ask for instead, best first, when this mode is not advertised. const fn fallbacks(self) -> [Mode; 2] { match self { @@ -185,7 +206,7 @@ pub(crate) fn want(hw_cursor: bool, backend: &str) -> Mode { #[cfg(target_os = "linux")] impl Mode { - fn to_ashpd(self) -> ashpd::desktop::screencast::CursorMode { + pub(crate) fn to_ashpd(self) -> ashpd::desktop::screencast::CursorMode { use ashpd::desktop::screencast::CursorMode; match self { Mode::Hidden => CursorMode::Hidden, @@ -198,12 +219,17 @@ impl Mode { /// Ask the portal what it supports, run the ladder, and hand back the mode to put in /// `SelectSources`. Infallible by construction: a backend we cannot interrogate gets `Embedded`, /// the mode that predates the property and that every implementation has always had. +/// +/// Returns OUR [`Mode`], not ashpd's — the caller converts with [`Mode::to_ashpd`] for the request +/// and carries the value out of the portal thread, because what was negotiated (as opposed to +/// asked for) governs how the session's cursor behaves for its whole life. See +/// [`Mode::delivers_metadata`]. #[cfg(target_os = "linux")] pub(crate) async fn negotiate( proxy: &ashpd::desktop::screencast::Screencast, hw_cursor: bool, backend: &str, -) -> ashpd::desktop::screencast::CursorMode { +) -> Mode { let want = want(hw_cursor, backend); let advertised = match proxy.available_cursor_modes().await { Ok(avail) => avail.bits(), @@ -216,7 +242,7 @@ pub(crate) async fn negotiate( error = %e, "ScreenCast: AvailableCursorModes query failed — requesting Embedded cursor" ); - return Mode::Embedded.to_ashpd(); + return Mode::Embedded; } }; let choice = pick(advertised, want); @@ -238,7 +264,7 @@ pub(crate) async fn negotiate( (requesting it anyway would close the session)" ), } - choice.mode.to_ashpd() + choice.mode } #[cfg(test)] @@ -286,6 +312,21 @@ mod tests { assert_eq!(c.wanted, Some(Mode::Metadata)); } + /// The consumer-facing half of the same incident: xdph negotiates `3` down to `Embedded`, and + /// under Embedded no `SPA_META_Cursor` ever arrives — so a host that reads "no cursor overlay" + /// as "the pointer has not reached the streamed output" (true on Mutter, which suppresses + /// metadata off-view) re-centres the user's pointer forever. Field report 2026-08-14: the seat + /// pointer warped to centre once a second for the full park cap on a working Hyprland stream. + #[test] + fn only_metadata_can_deliver_a_cursor_overlay() { + assert!(Mode::Metadata.delivers_metadata()); + assert!(!Mode::Embedded.delivers_metadata()); + assert!(!Mode::Hidden.delivers_metadata()); + // The negotiated mode is what governs, not the wanted one: this is the exact ladder result + // on xdph/xdpw, and it says "no overlay is ever coming" even though metadata was requested. + assert!(!pick(3, Mode::Metadata).mode.delivers_metadata()); + } + /// The same portal, a session with no cursor channel: already asking for what exists, so the /// fix must not perturb it. #[test] diff --git a/crates/pf-vdisplay/src/vdisplay/linux/portal_picker.rs b/crates/pf-vdisplay/src/vdisplay/linux/portal_picker.rs new file mode 100644 index 00000000..464e0409 --- /dev/null +++ b/crates/pf-vdisplay/src/vdisplay/linux/portal_picker.rs @@ -0,0 +1,153 @@ +//! The line we feed xdg-desktop-portal-hyprland's **custom picker** to select an output headlessly. +//! +//! xdph has no headless source-selection API: it runs `screencopy:custom_picker_binary` and parses +//! one line from its stdout. The Hyprland backend points that at a shim which cats a per-session +//! file, and this module is the format of what goes in the file — a wire format with no schema, no +//! validation and no error report, whose only observable failure is a line in the portal's log. +//! +//! Declared unconditionally although only `hyprland.rs` calls it, for the reason `portal_config` and +//! `portal_cursor` give: this is pure string handling whose tests are the only place its behaviour +//! is checkable without a compositor in front of you, so they run on every platform's CI rather than +//! only on the leg that compiles `mod hyprland`. That is not hypothetical here — the bug below +//! shipped, and the one test that existed for it passed the whole time. + +/// The picker line selecting monitor `name`: `[SELECTION]/`, with **empty flags**. +/// +/// 🛑 THE `/` IS MANDATORY AND WE USED TO OMIT IT. xdph splits the line on the FIRST `/` into flags +/// and selection ([xdph 1.3.12] `src/shared/ScreencopyShared.cpp:86-87`): +/// +/// ```text +/// const auto FLAGS = SELECTION.substr(0, SELECTION.find_first_of('/')); +/// const auto SEL = SELECTION.substr(SELECTION.find_first_of('/') + 1); +/// ``` +/// +/// With no `/` anywhere, `find_first_of` returns `npos`, so `FLAGS` becomes the WHOLE payload — and +/// `SEL` becomes the whole payload too, purely because `npos + 1` wraps to `0`. The output name +/// therefore still parsed correctly, which is exactly why this survived: the only thing it broke was +/// the flag loop (`:89-94`), which then walked `screen:` one character at a time — +/// +/// ```text +/// [screencopy] unknown flag from share-picker: s +/// [screencopy] unknown flag from share-picker: c +/// [screencopy] unknown flag from share-picker: e … one line per character +/// ``` +/// +/// — and, because `sc*r*een` contains an `r`, which is xdph's "allow restore token" flag, set +/// `data.allowToken = true`. xdph then answered every `Start` with a `restore_data` + +/// `persist_mode: 2` we never asked for (we request `PersistMode::DoNot`), which is the +/// `[screencopy] Sent restore token to …` on every single session in the field log. +/// +/// The reference picker prints the separator unconditionally +/// (`hyprland-share-picker/main.cpp:133-136`): +/// +/// ```text +/// std::cout << "[SELECTION]"; +/// std::cout << (ALLOWTOKENBUTTON->isChecked() ? "r" : ""); +/// std::cout << "/"; +/// std::cout << "screen:" << outputName.toStdString() << "\n"; +/// ``` +/// +/// so empty flags are spelled as a bare leading `/`, not as nothing at all. +/// +/// ⚠️ This was NOT the cause of the "only the first stream works" stall — see `hyprland.rs`'s +/// `StopGuard` for that. The sessions that streamed fine logged the identical flag spam and the +/// identical restore token, so it never discriminated. It is a real bug on its own terms and nothing +/// more. +/// +/// The trailing newline is equally load-bearing: xdph does `data.output.pop_back()` unconditionally +/// after `SEL.substr(7)` (`:96-100`), so without it the last character of the output name is eaten. +/// +/// [xdph 1.3.12]: https://github.com/hyprwm/xdg-desktop-portal-hyprland/blob/v1.3.12/src/shared/ScreencopyShared.cpp +pub(crate) fn selection_line(name: &str) -> String { + format!("[SELECTION]/screen:{name}\n") +} + +#[cfg(test)] +mod tests { + use super::*; + + /// xdph 1.3.12's parser (`ScreencopyShared.cpp:82-100`), transcribed — including the `npos` + /// arithmetic, which is the entire subtlety. Returns `(flags, output)`, or `None` where xdph + /// would fall through to its interactive picker. + /// + /// Transcribed rather than asserted on the string, because the bug this catches is invisible in + /// the string: the old line yielded the RIGHT OUTPUT NAME while handing xdph the whole selection + /// as a FLAG STRING. Only running its parser tells the two apart. + fn xdph_parse(picker_stdout: &str) -> Option<(String, String)> { + // `if (!RETVAL.contains("[SELECTION]")) return data;` — a default `SSelectionData`, i.e. + // TYPE_INVALID, which makes `SelectSources` fail. + let marker = picker_stdout.find("[SELECTION]")?; + let selection = &picker_stdout[marker + "[SELECTION]".len()..]; + // `substr(0, npos)` is the whole string, and `substr(npos + 1)` is `substr(0)` — also the + // whole string. Unsigned wraparound, not a special case in xdph. + let (flags, sel) = match selection.find('/') { + Some(i) => (&selection[..i], &selection[i + 1..]), + None => (selection, selection), + }; + let name = sel.strip_prefix("screen:")?; + // `data.output.pop_back()` — unconditional, hence the mandatory trailing newline. + let mut output = name.to_string(); + output.pop(); + Some((flags.to_string(), output)) + } + + /// The three load-bearing parts of the line, pinned as bytes. + #[test] + fn the_line_carries_marker_empty_flags_separator_and_newline() { + assert_eq!(selection_line("PF-1"), "[SELECTION]/screen:PF-1\n"); + } + + /// What xdph actually makes of our line: the exact output, and NO flags. + #[test] + fn xdph_reads_our_line_as_an_output_with_no_flags() { + for name in ["PF-1", "PF-1620-1", "HDMI-A-1", "DP-2"] { + let (flags, output) = xdph_parse(&selection_line(name)).expect("xdph parses our line"); + assert_eq!(output, name, "xdph must recover the exact output name"); + assert_eq!(flags, "", "we ask for no flags at all"); + assert!( + !flags.contains('r'), + "an `r` in the flags makes xdph hand back restore_data + persist_mode=2 we never \ + requested (Screencopy.cpp:261-267)" + ); + } + } + + /// The regression itself, so it cannot come back by "simplifying" the leading `/` away: the line + /// we used to send parsed the whole selection as flags, `r` included. + #[test] + fn without_the_separator_the_whole_selection_becomes_flags() { + let (flags, output) = xdph_parse("[SELECTION]screen:PF-1620-1\n").expect("still parses"); + assert_eq!( + output, "PF-1620-1", + "the output name did survive — which is precisely why this hid for so long" + ); + assert_eq!( + flags, "screen:PF-1620-1\n", + "…while the entire selection was handed to the flag loop" + ); + assert!( + flags.contains('r'), + "the `r` of `sc*r*een` is xdph's allow-restore-token flag" + ); + } + + /// Without the trailing newline xdph's unconditional `pop_back()` eats a character of the name — + /// a silently wrong output, not an error. + #[test] + fn the_trailing_newline_is_what_pop_back_consumes() { + assert!(selection_line("PF-1620-1").ends_with('\n')); + let (_, truncated) = xdph_parse("[SELECTION]/screen:PF-1620-1").expect("parses"); + assert_eq!( + truncated, "PF-1620-", + "pop_back() takes the last real character" + ); + } + + /// A line with no marker at all is xdph's documented empty-read fallback: it prompts instead. The + /// shim relies on this when no session has written the selection file. + #[test] + fn an_empty_read_is_not_a_selection() { + assert!(xdph_parse("").is_none()); + assert!(xdph_parse("screen:PF-1\n").is_none()); + } +} diff --git a/crates/pf-vdisplay/src/vdisplay/linux/portal_rt.rs b/crates/pf-vdisplay/src/vdisplay/linux/portal_rt.rs new file mode 100644 index 00000000..39ecb653 --- /dev/null +++ b/crates/pf-vdisplay/src/vdisplay/linux/portal_rt.rs @@ -0,0 +1,50 @@ +//! The ONE tokio runtime every portal handshake runs on, for the life of the process. +//! +//! 🛑🛑🛑 This exists because of a lifetime bug that cost a full day of misdiagnosis, so the reason +//! is written down rather than left to be rediscovered. +//! +//! ashpd caches its D-Bus connection **process-globally** — `static SESSION: OnceLock` +//! (ashpd 0.13.13, `src/proxy.rs:27`). The first `Screencast::new()` in the process creates that +//! connection, and zbus spawns the connection's background reader as a task **on whichever tokio +//! runtime happens to be current at that moment**. +//! +//! Each backend used to build its own multi-thread runtime per cast and drop it at teardown. So the +//! FIRST cast of a host process created the cached connection on a runtime that was then destroyed +//! when that cast ended — and the `OnceLock` went on handing the same, now-executor-less connection +//! to every later `Screencast::new()`, which then awaited a reply nothing was left alive to read. +//! +//! MEASURED 2026-08-14 (Hyprland 0.55.4 + xdph 1.3.12): the first cast of a host process streamed; +//! every cast after it hung, in a process whose surviving cast thread sat in `futex_do_wait` inside +//! runtime shutdown. The discriminator that pins it on us rather than on the compositor stack: a +//! freshly spawned process completed the identical handshake against the identical xdph, repeatedly, +//! while the long-lived host could complete none — and xdph itself was idle (28 ms of CPU). +//! +//! ⚠ Therefore: **never build a per-cast runtime, and never drop this one.** A `OnceLock` that is +//! only ever read keeps the connection's reader alive for the process lifetime, which is exactly as +//! long as the cached connection itself lives. `block_on` takes `&self`, so every cast thread can +//! park on this one runtime concurrently. + +use std::sync::OnceLock; +use tokio::runtime::Runtime; + +/// Build failures are reported to the caller rather than panicking: a host that cannot build a +/// runtime should fail the cast with a reason, not abort the process. +static PORTAL_RT: OnceLock> = OnceLock::new(); + +/// The shared portal runtime, or the error from trying to build it. +/// +/// Multi-thread with 2 workers: the zbus background reader must be pumped *across* the +/// `create_session` → `select_sources` → `start` handshake while a cast thread blocks on it, which a +/// current-thread runtime cannot do. +pub(crate) fn portal_runtime() -> Result<&'static Runtime, String> { + match PORTAL_RT.get_or_init(|| { + tokio::runtime::Builder::new_multi_thread() + .worker_threads(2) + .thread_name("punktfunk-portal-rt") + .enable_all() + .build() + }) { + Ok(rt) => Ok(rt), + Err(e) => Err(format!("build the shared portal runtime: {e}")), + } +} diff --git a/crates/pf-vdisplay/src/vdisplay/linux/wlroots.rs b/crates/pf-vdisplay/src/vdisplay/linux/wlroots.rs index 4e6b341f..d241ec4f 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/wlroots.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/wlroots.rs @@ -11,8 +11,10 @@ //! (`~/.config/xdg-desktop-portal-wlr/config`, written once + portal restarted on change) //! sets `chooser_type=simple` with a `chooser_cmd` that cats the chooser file, which we //! write per session (`Monitor: ` — xdpw 0.8 parses that prefix strictly). -//! 4. Teardown is RAII: drop stops the portal thread (its zbus connection ends the cast) and -//! runs `swaymsg output unplug` (headless outputs support unplug since sway 1.8). +//! 4. Teardown is RAII **and ordered**: drop closes the ScreenCast session and WAITS for the portal +//! to confirm it, and only then runs `swaymsg output unplug` (headless outputs support +//! unplug since sway 1.8). See [`StopGuard`] — and the long root-cause note on `hyprland.rs`'s +//! copy, which is where this was measured. //! //! Requirements: the host runs inside the sway session's environment (`SWAYSOCK` for swaymsg, //! and the portal activation env — `WAYLAND_DISPLAY`/`XDG_CURRENT_DESKTOP=sway` imported into @@ -67,11 +69,18 @@ pub struct WlrootsDisplay { /// never be served out-of-band: it now degrades to `Embedded` and streams, where it used to /// cancel the cast and hand the client a black screen. hw_cursor: bool, + /// What the portal actually gave us on the most recent [`create`](VirtualDisplay::create) — see + /// [`VirtualDisplay::last_portal_cursor_mode`], which is how the host learns that a cursor + /// overlay is never coming instead of inferring it from an absence. + last_cursor_mode: Option, } impl WlrootsDisplay { pub fn new() -> Result { - Ok(WlrootsDisplay { hw_cursor: false }) + Ok(WlrootsDisplay { + hw_cursor: false, + last_cursor_mode: None, + }) } } @@ -100,6 +109,10 @@ impl VirtualDisplay for WlrootsDisplay { self.hw_cursor } + fn last_portal_cursor_mode(&self) -> Option { + self.last_cursor_mode + } + fn create(&mut self, mode: Mode) -> Result { warn_topology_is_extend_only(); // Snapshot → create → identify, all under CREATE_LOCK. sway names the headless output @@ -146,16 +159,21 @@ impl VirtualDisplay for WlrootsDisplay { // its own thread (it parks to keep the cast alive, like the other backends). Serialized: // the chooser is one per-user file, so a concurrent session's write between ours and xdpw's // read would silently capture the wrong output (see `SELECTION_LOCK`). - let (fd, node_id, stop) = { + let (fd, node_id, cursor_mode, stop) = { let _sel = SELECTION_LOCK.lock().unwrap_or_else(|e| e.into_inner()); select_and_cast(&name, self.hw_cursor)? }; + // Latched for `last_portal_cursor_mode`: xdpw refuses metadata by construction, so this is + // `embedded` whatever we asked for, and the session's whole cursor behaviour follows from + // that fact rather than from `hw_cursor`. + self.last_cursor_mode = Some(cursor_mode); tracing::info!( node_id, output = %name, w = mode.width, h = mode.height, hz = mode.refresh_hz, + cursor = cursor_mode.name(), "sway headless output ready" ); Ok(VirtualOutput { @@ -173,24 +191,75 @@ impl VirtualDisplay for WlrootsDisplay { reused_gen: None, pool_gen: None, expect_exact_dims: false, + // Same EXTEND problem as Hyprland: on a sway session with real heads this `HEADLESS-N` + // sits beside them, and absolute input must be aimed at it by name. `swaymsg`'s output + // name is the head's `wl_output.name`, which is what the injector matches. + output_name: Some(name), }) } } -/// Drop order matters: stop the portal thread first (zbus connection drop ends the cast), -/// then unplug the output (fields drop in declaration order). +/// Drop order matters, and it is the whole fix: [`StopGuard`] **blocks until the ScreenCast session +/// is actually closed**, and only then does [`OutputGuard`] unplug the output (fields drop in +/// declaration order). This used to unplug first — see [`StopGuard`]. struct Keepalive { _stop: StopGuard, _output: OutputGuard, } -/// Dropping this ends the portal keepalive thread, closing its zbus connection — the portal -/// then tears the screencast session down. -struct StopGuard(Arc); +/// How long teardown waits for the portal to confirm the ScreenCast session is closed before giving +/// up and unplugging the output anyway. See `hyprland.rs`'s twin. +const CAST_CLOSE_BUDGET: Duration = Duration::from_secs(3); + +/// Ceiling on the whole ScreenCast handshake, under the caller's 20 s wait — see the note at the +/// handshake, and the longer one on `hyprland.rs`'s copy. +const HANDSHAKE_BUDGET: Duration = Duration::from_secs(15); + +/// Ends the cast: signals the portal thread, then **waits for it to have closed the ScreenCast +/// session**, so the caller may safely unplug the output afterwards. +/// +/// 🛑 THE WAIT IS THE POINT. Root-caused on the Hyprland leg (see the long note on `hyprland.rs`'s +/// `StopGuard`, which carries the measurements); the defect is the same here, and this is NOT an +/// assumption of symmetry — xdpw was read to confirm it, against `emersion/xdg-desktop-portal-wlr`: +/// +/// * **Only `Close` tears a session down.** `src/core/session.c` gives the session object exactly +/// one method — `SD_BUS_METHOD("Close", …, method_close, …)` — and nothing else calls +/// `xdpw_session_destroy` for a live cast. Like xdph, xdpw has no peer-vanished watcher of its own +/// and depends entirely on xdg-desktop-portal's `peer_died_cb` calling `Close` for us, which +/// happens only after our bus name goes away, asynchronously, and therefore after the old +/// `StopGuard` had already let `OutputGuard` unplug the output. +/// * **The same unbounded busy-wait is waiting for it.** `src/screencast/screencast.c:599-605`: +/// `while (cast->node_id == SPA_ID_INVALID) { pw_loop_iterate(state->pw_loop, 0); }` — timeout 0, +/// i.e. non-blocking, i.e. a hot spin on the portal's only loop with no escape if the stream never +/// gets a node id. xdph's copy (`Screencopy.cpp:307-313`) is this code; that is the one measured +/// pinning a core solid until it was restarted. +/// +/// So sway's `output unplug` yanks a captured output out from under a live session exactly the way +/// Hyprland's `output remove` did. Whether xdpw wedges *identically* has not been observed on glass +/// — no sway box was available — but the two preconditions are present in its source, and closing +/// the session before unplugging is the correct order regardless of what the backend does with it. +struct StopGuard { + stop: Arc, + /// Signalled by the portal thread once it has closed the ScreenCast session. `None` when no cast + /// was ever established — nothing to close, and nothing worth spending the budget on. + closed: Option>, +} impl Drop for StopGuard { fn drop(&mut self) { - self.0.store(true, Ordering::Relaxed); + self.stop.store(true, Ordering::Relaxed); + let Some(closed) = self.closed.take() else { + return; + }; + match closed.recv_timeout(CAST_CLOSE_BUDGET) { + // Closed, or the thread is gone without confirming — either way nothing holds the cast. + Ok(()) | Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {} + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => tracing::warn!( + budget_s = CAST_CLOSE_BUDGET.as_secs(), + "the ScreenCast session did not close in time — unplugging the output underneath \ + it; the next cast may find the portal busy" + ), + } } } @@ -346,7 +415,10 @@ impl Drop for ChooserFile { /// Point xdpw's chooser at `output` and run the ScreenCast handshake, returning the portal fd + /// node id and the guard that stops the cast. The caller must hold [`SELECTION_LOCK`]. -fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, StopGuard)> { +fn select_and_cast( + output: &str, + hw_cursor: bool, +) -> Result<(OwnedFd, u32, crate::portal_cursor::Mode, StopGuard)> { ensure_xdpw_config()?; let chooser = chooser_file(); std::fs::write(&chooser, format!("Monitor: {output}\n")) @@ -354,12 +426,21 @@ fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, StopG // Owned from the write on: every arm below (and every `?`) leaves the handshake, which is the // only thing that reads it. let _chooser = ChooserFile(chooser); - let (setup_tx, setup_rx) = std::sync::mpsc::channel::>(); + // The NEGOTIATED cursor mode rides back with the fd and node id: it is decided inside the + // portal thread (only there is the proxy to ask), and nothing downstream can re-derive it — + // `hw_cursor` is the request, not the answer. + let (setup_tx, setup_rx) = + std::sync::mpsc::channel::>(); + // The teardown handshake: the thread signals this once it has closed the ScreenCast session, and + // `StopGuard::drop` waits on it before the output is unplugged (see `StopGuard`). Kept a + // SEPARATE channel from the setup one above — it fires at the other end of the cast's life, + // long after `setup_rx` has been consumed. + let (closed_tx, closed_rx) = std::sync::mpsc::channel::<()>(); let stop = Arc::new(AtomicBool::new(false)); let stop_thread = stop.clone(); thread::Builder::new() .name("punktfunk-wlr-cast".into()) - .spawn(move || portal_thread(setup_tx, stop_thread, hw_cursor)) + .spawn(move || portal_thread(setup_tx, closed_tx, stop_thread, hw_cursor)) .context("spawn wlroots portal thread")?; // Built BEFORE the wait so EVERY error arm below sets the flag on its way out — as Mutter's // `create` does. Returning the bare `Arc` and letting the CALLER wrap it left the two failure @@ -368,9 +449,13 @@ fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, StopG // parks forever on `while !stop`, holding a live ScreenCast session, its zbus connection, an // `OwnedFd` and a 2-worker tokio runtime — one more set per slow-portal connect, for the host's // lifetime, against an output that no longer exists. - let guard = StopGuard(stop); + let mut guard = StopGuard { stop, closed: None }; match setup_rx.recv_timeout(Duration::from_secs(20)) { - Ok(Ok((fd, node_id))) => Ok((fd, node_id, guard)), + Ok(Ok((fd, node_id, cursor_mode))) => { + // A cast exists now, so teardown has something to close and must wait for it. + guard.closed = Some(closed_rx); + Ok((fd, node_id, cursor_mode, guard)) + } Ok(Err(e)) => bail!("ScreenCast portal on {output} failed: {e}"), Err(_) => bail!("timed out waiting for the ScreenCast portal on {output}"), } @@ -387,10 +472,11 @@ pub(crate) fn stream_existing_output( hw_cursor: bool, ) -> Result { let _sel = SELECTION_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - let (fd, node_id, stop) = select_and_cast(connector, hw_cursor)?; + let (fd, node_id, cursor_mode, stop) = select_and_cast(connector, hw_cursor)?; Ok(crate::mirror::MirrorStream { node_id, remote_fd: Some(fd), + cursor_mode: Some(cursor_mode), keepalive: Box::new(stop), }) } @@ -513,7 +599,8 @@ fn ensure_xdpw_config() -> Result<()> { /// reports the fd + node id and parks until stopped — the zbus connection is the cast's /// lifetime). xdpw answers the source selection via the chooser, no dialog. fn portal_thread( - setup_tx: Sender>, + setup_tx: Sender>, + closed_tx: Sender<()>, stop: Arc, hw_cursor: bool, ) { @@ -523,14 +610,13 @@ fn portal_thread( // Multi-thread runtime: the zbus background reader must be pumped across the // create_session → select_sources → start handshake (see capture/linux.rs). - let rt = match tokio::runtime::Builder::new_multi_thread() - .worker_threads(2) - .enable_all() - .build() - { + // The SHARED, never-dropped runtime — see [`crate::portal_rt`] and the long note on + // `hyprland.rs`'s copy: a per-cast runtime kills ashpd's process-global cached connection when + // the cast ends, and every later handshake in the process then hangs. + let rt = match crate::portal_rt::portal_runtime() { Ok(rt) => rt, Err(e) => { - let _ = setup_tx.send(Err(format!("build tokio runtime: {e}"))); + let _ = setup_tx.send(Err(e)); return; } }; @@ -538,9 +624,20 @@ fn portal_thread( rt.block_on(async move { let result: Result<()> = async { - let proxy = Screencast::new().await.context( - "connect ScreenCast portal (is xdg-desktop-portal running with the wlr backend?)", - )?; + // Bounded, like `hyprland.rs`'s copy: an orphaned cached connection hangs HERE, before + // any handshake call, so a bound that starts later never fires. + let connect = async { + Screencast::new().await.context( + "connect ScreenCast portal (is xdg-desktop-portal running with the wlr backend?)", + ) + }; + let proxy = match tokio::time::timeout(HANDSHAKE_BUDGET, connect).await { + Ok(v) => v?, + Err(_) => bail!( + "connecting to the ScreenCast portal did not return within {}s", + HANDSHAKE_BUDGET.as_secs() + ), + }; // NEGOTIATED against what xdpw advertises, never asserted from `hw_cursor` alone — see // the xdph copy in `hyprland.rs` for the incident. xdpw is the sharper case: its // screencast.c refuses the mode outright — @@ -549,51 +646,91 @@ fn portal_thread( // — so EVERY cursor-forward session on this backend asked for a mode that cancelled the // cast. Different wording from xdph's "unavailable cursor mode 4", same dead session. let cursor_mode = crate::portal_cursor::negotiate(&proxy, hw_cursor, "xdpw").await; - let session = proxy - .create_session(Default::default()) - .await - .context("create_session")?; - proxy - .select_sources( - &session, - SelectSourcesOptions::default() - .set_cursor_mode(cursor_mode) - // xdpw offers MONITOR only; the chooser picks our output. - .set_sources(BitFlags::from_flag(SourceType::Monitor)) - .set_multiple(false) - .set_persist_mode(PersistMode::DoNot), - ) - .await - .context("select_sources")? - .response() - .context("select_sources rejected")?; - let streams = proxy - .start(&session, None, Default::default()) - .await - .context("start cast")? - .response() - .context("start response (chooser declined? check the xdpw config/chooser file)")?; - let stream = streams - .streams() - .first() - .context("portal returned no streams")? - .clone(); - let node_id = stream.pipe_wire_node_id(); - let fd = proxy - .open_pipe_wire_remote(&session, Default::default()) - .await - .context("open_pipe_wire_remote")?; + // Bounded for the same reason as `hyprland.rs`'s copy (the long note lives there): an + // await on a wedged portal never returns, the `stop` flag is only read by the park loop + // further down, so the thread leaks — and a leaked half-handshake poisons every later + // portal request from this process. xdpw has the identical unbounded node-id spin as + // xdph (`screencast.c`), so it can wedge the same way. + let handshake = async { + let session = proxy + .create_session(Default::default()) + .await + .context("create_session")?; + proxy + .select_sources( + &session, + SelectSourcesOptions::default() + .set_cursor_mode(cursor_mode.to_ashpd()) + // xdpw offers MONITOR only; the chooser picks our output. + .set_sources(BitFlags::from_flag(SourceType::Monitor)) + .set_multiple(false) + .set_persist_mode(PersistMode::DoNot), + ) + .await + .context("select_sources")? + .response() + .context("select_sources rejected")?; + let streams = proxy + .start(&session, None, Default::default()) + .await + .context("start cast")? + .response() + .context( + "start response (chooser declined? check the xdpw config/chooser file)", + )?; + let stream = streams + .streams() + .first() + .context("portal returned no streams")? + .clone(); + let node_id = stream.pipe_wire_node_id(); + let fd = proxy + .open_pipe_wire_remote(&session, Default::default()) + .await + .context("open_pipe_wire_remote")?; + Ok::<_, anyhow::Error>((session, fd, node_id)) + }; + let (session, fd, node_id) = + match tokio::time::timeout(HANDSHAKE_BUDGET, handshake).await { + Ok(v) => v?, + Err(_) => bail!( + "the ScreenCast portal did not complete the handshake within {}s — \ + abandoning it instead of parking this thread on it forever (a hung \ + request poisons every later one from this process)", + HANDSHAKE_BUDGET.as_secs() + ), + }; setup_tx - .send(Ok((fd, node_id))) + .send(Ok((fd, node_id, cursor_mode))) .map_err(|_| anyhow!("virtual-output opener went away"))?; - // Park, keeping `proxy` + `session` (the zbus connection) alive until stopped — - // the cast is torn down when the connection drops. + // Park, keeping `proxy` + `session` alive until stopped. Polled at 20 ms rather than the + // 200 ms this used to use, because teardown now WAITS on what follows. let _keep_alive = (&proxy, &session); while !stop.load(Ordering::Relaxed) { - tokio::time::sleep(Duration::from_millis(200)).await; + tokio::time::sleep(Duration::from_millis(20)).await; } + + // 🛑 CLOSE THE SESSION, AND CLOSE IT *BEFORE* THE OUTPUT IS UNPLUGGED. `Session.Close` is + // the only thing that ends an xdpw session (`src/core/session.c`); dropping the + // connection and trusting the peer to notice is not the contract. The caller is blocked + // in `StopGuard::drop` on the signal below — see `StopGuard`. Bounded, so an + // already-wedged portal cannot hang teardown with it. + match tokio::time::timeout(CAST_CLOSE_BUDGET, session.close()).await { + Ok(Ok(())) => {} + Ok(Err(e)) => tracing::warn!( + error = %e, + "closing the ScreenCast session failed — the next cast may find the portal busy" + ), + Err(_) => tracing::warn!( + budget_s = CAST_CLOSE_BUDGET.as_secs(), + "the ScreenCast portal did not answer Session.Close in time — it is probably \ + already wedged" + ), + } + // Release the teardown. Best-effort: the receiver is gone if the caller already gave up. + let _ = closed_tx.send(()); Ok(()) } .await; diff --git a/crates/pf-vdisplay/src/vdisplay/mirror.rs b/crates/pf-vdisplay/src/vdisplay/mirror.rs index 0e524f5f..f0a32034 100644 --- a/crates/pf-vdisplay/src/vdisplay/mirror.rs +++ b/crates/pf-vdisplay/src/vdisplay/mirror.rs @@ -31,6 +31,12 @@ use anyhow::{bail, Context, Result}; pub(crate) struct MirrorStream { pub node_id: u32, pub remote_fd: Option, + /// The cursor mode the xdg ScreenCast portal NEGOTIATED for this recording, for the two + /// portal-based backends; `None` for the compositor-protocol ones (KWin/Mutter/gamescope), + /// which get what they ask for. Reported on to the host as + /// [`VirtualDisplay::last_portal_cursor_mode`] — same split as `remote_fd` above, and for the + /// same reason: only the portal path has an answer that can differ from the request. + pub cursor_mode: Option, /// Dropping this ends the recording. It never owns the monitor — we did not create it. pub keepalive: Box, } @@ -40,6 +46,9 @@ pub struct MirrorDisplay { compositor: Compositor, connector: String, hw_cursor: bool, + /// What the portal gave the most recent [`create`](VirtualDisplay::create), when this mirror + /// delegated to a portal-based backend. See [`VirtualDisplay::last_portal_cursor_mode`]. + last_cursor_mode: Option, } impl MirrorDisplay { @@ -48,6 +57,7 @@ impl MirrorDisplay { compositor, connector, hw_cursor: false, + last_cursor_mode: None, }) } } @@ -65,6 +75,10 @@ impl VirtualDisplay for MirrorDisplay { self.hw_cursor } + fn last_portal_cursor_mode(&self) -> Option { + self.last_cursor_mode + } + fn poolable_now(&self) -> bool { // Never. `create` below always reports `DisplayOwnership::External` — we did not make this // head and must not keep it — so the registry never pools a mirror, and the trait's `true` @@ -125,10 +139,15 @@ impl VirtualDisplay for MirrorDisplay { ), }; + // Latched for `last_portal_cursor_mode` — the delegate's verdict is this mirror's verdict. + self.last_cursor_mode = stream.cursor_mode; + // NOTE: aiming absolute input at this head is the HOST's job, not ours — this crate must // not depend on pf-inject (see the crate doc: "never on capture/inject"). The host sets the // anchor from the same pin at startup; §7.2 of the design doc explains why it is host-level - // rather than set here per session. + // rather than set here per session. We only CARRY the head's name out (`output_name` + // below), which is what the wlr injector needs to bind its virtual pointer to this head — + // the libei anchor above cannot serve it, because that backend selects by region. tracing::info!( connector = %target.connector, mode = %target.mode_label(), @@ -145,6 +164,9 @@ impl VirtualDisplay for MirrorDisplay { out.remote_fd = stream.remote_fd; // Never pooled, never lingered, never made primary/exclusive: we don't own this head. out.ownership = DisplayOwnership::External; + // The head absolute input maps into is the one we mirror — its connector IS its + // `wl_output.name` on the wlroots/Hyprland backends, where the injector matches on it. + out.output_name = Some(target.connector.clone()); Ok(out) } } diff --git a/crates/pf-vdisplay/src/vdisplay/registry.rs b/crates/pf-vdisplay/src/vdisplay/registry.rs index 7f3d32f9..4b5472af 100644 --- a/crates/pf-vdisplay/src/vdisplay/registry.rs +++ b/crates/pf-vdisplay/src/vdisplay/registry.rs @@ -302,6 +302,14 @@ mod pool { pub(super) keepalive: Box, pub(super) node_id: u32, pub(super) preferred_mode: Option<(u32, u32, u32)>, + /// The compositor's name for this output ([`VirtualOutput::output_name`]) — the identity the + /// host aims absolute input with. Kept across a keep-alive reuse for the same reason + /// `preferred_mode` is: the reused display IS the same head, so the output the caller is + /// handed must answer with the same name a fresh create would. No poolable backend sets it + /// today — the ones that do are all passed through unpooled (Hyprland/sway carry a portal + /// fd, a mirror is `External`) — so this only exists so that stops being a silent trap the + /// day one does. + pub(super) output_name: Option, pub(super) mode: Mode, pub(super) backend: &'static str, /// The identity slot the backend resolved for this display (KWin per-slot naming; `None` for @@ -601,6 +609,7 @@ mod pool { keepalive: Box::new(()), node_id: 0, preferred_mode: None, + output_name: None, mode: Mode { width: 1920, height: 1080, @@ -1083,6 +1092,7 @@ mod linux { fn output_for( node_id: u32, preferred_mode: Option<(u32, u32, u32)>, + output_name: Option, generation: u64, quit: Arc, reused: bool, @@ -1093,6 +1103,8 @@ mod linux { preferred_mode, Box::new(DisplayLease { generation, quit }), ); + // The head is the same one the entry was created for, so it answers with the same name. + out.output_name = output_name; // A2: tell the pipeline builder this was a REUSED kept display, so a first-frame failure can // `mark_failed(generation)` (tear the corpse down) rather than re-wedge the retry loop on the same node. out.reused_gen = reused.then_some(generation); @@ -1176,6 +1188,7 @@ mod linux { let generation = r.generation.fetch_add(1, Ordering::Relaxed); es[idx].generation = generation; let preferred_mode = es[idx].preferred_mode; + let output_name = es[idx].output_name.clone(); tracing::info!( backend, node_id, @@ -1184,6 +1197,7 @@ mod linux { ReuseOutcome::Reused(output_for( node_id, preferred_mode, + output_name, generation, quit.clone(), true, @@ -1279,6 +1293,7 @@ mod linux { let node_id = real.node_id; let preferred_mode = real.preferred_mode; + let output_name = real.output_name.clone(); // Fresh creates only: the backend may have birthed the output at a sacrificial mode whose // stream must renegotiate before frames count (KWin >60 Hz — see backend.rs). A REUSED kept // display already renegotiated in its prior session (the producer's rebuilt offer persists @@ -1295,6 +1310,7 @@ mod linux { keepalive: real.keepalive, node_id, preferred_mode, + output_name: output_name.clone(), mode, backend, identity_slot, @@ -1349,7 +1365,14 @@ mod linux { if (position.x, position.y) != (0, 0) { vd.apply_position(position.x, position.y); } - let mut out = output_for(node_id, preferred_mode, generation, quit, false); + let mut out = output_for( + node_id, + preferred_mode, + output_name, + generation, + quit, + false, + ); out.expect_exact_dims = expect_exact_dims; Ok(out) } diff --git a/crates/punktfunk-host/src/capture.rs b/crates/punktfunk-host/src/capture.rs index 045b5147..68914df3 100644 --- a/crates/punktfunk-host/src/capture.rs +++ b/crates/punktfunk-host/src/capture.rs @@ -135,6 +135,17 @@ pub fn capture_virtual_output( // handshake already resolved that through [`capturer_supports_hdr_for`] before the Welcome, // so passing it through here is the whole of this arm's HDR logic. It used to be dropped on // the floor, which is what kept the Linux native plane at 8 bits. + // + // Aim the wlr injector's absolute mapping (abs-mouse, and `park_pointer`'s opening warp) at + // THIS head — the Linux counterpart of the `set_stream_target` call in the Windows arm below. + // The wlroots virtual pointer maps `motion_absolute` onto the `wl_output` it was created with, + // and on the EXTEND backends (Hyprland, sway) the streamed head sits BESIDE the operator's, so + // without this every absolute sample landed on their screen and the cursor never entered the + // stream at all. `None` (KWin/Mutter/gamescope, none of which inject through that backend) + // CLEARS the slot rather than leaving a stale name: one compositor serves the whole host, so a + // `None` here means no session on this host wants a named binding — e.g. a Game-Mode switch + // from a Hyprland desktop to gamescope, after which the old `PF-…` name means nothing. + crate::inject::set_stream_output(vout.output_name.clone()); pf_capture::open_virtual_output( vout.remote_fd, vout.node_id, diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 0ff3df79..aa95b462 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -1347,8 +1347,19 @@ pub(super) struct SessionContext { /// its embedded mode paints nothing either) the stream has NO cursor at all, in both the /// embedded and the cursor-channel composite models. Parking once per (re)built display — and /// again on the mid-stream flip to the capture model, which heals a pointer that drifted off the -/// output's edge — pins the pointer to the surface the client actually sees. A desktop-model -/// client overrides it with its first absolute move, so the jump is invisible in practice. +/// output's edge — pins the pointer to the surface the client actually sees. +/// +/// **Retried only for a relative-only client.** The schedule used to repeat this for every session, +/// on the theory that "a desktop-model client overrides it with its first absolute move, so the +/// jump is invisible in practice". One park at bring-up is; a *repeat* is not. A desktop-model +/// client sends absolute positions — the very same [`MouseMoveAbs`] this synthesizes, only aimed +/// where the user is actually pointing — so once those are flowing the park has nothing left to add +/// and every later attempt is a visible yank to centre that fights them (field report 2026-08-14). +/// The schedule below therefore keeps the single bring-up park for such a client (so its first +/// click cannot land on the monitor the seat pointer was left on) and drops the retry; the +/// capture-model flip re-arms the full schedule. +/// +/// [`MouseMoveAbs`]: punktfunk_core::input::InputKind::MouseMoveAbs #[cfg(target_os = "linux")] fn park_pointer(input_tx: &std::sync::mpsc::SyncSender, w: u32, h: u32) { let ev = punktfunk_core::input::InputEvent { @@ -1375,6 +1386,65 @@ fn park_pointer(input_tx: &std::sync::mpsc::SyncSender bool { + let Some(negotiated) = vd.last_portal_cursor_mode() else { + return true; + }; + if negotiated.delivers_metadata() { + return true; + } + if *metadata_composite { + *metadata_composite = false; + tracing::info!( + negotiated = negotiated.name(), + "the portal negotiated a cursor mode that carries no cursor metadata — dropping the \ + host composite; the pointer in this stream is the compositor's own, burnt into the \ + frames" + ); + } + false +} + pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option) -> Result<()> { // This thread runs the capture+encode loop (single-process — the only topology: Linux portal / // synthetic, Windows in-process IDD-push). Elevate it so a CPU-heavy game can't deschedule our GPU @@ -1705,6 +1775,11 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option { if !composite_saw_none { composite_saw_none = true; + // NOT necessarily "cursorless", which this line used to assert: + // where the portal negotiated Embedded (the whole wlr family) no + // `SPA_META_Cursor` is ever sent and the compositor's own pointer + // is already in the pixels — the host blend has nothing to add. + // `settle_portal_cursor` logs which of the two this session is. tracing::info!( "host-composite active but the capture has no live cursor \ - overlay yet (no SPA_META_Cursor bitmap) — the stream is \ - cursorless until one arrives" + overlay (no SPA_META_Cursor bitmap) — nothing for the encoder \ + blend to draw; the pointer, if any, is the compositor's own" ); } } @@ -3189,14 +3283,32 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option= next_park_at { - let composite_starved = ((cursor_fwd.is_some() - && !cursor_client_draws.load(Ordering::Relaxed)) + let client_steers = cursor_fwd.is_some() && cursor_client_draws.load(Ordering::Relaxed); + let unconditional = if client_steers { 1 } else { 2 }; + // Never true while `client_steers`: the channel term excludes it outright, and + // `metadata_composite` implies no channel at all. + let composite_starved = ((cursor_fwd.is_some() && !client_steers) || metadata_composite) - && capturer.cursor().is_none(); - if park_attempts < 2 || composite_starved { + && capturer.cursor().is_none() + && no_overlay_means_off_output; + if park_attempts < unconditional || composite_starved { park_pointer(&input_tx, frame.width, frame.height); park_attempts += 1; next_park_at = std::time::Instant::now() + std::time::Duration::from_secs(1); } else { - // Settled (overlay flowing, or the client draws): stop scheduling until a - // rebuild or a capture-model flip re-arms it. + // Settled (the client steers, the overlay is flowing, or its absence carries no + // information here): stop scheduling until a rebuild or a capture-model flip + // re-arms it. park_attempts = PARK_ATTEMPTS_MAX; } } @@ -5115,4 +5232,65 @@ mod tests { "a +2 ms offset must shift the next target by +2 ms mod P, got {shift}" ); } + + /// The 2026-08-14 Hyprland field report, in one function: xdph advertises `Hidden|Embedded`, + /// so a session that asked for cursor metadata is served **Embedded** — no `SPA_META_Cursor` + /// is ever sent, whatever the pointer does. The host must then (a) stop planning a metadata + /// composite it can never feed, and (b) stop reading "no cursor overlay" as "the seat pointer + /// is not on the streamed output" — the inference that re-centred the user's pointer once a + /// second for the whole park cap. + /// + /// The `None` case is the regression guard for GNOME: Mutter is served the cursor mode it + /// asks for through its own protocol and DOES suppress metadata while the pointer is off the + /// recorded view, which is the signal `park_pointer` exists for. Nothing here may touch it. + #[cfg(target_os = "linux")] + #[test] + fn an_embedded_portal_voids_both_the_composite_and_the_starvation_signal() { + struct Fake(Option); + impl crate::vdisplay::VirtualDisplay for Fake { + fn name(&self) -> &'static str { + "fake" + } + fn create( + &mut self, + _mode: pf_vdisplay::Mode, + ) -> anyhow::Result { + anyhow::bail!("this test never creates a display") + } + fn last_portal_cursor_mode(&self) -> Option { + self.0 + } + } + + // The whole wlr family, today: metadata wanted, Embedded served. + let mut composite = true; + assert!(!settle_portal_cursor( + &Fake(Some(pf_vdisplay::PortalCursorMode::Embedded)), + &mut composite + )); + assert!(!composite, "the composite can never be fed — drop it"); + + // Hidden is the same story from the other end: no pointer, so no overlay, so no signal. + let mut composite = true; + assert!(!settle_portal_cursor( + &Fake(Some(pf_vdisplay::PortalCursorMode::Hidden)), + &mut composite + )); + assert!(!composite); + + // A portal that really does serve metadata (xdph ≥ #366, or the portal path on + // KWin/Mutter): everything stays exactly as it was. + let mut composite = true; + assert!(settle_portal_cursor( + &Fake(Some(pf_vdisplay::PortalCursorMode::Metadata)), + &mut composite + )); + assert!(composite); + + // Not portal-negotiated at all — KWin `zkde_screencast`, Mutter `RecordVirtual`, + // gamescope, Windows. THE no-regression case. + let mut composite = true; + assert!(settle_portal_cursor(&Fake(None), &mut composite)); + assert!(composite); + } }