From fbbfce9b0e499cc263ad9044876dea1d42120ba6 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 13 Aug 2026 20:50:09 +0200 Subject: [PATCH] fix(console-ui): Skia sized its function table to the loader, not to what we promised The skia-safe 0.87 -> 0.99 move swapped `BackendContext::new` for `new_builder(..., None)` and recorded the `None` as "byte-for-byte what the (now removed) `BackendContext::new` did". That is true of the VALUE and false of the BEHAVIOUR. `None` leaves Skia's `fMaxAPIVersion` at its `0` sentinel, and the newer Skia acts on that sentinel by falling back to `vkEnumerateInstanceVersion()` -- the LOADER's ceiling, not ours. The presenter declares 1.3; a current Mesa answers 1.4 (1.4.321 on SteamOS 3.7, host and inside the flatpak sandbox alike). Skia then validates a 1.4 function table against an instance that only ever promised 1.3, `vkGetDeviceProcAddr` returns null for the entry points in between, validation fails, and `make_vulkan` hands back `None`. At 0.87 the same sentinel was inert, because that Skia knew nothing of Vulkan 1.4 -- which is why this surfaced the moment 0.28.0 landed. `run.rs` makes an overlay that cannot init fatal for `--browse`, so on the Steam Deck the console home died on update: the Decky panel's button and the gamepad-UI library shortcut both launch `PF_BROWSE=1`, and neither would open. In a stream the same failure only warns, so those sessions quietly lost their stats OSD and capture HUD instead. `pf-presenter`'s `vk` module is `cfg(any(linux, windows))`, so this was never Deck-specific. The presenter now publishes the version an overlay may size itself to as `SharedDevice::api_version`, and `SkiaOverlay::init` passes it instead of `None`. It is `min(what we declared, what the loader reports)`: taking the loader's number alone is this bug, and taking ours alone would break the mirror case, where a 1.1+ loader accepts our 1.3 `apiVersion` as intent even when it cannot deliver 1.3. Three unit tests pin both directions and the no-answer case. The three `API_VERSION_1_3` spellings in setup.rs now read the one constant, so the number the overlay is told can no longer drift from the number we asked for. Measured on the Deck (RADV VANGOGH, loader 1.4.321) with a standalone repro against the shipped crate -- the client build is not needed to see it: vkEnumerateInstanceVersion() -> 1.4.321 ; VkApplicationInfo -> 1.3.0 max_api_version = None => DirectContext NULL max_api_version = Some(1.3) => DirectContext OK Verified: cargo fmt --all --check; and in the pf-lxcheck2 x86_64 container, cargo build + cargo clippy --all-targets -- -D warnings for pf-console-ui and pf-presenter, plus cargo test -p pf-presenter (46 passed). Note that `cargo check -p pf-console-ui` on macOS is vacuous -- every mod in that crate is cfg(linux|windows), so it compiles nothing there. --- crates/pf-console-ui/src/skia_overlay.rs | 21 +++++-- crates/pf-presenter/src/overlay.rs | 11 ++++ crates/pf-presenter/src/vk/mod.rs | 79 ++++++++++++++++++++++++ crates/pf-presenter/src/vk/setup.rs | 8 ++- 4 files changed, 111 insertions(+), 8 deletions(-) diff --git a/crates/pf-console-ui/src/skia_overlay.rs b/crates/pf-console-ui/src/skia_overlay.rs index 1d134eca..20e99ce4 100644 --- a/crates/pf-console-ui/src/skia_overlay.rs +++ b/crates/pf-console-ui/src/skia_overlay.rs @@ -245,11 +245,22 @@ impl Overlay for SkiaOverlay { shared.queue_family_index as usize, ), &get_proc, - // `None` leaves Skia's `fMaxAPIVersion` at its `0` sentinel, so it caps entry-point - // validation at whatever `vkEnumerateInstanceVersion()` reports — byte-for-byte what - // the (now removed) `BackendContext::new` did. The presenter owns the instance and its - // `VkApplicationInfo`, so pinning a version here would just duplicate its choice. - None, + // 🛑 MUST be the presenter's declared version, never `None`. + // + // `None` leaves Skia's `fMaxAPIVersion` at its `0` sentinel, which makes Skia fall + // back to `vkEnumerateInstanceVersion()` — the LOADER's ceiling, not ours. Those are + // not the same number: the presenter asks for 1.3, while a current Mesa loader answers + // 1.4 (1.4.321 on SteamOS 3.7). Skia then validates a 1.4 function table against an + // instance that only ever promised 1.3, `vkGetDeviceProcAddr` returns null for the + // entry points above 1.3, validation fails, and `make_vulkan` hands back `None` — so + // the console UI refuses to start and `--browse` dies with it. + // + // ⚠ The `0` sentinel was harmless at skia-safe 0.87 (that Skia knew nothing of 1.4, so + // clamping to the loader was a no-op) and the 0.99 migration preserved it as + // "byte-for-byte what `BackendContext::new` did" — true of the VALUE, false of the + // BEHAVIOUR. It shipped in 0.28.0 and took the Deck's launcher out. 0.99's own doc for + // this parameter says it should match `VkApplicationInfo::apiVersion`; this is that. + Some(skvk::Version::from(shared.api_version)), ); // SAFETY: the instance/physical-device/device handles come from `shared`, which owns them // and outlives this backend context, and `get_proc` above resolves through those same diff --git a/crates/pf-presenter/src/overlay.rs b/crates/pf-presenter/src/overlay.rs index 13748e52..0e28e576 100644 --- a/crates/pf-presenter/src/overlay.rs +++ b/crates/pf-presenter/src/overlay.rs @@ -26,6 +26,17 @@ pub struct SharedDevice { /// with [`pf_client_core::video::QueueLock::guard`], whose RAII form is what every /// Rust caller wants. pub queue_lock: std::sync::Arc, + /// The Vulkan version an overlay renderer may size its function table to — the lower of + /// [`crate::vk::INSTANCE_API_VERSION`] (what `VkApplicationInfo::apiVersion` declared for + /// `instance`) and what the loader provides. + /// + /// **Cap yourself here; do not ask the loader yourself.** Entry points above this version + /// were never promised to us — `vkGetDeviceProcAddr` returns null for them — so a renderer + /// that probes `vkEnumerateInstanceVersion` instead (a current Mesa answers 1.4 where we + /// asked for 1.3) validates a function table it can never fill and refuses to start. That + /// is exactly how the Skia console UI died in 0.28.0; see the note in `pf-console-ui`'s + /// `SkiaOverlay::init`. + pub api_version: u32, } /// What the overlay may draw this frame — composed by the run loop from session state. diff --git a/crates/pf-presenter/src/vk/mod.rs b/crates/pf-presenter/src/vk/mod.rs index 814cb780..8828d63a 100644 --- a/crates/pf-presenter/src/vk/mod.rs +++ b/crates/pf-presenter/src/vk/mod.rs @@ -43,6 +43,24 @@ mod setup; pub use setup::{list_adapters, probe_decode, AdapterDecode, PresentPref}; +/// The Vulkan version every instance this crate creates declares in +/// `VkApplicationInfo::apiVersion`. +/// +/// 1.3 because Vulkan Video decode and PyroWave's compute kernels both need a 1.3 device. +/// It is deliberately a CEILING as well as a floor: the loader is routinely newer (Mesa 26 +/// answers `vkEnumerateInstanceVersion` with 1.4), but we only ever promised 1.3, so the +/// entry points above it are not ours to call. Anything that must know how far the device +/// side reaches — notably an overlay renderer sizing its own function table — reads this +/// through [`crate::overlay::SharedDevice::api_version`] rather than asking the loader. +pub const INSTANCE_API_VERSION: u32 = vk::API_VERSION_1_3; + +/// The clamp behind [`Presenter::overlay_api_version`], split out so the decision is provable +/// without a device: the answer is the lower of what we declared and what the loader reports, +/// and a loader too old to answer at all (`None`) can only be a 1.0 one. +fn overlay_api_version_of(declared: u32, loader: Option) -> u32 { + declared.min(loader.unwrap_or(vk::API_VERSION_1_0)) +} + /// The video-format probe behind [`AdapterDecode::formats`], re-exported so a caller /// that prints the report does not need its own `pf-vkdecode` dependency (and cannot /// end up printing a DIFFERENT crate version's idea of the flag names). @@ -387,8 +405,30 @@ impl Presenter { queue: self.queue, queue_family_index: self.qfi, queue_lock: self.queue_lock.clone(), + api_version: self.overlay_api_version(), } } + + /// The Vulkan version an overlay renderer may size its function table to: the LOWER of + /// what our instance declared ([`INSTANCE_API_VERSION`]) and what the loader actually + /// provides. + /// + /// Both halves are load-bearing, in opposite directions. Taking only the loader's number + /// is the bug that killed the console UI in 0.28.0 — Mesa answers 1.4 where we asked for + /// 1.3, and the entry points in between resolve to null. Taking only ours would break the + /// mirror case: a loader older than 1.3 still accepts our 1.3 instance (1.1+ loaders treat + /// `apiVersion` as intent, not a contract), and claiming 1.3 to a renderer there promises + /// functions the loader has never heard of. The minimum is the only number that is true on + /// both sides. + fn overlay_api_version(&self) -> u32 { + // SAFETY: per the Vulkan contract above - `vkEnumerateInstanceVersion` is a global + // command taking no handles, resolved through the loaded entry that owns it; it writes + // one `u32` local. Absent (a 1.0 loader) it reports `None` rather than failing. + let loader = unsafe { self.entry.try_enumerate_instance_version() } + .ok() + .flatten(); + overlay_api_version_of(INSTANCE_API_VERSION, loader) + } } impl Drop for Presenter { @@ -449,3 +489,42 @@ impl Drop for Presenter { let _ = &self.entry; } } + +#[cfg(test)] +mod tests { + use super::*; + + /// The 0.28.0 regression, as an assertion: a loader NEWER than the version we declared + /// must not raise the cap. Skia sized its function table to the loader's 1.4 here, then + /// could not resolve the entry points our 1.3 instance never exposed, and the console UI + /// refused to start (Steam Deck, Mesa loader 1.4.321). + #[test] + fn a_newer_loader_never_raises_the_cap() { + let loader = vk::make_api_version(0, 1, 4, 321); + assert_eq!( + overlay_api_version_of(INSTANCE_API_VERSION, Some(loader)), + INSTANCE_API_VERSION + ); + } + + /// The mirror case, which is why this is a `min` and not "just use ours": a 1.1+ loader + /// accepts our 1.3 `apiVersion` as intent even when it cannot deliver 1.3, so promising + /// 1.3 to the overlay there would name functions the loader has never heard of. + #[test] + fn an_older_loader_lowers_the_cap() { + let loader = vk::make_api_version(0, 1, 2, 198); + assert_eq!( + overlay_api_version_of(INSTANCE_API_VERSION, Some(loader)), + loader + ); + } + + /// No `vkEnumerateInstanceVersion` at all is the one thing it can mean: a 1.0 loader. + #[test] + fn a_loader_that_cannot_answer_is_1_0() { + assert_eq!( + overlay_api_version_of(INSTANCE_API_VERSION, None), + vk::API_VERSION_1_0 + ); + } +} diff --git a/crates/pf-presenter/src/vk/setup.rs b/crates/pf-presenter/src/vk/setup.rs index 9930ceaf..baa35c55 100644 --- a/crates/pf-presenter/src/vk/setup.rs +++ b/crates/pf-presenter/src/vk/setup.rs @@ -155,9 +155,11 @@ impl Presenter { // 1.3: Vulkan Video decode and PyroWave's compute kernels both need a 1.3 // device, and the instance version caps what the device can report (any current // loader accepts 1.3 regardless of device support; device-level gating is below). + // `SharedDevice::api_version` republishes this constant to the overlay — keep the + // two the same by construction rather than by two spellings of `API_VERSION_1_3`. let app_info = vk::ApplicationInfo::default() .application_name(&app_name) - .api_version(vk::API_VERSION_1_3); + .api_version(super::INSTANCE_API_VERSION); // HDR10 presentation needs the extended colorspaces at the INSTANCE level. let mut instance_extensions: Vec = instance_extensions.to_vec(); let inst_available = @@ -749,7 +751,7 @@ pub fn probe_decode() -> Result> { let app_name = CString::new("punktfunk-session").unwrap(); let app_info = vk::ApplicationInfo::default() .application_name(&app_name) - .api_version(vk::API_VERSION_1_3); + .api_version(super::INSTANCE_API_VERSION); // SAFETY: per the Vulkan contract above - a create/allocate call on the live device, over // builder structs that are locals outliving the call; the handle it returns is owned by the // value being built here. @@ -902,7 +904,7 @@ pub fn list_adapters() -> Result> { let app_name = CString::new("punktfunk-session").unwrap(); let app_info = vk::ApplicationInfo::default() .application_name(&app_name) - .api_version(vk::API_VERSION_1_3); + .api_version(super::INSTANCE_API_VERSION); // SAFETY: per the Vulkan contract above - the Vulkan handles used here are owned by this type // and live for the call, and every builder struct is a local that outlives it. let instance = unsafe { -- 2.54.0