From 0015108e4fc638dca64123d0eea8b791ba9e983c Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Thu, 30 Jul 2026 11:42:42 -0300 Subject: [PATCH] drm: close the round-8 review findings - the .so contract check in the drm workflow runs under strict mode: without set -e the trailing ::notice echo returned 0 and masked the `test "$missing" -eq 0` assertion, so the step passed even with a missing loader symbol or a CPU-only stub. the two extraction pipelines get an explicit rescue so a zero-match grep still reaches the ::error guard that explains WHY instead of dying silently. - the pipewire-fallback geometry guard no longer compares the physical drm size against the portal rect on a single-display host: the rect is the compositor's LOGICAL size, so on a scaled output the two legitimately disagree (2880x1800 vs 1440x900) and the guard rejected the one valid fallback, restart-looping the display instead of degrading. on a single-display host the whole-desktop stream is that display by construction, so only the position has to agree; the size check stays on multi-monitor hosts, where it is what tells one connector apart from the full-desktop rect. --- .github/workflows/drm-capture.yml | 11 +++++++++-- src/server/wayland.rs | 21 ++++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.github/workflows/drm-capture.yml b/.github/workflows/drm-capture.yml index 9f8b9059f..9706c7ae1 100644 --- a/.github/workflows/drm-capture.yml +++ b/.github/workflows/drm-capture.yml @@ -181,6 +181,10 @@ jobs: - name: Assert the .so contract (EGL enabled, loader symbols present) shell: bash run: | + # Strict mode is load-bearing here: without it the trailing ::notice echo would return 0 + # and mask the `test "$missing" -eq 0` assertion, so the step would pass with a missing + # loader symbol or a CPU-only stub. (pipefail also keeps the grep -c pipelines honest.) + set -euo pipefail SO="$(cat so_path)" echo "checking $SO" missing=0 @@ -189,9 +193,12 @@ jobs: # silently dropped from the loop), and the count is asserted below so a refactor of the # loader away from b"..." literals cannot quietly turn this whole check into a no-op that # iterates zero times and passes. + # `|| true` on the extraction pipelines: under set -e/pipefail a zero-match grep would + # abort the script before the explicit ::error guard below can say WHY it failed; the + # guard on nsyms is the intended reporter for that case. syms=$(grep -oE 'b"drmtap_[a-z0-9_]+"' libs/scrap/src/common/drmtap_dl.rs \ - | sed 's/^b"//; s/"$//' | sort -u) - nsyms=$(echo "$syms" | grep -c .) + | sed 's/^b"//; s/"$//' | sort -u || true) + nsyms=$(echo "$syms" | grep -c . || true) if [ "$nsyms" -lt 13 ]; then echo "::error::extracted only $nsyms loader symbols from drmtap_dl.rs (expected >= 13); the extraction pattern no longer matches the loader" missing=1 diff --git a/src/server/wayland.rs b/src/server/wayland.rs index 9982fc885..40869903d 100644 --- a/src/server/wayland.rs +++ b/src/server/wayland.rs @@ -452,13 +452,24 @@ pub(super) fn get_capturer_for_display( // this guard is skipped, preserving upstream behavior exactly. #[cfg(feature = "drm")] if super::drm_capturer::is_available_cached() { - if let Some(advertised) = super::drm_capturer::get_display_infos() - .and_then(|l| l.get(display_idx).cloned()) - { + let (advertised, single_display) = match super::drm_capturer::get_display_infos() { + Some(list) => (list.get(display_idx).cloned(), list.len() == 1), + None => (None, false), + }; + if let Some(advertised) = advertised { + // The advertised DRM geometry is PHYSICAL while the PipeWire rect is the + // compositor's LOGICAL size (try_fix_logical_size), so on a scaled output the + // two sizes legitimately disagree (2880x1800 vs 1440x900) even when the stream + // IS this display. On a single-display host the whole-desktop stream is this + // display by construction, so only the position has to agree there; comparing + // the physical size too rejected the one valid fallback and restart-looped the + // display instead of degrading. On a multi-monitor host the size check stays: + // it is what tells one connector apart from the full-desktop rect. let consistent = advertised.x == rect.0 .0 && advertised.y == rect.0 .1 - && advertised.width as usize == rect.1 - && advertised.height as usize == rect.2; + && (single_display + || (advertised.width as usize == rect.1 + && advertised.height as usize == rect.2)); if !consistent { bail!( "drm display {} demoted with no geometry-consistent PipeWire stream (advertised {}x{}+{}+{} vs stream {}x{}+{}+{}); advertised offline",