From 6572514eb18dc4803f567bc2fd34f4b5c65c3565 Mon Sep 17 00:00:00 2001 From: Joseph <56056764+00xJS@users.noreply.github.com> Date: Tue, 3 Nov 2026 17:55:02 -0700 Subject: [PATCH] GPU: size the memory cap from the card's video memory on Linux (fixes #1487) (#1706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Linux, the discrete-GPU memory cap now comes from the card's video memory: min(3 GiB, ¾ of amdgpu's `mem_info_vram_total`), so a 2 GB RX 550 gets 1.5 GiB. The card is matched by PCI address, falling back to the smallest card with the same ids. Also folds in CodeRabbit follow-ups from #1856 (PatchMatch flags buffer limit preflight), #1490 (arbitrary-map index wrap), #1836 (round_away for |x| >= 2^23) and #3025 (saturating FrameCache byte accounting). Fixes #1487 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> --- crates/gpu/src/fx_tone.rs | 20 ++++- crates/gpu/src/inpaint.rs | 35 ++++++++ crates/gpu/src/memcap.rs | 128 +++++++++++++++++++++++++---- crates/gpu/src/shaders/common.wgsl | 4 + crates/gpu/src/tests.rs | 28 +++++++ crates/ui-egui/src/frames.rs | 4 +- 6 files changed, 198 insertions(+), 21 deletions(-) diff --git a/crates/gpu/src/fx_tone.rs b/crates/gpu/src/fx_tone.rs index f6e7062e2..0274cdb0f 100644 --- a/crates/gpu/src/fx_tone.rs +++ b/crates/gpu/src/fx_tone.rs @@ -190,7 +190,9 @@ fn arbitrary_map_table(map: &[f32], phase: f64) -> Option> { } let at = |v: f32| { let x = (v as f64 * scale + offset).rem_euclid(n as f64); - let i = x.floor() as usize; + // `rem_euclid` can round a tiny negative phase up to `n`: wrap the index like the CPU + // lookup (`color3::arbitrary_map`) so both endpoints come from the start of the map. + let i = x.floor() as usize % n; let a = *map.get(i).unwrap_or(&0.0); let b = *map.get((i + 1).min(n - 1)).unwrap_or(&a); (a + (b - a) * (x - x.floor()) as f32, (b - a) * scale as f32) @@ -435,3 +437,19 @@ fn point(e: &mut Enc, id: &str, ctx: &EffectCtx, b: GBuf) -> Option { } run(e, &p, b, data) } + +#[cfg(test)] +mod map_tests { + use super::arbitrary_map_table; + + /// A phase just below zero rounds `rem_euclid` up to `n` at input 0: the table must hold the + /// map's first entry and the slope towards its second, as the CPU lookup does. + #[test] + fn arbitrary_map_wraps_a_phase_rounded_to_the_end() { + let map = [0.25f32, 0.75, 1.0]; + let table = arbitrary_map_table(&map, -1e-300).unwrap(); + assert_eq!(table[0], 0.0); + assert_eq!(table[1], 0.25, "value at input 0 is map[0]"); + assert_eq!(table[2], 0.5 * 2.0, "slope from map[0] to map[1], times n - 1"); + } +} diff --git a/crates/gpu/src/inpaint.rs b/crates/gpu/src/inpaint.rs index 25cbc31b9..8adbc3069 100644 --- a/crates/gpu/src/inpaint.rs +++ b/crates/gpu/src/inpaint.rs @@ -577,6 +577,12 @@ pub(crate) fn patch_image(g: &GpuContext, img: &Image, hole: &[bool], init: Opti /// The plan of one level: its flags (`bit 0` hole, `bit 1` a target centre) and the dispatch /// bounds of its targets, holes and field. fn plan_level(e: &mut Enc, hole: &[bool], w: u32, h: u32, patch: usize) -> Option { + // Texture and buffer limits are independent: a level that fits as a texture can still need a + // flags buffer over the device's storage binding limit, and that validation error would + // retire the context. Decline the GPU path instead (the fill runs on the CPU). + if !flags_fit(&e.g.device.limits(), w, h) { + return None; + } let (iw, ih) = (w as usize, h as usize); let r = radius(patch, w, h); let near = dilate(hole, iw, ih, r as usize); @@ -585,6 +591,16 @@ fn plan_level(e: &mut Enc, hole: &[bool], w: u32, h: u32, patch: usize) -> Optio Some(Lv { w, h, r, near: near_bb, holes: bbox(hole, iw, ih)?, field: grow(near_bb, r + 1, w, h), flags: e.data(&flags) }) } +/// Whether a level's `w × h` f32 flags buffer (at least the 16 bytes [`Enc::data`] allocates) +/// can be created and bound as one storage buffer on a device with `limits`. +fn flags_fit(limits: &wgpu::Limits, w: u32, h: u32) -> bool { + u64::from(w) + .checked_mul(u64::from(h)) + .and_then(|n| n.checked_mul(4)) + .map(|b| b.max(16)) + .is_some_and(|b| b <= limits.max_buffer_size && b <= limits.max_storage_buffer_binding_size) +} + /// The PatchMatch run at one level: the random stream's seed and the EM round and iteration /// counts. struct PmRun { @@ -727,3 +743,22 @@ pub(crate) fn distance_histogram(g: &GpuContext, field: &Image, near: &[bool], w } out } + +#[cfg(test)] +mod limit_tests { + use super::flags_fit; + + /// A level whose flags buffer is over the storage binding limit is declined before it is + /// created (the default 128 MiB binding holds 33,554,432 flags: 8192 × 4096). + #[test] + fn flags_buffer_is_checked_against_the_device_limits() { + let l = wgpu::Limits::default(); + assert!(flags_fit(&l, 1, 1)); + assert!(flags_fit(&l, 8192, 4096)); + assert!(!flags_fit(&l, 8192, 4097)); + assert!(!flags_fit(&l, 16384, 16384)); + let small = wgpu::Limits { max_buffer_size: 1024, ..wgpu::Limits::default() }; + assert!(flags_fit(&small, 16, 16)); + assert!(!flags_fit(&small, 16, 17)); + } +} diff --git a/crates/gpu/src/memcap.rs b/crates/gpu/src/memcap.rs index 1c2ce6456..9c745322f 100644 --- a/crates/gpu/src/memcap.rs +++ b/crates/gpu/src/memcap.rs @@ -10,10 +10,13 @@ //! be reclaimed; a warning is logged once. //! //! wgpu has no safe query for the adapter's memory size (`max_buffer_size` is capped at `i32::MAX` -//! on Direct3D 12 and Linux/Mesa Vulkan), so the default cap is conservative per device type; +//! on Direct3D 12 and Linux/Mesa Vulkan), so the default cap is conservative per device type, +//! and lower on a discrete card whose memory the OS reports without `unsafe` (Linux's amdgpu +//! driver, in sysfs: a 2–4 GB card ran out of memory and took the system down, #1487); //! `EFFECTCRAFT_GPU_MEMCAP_MB` overrides it (0 = off). Backends without an allocator report //! (Metal, WebGPU, GL) are never gated. +use std::path::Path; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering::Relaxed}; use std::time::Duration; @@ -44,11 +47,11 @@ pub(crate) struct MemCap { impl MemCap { pub(crate) fn new(info: &wgpu::AdapterInfo) -> MemCap { let env = std::env::var(MEMCAP_ENV).ok(); - Self::from_env(env.as_deref(), info.device_type) + Self::from_env(env.as_deref(), info.device_type, vram(info)) } - fn from_env(var: Option<&str>, device_type: wgpu::DeviceType) -> MemCap { - let cap = cap_from_env(var, device_type); + fn from_env(var: Option<&str>, device_type: wgpu::DeviceType, vram: Option) -> MemCap { + let cap = cap_from_env(var, device_type, vram); MemCap { cap, hysteresis: default_hysteresis(cap), @@ -145,12 +148,12 @@ impl MemCap { /// The cap from the `EFFECTCRAFT_GPU_MEMCAP_MB` value: whole MiB (0 turns the gate off), or the /// default for `device_type` when unset, or (with a warning) when not a number or too large. -fn cap_from_env(var: Option<&str>, device_type: wgpu::DeviceType) -> u64 { - let Some(v) = var else { return default_cap(device_type) }; +fn cap_from_env(var: Option<&str>, device_type: wgpu::DeviceType, vram: Option) -> u64 { + let Some(v) = var else { return default_cap(device_type, vram) }; match v.trim().parse::().ok().and_then(|mb| mb.checked_mul(MIB)) { Some(cap) => cap, None => { - let cap = default_cap(device_type); + let cap = default_cap(device_type, vram); log::warn!(target: "effectcraft_gpu", "{MEMCAP_ENV}={v:?} is not a whole number of MiB that fits in 64 bits: using {} MiB", cap / MIB); cap } @@ -159,15 +162,59 @@ fn cap_from_env(var: Option<&str>, device_type: wgpu::DeviceType) -> u64 { /// Discrete cards: 3 GiB, low enough to fire on 4–8 GB cards with headroom for the driver and /// desktop while leaving ~1 GiB above the default 1.5 GiB upload cache + 256 MB pool (larger -/// cards can raise it with `EFFECTCRAFT_GPU_MEMCAP_MB`). Integrated and software adapters share -/// system memory: 2 GiB. -fn default_cap(t: wgpu::DeviceType) -> u64 { +/// cards can raise it with `EFFECTCRAFT_GPU_MEMCAP_MB`), and at most three quarters of the card's +/// memory when it is known (`vram`), so a 2 GB card gets 1.5 GiB (#1487). Integrated and software +/// adapters share system memory: 2 GiB, whatever their carve-out. +fn default_cap(t: wgpu::DeviceType, vram: Option) -> u64 { match t { - wgpu::DeviceType::DiscreteGpu => 3 * GIB, + wgpu::DeviceType::DiscreteGpu => vram.map_or(3 * GIB, |v| (v / 4).saturating_mul(3).min(3 * GIB)), _ => 2 * GIB, } } +/// The adapter's dedicated memory in bytes, where the OS reports it without `unsafe`. +fn vram(info: &wgpu::AdapterInfo) -> Option { + if !cfg!(target_os = "linux") || info.device_type != wgpu::DeviceType::DiscreteGpu { + return None; + } + let bytes = vram_in_sysfs(Path::new("/sys/class/drm"), info.vendor, info.device, &info.device_pci_bus_id); + if let Some(b) = bytes { + log::info!(target: "effectcraft_gpu", "gpu {}: {} MiB of video memory (sysfs)", info.name, b >> 20); + } + bytes +} + +/// `mem_info_vram_total` (amdgpu) of the DRM card under `drm` whose PCI vendor and device ids are +/// `vendor` and `device`. Connector entries (`card1-DP-1`) are skipped. Variants of one model share +/// those ids (the RX 550 ships with 2 and 4 GB), so the card at PCI address `bus` (`uevent`'s +/// `PCI_SLOT_NAME`, wgpu's `device_pci_bus_id`) wins; without one, the smallest matching card. +fn vram_in_sysfs(drm: &Path, vendor: u32, device: u32, bus: &str) -> Option { + let id = |path: &Path| std::fs::read_to_string(path).ok().and_then(|s| u32::from_str_radix(s.trim().trim_start_matches("0x"), 16).ok()); + let slot = |dev: &Path| { + std::fs::read_to_string(dev.join("uevent")).ok()?.lines().find_map(|l| l.strip_prefix("PCI_SLOT_NAME=").map(|s| s.trim().to_ascii_lowercase())) + }; + let mut cards: Vec<_> = std::fs::read_dir(drm) + .ok()? + .flatten() + .map(|e| e.path()) + .filter(|p| p.file_name().and_then(|n| n.to_str()).is_some_and(|n| n.starts_with("card") && !n.contains('-'))) + .collect(); + cards.sort(); + let found: Vec<(Option, u64)> = cards + .iter() + .filter_map(|card| { + let dev = card.join("device"); + if id(&dev.join("vendor")) != Some(vendor) || id(&dev.join("device")) != Some(device) { + return None; + } + let bytes = std::fs::read_to_string(dev.join("mem_info_vram_total")).ok()?.trim().parse::().ok().filter(|&b| b > 0)?; + Some((slot(&dev), bytes)) + }) + .collect(); + let bus = bus.trim().to_ascii_lowercase(); + found.iter().find(|(s, _)| !bus.is_empty() && s.as_deref() == Some(bus.as_str())).map(|&(_, b)| b).or_else(|| found.iter().map(|&(_, b)| b).min()) +} + /// Minimum reclaimable bytes before a wait is worth draining an encoder (1/8 of the cap, /// clamped between 1 MiB and 256 MiB, or 1/4 of caps smaller than 4 MiB). fn default_hysteresis(cap: u64) -> u64 { @@ -180,20 +227,20 @@ mod tests { #[test] fn defaults_and_cadence() { - assert_eq!(default_cap(wgpu::DeviceType::DiscreteGpu), 3 * GIB); - assert_eq!(default_cap(wgpu::DeviceType::IntegratedGpu), 2 * GIB); - let m = MemCap::from_env(None, wgpu::DeviceType::IntegratedGpu); + assert_eq!(default_cap(wgpu::DeviceType::DiscreteGpu, None), 3 * GIB); + assert_eq!(default_cap(wgpu::DeviceType::IntegratedGpu, None), 2 * GIB); + let m = MemCap::from_env(None, wgpu::DeviceType::IntegratedGpu, None); assert_eq!(m.cap(), 2 * GIB); let due = (0..2 * EVERY).filter(|_| m.due()).count(); assert_eq!(due, 2); - let off = MemCap::from_env(Some("0"), wgpu::DeviceType::DiscreteGpu); + let off = MemCap::from_env(Some("0"), wgpu::DeviceType::DiscreteGpu, None); assert_eq!(off.cap(), 0); assert!((0..EVERY).all(|_| !off.due())); } #[test] fn override_is_whole_mib_or_the_default() { - let cap = |v| MemCap::from_env(Some(v), wgpu::DeviceType::DiscreteGpu).cap(); + let cap = |v| MemCap::from_env(Some(v), wgpu::DeviceType::DiscreteGpu, None).cap(); assert_eq!(cap("2048"), 2 * GIB); assert_eq!(cap(" 8 "), 8 * MIB); assert_eq!(cap("17592186044415"), 17592186044415 * MIB, "the largest value that fits"); @@ -204,7 +251,7 @@ mod tests { #[test] fn hysteresis_skips_non_reclaimable_and_unfreed_memory() { - let m = MemCap::from_env(Some("2048"), wgpu::DeviceType::IntegratedGpu); + let m = MemCap::from_env(Some("2048"), wgpu::DeviceType::IntegratedGpu, None); // Upload cache + pool + live frames (2.15 GiB) sit over the 2 GiB cap with only 50 MiB // reclaimable: waiting would free almost nothing, so the gate must stay quiet. let kept = 2150 * MIB; @@ -225,7 +272,7 @@ mod tests { #[test] fn kept_memory_over_the_cap_warns_once_and_never_waits() { - let m = MemCap::from_env(Some("8"), wgpu::DeviceType::IntegratedGpu); + let m = MemCap::from_env(Some("8"), wgpu::DeviceType::IntegratedGpu, None); // Cap 8 MiB: kept alone over the cap with reclaimable under the hysteresis → no wait, warn once. assert!(!m.should_wait(20 * MIB, 20 * MIB)); assert!(m.warned()); @@ -233,4 +280,49 @@ mod tests { assert!(!m.should_wait(20 * MIB, 19 * MIB + MIB / 2)); assert!(m.should_wait(20 * MIB, 4 * MIB), "16 MiB reclaimable is over the hysteresis"); } + + /// #1487: a 2–4 GB card gets three quarters of its memory instead of the 3 GiB default; a + /// larger card keeps the default, and an integrated GPU's carve-out doesn't lower its cap. + #[test] + fn known_video_memory_lowers_the_discrete_cap() { + let d = wgpu::DeviceType::DiscreteGpu; + assert_eq!(default_cap(d, Some(2 * GIB)), 3 * GIB / 2); + assert_eq!(default_cap(d, Some(4 * GIB)), 3 * GIB); + assert_eq!(default_cap(d, Some(16 * GIB)), 3 * GIB, "never above the default"); + assert_eq!(default_cap(wgpu::DeviceType::IntegratedGpu, Some(GIB / 2)), 2 * GIB); + assert_eq!(cap_from_env(None, d, Some(2 * GIB)), 3 * GIB / 2); + assert_eq!(cap_from_env(Some("lots"), d, Some(2 * GIB)), 3 * GIB / 2, "a bad override falls back to it"); + assert_eq!(cap_from_env(Some("4096"), d, Some(2 * GIB)), 4 * GIB, "the override still wins"); + } + + #[test] + fn video_memory_is_read_from_the_matching_drm_card() { + let dir = std::env::temp_dir().join(format!("effectcraft-memcap-{}", std::process::id())); + let card = |name: &str, vendor: &str, device: &str, vram: Option<&str>| { + let dev = dir.join(name).join("device"); + std::fs::create_dir_all(&dev).unwrap(); + std::fs::write(dev.join("vendor"), vendor).unwrap(); + std::fs::write(dev.join("device"), device).unwrap(); + if let Some(v) = vram { + std::fs::write(dev.join("mem_info_vram_total"), v).unwrap(); + } + }; + // An integrated Intel GPU (no amdgpu file), the RX 550 and a connector entry. + card("card0", "0x8086\n", "0x9bc5\n", None); + card("card1", "0x1002\n", "0x699f\n", Some("4294967296\n")); + std::fs::create_dir_all(dir.join("card1-DP-1")).unwrap(); + assert_eq!(vram_in_sysfs(&dir, 0x1002, 0x699f, ""), Some(4 * GIB)); + assert_eq!(vram_in_sysfs(&dir, 0x8086, 0x9bc5, ""), None, "no mem_info_vram_total"); + assert_eq!(vram_in_sysfs(&dir, 0x1002, 0x73bf, ""), None, "no such card"); + assert_eq!(vram_in_sysfs(&dir.join("missing"), 0x1002, 0x699f, ""), None); + // A 2 GB RX 550 next to the 4 GB one: same ids, told apart by PCI address. + card("card2", "0x1002\n", "0x699f\n", Some("2147483648\n")); + std::fs::write(dir.join("card1/device/uevent"), "DRIVER=amdgpu\nPCI_SLOT_NAME=0000:01:00.0\n").unwrap(); + std::fs::write(dir.join("card2/device/uevent"), "DRIVER=amdgpu\nPCI_SLOT_NAME=0000:03:00.0\n").unwrap(); + assert_eq!(vram_in_sysfs(&dir, 0x1002, 0x699f, "0000:01:00.0"), Some(4 * GIB)); + assert_eq!(vram_in_sysfs(&dir, 0x1002, 0x699f, "0000:03:00.0"), Some(2 * GIB)); + assert_eq!(vram_in_sysfs(&dir, 0x1002, 0x699f, ""), Some(2 * GIB), "unknown address: the smaller card"); + assert_eq!(vram_in_sysfs(&dir, 0x1002, 0x699f, "0000:09:00.0"), Some(2 * GIB), "no such address: the smaller card"); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/crates/gpu/src/shaders/common.wgsl b/crates/gpu/src/shaders/common.wgsl index 1d8a701b6..5541a42c4 100644 --- a/crates/gpu/src/shaders/common.wgsl +++ b/crates/gpu/src/shaders/common.wgsl @@ -247,6 +247,10 @@ fn powz(x: f32, y: f32) -> f32 { // that feeds `x - t` into one fma (AMD does), skipping the product's rounding, so `5/6 · 3`, // which rounds to exactly 2.5 on the CPU, came out 0.49999994 above 2 and rounded down (#1605). fn round_away(x: f32) -> f32 { + // From 2^23 up every f32 is an integer, and `t + 0.5` would round to even (8388608 → 8388609). + if (abs(x) >= 8388608.0) { + return x; + } let t = trunc(x); if (x >= 0.0) { return select(t, t + 1.0, x >= t + 0.5); diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 96e49dd7d..35694e8a8 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -69,6 +69,10 @@ fn math_probe(@builtin(global_invocation_id) gid: vec3) { textureStore(out, p, vec4(q, fxs_posterize_bin(q, P.u[0].y), 0.0, 1.0)); return; } + if (P.u[0].x == 2u) { + textureStore(out, p, vec4(round_away(v), 0.0, 0.0, 1.0)); + return; + } textureStore(out, p, vec4(sin_p(v), cos_p(v), 0.0, 1.0)); } "#; @@ -127,6 +131,30 @@ fn alpha_division_and_posterization_match_f32_rounding() { } } +/// `round_away` is `f32::round`, also from 2^23 up where every value is already an integer +/// (integer IDs there must not merge: 8388608 and 8388609 both rounded to 8388609). +#[test] +fn round_away_matches_f32_round_for_large_values() { + let Some(g) = gpu() else { return }; + let mut values = vec![]; + for m in [1.0f32, -1.0] { + for v in [0.0f32, 0.5, 1.5, 2.5, 0.49999997, 4194303.5, 8388607.5, 8388608.0, 8388609.0, 8388610.0, 16777216.0, 3.0e9] { + values.push([m * v, 0.0, 0.0, 1.0]); + } + } + let img = Image { width: values.len() as u32, height: 1, data: values.clone() }; + let input = g.ctx.upload_image(&img).unwrap(); + let mut e = crate::context::Enc::new(&g.ctx); + let out = e.scratch(img.width, 1); + let mut params = crate::context::Params::default(); + params.u[0] = [2, 0, 0, 0]; + e.pixels("math_probe", ¶ms, &input, None, &out, None); + let result = e.download(&out).unwrap(); + for (v, got) in values.iter().zip(result.data) { + assert_eq!(v[0].round(), got[0], "round_away({})", v[0]); + } +} + /// Prove that the numerical fixes run on the device, without per-effect image /// downloads or CPU fallback. Compiler-limited DX12 devices retain their tested /// fallback contract; these kernels must compile on the Vulkan test adapters. diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 6a98abfe6..9aad2a81c 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -235,9 +235,9 @@ impl Cache { self.gpu_pressure.is_none_or(|last| last.elapsed() >= GPU_PRESSURE_COOLDOWN) } fn forget(&mut self, img: &FrameImage) { - self.bytes -= img.bytes(); + self.bytes = self.bytes.saturating_sub(img.bytes()); if img.is_gpu() { - self.gpu_bytes -= img.bytes(); + self.gpu_bytes = self.gpu_bytes.saturating_sub(img.bytes()); } } fn remove(&mut self, k: &FrameKey) {