fix(pf-vdisplay): verify the size KWin actually built the virtual output at (≤60 Hz path) #194
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: unom/punktfunk#194
Reference in New Issue
Block a user
Found via a field report: an Xbox Moonlight client negotiated 4K60 against a KWin host and the session captured 1920x1080.
The defect
kwin.rscreate()asked KWin for 3840x2160, KWin built something else, and nothing compared the two. Only the>60 Hzarm ever read anything back — and it gets that incidentally, because it installs a custom mode. The≤60 Hzarm installs nothing, which is exactly why it never noticed.The line that should have caught this is the one that hid it.
spawn_voutreturns a node id, never a size, sowas echoing the request. The field log stated
3840x2160while the output was 1080p, and the first pass at diagnosing this was done against that number. It now logsrequested_w/requested_h, with the readback underneath it.Blast radius was wider than the stream size.
final_dimscarried the unverified request forward, andapply_topology,clear_replication_sourceandresolve_kscreen_addrall resolve by dims — so on that session they were hunting for a 4K output that did not exist, and the stream was left neither primary nor de-mirrored. The encoder then opened at the captured size, handing the client a bitstream that disagreed with the resolution it had configured its decoder from.Suspected trigger is KWin restoring per-output mode/scale from
kwinoutputconfig.json, which is keyed by output name — and ours is deliberately stable across sessions so KDE reapplies that client's scaling (Stage 3). The feature and the failure are the same mechanism.The fix
kwin_output_mgmt::actual_dims()reads the output's real mode + scale. Resolution is by name alone, so it declines unless exactly one output carries our prefix: two means a supersede is in flight, and the dims filter is the only thing that can tell the replacement from the predecessor whose name it reuses. Failing closed keeps this a pure addition — the honoured-request path builds no configuration and applies nothing, byte-for-byte the behaviour this arm always had.set_custom_modeinstall+select the sacrificial birth already uses (an output at a size we don't want, moved to one we do), and armexpect_exact_dimsso the capturer holds frames until the screencast renegotiates. Requests 60 Hz, notmode.refresh_hz— only the size is wrong here, and asking for the client's rate would install a 30 Hz mode for a 30 fps client and throttle the compositor to it.mode_satisfies()extracts the acceptance predicate both arms now share, so they cannot drift on what "we got what we asked for" means.Deliberately NOT done
The stream-side warning is reworded but stays non-fatal. Mirroring a pinned monitor streams a size the client never negotiated by design — §7.3, a panel runs at the mode its owner set and the client scales, and
open_gs_mirror_sourcepasses the client's mode purely "so the argument stays honest". Hard-failing on the mismatch would have broken every mirror session. The message now names both causes and states what the client actually does with the stream.Scope note
This does not claim to close the Xbox disconnect it was found through. That client's IDR storm begins ~4.6 s after the first frame, and all ~120 requests were
0x0302(Request-IDR) — never once0x0301(reference invalidation); a decoder simply unable to handle the size would fail at frame 1, not 4.6 s in. The 1080p-instead-of-4K is a real defect on its own terms and is what this fixes. Discriminator for the remaining question: have the reporter stream at 1920x1080 — if it still storms, it is not the size.Verification
scripts/xcheck.sh linux check+clippy— cleanpunktfunk-host --all-targetsinpf-lxcheck2— clean, and verified non-vacuous (Compiling punktfunk-hostpresent; an earlier run had silently treated the crate as fresh and was discarded)cargo fmt --checkclean on both cratesA 4K60 GameStream session captured 1920x1080. `create()` asked KWin for 3840x2160, KWin built something else, and nothing compared the two: only the >60 Hz arm read anything back, and it gets that for free because it installs a custom mode. The ≤60 Hz arm installs nothing, which is exactly why it never noticed. The line that should have caught it was the one that hid it. `spawn_vout` returns a node id, never a size, so tracing::info!(node_id, width, height, "KWin virtual output ready") was echoing the REQUEST — the field log stated 3840x2160 while the output was 1080p, and the first pass at diagnosing this was done against that number. It now logs `requested_w`/`requested_h`, and the readback sits under it. Unverified, the mismatch was silent and total. `final_dims` carried the request forward, so `apply_topology`, `clear_replication_source` and `resolve_kscreen_addr` — all of which resolve by dims — quietly missed their own output, leaving the stream neither primary nor de-mirrored; and the encoder opened at the captured size, handing the client a bitstream that disagreed with the resolution it had configured its decoder from. Suspected trigger is KWin restoring per-output mode/scale from kwinoutputconfig.json, which is keyed by output NAME — and ours is deliberately stable across sessions so KDE reapplies that client's scaling (Stage 3). The feature and the failure are the same mechanism. - `kwin_output_mgmt::actual_dims()` reads the output's real mode + scale. Resolution is by name alone, so it declines unless EXACTLY one output carries our prefix: two means a supersede is in flight, and the dims filter is the only thing that can tell the replacement from the predecessor whose name it reuses. Failing closed keeps this a pure addition. - On a mismatch, re-assert the requested mode through the same `set_custom_mode` install+select the sacrificial birth already uses (an output at a size we don't want, moved to one we do) and arm `expect_exact_dims` so the capturer holds frames until the screencast renegotiates. 60 Hz is requested, not `mode.refresh_hz`: only the size is wrong here, and asking for the client's rate would install a 30 Hz mode for a 30 fps client. - If KWin refuses the correction, report the size that is REALLY there rather than the request, so the dims-keyed resolves and the encoder key on reality, and say in the log how to clear the stored entry. - Scale is logged, never corrected — a non-unity scale here is the Stage 3 feature working, not a fault. - `mode_satisfies()` extracts the acceptance predicate both arms now share, so they cannot drift into disagreeing about what "we got what we asked for" means. Tested: a restored 1080p does not pass for a 4K request, a CVT-aligned width does, and the slack is bounded, one-sided and width-only. The stream-side warning is reworded but deliberately still NOT fatal: mirroring a pinned monitor streams a size the client never negotiated BY DESIGN (§7.3 — a panel runs at the mode its owner set and the client scales), so refusing the mismatch would break every mirror session. It now names both causes and states what the client actually does with the stream. Does not claim to close the Xbox Moonlight disconnect it was found through: that client's IDR storm begins ~4.6 s after the first frame, which a decoder simply unable to handle the size would not do. The 1080p-instead-of-4K is a real defect on its own terms and is what this fixes.