refactor(win-display): the CCD helpers were unsafe fn for serialization, not soundness

`pf-win-display`'s 21 CCD/GDI helpers were `pub unsafe fn` without a memory-safety
obligation between them. Every one takes `Copy` scalars or borrowed Rust data, returns
owned values, and discharges its FFI preconditions internally —
`retry_set_display_config` even binds the input desktop itself, which is the one thing
a caller could plausibly get wrong. Their own `# Safety` sections say as much: one
reads "safe to call from any thread", and the rest say "call under the manager `state`
lock". That is a SERIALIZATION requirement — calling one unlocked races the topology
mutator and returns a stale answer, a correctness bug, not undefined behaviour.

Encoding it with `unsafe fn` cost 55 `unsafe` blocks across THREE crates whose proofs
could only restate "this takes a u32 and returns a String". That is how `unsafe` stops
meaning anything: the blocks that wrap a real FFI obligation read exactly like the ones
that wrap nothing. The requirement is now prose on the functions that have it, and
`unsafe` marks only the FFI calls inside the helpers.

win_display.rs 97 -> 63 `unsafe`, manager.rs 56 -> 30, pf_vdisplay.rs 34 -> 32, plus 10
sites in pf-capture's IDD-push paths. 55 blocks and 55 orphaned `// SAFETY:` proofs
gone; `#![allow(clippy::missing_safety_doc)]` with them, since nothing is `pub unsafe
fn` any more.

Two things the compiler handed back once the blocks went, both of which existed only to
carry a proof: the `let-else` parens in `force_mode_reenumeration` and
`apply_source_positions`, and the nested `if` in `wait_mode_settled` — which is `&&`
again, still short-circuiting, so the second CCD query on a 25 ms poll is unchanged.

