Merge pull request 'Every NVIDIA gamescope HDR stream had red and blue swapped — and a sysext step added in a release was unreachable forever' (#143) from worktree-hdr-rb-swap-nvidia into main

Reviewed-on: #143

crates/pf-capture/src/linux/pipewire.rs

@@ -4,6 +4,7 @@ use super::pw_cursor::{composite_cursor, update_cursor_meta, CursorState};
 use super::pw_pods::{
     build_cursor_meta_param, build_default_format_obj, build_dmabuf_buffers, build_dmabuf_format,
     build_hdr_dmabuf_format, build_mappable_buffers, build_shm_only_buffers, serialize_pod,
+    HDR_FORMAT_ORDER,
 };
 use super::{CapturedFrame, DmabufFrame, FramePayload, PixelFormat, ZeroCopyPolicy};
 use anyhow::{Context, Result};
@@ -1850,13 +1851,15 @@ pub fn pipewire_thread(
     // negotiation-timeout path latches the process-wide SDR downgrade if nothing matches.
     let format_pods: Vec<Vec<u8>> = if want_hdr {
         tracing::info!(
-            "HDR capture: offering xRGB_210LE/xBGR_210LE LINEAR dmabufs with MANDATORY \
+            "HDR capture: offering xBGR_210LE/xRGB_210LE LINEAR dmabufs with MANDATORY \
              BT.2020 + SMPTE-2084 (PQ) colorimetry (GNOME 50+ monitor stream)"
         );
-        vec![
-            build_hdr_dmabuf_format(VideoFormat::xRGB_210LE, preferred)?,
-            build_hdr_dmabuf_format(VideoFormat::xBGR_210LE, preferred)?,
-        ]
+        // ⚠ Order is the whole fix — see the NVIDIA note on `HDR_FORMAT_ORDER`. The first
+        // compatible consumer pod wins, so this is what a gamescope session actually lands on.
+        HDR_FORMAT_ORDER
+            .iter()
+            .map(|fmt| build_hdr_dmabuf_format(*fmt, preferred))
+            .collect::<Result<Vec<_>>>()?
     } else if want_dmabuf {
         let mut pods = Vec::with_capacity(if prefer_native_nv12 { 2 } else { 1 });
         if prefer_native_nv12 {

crates/pf-capture/src/linux/pw_pods.rs

@@ -121,6 +121,38 @@ pub(super) fn build_dmabuf_format(
 /// SDR — the same outcome as not offering HDR.
 const SPA_VIDEO_TRANSFER_SMPTE2084: u32 = 14;
 
+/// The two 10-bit PQ formats an HDR session offers, **in negotiation order**. The order is not a
+/// style choice — on NVIDIA it is the difference between correct colour and red/blue swapped.
+///
+/// `xBGR_210LE` (DRM `XBGR2101010`, Vulkan `A2B10G10R10_UNORM_PACK32`) comes FIRST because the
+/// first compatible consumer pod wins, and it is the only one gamescope fills correctly on every
+/// vendor:
+///
+/// * `A2R10G10B10_UNORM_PACK32` **linear-tiled storage** is an optional Vulkan feature that
+///   NVIDIA does not implement. gamescope's capture textures are mappable, hence linear, so on
+///   NVIDIA its composite `imageStore` into that image lands in XBGR order — the bytes come out
+///   byte-reversed while the buffer is still LABELLED `XRGB2101010`.
+/// * The host believes the label: `xRGB_210LE → PixelFormat::X2Rgb10 →`
+///   `NV_ENC_BUFFER_FORMAT_ARGB10`. Every mapping in that chain is individually correct, which is
+///   exactly why the bug is invisible from this side — the *content* is what's wrong.
+/// * Upstream gamescope hit the same wall and fixed it with `vulkan_get_rgb10_capture_format()`,
+///   which probes `linearTilingFeatures` for STORAGE+SAMPLED and falls back to `XBGR2101010`.
+///   That landed AFTER 3.16.25, so the pinned `punktfunk-gamescope` (3.16.25-7-g60561e2 +pfhdr4)
+///   predates it and cannot self-correct — hence fixing the preference host-side, where it ships
+///   in the host binary with no gamescope rebuild.
+///
+/// Preferring xBGR costs nothing anywhere else: `A2B10G10R10_UNORM_PACK32` is the universally
+/// supported packed-10 format (it is the standard HDR10 swapchain format), it is what upstream
+/// falls back to, and `X2Bgr10` has a first-class encoder path (NVENC `ABGR10`, VAAPI
+/// `X2BGR10LE`). `xRGB_210LE` stays as the second pod so a producer that somehow offers only it
+/// can still negotiate HDR rather than falling off to the SDR downgrade.
+///
+/// ⚠ The real fix belongs upstream in the patch set: `spa_format_to_drm()` should offer only the
+/// format `vulkan_get_rgb10_capture_format()` reports. Until the gamescope pin moves past that
+/// commit, THIS ORDER is what keeps NVIDIA HDR sessions correct — do not "tidy" it.
+pub(super) const HDR_FORMAT_ORDER: [VideoFormat; 2] =
+    [VideoFormat::xBGR_210LE, VideoFormat::xRGB_210LE];
+
 pub(super) fn build_hdr_dmabuf_format(
     format: VideoFormat,
     preferred: Option<(u32, u32, u32)>,
@@ -596,4 +628,37 @@ mod tests {
         // The minimum must not exceed what producers already serve, or the ask becomes a demand.
         const { assert!(POOL_MIN <= 2) };
     }
+
+    /// xBGR_210LE must be offered FIRST, and this is a correctness test, not a style one.
+    ///
+    /// The first compatible consumer pod wins the negotiation. Leading with `xRGB_210LE` makes an
+    /// NVIDIA gamescope session land on `XRGB2101010`, whose linear-tiled `A2R10G10B10` storage
+    /// NVIDIA does not support — gamescope's composite `imageStore` writes XBGR bytes under an
+    /// XRGB label and the whole stream comes out with red and blue swapped. Every format mapping
+    /// on the host side is individually correct, so nothing downstream can detect it.
+    ///
+    /// Field-confirmed 2026-08-09 on the RTX 5070 Ti Bazzite host with 0.26.0. See the
+    /// [`HDR_FORMAT_ORDER`] docs for the upstream fix this predates.
+    #[test]
+    fn hdr_offers_xbgr_before_xrgb() {
+        assert_eq!(
+            HDR_FORMAT_ORDER[0],
+            VideoFormat::xBGR_210LE,
+            "xBGR_210LE must be offered first — leading with xRGB_210LE swaps red and blue on \
+             every NVIDIA gamescope HDR session"
+        );
+        assert_eq!(
+            HDR_FORMAT_ORDER[1],
+            VideoFormat::xRGB_210LE,
+            "xRGB_210LE stays as the fallback pod so a producer offering only it can still \
+             negotiate HDR instead of dropping to the SDR downgrade"
+        );
+        // Both must still build: the order is a preference, never a removal.
+        for fmt in HDR_FORMAT_ORDER {
+            assert!(
+                !build_hdr_dmabuf_format(fmt, None).unwrap().is_empty(),
+                "{fmt:?} must still produce a format pod"
+            );
+        }
+    }
 }

packaging/bazzite/punktfunk-sysext.sh

@@ -55,7 +55,9 @@ sy5uhYGZD6lMJ4uZAQC7W81H2gHlTDTA2Nq35HKW9IOU+Ll2c9fqa7fAIKf9Bg==
 usage() {
   sed -n 's/^#\( \|$\)//p' "$0" | sed -n '1,20p'
   echo "usage: punktfunk-sysext install [--channel stable|canary] [--from-file X.raw]"
-  echo "       punktfunk-sysext update [--from-file X.raw] | status | remove"
+  echo "       punktfunk-sysext update [--from-file X.raw] | reapply | status | remove"
+  echo "       reapply: re-run the host-state steps a sysext image cannot carry (groups, /etc"
+  echo "                mirrors, udev, sysctl, modules) without reinstalling the image."
   exit "${1:-0}"
 }
 need_root() { [ "$(id -u)" = 0 ] || { echo "run as root (sudo)" >&2; exit 1; }; }
@@ -174,6 +176,17 @@ post_merge() {
   # 'input': writing 'attach' materialises an arbitrary emulated USB device (review 2026-08-05 M-4),
   # so it stays a group users join on purpose — see `ujust add-user-to-input-group` for the other one.
   getent group punktfunk >/dev/null 2>&1 || groupadd --system punktfunk 2>/dev/null || :
+  # Creating the group is necessary but NOT sufficient, and the difference is invisible until a
+  # stream fails: `pf-dm-helper` gates on MEMBERSHIP, so a host whose user never joined gets
+  # "stopping the display manager needs privilege" on every managed takeover — sddm's autologin
+  # Relogin loop then churns logind sessions for the whole stream. Joining stays opt-in (writing
+  # vhci `attach` materialises an arbitrary emulated USB device), so say so instead of doing it.
+  local _pf_user="${SUDO_USER:-}"
+  if [ -n "$_pf_user" ] && ! id -nG "$_pf_user" 2>/dev/null | tr ' ' '\n' | grep -qx punktfunk; then
+    echo "!! $_pf_user is not in the 'punktfunk' group — the managed gamescope takeover cannot stop"
+    echo "!! the display manager, and the virtual Steam Deck pad cannot attach. To opt in:"
+    echo "!!     sudo usermod -aG punktfunk $_pf_user"
+  fi
   modprobe vhci-hcd 2>/dev/null || :
   # Re-fire the vhci rule against the (possibly already-present) controller so attach/detach pick up
   # the input-group ownership even when the module's original add event predated the reloaded rule.
@@ -265,7 +278,22 @@ cmd_update() {
   [ -n "$l" ] || { echo "no image in the feed $(feed_url)" >&2; exit 1; }
   ver="${l%% *}"
   if [ "$ver" = "$cur" ] && merged; then
-    echo "already on $cur (channel $(channel)) — nothing to do."
+    # NOT "nothing to do": re-run post_merge. Every step in it is idempotent, and skipping it here
+    # is how host state silently rots one release behind the image.
+    #
+    # The trap, field-proven on a Bazzite host that took 0.25.0 -> 0.26.0 (2026-08-09): an upgrade
+    # is driven by the script from the OLD image — this file is replaced by the very
+    # `systemd-sysext refresh` that runs mid-upgrade — so a post_merge step ADDED in the new
+    # release is executed by nobody. The old script doesn't have it, and the new script never gets
+    # a turn, because from then on `update` matches this branch and returns. The step is then
+    # permanently unreachable on exactly the installs that need it.
+    #
+    # That cost the `punktfunk` group (added to post_merge in 0.26.0): it was never created, so
+    # `pf-dm-helper` refused every caller — it gates on membership — and every managed gamescope
+    # takeover fell back to "stopping the display manager needs privilege", leaving sddm's autologin
+    # Relogin loop churning for the whole stream.
+    echo "already on $cur (channel $(channel)) — re-applying host state."
+    post_merge
     return
   fi
   echo "updating: ${cur:-<none>} -> $ver"
@@ -311,6 +339,7 @@ cmd_remove() {
 case "${1:-}" in
   install) shift; cmd_install "$@" ;;
   update)  shift; cmd_update "$@" ;;
+  reapply) shift; need_root; post_merge ;;
   status)  shift; cmd_status ;;
   remove)  shift; cmd_remove ;;
   *) usage ;;