refactor(windows-host): delete the SudoVDA backend — pf-vdisplay is the sole vdisplay (Goal 2)
Goal 2 ("drop every trace of SudoVDA") is done. The SudoVDA driver is no longer
shipped (only pf-vdisplay; the old vdisplay-driver tree was deleted in a2bd0cd),
and F1 (d638a93/e60cda3) already moved the display-utility helpers out of the
backend into neutral modules (win_adapter/win_display), breaking the reach-in.
So the backend is now cleanly removable:
- Deleted crates/punktfunk-host/src/vdisplay/windows/sudovda.rs (350 lines: the
SudoVdaDisplay VirtualDisplay impl + its VdisplayDriver/probe).
- vdisplay::open()/probe() are now unconditional pf-vdisplay; deleted the
windows_use_pf_vdisplay() backend selector. open() now ensure!s
pf_vdisplay::is_available() with a clear "driver not installed" error instead
of the old silent SudoVDA fallback (no fallback driver exists anymore).
- Scrubbed the dangling references to the deleted symbols (manager/sendinput/dxgi
comments, the config + host.env PUNKTFUNK_VDISPLAY docs); the var stays as an
informational forward-seam. Updated the F1 module docs (Goal 2 now done).
All changes are #[cfg(windows)] except the config doc; Linux clippy
-p punktfunk-host -D warnings clean; zero `sudovda::`/`SudoVdaDisplay` code refs
remain (comments only). Windows build is CI-gated.
Scorecard Goal 2 -> DONE; recorded the E1 "do NOT do it" stability decision in
windows-host-rewrite.md §4 (the process-global driver design is sound given
ProcessSharingDisabled; a device-owned variant adds a use-after-free window for
no gain).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -33,7 +33,7 @@ which kept the live-validated host working at every step. The driver, by contras
|
||||
| Item | Status | Evidence |
|
||||
|---|---|---|
|
||||
| **Goal 1** — clean, layered host architecture | ✅ **DONE** | `config.rs` (`HostConfig`), `session_plan.rs` (`SessionPlan`), `SessionContext`, `windows/`+`linux/` confinement (`38c68c3`), `VirtualDisplayManager` (§2.5), `EncoderCaps` (`0ccd0fe`) |
|
||||
| **Goal 2** — drop every trace of SudoVDA | 🟡 **PARTIAL** | reach-in **decoupled** (F1: `d638a93`/`e60cda3` → `win_adapter`/`win_display`); `sudovda.rs` still present as a fallback backend — deletable now, not yet deleted |
|
||||
| **Goal 2** — drop every trace of SudoVDA | ✅ **DONE** | reach-in decoupled (F1: `d638a93`/`e60cda3` → `win_adapter`/`win_display`), then the `sudovda.rs` backend + the dual-backend select **deleted** (this branch) — pf-vdisplay is the sole Windows virtual-display backend |
|
||||
| **Goal 3** — minimize `unsafe` + P0 lints | 🟡 **PARTIAL** | driver `deny(unsafe_op_in_unsafe_fn)` (`a755d6e`); host crate has **no** P0 lints yet; `OwnedHandle` adopted in `manager.rs`/`pf_vdisplay.rs`/`sudovda.rs`, **not** `idd_push.rs` |
|
||||
| **M0** — proto ABI + driver toolchain + `/INTEGRITYCHECK` + `iddcx` | ✅ **DONE** | `pf-vdisplay-proto`; vendored `windows-drivers-rs` 0.5.1; `clear-force-integrity.ps1`; CI-green |
|
||||
| **M1** — new IddCx driver, first light + HDR | ✅ **DONE (on-glass)** | STEP 0–8 (`d7a9fbf`…`cd59151`); HDR live ("Mac connects WITH HDR", `6399d28`) |
|
||||
@@ -220,21 +220,33 @@ These are expensive empirical wins; keep them intact when touching the code:
|
||||
sustained churn on the RTX box, so this needs an **on-glass reconnect-storm A/B** to confirm (the box is
|
||||
ephemeral). Keep `packaging/windows/reset-pf-vdisplay.ps1` as the recovery until validated.
|
||||
|
||||
**P2 — hygiene / architecture completion**
|
||||
**P2 — hygiene / architecture completion** (the unsafe-reduction + stability priority)
|
||||
4. **D1-host — host-crate P0 lints.** Add `#![deny(unsafe_op_in_unsafe_fn)]` +
|
||||
`#![warn(clippy::undocumented_unsafe_blocks)]` to the host crate and fix the fallout (large, mechanical,
|
||||
touches Linux + Windows `unsafe`). Do it incrementally per-subsystem. The driver already has the deny.
|
||||
5. **D2 — `OwnedHandle` in `idd_push.rs`.** `map`/`event`/`dbg_map` are still raw `HANDLE` closed in `Drop`;
|
||||
wrap in `std::os::windows::io::OwnedHandle` (RAII close, fixes leak-on-error). `manager.rs` already shows
|
||||
the pattern.
|
||||
6. **Goal 2 — delete `sudovda.rs`.** The reach-in is fully decoupled (F1); the backend is now a thin,
|
||||
deletable fallback. Retire it (and the `PUNKTFUNK_VDISPLAY=sudovda` path) to finish "drop SudoVDA."
|
||||
7. **E1 — finish the driver ownership refactor.** Move the process-globals
|
||||
(`MONITOR_MODES`/`NEXT_ID`/`ADAPTER`/`DEVICE_POOL`) into a `DeviceContext`; wire `EvtCleanupCallback` on
|
||||
the `IDDCX_MONITOR` object (today only the WDFDEVICE has it); collapse the 3-key monitor identity. This
|
||||
is the prerequisite to `max_concurrent>1` on Windows + removes the host-side preempt dance. **On-glass
|
||||
gated** (must instrument that `MonitorContext::Drop` actually fires on this UMDF/IddCx stack; keep the
|
||||
explicit REMOVE path as fallback if it doesn't).
|
||||
`#![warn(clippy::undocumented_unsafe_blocks)]` to the host crate and fix the fallout (~30 of the 52
|
||||
`unsafe fn`s need an inner `unsafe {}`). Stage it **per-module, Linux-first** (item-level `#[deny]` on
|
||||
`linux/zerocopy/cuda.rs`/`egl.rs`, `encode/linux/vaapi.rs` — locally verifiable), then the Windows
|
||||
modules (CI-gated), then promote to crate-level. The driver already has the deny.
|
||||
5. **D2 — `OwnedHandle` rollout.** Highest-impact first: `windows/service.rs` (the `AtomicIsize`
|
||||
STOP/SESSION event smuggling + the Job + the `PROCESS_INFORMATION` handles across 5 cleanup arms — deletes
|
||||
~15 manual `CloseHandle`), then `capture/windows/idd_push.rs` (`map`/`event`/`dbg_map`), then the gamepad
|
||||
shm handles. `manager.rs`/`pf_vdisplay.rs` already use `OwnedHandle` (the pattern). RAII close + fixes
|
||||
leak-on-error.
|
||||
6. **Driver unsafe levers** (the driver is already `deny`-clean with per-site SAFETY; these *reduce count*):
|
||||
a `pod_init!` macro for the ~11 `mem::zeroed()` POD inits (Linux-verifiable as a macro), one audited
|
||||
`ThreadBound<T>`/`SendPtr<T>` replacing the 8 scattered `unsafe impl Send`, a generic IOCTL dispatch
|
||||
helper in `control.rs`, and a `KeyedMutexGuard`/`AcquiredSurface` RAII for the frame-transport hot loop
|
||||
(needs an on-glass latency check). ~157 → ~105.
|
||||
7. **M6 scaffolding cleanup** — delete the bring-up diagnostics (`spawn_observer`/`DebugBlock` in
|
||||
`idd_push.rs`) and, once full parity is proven on glass, the host monoliths.
|
||||
|
||||
**Explicitly NOT doing (stability decision): E1 — driver `DeviceContext` ownership + per-`IDDCX_MONITOR`
|
||||
`EvtCleanupCallback`.** The current process-global design is *sound*: IddCx DDIs receive only an
|
||||
`IDDCX_MONITOR` handle (never the WDFDEVICE/context), and `ProcessSharingDisabled` makes one devnode = one
|
||||
host process that dies with the device. A "device-owned" variant would *add* a use-after-free window (the
|
||||
watchdog races device cleanup) for no gain, and the per-monitor cleanup callback isn't reliably reachable
|
||||
on this UMDF/IddCx stack. Cleanup is already deterministic (WDFDEVICE `EvtCleanupCallback` +
|
||||
`cleanup_for_device_removal` + the host-gone watchdog). **Revisit only if `max_concurrent>1` on Windows is
|
||||
actually needed.** (`monitor.rs` documents this rationale at the `MONITOR_MODES` static.)
|
||||
8. **M6 scaffolding cleanup** — delete the bring-up diagnostics (`spawn_observer`/`DebugBlock` in
|
||||
`idd_push.rs`) and, once full parity is proven on glass, the host monoliths.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user