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 {