Scope note: the sweep and the plan both scoped this as two crates. It is three —
pf-capture calls the same helpers from 10 sites, and `unused_unsafe` under `-D warnings`
makes that non-optional. Found by re-deriving the count, which the plan asked for
because the underlying finding had been REFUTED in verification; the refutation was
wrong about the magnitude (22 of manager.rs's 45 blocks, against its claimed 24).

Verified on the Windows runner: clippy --all-targets clean over pf-frame,
pf-win-display, pf-capture and pf-vdisplay; pf-vdisplay 55 passed, pf-capture 18 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-28 19:46:15 +02:00
co-authored by Claude Opus 5
parent 029979e824
commit 50a153b72a
11 changed files with 101 additions and 214 deletions
+4 -13
View File
@@ -791,13 +791,8 @@ impl IddPushCapturer {
// Reading back immediately can catch a flip that has not settled yet; that costs one // Reading back immediately can catch a flip that has not settled yet; that costs one
// debounce cycle (the poller re-samples in ~250 ms) and never a wrong ring, which is why // debounce cycle (the poller re-samples in ~250 ms) and never a wrong ring, which is why
// this does not block the frame path on a settle poll the way `open_on` does. // this does not block the frame path on a settle poll the way `open_on` does.
// SAFETY: both are `unsafe fn`s over CCD DisplayConfig; each takes a copy of the plain let requested = pf_win_display::win_display::set_advanced_color(self.target_id, want);
// `u32` target id (plus a `bool`), forms no lasting borrow, and returns an owned value. let observed = pf_win_display::win_display::advanced_color_enabled(self.target_id);
let requested =
unsafe { pf_win_display::win_display::set_advanced_color(self.target_id, want) };
// SAFETY: as above — a read-only CCD query over a copy of the plain `u32` target id.
let observed =
unsafe { pf_win_display::win_display::advanced_color_enabled(self.target_id) };
// A failed READ is not evidence of a failed flip — keep the poller's sample then. // A failed READ is not evidence of a failed flip — keep the poller's sample then.
now.hdr = observed.unwrap_or(now.hdr); now.hdr = observed.unwrap_or(now.hdr);
if now.hdr != want && !self.hdr_pin_warned { if now.hdr != want && !self.hdr_pin_warned {
@@ -1129,9 +1124,7 @@ impl IddPushCapturer {
if !self.display_hdr { if !self.display_hdr {
return; return;
} }
// SAFETY: `sdr_white_level_scale` is an `unsafe fn` running a read-only CCD query over owned let queried = pf_win_display::win_display::sdr_white_level_scale(self.target_id);
// local buffers; the `Copy` target id crosses by value and it returns an owned value.
let queried = unsafe { pf_win_display::win_display::sdr_white_level_scale(self.target_id) };
self.sdr_white_scale = queried.unwrap_or(self.sdr_white_scale); self.sdr_white_scale = queried.unwrap_or(self.sdr_white_scale);
tracing::info!( tracing::info!(
target_id = self.target_id, target_id = self.target_id,
@@ -1830,9 +1823,7 @@ impl Capturer for IddPushCapturer {
// driver's stash (measured: new_fps=0 forever after the re-attach). CDS_RESET forces a // driver's stash (measured: new_fps=0 forever after the re-attach). CDS_RESET forces a
// real mode-set at the CURRENT mode — the same lever bring-up's ADD path relies on — // real mode-set at the CURRENT mode — the same lever bring-up's ADD path relies on —
// and the ring recreate below then re-attaches after that churn, not before it. // and the ring recreate below then re-attaches after that churn, not before it.
// SAFETY: `resolve_gdi_name` runs the CCD query FFI over a `Copy` target id (owned match pf_win_display::win_display::resolve_gdi_name(self.target_id) {
// return) — same contract as every sibling call in this file.
match unsafe { pf_win_display::win_display::resolve_gdi_name(self.target_id) } {
Some(gdi) => { Some(gdi) => {
if !pf_win_display::win_display::force_mode_reset(&gdi) { if !pf_win_display::win_display::force_mode_reset(&gdi) {
tracing::warn!( tracing::warn!(
@@ -69,17 +69,13 @@ pub(super) fn kick_dwm_compose(target_id: u32) {
let mut pos = POINT::default(); let mut pos = POINT::default();
// SAFETY: plain FFI; `pos` is a valid out-param for this synchronous call. // SAFETY: plain FFI; `pos` is a valid out-param for this synchronous call.
let have_pos = unsafe { GetCursorPos(&mut pos) }.is_ok(); let have_pos = unsafe { GetCursorPos(&mut pos) }.is_ok();
// SAFETY: `source_desktop_rect` only runs the CCD QueryDisplayConfig FFI over owned local let rect = pf_win_display::win_display::source_desktop_rect(target_id);
// buffers; the `Copy` target id crosses by value.
let rect = unsafe { pf_win_display::win_display::source_desktop_rect(target_id) };
// HID-first (see the doc comment): the registered virtual-mouse kick works from any // HID-first (see the doc comment): the registered virtual-mouse kick works from any
// session/desktop and wakes an off display. Both geometries come from CCD (global database), // session/desktop and wakes an off display. Both geometries come from CCD (global database),
// NOT per-session GDI metrics, so the aim is right even from a non-console session. Fall // NOT per-session GDI metrics, so the aim is right even from a non-console session. Fall
// through to SendInput only when the hook isn't registered / the mouse isn't up. // through to SendInput only when the hook isn't registered / the mouse isn't up.
if let (Some(kick), Some(rect)) = (crate::HID_COMPOSE_KICK.get(), rect) { if let (Some(kick), Some(rect)) = (crate::HID_COMPOSE_KICK.get(), rect) {
// SAFETY: `desktop_bounds` only runs the CCD QueryDisplayConfig FFI over owned local let bounds = pf_win_display::win_display::desktop_bounds();
// buffers.
let bounds = unsafe { pf_win_display::win_display::desktop_bounds() };
if let Some(bounds) = bounds { if let Some(bounds) = bounds {
if kick(rect, bounds) { if kick(rect, bounds) {
return; return;
@@ -72,9 +72,7 @@ impl CursorShared {
MappedSection { handle: map, view } MappedSection { handle: map, view }
}; };
// Desktop origin of this monitor's source — for the desktop→frame coordinate shift. // Desktop origin of this monitor's source — for the desktop→frame coordinate shift.
// SAFETY: `source_desktop_rect` only runs the CCD QueryDisplayConfig FFI over owned let rect = pf_win_display::win_display::source_desktop_rect(target_id);
// locals (same call the compose-kick path makes).
let rect = unsafe { pf_win_display::win_display::source_desktop_rect(target_id) };
let origin = rect.map(|(x, y, _w, _h)| (x, y)).unwrap_or((0, 0)); let origin = rect.map(|(x, y, _w, _h)| (x, y)).unwrap_or((0, 0));
Ok(CursorShared { Ok(CursorShared {
section, section,
@@ -187,10 +187,7 @@ fn run(
// transient CCD failure must not park the pointer at a `(0, 0, 0, 0)` rect, which would // transient CCD failure must not park the pointer at a `(0, 0, 0, 0)` rect, which would
// report every position invisible. // report every position invisible.
// //
// SAFETY: `source_desktop_rect` is an `unsafe fn` running the read-only CCD let fresh = pf_win_display::win_display::source_desktop_rect(target_id);
// `QueryDisplayConfig` over owned local buffers; the `Copy` target id crosses by value
// and it returns owned `(x, y, w, h)` values, borrowing nothing.
let fresh = unsafe { pf_win_display::win_display::source_desktop_rect(target_id) };
if let Some(fresh) = fresh { if let Some(fresh) = fresh {
if fresh != rect { if fresh != rect {
tracing::info!( tracing::info!(
@@ -59,14 +59,10 @@ impl DescriptorPoller {
let mut last_slow_log: Option<Instant> = None; let mut last_slow_log: Option<Instant> = None;
while !stop_t.load(Ordering::Relaxed) { while !stop_t.load(Ordering::Relaxed) {
let t = Instant::now(); let t = Instant::now();
// SAFETY: both are read-only CCD queries taking only a copy of the plain `u32` let (hdr, res) = (
// target id (see their own SAFETY docs); nothing is borrowed across the calls.
let (hdr, res) = unsafe {
(
pf_win_display::win_display::advanced_color_enabled(target_id), pf_win_display::win_display::advanced_color_enabled(target_id),
pf_win_display::win_display::active_resolution(target_id), pf_win_display::win_display::active_resolution(target_id),
) );
};
let took = t.elapsed(); let took = t.elapsed();
if took >= Self::SLOW if took >= Self::SLOW
&& last_slow_log.is_none_or(|t| t.elapsed() >= Duration::from_secs(10)) && last_slow_log.is_none_or(|t| t.elapsed() >= Duration::from_secs(10))
@@ -281,11 +281,8 @@ impl IddPushCapturer {
// a fullscreen game can hold the virtual display at a different mode (esp. across a reconnect), so // a fullscreen game can hold the virtual display at a different mode (esp. across a reconnect), so
// matching the actual mode lets the first frame flow instead of being dropped (game-capture bug // matching the actual mode lets the first frame flow instead of being dropped (game-capture bug
// GB1). Falls back to the negotiated mode when the CCD read is unavailable. // GB1). Falls back to the negotiated mode when the CCD read is unavailable.
// SAFETY: `active_resolution` is an `unsafe fn` (Win32 CCD `QueryDisplayConfig`) that takes only a let (w, h) =
// copy of the plain `u32` CCD target id and returns owned `(w, h)` values; it forms no borrows from pf_win_display::win_display::active_resolution(target.target_id).unwrap_or((pw, ph));
// us and validates the id internally, returning `None` on any failure (handled by `unwrap_or`).
let (w, h) = unsafe { pf_win_display::win_display::active_resolution(target.target_id) }
.unwrap_or((pw, ph));
if (w, h) != (pw, ph) { if (w, h) != (pw, ph) {
tracing::info!( tracing::info!(
target_id = target.target_id, target_id = target.target_id,
@@ -193,9 +193,7 @@ enum ShrinkAction {
fn poll_gdi_name(target_id: u32) -> Option<String> { fn poll_gdi_name(target_id: u32) -> Option<String> {
for _ in 0..60 { for _ in 0..60 {
thread::sleep(Duration::from_millis(50)); thread::sleep(Duration::from_millis(50));
// SAFETY: `resolve_gdi_name` is `unsafe` for its CCD FFI; it takes a plain `Copy` `u32` if let Some(n) = resolve_gdi_name(target_id) {
// target id by value and returns an owned `String`, so no caller memory is borrowed.
if let Some(n) = unsafe { resolve_gdi_name(target_id) } {
return Some(n); return Some(n);
} }
} }
@@ -380,9 +378,7 @@ pub fn force_recommit() -> bool {
return false; return false;
}; };
let _guard = m.state.lock().unwrap(); let _guard = m.state.lock().unwrap();
// SAFETY: `force_mode_reenumeration`'s contract is "call under the manager `state` lock"; pf_win_display::win_display::force_mode_reenumeration()
// held above. The call reads + re-applies the current CCD config over owned locals.
unsafe { pf_win_display::win_display::force_mode_reenumeration() }
} }
/// Best-effort "is this WUDFHost pid still alive?" — the monitor-liveness probe for the JOIN path. /// Best-effort "is this WUDFHost pid still alive?" — the monitor-liveness probe for the JOIN path.
@@ -561,9 +557,7 @@ impl VirtualDisplayManager {
// Let the OS finish the ASYNC monitor departure before the next ADD; a back-to-back // Let the OS finish the ASYNC monitor departure before the next ADD; a back-to-back
// REMOVE→ADD races the teardown and the ADD IOCTL is rejected under reconnect churn. // REMOVE→ADD races the teardown and the ADD IOCTL is rejected under reconnect churn.
// Verified-state wait, ceiling = the old fixed 400 ms settle (latency plan P0.3). // Verified-state wait, ceiling = the old fixed 400 ms settle (latency plan P0.3).
// SAFETY: CCD query FFI over a `Copy` target id, under the held `state` lock. let departed = wait_target_departed(old_target, Duration::from_millis(400));
let departed =
unsafe { wait_target_departed(old_target, Duration::from_millis(400)) };
if !departed { if !departed {
tracing::debug!( tracing::debug!(
old_target, old_target,
@@ -598,8 +592,7 @@ impl VirtualDisplayManager {
// is exclusively owned here — no aliasing. // is exclusively owned here — no aliasing.
unsafe { self.teardown_removed(dev, &mut inner, mon) }; unsafe { self.teardown_removed(dev, &mut inner, mon) };
// Same async-departure settle as the reconnect preempt above (verified wait, P0.3). // Same async-departure settle as the reconnect preempt above (verified wait, P0.3).
// SAFETY: CCD query FFI over a `Copy` target id, under the held `state` lock. let _ = wait_target_departed(old_target, Duration::from_millis(400));
let _ = unsafe { wait_target_departed(old_target, Duration::from_millis(400)) };
} }
} }
@@ -902,10 +895,7 @@ impl VirtualDisplayManager {
if keep.is_empty() { if keep.is_empty() {
continue; continue;
} }
// SAFETY: `count_other_active` runs the CCD QueryDisplayConfig FFI over a let survivors = count_other_active(&keep).unwrap_or(0);
// borrowed slice of `Copy` target ids (owned result), under the `state` lock
// (`inner` guard held to the end of this iteration).
let survivors = unsafe { count_other_active(&keep) }.unwrap_or(0);
if survivors == 0 { if survivors == 0 {
if fighting > 0 { if fighting > 0 {
tracing::info!( tracing::info!(
@@ -947,16 +937,7 @@ impl VirtualDisplayManager {
"re-asserting exclusive topology" "re-asserting exclusive topology"
), ),
} }
// SAFETY: `isolate_displays_ccd` drives the CCD query/apply FFI over a let _ = isolate_displays_ccd(&keep);
// borrowed slice of `Copy` target ids, under the `state` lock — the sole
// topology mutator. The returned snapshot is discarded (the group restores
// the FIRST member's). The FULL isolate on purpose — its forced re-commit
// (SDC_FORCE_MODE_ENUMERATION → COMMIT_MODES → ASSIGN_SWAPCHAIN) is
// load-bearing here exactly as at first isolate: after the eviction's
// topology change the OS stops presenting to the virtual display until a
// forced mode re-commit restarts it (on-glass: a gentle supplied-config
// eviction left capture receiving ONE stashed frame and then nothing).
let _ = unsafe { isolate_displays_ccd(&keep) };
// That same forced re-commit hands the live IDD path a fresh swap-chain, // That same forced re-commit hands the live IDD path a fresh swap-chain,
// orphaning the session's capture ring — announce it so the session rebuilds // orphaning the session's capture ring — announce it so the session rebuilds
// its capture attachment (same-mode ring recreate + driver re-attach + fresh // its capture attachment (same-mode ring recreate + driver re-attach + fresh
@@ -1014,9 +995,7 @@ impl VirtualDisplayManager {
.zip(&placements) .zip(&placements)
.map(|(&(_, _, target, _), p)| (target, p.x, p.y)) .map(|(&(_, _, target, _), p)| (target, p.x, p.y))
.collect(); .collect();
// SAFETY: `apply_source_positions` only drives the CCD query/apply FFI with owned local pf_win_display::win_display::apply_source_positions(&positions);
// buffers, under the `state` lock — the sole topology mutator.
unsafe { pf_win_display::win_display::apply_source_positions(&positions) };
for (&(slot, ..), p) in ordered.iter().zip(&placements) { for (&(slot, ..), p) in ordered.iter().zip(&placements) {
if let Some( if let Some(
SlotState::Active { mon, .. } SlotState::Active { mon, .. }
@@ -1071,14 +1050,11 @@ impl VirtualDisplayManager {
if let Some(n) = poll_gdi_name(target_id) { if let Some(n) = poll_gdi_name(target_id) {
return Some(n); return Some(n);
} }
// SAFETY: `force_extend_topology` only calls `SetDisplayConfig` (CCD) with no borrowed memory. force_extend_topology();
unsafe { force_extend_topology() };
if let Some(n) = poll_gdi_name(target_id) { if let Some(n) = poll_gdi_name(target_id) {
return Some(n); return Some(n);
} }
// SAFETY: `activate_target_path` runs the CCD query/apply FFI with owned local buffers; the if pf_win_display::win_display::activate_target_path(target_id) {
// `Copy` target id is passed by value, under the `state` lock — the sole topology mutator.
if unsafe { pf_win_display::win_display::activate_target_path(target_id) } {
if let Some(n) = poll_gdi_name(target_id) { if let Some(n) = poll_gdi_name(target_id) {
return Some(n); return Some(n);
} }
@@ -1170,11 +1146,7 @@ impl VirtualDisplayManager {
if crate::policy::prefs().ddc_power_off() { if crate::policy::prefs().ddc_power_off() {
inner.group.ddc_panels_off = crate::ddc::panel_off_except(n); inner.group.ddc_panels_off = crate::ddc::panel_off_except(n);
} }
// SAFETY: `isolate_displays_ccd` is `unsafe` for its CCD topology FFI; it inner.group.ccd_saved = isolate_displays_ccd(&keep);
// takes a borrowed slice of `Copy` target ids (alive across the call) and
// returns an owned `SavedConfig`, under the `state` lock — the sole
// topology mutator.
inner.group.ccd_saved = unsafe { isolate_displays_ccd(&keep) };
// EXPERIMENTAL `pnp_disable_monitors` policy axis: AFTER the isolate took, // EXPERIMENTAL `pnp_disable_monitors` policy axis: AFTER the isolate took,
// additionally disable the deactivated monitors' PnP devnodes (persistent // additionally disable the deactivated monitors' PnP devnodes (persistent
// across hot-plug re-arrival) so a standby monitor/TV's periodic wake // across hot-plug re-arrival) so a standby monitor/TV's periodic wake
@@ -1199,9 +1171,7 @@ impl VirtualDisplayManager {
// Grown set: re-isolate so the fresh member joins the composited set // Grown set: re-isolate so the fresh member joins the composited set
// (its auto-activate may have lit nothing extra to deactivate, but the // (its auto-activate may have lit nothing extra to deactivate, but the
// re-commit also drives COMMIT_MODES for the new path). // re-commit also drives COMMIT_MODES for the new path).
// SAFETY: as above — borrowed slice of Copy ids, owned return, under the let snap = isolate_displays_ccd(&keep);
// `state` lock.
let snap = unsafe { isolate_displays_ccd(&keep) };
// Normally DISCARDED — the group restores the FIRST member's snapshot. // Normally DISCARDED — the group restores the FIRST member's snapshot.
// But if the first member's isolate FAILED, there is no first snapshot, // But if the first member's isolate FAILED, there is no first snapshot,
// and this one just deactivated the physicals with nothing able to put // and this one just deactivated the physicals with nothing able to put
@@ -1233,10 +1203,8 @@ impl VirtualDisplayManager {
// persistence DB, RESETTING a 120 Hz panel to 60 Hz. So force-EXTEND only when the // persistence DB, RESETTING a 120 Hz panel to 60 Hz. So force-EXTEND only when the
// virtual is currently sole; otherwise skip straight to the reposition, which // virtual is currently sole; otherwise skip straight to the reposition, which
// re-supplies each physical's QUERIED mode verbatim (preserving its refresh). // re-supplies each physical's QUERIED mode verbatim (preserving its refresh).
// SAFETY: `count_other_active` runs the CCD QueryDisplayConfig FFI (borrowed
// slice of Copy ids, owned result), under the `state` lock.
let already_extended = let already_extended =
unsafe { count_other_active(&[added.target_id]) }.unwrap_or(0) > 0; count_other_active(&[added.target_id]).unwrap_or(0) > 0;
if already_extended { if already_extended {
tracing::info!( tracing::info!(
"display topology=primary — a physical display is already active; \ "display topology=primary — a physical display is already active; \
@@ -1244,14 +1212,10 @@ impl VirtualDisplayManager {
virtual primary" virtual primary"
); );
} else { } else {
// SAFETY: `force_extend_topology` drives the CCD topology FFI (no args, no force_extend_topology();
// borrowed memory), under the `state` lock — the sole topology mutator.
unsafe { force_extend_topology() };
thread::sleep(Duration::from_millis(300)); thread::sleep(Duration::from_millis(300));
} }
// SAFETY: `set_virtual_primary_ccd` takes the `Copy` target id by value and returns inner.group.ccd_saved = set_virtual_primary_ccd(added.target_id);
// an owned `SavedConfig` (no borrowed memory crosses), under the `state` lock.
inner.group.ccd_saved = unsafe { set_virtual_primary_ccd(added.target_id) };
} }
Topology::Primary => { Topology::Primary => {
// A sibling already holds primary (the group's designated member) — the new // A sibling already holds primary (the group's designated member) — the new
@@ -1271,10 +1235,7 @@ impl VirtualDisplayManager {
// 1500 ms sleep (a rejected mode / slow third-party CCD-lock holder burns the // 1500 ms sleep (a rejected mode / slow third-party CCD-lock holder burns the
// ceiling and proceeds, exactly like the sleep it replaces). // ceiling and proceeds, exactly like the sleep it replaces).
let settle_start = std::time::Instant::now(); let settle_start = std::time::Instant::now();
// SAFETY: CCD/GDI query FFI over a `Copy` target id, under the held `state` lock. let settled = wait_mode_settled(added.target_id, mode, Duration::from_millis(1500));
let settled = unsafe {
wait_mode_settled(added.target_id, mode, Duration::from_millis(1500))
};
tracing::info!( tracing::info!(
settle_ms = settle_start.elapsed().as_millis() as u64, settle_ms = settle_start.elapsed().as_millis() as u64,
verified = settled, verified = settled,
@@ -1391,8 +1352,7 @@ impl VirtualDisplayManager {
// SAFETY: `dev` is the live control handle (this fn's contract); `update_modes` // SAFETY: `dev` is the live control handle (this fn's contract); `update_modes`
// forwards it to a synchronous IOCTL with owned/borrowed locals only. // forwards it to a synchronous IOCTL with owned/borrowed locals only.
unsafe { self.driver.update_modes(dev, &mon.key, mode) }?; unsafe { self.driver.update_modes(dev, &mon.key, mode) }?;
// SAFETY: CCD query/apply FFI under the held `state` lock (this fn's contract). pf_win_display::win_display::force_mode_reenumeration();
unsafe { pf_win_display::win_display::force_mode_reenumeration() };
if !pf_win_display::win_display::wait_mode_advertised( if !pf_win_display::win_display::wait_mode_advertised(
&gdi, &gdi,
mode, mode,
@@ -1414,9 +1374,7 @@ impl VirtualDisplayManager {
// Verified-state settle (P0.2): the same committed-state predicate as the create paths. A // Verified-state settle (P0.2): the same committed-state predicate as the create paths. A
// mode set that did not commit within the ceiling routes to the re-arrival fallback. // mode set that did not commit within the ceiling routes to the re-arrival fallback.
let settle_start = Instant::now(); let settle_start = Instant::now();
// SAFETY: CCD/GDI query FFI over a `Copy` target id, under the held `state` lock. let settled = wait_mode_settled(mon.target_id, mode, Duration::from_millis(1500));
let settled =
unsafe { wait_mode_settled(mon.target_id, mode, Duration::from_millis(1500)) };
if !settled { if !settled {
anyhow::bail!( anyhow::bail!(
"in-place mode set did not commit within 1.5s (advertised after {advertised_ms} ms)" "in-place mode set did not commit within 1.5s (advertised after {advertised_ms} ms)"
@@ -1486,8 +1444,7 @@ impl VirtualDisplayManager {
// the old fixed 400 ms settle (latency plan P0.3); the driver's ghost-reap ADD retry remains // the old fixed 400 ms settle (latency plan P0.3); the driver's ghost-reap ADD retry remains
// the backstop for a departure the CCD reports early. // the backstop for a departure the CCD reports early.
let depart_start = std::time::Instant::now(); let depart_start = std::time::Instant::now();
// SAFETY: CCD query FFI over a `Copy` target id, under the held `state` lock. let departed = wait_target_departed(old.target_id, Duration::from_millis(400));
let departed = unsafe { wait_target_departed(old.target_id, Duration::from_millis(400)) };
tracing::info!( tracing::info!(
depart_ms = depart_start.elapsed().as_millis() as u64, depart_ms = depart_start.elapsed().as_millis() as u64,
verified = departed, verified = departed,
@@ -1522,10 +1479,7 @@ impl VirtualDisplayManager {
// Topology settle before capture reopens: verified-state wait, ceiling = the old // Topology settle before capture reopens: verified-state wait, ceiling = the old
// fixed 1500 ms sleep (latency plan P0.2 — the re-arrival twin). // fixed 1500 ms sleep (latency plan P0.2 — the re-arrival twin).
let settle_start = std::time::Instant::now(); let settle_start = std::time::Instant::now();
// SAFETY: CCD/GDI query FFI over a `Copy` target id, under the held `state` lock. let settled = wait_mode_settled(added.target_id, mode, Duration::from_millis(1500));
let settled = unsafe {
wait_mode_settled(added.target_id, mode, Duration::from_millis(1500))
};
tracing::info!( tracing::info!(
settle_ms = settle_start.elapsed().as_millis() as u64, settle_ms = settle_start.elapsed().as_millis() as u64,
verified = settled, verified = settled,
@@ -1574,16 +1528,14 @@ impl VirtualDisplayManager {
// snapshot is DISCARDED — the group keeps the first member's (design §6.1). // snapshot is DISCARDED — the group keeps the first member's (design §6.1).
let mut keep = inner.target_ids(); let mut keep = inner.target_ids();
keep.push(new_target); keep.push(new_target);
// SAFETY: borrowed slice of Copy target ids, owned return, under the `state` lock. let _ = isolate_displays_ccd(&keep);
let _ = unsafe { isolate_displays_ccd(&keep) };
} }
Topology::Primary => { Topology::Primary => {
// Make the new target primary again (its predecessor held primary), preserving the // Make the new target primary again (its predecessor held primary), preserving the
// original restore snapshot: `set_virtual_primary_ccd` recaptures one, so save + restore // original restore snapshot: `set_virtual_primary_ccd` recaptures one, so save + restore
// the group's around the call. // the group's around the call.
let keep_saved = inner.group.ccd_saved.take(); let keep_saved = inner.group.ccd_saved.take();
// SAFETY: `Copy` target id by value, owned return, under the `state` lock. let _ = set_virtual_primary_ccd(new_target);
let _ = unsafe { set_virtual_primary_ccd(new_target) };
inner.group.ccd_saved = keep_saved; inner.group.ccd_saved = keep_saved;
} }
Topology::Extend | Topology::Auto => { Topology::Extend | Topology::Auto => {
@@ -1645,14 +1597,7 @@ impl VirtualDisplayManager {
// displays. // displays.
inner.group.ccd_exclusive = false; inner.group.ccd_exclusive = false;
if let Some(saved) = inner.group.ccd_saved.take() { if let Some(saved) = inner.group.ccd_saved.take() {
// SAFETY: `saved` is a `SavedConfig` this manager captured from restore_displays_ccd(&saved);
// `isolate_displays_ccd`/`set_virtual_primary_ccd` on this same box (it can be
// produced no other way), so its path+mode arrays are a self-consistent CCD
// topology and are passed with their own lengths. `restore_displays_ccd` binds
// itself to the input desktop internally (`retry_set_display_config`), which is
// the one thing its callers could otherwise get wrong. The `take()` above makes
// this the only consumer of that snapshot, so no second restore can race it.
unsafe { restore_displays_ccd(&saved) };
} }
// EXPERIMENTAL `ddc_power_off` wake. OUTSIDE the `ccd_saved` gate, for the same reason // EXPERIMENTAL `ddc_power_off` wake. OUTSIDE the `ccd_saved` gate, for the same reason
// `pnp_disabled` is above it: the panels were commanded dark BEFORE the isolate, and // `pnp_disabled` is above it: the panels were commanded dark BEFORE the isolate, and
@@ -1680,10 +1625,7 @@ impl VirtualDisplayManager {
// the group keeps the first member's. // the group keeps the first member's.
ShrinkAction::Reisolate => { ShrinkAction::Reisolate => {
let keep = inner.target_ids(); let keep = inner.target_ids();
// SAFETY: `isolate_displays_ccd` only drives the CCD query/apply FFI over a let _ = isolate_displays_ccd(&keep);
// borrowed slice of Copy target ids, under the `state` lock — the sole
// topology mutator.
let _ = unsafe { isolate_displays_ccd(&keep) };
} }
// Re-promote a survivor rather than leaving the desktop's primary on a target that // Re-promote a survivor rather than leaving the desktop's primary on a target that
// is about to be REMOVEd. Same save/restore-the-snapshot dance as // is about to be REMOVEd. Same save/restore-the-snapshot dance as
@@ -1692,8 +1634,7 @@ impl VirtualDisplayManager {
ShrinkAction::RepromotePrimary => { ShrinkAction::RepromotePrimary => {
if let Some(&survivor) = inner.target_ids().first() { if let Some(&survivor) = inner.target_ids().first() {
let keep_saved = inner.group.ccd_saved.take(); let keep_saved = inner.group.ccd_saved.take();
// SAFETY: `Copy` target id by value, owned return, under the `state` lock. let _ = set_virtual_primary_ccd(survivor);
let _ = unsafe { set_virtual_primary_ccd(survivor) };
inner.group.ccd_saved = keep_saved; inner.group.ccd_saved = keep_saved;
} }
} }
@@ -910,8 +910,7 @@ mod tests {
// Context probe: can this process see the CCD active-path set at all? (`None` = the query // Context probe: can this process see the CCD active-path set at all? (`None` = the query
// itself fails in this session/window-station — the whole ladder would be blind, and a // itself fails in this session/window-station — the whole ladder would be blind, and a
// "monitor never activated" verdict would be an artifact of the test context.) // "monitor never activated" verdict would be an artifact of the test context.)
// SAFETY: CCD query over an owned empty slice (test-only diagnostics). let active0 = pf_win_display::win_display::count_other_active(&[]);
let active0 = unsafe { pf_win_display::win_display::count_other_active(&[]) };
println!("spike: CCD active paths visible before create: {active0:?}"); println!("spike: CCD active paths visible before create: {active0:?}");
let mut vd = PfVdisplayDisplay::new().expect("open pf-vdisplay"); let mut vd = PfVdisplayDisplay::new().expect("open pf-vdisplay");
let first = vd let first = vd
@@ -943,8 +942,7 @@ mod tests {
.expect("no capture target") .expect("no capture target")
.target_id; .target_id;
let in_place = t1 == t2; let in_place = t1 == t2;
// SAFETY: CCD query over a Copy target id (test-only diagnostics). let active = pf_win_display::win_display::active_resolution(t2);
let active = unsafe { pf_win_display::win_display::active_resolution(t2) };
println!( println!(
"in-place resize spike: in_place={in_place} (target {t1} -> {t2}) took {resize_ms} ms, \ "in-place resize spike: in_place={in_place} (target {t1} -> {t2}) took {resize_ms} ms, \
active resolution now {active:?}" active resolution now {active:?}"
+1 -3
View File
@@ -193,9 +193,7 @@ fn push_event(kind: DisplayEventKind, detail: Option<String>) {
} }
fn refresh_inventory() { fn refresh_inventory() {
// SAFETY: `target_inventory` only runs read-only CCD queries over locals (see its SAFETY doc); let inv = crate::win_display::target_inventory();
// called from the listener thread, never the capture thread.
let inv = unsafe { crate::win_display::target_inventory() };
if !inv.is_empty() { if !inv.is_empty() {
state().lock().unwrap().inventory = inv; state().lock().unwrap().inventory = inv;
} }
+1 -3
View File
@@ -174,9 +174,7 @@ pub fn disable_for_deactivated(
/// active flags it reads are the settled ones. Journals like [`disable_for_deactivated`]; the /// active flags it reads are the settled ones. Journals like [`disable_for_deactivated`]; the
/// caller merges the returned ids into the same teardown list. /// caller merges the returned ids into the same teardown list.
pub fn disable_connected_inactive(keep_target_ids: &[u32]) -> Vec<String> { pub fn disable_connected_inactive(keep_target_ids: &[u32]) -> Vec<String> {
// SAFETY: `target_inventory` only runs read-only CCD queries over local buffers (see its let inventory = crate::win_display::target_inventory();
// docs); no borrowed memory crosses the call.
let inventory = unsafe { crate::win_display::target_inventory() };
let mut targets: Vec<(String, String)> = Vec::new(); let mut targets: Vec<(String, String)> = Vec::new();
for t in &inventory { for t in &inventory {
if t.active || !t.external_physical || keep_target_ids.contains(&t.target_id) { if t.active || !t.external_physical || keep_target_ids.contains(&t.target_id) {
+61 -84
View File
@@ -15,12 +15,17 @@
// below — including `restore_displays_ccd`, the call pf-vdisplay's teardown path depends on to give // below — including `restore_displays_ccd`, the call pf-vdisplay's teardown path depends on to give
// the operator their physical panels back. // the operator their physical panels back.
#![deny(unsafe_op_in_unsafe_fn)] #![deny(unsafe_op_in_unsafe_fn)]
// These CCD/GDI FFI helpers were `pub(crate) unsafe fn` before the pf-win-display carve (plan §W6), // The CCD/GDI helpers below are SAFE fns. They were `unsafe fn` for a decade of habit rather than a
// where `missing_safety_doc` stays silent; crossing the crate boundary makes them `pub` and would // memory-safety obligation: every one takes `Copy` scalars or borrowed Rust data, returns owned
// demand a `# Safety` heading on each. Their callers' obligations (call on the right desktop thread // values, and discharges its own FFI preconditions internally (`retry_set_display_config` even binds
// with a live OS target id) are stated in each fn's prose doc, and this is an internal // the input desktop itself). What their `# Safety` sections actually described — "call under the
// (publish=false) leaf — keep the pre-carve behavior rather than adding 12 formal headings. // manager `state` lock" — is a SERIALIZATION requirement: calling one unlocked races the topology
#![allow(clippy::missing_safety_doc)] // mutator and yields a stale answer, which is a correctness bug, not undefined behaviour.
//
// Encoding that with `unsafe fn` cost ~55 `unsafe` blocks across three crates whose proofs could
// only restate "this takes a u32 and returns a String", which is how `unsafe` stopped meaning
// anything here. The requirement is now prose on the functions that have it, and `unsafe` marks
// only the FFI calls inside.
// THE CCD CONTRACT, stated once — most `// SAFETY:` proofs below are an instance of it. // THE CCD CONTRACT, stated once — most `// SAFETY:` proofs below are an instance of it.
// //
@@ -94,7 +99,7 @@ use punktfunk_core::Mode;
/// OWN active path, so the rest of bring-up (`resolve_gdi_name` -> `set_active_mode` -> /// OWN active path, so the rest of bring-up (`resolve_gdi_name` -> `set_active_mode` ->
/// `isolate_displays_ccd`) proceeds. Best-effort + idempotent: a no-op on a single-display (already /// `isolate_displays_ccd`) proceeds. Best-effort + idempotent: a no-op on a single-display (already
/// sole/extended) box, so it is safe to call unconditionally. `rc == 0` is success. /// sole/extended) box, so it is safe to call unconditionally. `rc == 0` is success.
pub unsafe fn force_extend_topology() { pub fn force_extend_topology() {
// A topology flag with no supplied path/mode arrays tells the OS to recompute + apply that preset // A topology flag with no supplied path/mode arrays tells the OS to recompute + apply that preset
// for the currently-connected displays (the same code path DisplaySwitch.exe drives). // for the currently-connected displays (the same code path DisplaySwitch.exe drives).
let rc = crate::input_desktop::retry_set_display_config(|| { let rc = crate::input_desktop::retry_set_display_config(|| {
@@ -125,7 +130,7 @@ pub unsafe fn force_extend_topology() {
/// source not already driving another display — mode indices invalidated so `SDC_ALLOW_CHANGES` lets /// source not already driving another display — mode indices invalidated so `SDC_ALLOW_CHANGES` lets
/// the OS pick modes for the new path. Returns `true` when the apply reports success; the caller /// the OS pick modes for the new path. Returns `true` when the apply reports success; the caller
/// still re-polls [`resolve_gdi_name`] to confirm the path actually committed. /// still re-polls [`resolve_gdi_name`] to confirm the path actually committed.
pub unsafe fn activate_target_path(target_id: u32) -> bool { pub fn activate_target_path(target_id: u32) -> bool {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -228,7 +233,7 @@ pub unsafe fn activate_target_path(target_id: u32) -> bool {
/// Resolve the `\\.\DisplayN` GDI name for a virtual-display target id via the CCD API. Returns `None` /// Resolve the `\\.\DisplayN` GDI name for a virtual-display target id via the CCD API. Returns `None`
/// until the OS activates the target into the desktop topology (needs a real WDDM GPU; on a /// until the OS activates the target into the desktop topology (needs a real WDDM GPU; on a
/// GPU-less box this stays `None` even though ADD succeeded). /// GPU-less box this stays `None` even though ADD succeeded).
pub unsafe fn resolve_gdi_name(target_id: u32) -> Option<String> { pub fn resolve_gdi_name(target_id: u32) -> Option<String> {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -278,13 +283,9 @@ pub unsafe fn resolve_gdi_name(target_id: u32) -> Option<String> {
/// ACTUAL mode and polls it to recreate the ring when it changes — a fullscreen game can change the /// ACTUAL mode and polls it to recreate the ring when it changes — a fullscreen game can change the
/// virtual display's mode out from under the session-negotiated one (game-capture bug GB1). /// virtual display's mode out from under the session-negotiated one (game-capture bug GB1).
/// ///
/// # Safety /// Safe to call from any thread.
/// Calls the GDI/CCD APIs; safe to call from any thread. pub fn active_resolution(target_id: u32) -> Option<(u32, u32)> {
pub unsafe fn active_resolution(target_id: u32) -> Option<(u32, u32)> { let gdi = resolve_gdi_name(target_id)?;
// SAFETY: `resolve_gdi_name` is this module's own CCD helper: it takes `Copy` scalars / borrowed Rust
// data and returns an owned `String`, so every precondition is internal to it. This fn's own
// contract (call under the manager `state` lock) is what it needs.
let gdi = unsafe { resolve_gdi_name(target_id) }?;
let wname: Vec<u16> = gdi.encode_utf16().chain(std::iter::once(0)).collect(); let wname: Vec<u16> = gdi.encode_utf16().chain(std::iter::once(0)).collect();
let mut dm = DEVMODEW { let mut dm = DEVMODEW {
dmSize: size_of::<DEVMODEW>() as u16, dmSize: size_of::<DEVMODEW>() as u16,
@@ -310,20 +311,18 @@ pub unsafe fn active_resolution(target_id: u32) -> Option<(u32, u32)> {
/// class) burns the ceiling and proceeds — behavior identical to the fixed sleep it replaces. /// class) burns the ceiling and proceeds — behavior identical to the fixed sleep it replaces.
/// Returns `true` when the state verified (typical: one or two 25 ms polls), `false` on ceiling. /// Returns `true` when the state verified (typical: one or two 25 ms polls), `false` on ceiling.
/// ///
/// # Safety /// Call under the manager `state` lock like the callers it serve — a *serialization* requirement,
/// Runs the CCD/GDI query FFI; call under the manager `state` lock like the callers it serves. /// not a soundness one: reading topology unlocked races the mutator and yields a stale answer.
pub unsafe fn wait_mode_settled(target_id: u32, mode: Mode, ceiling: std::time::Duration) -> bool { pub fn wait_mode_settled(target_id: u32, mode: Mode, ceiling: std::time::Duration) -> bool {
let deadline = std::time::Instant::now() + ceiling; let deadline = std::time::Instant::now() + ceiling;
loop { loop {
// SAFETY: CCD/GDI FFI over a `Copy` target id, owned return — this fn's own contract // `&&` short-circuits, so the second CCD query still does not run while the target has no
// (under the manager `state` lock) is what it needs. // active path at all — this polls every 25 ms. (It was nested only so each call could carry
if unsafe { resolve_gdi_name(target_id) }.is_some() { // its own `unsafe` proof; both are safe fns now.)
// SAFETY: as above; `active_resolution` is this module's own helper over the same id. if resolve_gdi_name(target_id).is_some()
// Nested rather than `&&`-joined so it stays SHORT-CIRCUIT: this polls every 25 ms, and && active_resolution(target_id) == Some((mode.width, mode.height))
// the second CCD query must not run while the target has no active path at all. {
if unsafe { active_resolution(target_id) } == Some((mode.width, mode.height)) { return true;
return true;
}
} }
if std::time::Instant::now() >= deadline { if std::time::Instant::now() >= deadline {
return false; return false;
@@ -338,12 +337,12 @@ pub unsafe fn wait_mode_settled(target_id: u32, mode: Mode, ceiling: std::time::
/// advertised mode never became settable; the isolate/layout paths already re-commit with this /// advertised mode never became settable; the isolate/layout paths already re-commit with this
/// flag for the same "the OS won't re-evaluate unless told" class. Best-effort. /// flag for the same "the OS won't re-evaluate unless told" class. Best-effort.
/// ///
/// # Safety /// Call under the manager `state` lock — it is the sole topology mutator, and concurrent applies
/// Runs the CCD query/apply FFI; call under the manager `state` lock (sole topology mutator). /// fight each other. A serialization requirement, not a soundness one.
pub unsafe fn force_mode_reenumeration() -> bool { pub fn force_mode_reenumeration() -> bool {
// SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned // SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned
// `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all. // `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all.
let Some((paths, modes)) = (unsafe { query_active_config() }) else { let Some((paths, modes)) = query_active_config() else {
return false; return false;
}; };
// SAFETY: the CCD contract at the top of this file — the path/mode arrays go over as // SAFETY: the CCD contract at the top of this file — the path/mode arrays go over as
@@ -445,14 +444,12 @@ pub fn wait_mode_advertised(gdi_name: &str, mode: Mode, ceiling: std::time::Dura
/// for that rare race, exactly as it was for a settle that expired. Returns `true` when departure /// for that rare race, exactly as it was for a settle that expired. Returns `true` when departure
/// was observed, `false` on ceiling. /// was observed, `false` on ceiling.
/// ///
/// # Safety /// Call under the manager `state` lock like the callers it serves (serialization, not soundness).
/// Runs the CCD query FFI; call under the manager `state` lock like the callers it serves. pub fn wait_target_departed(target_id: u32, ceiling: std::time::Duration) -> bool {
pub unsafe fn wait_target_departed(target_id: u32, ceiling: std::time::Duration) -> bool {
let deadline = std::time::Instant::now() + ceiling; let deadline = std::time::Instant::now() + ceiling;
let mut absent_streak = 0u32; let mut absent_streak = 0u32;
loop { loop {
// SAFETY: CCD FFI over a `Copy` target id, owned return, under the caller's `state` lock. if resolve_gdi_name(target_id).is_none() {
if unsafe { resolve_gdi_name(target_id) }.is_none() {
absent_streak += 1; absent_streak += 1;
if absent_streak >= 2 { if absent_streak >= 2 {
return true; return true;
@@ -471,7 +468,7 @@ pub unsafe fn wait_target_departed(target_id: u32, ceiling: std::time::Duration)
/// secure (Winlogon) desktop makes it render SDR/composed so DXGI Desktop Duplication can capture it /// secure (Winlogon) desktop makes it render SDR/composed so DXGI Desktop Duplication can capture it
/// (the HDR fullscreen independent-flip otherwise storms `ACCESS_LOST` → black); re-enable on return so /// (the HDR fullscreen independent-flip otherwise storms `ACCESS_LOST` → black); re-enable on return so
/// WGC keeps HDR on the normal desktop. Returns true on a successful `DisplayConfigSetDeviceInfo`. /// WGC keeps HDR on the normal desktop. Returns true on a successful `DisplayConfigSetDeviceInfo`.
pub unsafe fn set_advanced_color(target_id: u32, enable: bool) -> bool { pub fn set_advanced_color(target_id: u32, enable: bool) -> bool {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -533,7 +530,7 @@ pub unsafe fn set_advanced_color(target_id: u32, enable: bool) -> bool {
/// list (both happen transiently during a display-topology re-probe): the caller decides the fallback — /// list (both happen transiently during a display-topology re-probe): the caller decides the fallback —
/// the capture loop's poller keeps the last known value, since reading a blip as "HDR off" used to cost /// the capture loop's poller keeps the last known value, since reading a blip as "HDR off" used to cost
/// an HDR session TWO spurious ring recreates (false, then true again a poll later). /// an HDR session TWO spurious ring recreates (false, then true again a poll later).
pub unsafe fn advanced_color_enabled(target_id: u32) -> Option<bool> { pub fn advanced_color_enabled(target_id: u32) -> Option<bool> {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -587,9 +584,8 @@ pub unsafe fn advanced_color_enabled(target_id: u32) -> Option<bool> {
/// slider default alone is ~2.5x). `None` = query failed / target not active (callers keep /// slider default alone is ~2.5x). `None` = query failed / target not active (callers keep
/// their last value or 1.0). /// their last value or 1.0).
/// ///
/// # Safety /// Read-only, over owned locals — same shape as [`advanced_color_enabled`].
/// Runs the read-only CCD query FFI over owned locals (same shape as [`advanced_color_enabled`]). pub fn sdr_white_level_scale(target_id: u32) -> Option<f32> {
pub unsafe fn sdr_white_level_scale(target_id: u32) -> Option<f32> {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -904,7 +900,7 @@ const DISPLAYCONFIG_PATH_MODE_IDX_INVALID: u32 = 0xffff_ffff;
/// Query the current ACTIVE display config (paths + modes), truncated to the real counts. `None` on /// Query the current ACTIVE display config (paths + modes), truncated to the real counts. `None` on
/// API failure. Shared by [`isolate_displays_ccd`] (snapshot + per-attempt re-query) and /// API failure. Shared by [`isolate_displays_ccd`] (snapshot + per-attempt re-query) and
/// [`count_other_active`]. /// [`count_other_active`].
unsafe fn query_active_config() -> Option<SavedConfig> { fn query_active_config() -> Option<SavedConfig> {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -939,10 +935,8 @@ unsafe fn query_active_config() -> Option<SavedConfig> {
/// that would still be lit besides the managed virtual set. `None` on query failure. Used to VERIFY /// that would still be lit besides the managed virtual set. `None` on query failure. Used to VERIFY
/// isolation actually took, and (in the `primary` topology) to detect a physical that is ALREADY /// isolation actually took, and (in the `primary` topology) to detect a physical that is ALREADY
/// active so we can skip a force-EXTEND that would reset its refresh. /// active so we can skip a force-EXTEND that would reset its refresh.
pub unsafe fn count_other_active(keep_target_ids: &[u32]) -> Option<u32> { pub fn count_other_active(keep_target_ids: &[u32]) -> Option<u32> {
// SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned let (paths, _) = query_active_config()?;
// `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all.
let (paths, _) = unsafe { query_active_config() }?;
Some( Some(
paths paths
.iter() .iter()
@@ -1010,7 +1004,7 @@ fn utf16z_str(buf: &[u16]) -> String {
/// matrix to unique targets) with its name, connector class and active state. Read-only CCD; can /// matrix to unique targets) with its name, connector class and active state. Read-only CCD; can
/// briefly serialize on the display-config lock during topology churn — callers must keep it OFF /// briefly serialize on the display-config lock during topology churn — callers must keep it OFF
/// the capture thread (`display_events` runs it on its own listener thread and caches). /// the capture thread (`display_events` runs it on its own listener thread and caches).
pub unsafe fn target_inventory() -> Vec<TargetInventory> { pub fn target_inventory() -> Vec<TargetInventory> {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -1101,20 +1095,16 @@ pub unsafe fn target_inventory() -> Vec<TargetInventory> {
/// later returns). Returns the original active config to restore on teardown. /// later returns). Returns the original active config to restore on teardown.
// pub so vdisplay::pf_vdisplay can reuse this backend-neutral CCD isolation helper // pub so vdisplay::pf_vdisplay can reuse this backend-neutral CCD isolation helper
// (it operates on real OS target ids — a pf-vdisplay monitor's target_id qualifies). // (it operates on real OS target ids — a pf-vdisplay monitor's target_id qualifies).
pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option<SavedConfig> { pub fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option<SavedConfig> {
// Snapshot the ORIGINAL active config ONCE for restore-on-teardown, before any changes. // Snapshot the ORIGINAL active config ONCE for restore-on-teardown, before any changes.
// SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned let saved = query_active_config()?;
// `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all.
let saved = unsafe { query_active_config() }?;
// Deactivate every non-keep display, then VERIFY and RETRY. A field-reported bug had a physical // Deactivate every non-keep display, then VERIFY and RETRY. A field-reported bug had a physical
// monitor STAY ACTIVE in exclusive mode, so we don't trust a single SetDisplayConfig: re-query the // monitor STAY ACTIVE in exclusive mode, so we don't trust a single SetDisplayConfig: re-query the
// live topology each attempt and re-apply until ONLY the keep set is active. Secure-desktop // live topology each attempt and re-apply until ONLY the keep set is active. Secure-desktop
// correctness depends on this — the lock screen must not land on a stray panel while we stream. // correctness depends on this — the lock screen must not land on a stray panel while we stream.
for attempt in 1..=4u32 { for attempt in 1..=4u32 {
// SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned let (mut paths, mut modes) = query_active_config()?;
// `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all.
let (mut paths, mut modes) = unsafe { query_active_config() }?;
let mut others = 0u32; let mut others = 0u32;
for p in paths.iter_mut() { for p in paths.iter_mut() {
if keep_target_ids.contains(&p.targetInfo.id) { if keep_target_ids.contains(&p.targetInfo.id) {
@@ -1141,9 +1131,7 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option<SavedConfi
// contains a primary — an origin-less desktop is rejected 0x57 no matter which array // contains a primary — an origin-less desktop is rejected 0x57 no matter which array
// shape carries it (see `anchor_kept_sources_at_origin`). // shape carries it (see `anchor_kept_sources_at_origin`).
if others > 0 { if others > 0 {
// SAFETY: this module's own helper over two borrowed local `Vec`s; it only reorders anchor_kept_sources_at_origin(&paths, &mut modes);
// the source positions it is handed and calls no FFI of its own.
unsafe { anchor_kept_sources_at_origin(&paths, &mut modes) };
} }
// Commit the config. Even when nothing needed deactivating we re-commit: a legacy mode-set does // Commit the config. Even when nothing needed deactivating we re-commit: a legacy mode-set does
// NOT drive the IddCx adapter's EVT_IDD_CX_ADAPTER_COMMIT_MODES, and without COMMIT_MODES the OS // NOT drive the IddCx adapter's EVT_IDD_CX_ADAPTER_COMMIT_MODES, and without COMMIT_MODES the OS
@@ -1161,9 +1149,7 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option<SavedConfi
// SDC_FORCE_MODE_ENUMERATION in case the driver rejects it combined with a real // SDC_FORCE_MODE_ENUMERATION in case the driver rejects it combined with a real
// topology change — an actual path removal drives COMMIT_MODES on its own, so the // topology change — an actual path removal drives COMMIT_MODES on its own, so the
// re-commit rationale doesn't need the flag here. // re-commit rationale doesn't need the flag here.
// SAFETY: this module's own helper — borrowed local `Vec`s in, owned `Vec`s out; no let (kp, km) = keep_only_supplied(&paths, &modes);
// caller obligation beyond the arrays being the ones `query_active_config` produced.
let (kp, km) = unsafe { keep_only_supplied(&paths, &modes) };
let mut esc = SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_ALLOW_CHANGES; let mut esc = SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_ALLOW_CHANGES;
if attempt < 4 { if attempt < 4 {
esc |= SDC_FORCE_MODE_ENUMERATION; esc |= SDC_FORCE_MODE_ENUMERATION;
@@ -1209,8 +1195,7 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option<SavedConfi
// VERIFY the OUTCOME (rc alone lies — a "successful" apply can leave a panel active): re-query // VERIFY the OUTCOME (rc alone lies — a "successful" apply can leave a panel active): re-query
// and confirm no non-keep display survived. Only then is the virtual set truly the sole desktop. // and confirm no non-keep display survived. Only then is the virtual set truly the sole desktop.
// SAFETY: this module's own CCD re-query over a borrowed `&[u32]`; owned return. let survivors = count_other_active(keep_target_ids).unwrap_or(0);
let survivors = unsafe { count_other_active(keep_target_ids) }.unwrap_or(0);
if survivors == 0 { if survivors == 0 {
tracing::info!("display isolate (CCD): target set {keep_target_ids:?} is the SOLE active desktop (attempt {attempt}/4, deactivated {others}, rc={rc:#x})"); tracing::info!("display isolate (CCD): target set {keep_target_ids:?} is the SOLE active desktop (attempt {attempt}/4, deactivated {others}, rc={rc:#x})");
return Some(saved); return Some(saved);
@@ -1221,8 +1206,7 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option<SavedConfi
// Name the survivors instead of assuming their kind — the field logs showed this path fire // Name the survivors instead of assuming their kind — the field logs showed this path fire
// with a sibling VIRTUAL display as the survivor (linger-expiry shrink), where the old // with a sibling VIRTUAL display as the survivor (linger-expiry shrink), where the old
// "a non-virtual display stayed active" wording sent the triage the wrong way. // "a non-virtual display stayed active" wording sent the triage the wrong way.
// SAFETY: this module's own CCD enumeration — no arguments, owned `Vec` return. let survivors: Vec<String> = target_inventory()
let survivors: Vec<String> = unsafe { target_inventory() }
.iter() .iter()
.filter(|t| t.active && !keep_target_ids.contains(&t.target_id)) .filter(|t| t.active && !keep_target_ids.contains(&t.target_id))
.map(|t| format!("{} {} \"{}\"", t.target_id, t.tech, t.friendly)) .map(|t| format!("{} {} \"{}\"", t.target_id, t.tech, t.friendly))
@@ -1248,7 +1232,7 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option<SavedConfi
/// dropped inactive entries were declared ignored anyway ("Only the paths within this array that /// dropped inactive entries were declared ignored anyway ("Only the paths within this array that
/// have the DISPLAYCONFIG_PATH_ACTIVE flag set are set"), so this shape asks for the identical /// have the DISPLAYCONFIG_PATH_ACTIVE flag set are set"), so this shape asks for the identical
/// topology — minus the array contents some driver/OS validation combos reject with 0x57. /// topology — minus the array contents some driver/OS validation combos reject with 0x57.
unsafe fn keep_only_supplied( fn keep_only_supplied(
paths: &[DISPLAYCONFIG_PATH_INFO], paths: &[DISPLAYCONFIG_PATH_INFO],
modes: &[DISPLAYCONFIG_MODE_INFO], modes: &[DISPLAYCONFIG_MODE_INFO],
) -> (Vec<DISPLAYCONFIG_PATH_INFO>, Vec<DISPLAYCONFIG_MODE_INFO>) { ) -> (Vec<DISPLAYCONFIG_PATH_INFO>, Vec<DISPLAYCONFIG_MODE_INFO>) {
@@ -1311,7 +1295,7 @@ fn remap_mode_idx(
/// (relative arrangement preserved) so the top-left-most lands exactly on the origin. A set that /// (relative arrangement preserved) so the top-left-most lands exactly on the origin. A set that
/// already covers `(0,0)` is left untouched, so a plain re-commit stays byte-identical and a /// already covers `(0,0)` is left untouched, so a plain re-commit stays byte-identical and a
/// user's negative-coordinate multi-monitor layout is never rearranged. /// user's negative-coordinate multi-monitor layout is never rearranged.
unsafe fn anchor_kept_sources_at_origin( fn anchor_kept_sources_at_origin(
paths: &[DISPLAYCONFIG_PATH_INFO], paths: &[DISPLAYCONFIG_PATH_INFO],
modes: &mut [DISPLAYCONFIG_MODE_INFO], modes: &mut [DISPLAYCONFIG_MODE_INFO],
) { ) {
@@ -1365,10 +1349,10 @@ unsafe fn anchor_kept_sources_at_origin(
/// Used by the IDD-push compose kick to dirty THE TARGET display: with parallel displays the /// Used by the IDD-push compose kick to dirty THE TARGET display: with parallel displays the
/// cursor sits on ONE of them, and a cursor wiggle only dirties that one — a sibling display's /// cursor sits on ONE of them, and a cursor wiggle only dirties that one — a sibling display's
/// kick must first know where to send the cursor (Stage W3 on-glass finding). /// kick must first know where to send the cursor (Stage W3 on-glass finding).
pub unsafe fn source_desktop_rect(target_id: u32) -> Option<(i32, i32, i32, i32)> { pub fn source_desktop_rect(target_id: u32) -> Option<(i32, i32, i32, i32)> {
// SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned // SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned
// `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all. // `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all.
let (paths, modes) = unsafe { query_active_config() }?; let (paths, modes) = query_active_config()?;
for p in &paths { for p in &paths {
if p.targetInfo.id != target_id || p.flags & DISPLAYCONFIG_PATH_ACTIVE == 0 { if p.targetInfo.id != target_id || p.flags & DISPLAYCONFIG_PATH_ACTIVE == 0 {
continue; continue;
@@ -1400,10 +1384,8 @@ pub unsafe fn source_desktop_rect(target_id: u32) -> Option<(i32, i32, i32, i32)
/// or the query fails. Used to normalize desktop coordinates into the virtual HID pointer's /// or the query fails. Used to normalize desktop coordinates into the virtual HID pointer's
/// absolute `0..=32767` axis space (win32k maps the device's logical extents onto the virtual /// absolute `0..=32767` axis space (win32k maps the device's logical extents onto the virtual
/// screen). /// screen).
pub unsafe fn desktop_bounds() -> Option<(i32, i32, i32, i32)> { pub fn desktop_bounds() -> Option<(i32, i32, i32, i32)> {
// SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned let (paths, modes) = query_active_config()?;
// `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all.
let (paths, modes) = unsafe { query_active_config() }?;
let mut acc: Option<(i32, i32, i32, i32)> = None; // (x0, y0, x1, y1) let mut acc: Option<(i32, i32, i32, i32)> = None; // (x0, y0, x1, y1)
for p in &paths { for p in &paths {
if p.flags & DISPLAYCONFIG_PATH_ACTIVE == 0 { if p.flags & DISPLAYCONFIG_PATH_ACTIVE == 0 {
@@ -1435,13 +1417,11 @@ pub unsafe fn desktop_bounds() -> Option<(i32, i32, i32, i32)> {
/// treats the source at `(0,0)` as primary, so auto-row's first member lands primary — the group's /// treats the source at `(0,0)` as primary, so auto-row's first member lands primary — the group's
/// designated member. Paths not named stay where they are. Best-effort: a failure leaves the OS /// designated member. Paths not named stay where they are. Best-effort: a failure leaves the OS
/// placement (mouse crossing may not match the layout table until the next apply). /// placement (mouse crossing may not match the layout table until the next apply).
pub unsafe fn apply_source_positions(positions: &[(u32, i32, i32)]) { pub fn apply_source_positions(positions: &[(u32, i32, i32)]) {
if positions.len() < 2 { if positions.len() < 2 {
return; // a single (or no) member sits at the origin — nothing to arrange return; // a single (or no) member sits at the origin — nothing to arrange
} }
// SAFETY: `query_active_config` is this module's own CCD helper: it takes nothing and returns owned let Some((paths, mut modes)) = query_active_config() else {
// `Vec`s built from a fresh `QueryDisplayConfig`, so it has no caller obligation at all.
let Some((paths, mut modes)) = (unsafe { query_active_config() }) else {
return; return;
}; };
// Dedup source-mode indices (a cloned group shares one) — same discipline as // Dedup source-mode indices (a cloned group shares one) — same discipline as
@@ -1504,7 +1484,7 @@ pub unsafe fn apply_source_positions(positions: &[(u32, i32, i32)]) {
/// atomic CCD `SetDisplayConfig` (NOT GDI `CDS_SET_PRIMARY`, which storms /// atomic CCD `SetDisplayConfig` (NOT GDI `CDS_SET_PRIMARY`, which storms
/// `DXGI_ERROR_MODE_CHANGE_IN_PROGRESS` when another display is live — see [`set_active_mode`]). /// `DXGI_ERROR_MODE_CHANGE_IN_PROGRESS` when another display is live — see [`set_active_mode`]).
/// Returns the original config to restore on teardown. /// Returns the original config to restore on teardown.
pub unsafe fn set_virtual_primary_ccd(keep_target_id: u32) -> Option<SavedConfig> { pub fn set_virtual_primary_ccd(keep_target_id: u32) -> Option<SavedConfig> {
let mut np = 0u32; let mut np = 0u32;
let mut nm = 0u32; let mut nm = 0u32;
// SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live // SAFETY: the CCD contract at the top of this file — `&mut np`/`&mut nm` are live
@@ -1612,7 +1592,7 @@ pub unsafe fn set_virtual_primary_ccd(keep_target_id: u32) -> Option<SavedConfig
/// Restore the topology saved by [`isolate_displays_ccd`] (teardown, before the virtual output is /// Restore the topology saved by [`isolate_displays_ccd`] (teardown, before the virtual output is
/// removed), re-activating the displays we deactivated. /// removed), re-activating the displays we deactivated.
// pub so vdisplay::pf_vdisplay can reuse this backend-neutral CCD restore helper. // pub so vdisplay::pf_vdisplay can reuse this backend-neutral CCD restore helper.
pub unsafe fn restore_displays_ccd(saved: &SavedConfig) { pub fn restore_displays_ccd(saved: &SavedConfig) {
let (paths, modes) = saved; let (paths, modes) = saved;
if paths.is_empty() { if paths.is_empty() {
return; return;
@@ -1644,8 +1624,7 @@ pub unsafe fn restore_displays_ccd(saved: &SavedConfig) {
// connected, fall back to the OS database EXTEND preset, which re-activates every connected // connected, fall back to the OS database EXTEND preset, which re-activates every connected
// display. Internal panels deliberately don't count as lit-able here — a closed clamshell // display. Internal panels deliberately don't count as lit-able here — a closed clamshell
// lid must not be forced back on. // lid must not be forced back on.
// SAFETY: this module's own CCD enumeration — no arguments, owned `Vec` return. let (connected, lit) = target_inventory()
let (connected, lit) = unsafe { target_inventory() }
.iter() .iter()
.filter(|t| t.external_physical) .filter(|t| t.external_physical)
.fold((0u32, 0u32), |(c, a), t| (c + 1, a + u32::from(t.active))); .fold((0u32, 0u32), |(c, a), t| (c + 1, a + u32::from(t.active)));
@@ -1653,8 +1632,6 @@ pub unsafe fn restore_displays_ccd(saved: &SavedConfig) {
tracing::warn!( tracing::warn!(
"display isolate (CCD): no external physical display active after the restore (rc={rc:#x}, connected={connected}) — forcing the EXTEND preset so the desk is not left dark" "display isolate (CCD): no external physical display active after the restore (rc={rc:#x}, connected={connected}) — forcing the EXTEND preset so the desk is not left dark"
); );
// SAFETY: this module's own topology preset — no arguments, and it binds its own force_extend_topology();
// `SetDisplayConfig` to the input desktop internally.
unsafe { force_extend_topology() };
} }
} }