From 9b241d9d7b4ba0fea9b46b1c5eaebf5c946cdca4 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 24 Jul 2026 10:21:47 +0200 Subject: [PATCH] fix(pf-win-display): anchor the kept sources at the desktop origin when isolating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 of the field-reported exclusive-topology 0x57, with the reporter's retest logs finally separating the variable: the SAME isolate call converged rc=0 whenever a kept member already sat at (0,0), and failed 0x57 on every shape — doomed-path-carried AND keep-only escalation — whenever the doomed physical held the origin. A committable config must still contain a primary (a source pinned exactly at (0,0)); deactivating the display that held it while the kept virtual stays at its EXTEND offset supplies an origin-less desktop, which Windows rejects wholesale. Translate the kept sources rigidly so the top-left-most lands on the origin; sets already covering (0,0) are untouched so plain re-commits stay byte-identical. Also from the same logs: - restore_displays_ccd now guarantees the desk is never left all-dark: the saved snapshot can be unappliable (0x64a — it pinned a virtual target incarnation removed before teardown) or apply cleanly yet re-light nothing (snapshotted while an earlier failed teardown had the physicals off). If no external physical is active after the apply while one is connected, fall back to the database EXTEND preset. Internal panels don't count — a closed lid must not be forced back on. - the final isolate failure now names the surviving targets instead of asserting 'a non-virtual display stayed active' — the logs showed a sibling VIRTUAL display as the survivor (linger-expiry shrink), and the wording sent triage the wrong way. Co-Authored-By: Claude Fable 5 --- crates/pf-win-display/src/win_display.rs | 104 +++++++++++++++++++++-- 1 file changed, 97 insertions(+), 7 deletions(-) diff --git a/crates/pf-win-display/src/win_display.rs b/crates/pf-win-display/src/win_display.rs index cc72b309..5d1a99d6 100644 --- a/crates/pf-win-display/src/win_display.rs +++ b/crates/pf-win-display/src/win_display.rs @@ -939,7 +939,7 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option Option 0 { + anchor_kept_sources_at_origin(&paths, &mut modes); + } // 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 // never calls ASSIGN_SWAPCHAIN, so the driver receives no frames. SDC_FORCE_MODE_ENUMERATION @@ -969,11 +976,11 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option 0 && attempt >= 2).then(|| { - // ESCALATION (attempt 2+): supply ONLY the keep paths. Field-reported (AMD + - // pf-vdisplay): carrying the doomed path in the array — inactive, modes unpinned — - // gets the whole config rejected 0x57 on EVERY retry, so the loop alone never - // converged; the same host applies the keep-only shape rc=0 whenever the topology - // database has already made the virtual display sole. The final attempt also drops + // ESCALATION (attempt 2+): supply ONLY the keep paths. Kept as belt-and-braces — + // the field 0x57 this was built for turned out to be the missing desktop origin + // (see `anchor_kept_sources_at_origin`), which rejected BOTH shapes identically; + // but should some other validator still choke on the full array, the minimal + // shape is the best last word. The final attempt also drops // 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 // re-commit rationale doesn't need the flag here. @@ -1023,7 +1030,18 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option = target_inventory() + .iter() + .filter(|t| t.active && !keep_target_ids.contains(&t.target_id)) + .map(|t| format!("{} {} \"{}\"", t.target_id, t.tech, t.friendly)) + .collect(); + tracing::error!( + "display isolate (CCD): failed to isolate target set {keep_target_ids:?} after 4 attempts — still active: [{}] (field-reported exclusive-mode bug)", + survivors.join(", ") + ); Some(saved) } @@ -1084,6 +1102,60 @@ fn remap_mode_idx( }) } +/// A committable desktop must still contain a PRIMARY — a source pinned exactly at the origin +/// `(0,0)`. Deactivating the display that held the origin (the physical, in the exclusive +/// topology) while the kept virtual stays pinned at its EXTEND offset supplies an origin-less +/// desktop, and Windows rejects that wholesale with 0x57 ERROR_INVALID_PARAMETER no matter the +/// array shape — the field box failed identically with the doomed path carried inactive AND with +/// the keep-only escalation, yet the very same call converged rc=0 whenever a kept member already +/// sat at `(0,0)`; the origin was the real variable all along. Translate the kept sources RIGIDLY +/// (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 +/// user's negative-coordinate multi-monitor layout is never rearranged. +unsafe fn anchor_kept_sources_at_origin( + paths: &[DISPLAYCONFIG_PATH_INFO], + modes: &mut [DISPLAYCONFIG_MODE_INFO], +) { + // Unique source-mode entries of the still-ACTIVE (kept) paths — clone-style pairs share one, + // and a shared entry must be translated once. + let mut idxs: Vec = Vec::new(); + for p in paths { + if p.flags & DISPLAYCONFIG_PATH_ACTIVE == 0 { + continue; + } + let idx = p.sourceInfo.Anonymous.modeInfoIdx as usize; + let Some(m) = modes.get(idx) else { continue }; + if m.infoType == DISPLAYCONFIG_MODE_INFO_TYPE_SOURCE && !idxs.contains(&idx) { + idxs.push(idx); + } + } + let positions: Vec<(i32, i32)> = idxs + .iter() + .map(|&i| { + let pos = modes[i].Anonymous.sourceMode.position; + (pos.x, pos.y) + }) + .collect(); + if positions.contains(&(0, 0)) { + return; // the kept set already holds the primary — don't touch a valid layout + } + // Lexicographic min over the actual positions — the anchor IS one of the kept sources, so + // after translation one source sits exactly at (0,0), which is what the validator wants. + let Some((ax, ay)) = positions.iter().copied().min() else { + return; // no pinned kept sources — placement is the OS's (SDC_ALLOW_CHANGES), nothing to anchor + }; + for &i in &idxs { + let sm = &mut modes[i].Anonymous.sourceMode; + sm.position.x -= ax; + sm.position.y -= ay; + } + tracing::info!( + "display isolate (CCD): kept source(s) re-anchored onto the desktop origin (primary) — the doomed display held (0,0) delta=({},{})", + -ax, + -ay + ); +} + /// The desktop-space rectangle `(x, y, w, h)` of `target_id`'s SOURCE — where this display's /// region lives in the desktop coordinate space. `None` while the target isn't an active path. /// Used by the IDD-push compose kick to dirty THE TARGET display: with parallel displays the @@ -1316,4 +1388,22 @@ pub unsafe fn restore_displays_ccd(saved: &SavedConfig) { sdc_access_denied_hint(rc) ); } + // GUARANTEE the desk is never left all-dark. The saved config can be unappliable (field + // rc=0x64a ERROR_BAD_CONFIGURATION: it pinned a virtual target incarnation that was since + // removed) or even apply rc=0 yet re-light nothing (snapshotted while an earlier failed + // teardown had the physicals off — the poisoned-snapshot chain from the field logs). Either + // way, if no external physical panel is active after the apply while at least one is + // 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 + // lid must not be forced back on. + let (connected, lit) = target_inventory() + .iter() + .filter(|t| t.external_physical) + .fold((0u32, 0u32), |(c, a), t| (c + 1, a + u32::from(t.active))); + if connected > 0 && lit == 0 { + 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" + ); + force_extend_topology(); + } }