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.
This commit is contained in:
Mariano Abad
2026-07-30 11:42:42 -03:00
parent b977d9b010
commit 0015108e4f
2 changed files with 25 additions and 7 deletions

View File

@@ -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

View File

@@ -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",