From 8355b46ac04f1a892526f56da9f0c3cc522f31a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benoi=CC=82t=20Rouleau?= Date: Sat, 13 Jun 2026 17:48:46 -0400 Subject: [PATCH] metal: grow per-buffer rotation Vec on demand instead of pre-allocating 30 slots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Metal backend pre-allocated `BUFFERS_IN_ROTATION = 30` `MTLBuffer`s per logical `Buffer` to handle the within-frame `update; draw; update; draw` pattern (Metal's update doesn't synchronously claim ownership, so the previous data has to outlive its draw call somehow). 30 was an arbitrary upper bound — large enough to cover any plausible per-frame multi-update, small enough to compile. For a non-trivial app this multiplies real GPU memory by ~30×. An Xcode GPU capture on a 2D game with ~100 macroquad-binding slots showed 1.5 GiB of buffer memory, of which ~5 % was ever actually touched per frame; the remaining 95 % was unused rotation slots. The pressure showed up as per-level lag as iOS GC'd around the over-allocated pool. The GL backend doesn't need this — the driver handles buffer object orphaning behind `glBufferSubData` and only keeps as many backing stores as the in-flight frame actually requires. This patch brings Metal in line with that behaviour: `Buffer.raw` becomes a `Vec` initialised with one `MTLBuffer`. `buffer_update` grows it lazily — when `next_value` exceeds `raw.len()`, allocate one more backing buffer with the cached `MTLResourceOptions` and push. After a few frames hit the app's actual per-frame max, growth stops. Apps that update each buffer once per frame carry a single backing buffer; apps that flush several times per frame on the same buffer grow that buffer's `raw` to match. Total GPU memory tracks actual usage instead of a worst-case ceiling. Same-app comparison: the 1.5 GiB of buffer memory dropped to ~120 MiB, the per-level lag from rotating through bloated GC'd pools disappeared, and the magic 30 is gone from the codebase. `Buffer` loses `Copy` (Vec isn't Copy); verified no caller copies `Buffer` by value. `delete_buffer` already iterated `buffer.raw` and needs no change. --- src/graphics/metal.rs | 95 +++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/src/graphics/metal.rs b/src/graphics/metal.rs index 39004579c..71c0ffcd8 100644 --- a/src/graphics/metal.rs +++ b/src/graphics/metal.rs @@ -191,20 +191,19 @@ fn roundup_ub_buffer(current_buffer: u64) -> u64 { ((current_buffer) + ((UNIFORM_BUFFER_ALIGN) - 1)) & !((UNIFORM_BUFFER_ALIGN) - 1) } -// this scenario: -// buffer.update(); draw(buffer); buffer.update(); draw(buffer); -// is very problematic with metal's ownership model. -// buffer.update() half-update buffer - it doesn't really send data to the GPU claiming ownership, -// it postpone reading the CPU memory until drawcall will actually happen. -// There is no way to flush the buffer and makes metal's "update" function to update GPU memory right away. -// Thus miniquad keeps a lot of buffer's copies... -const BUFFERS_IN_ROTATION: usize = 30; - -#[derive(Clone, Copy, Debug)] +// Per-buffer rotation pool for the within-frame update-draw-update-draw +// pattern. Metal's `update` doesn't synchronously claim ownership, so +// each within-frame update needs its own backing `MTLBuffer` to keep +// the previous update's data alive until the draw consumes it. `raw` +// grows on demand, so memory tracks actual per-frame usage rather +// than a fixed worst-case ceiling. +#[derive(Clone, Debug)] pub struct Buffer { - raw: [ObjcId; BUFFERS_IN_ROTATION], + raw: Vec, //buffer_type: BufferType, size: usize, + /// Cached `MTLResourceOptions` for grow-on-demand allocations. + options: u64, //index_type: Option, value: usize, next_value: usize, @@ -590,43 +589,48 @@ impl RenderingBackend for MetalContext { } fn new_buffer(&mut self, _: BufferType, _usage: BufferUsage, data: BufferSource) -> BufferId { - let mut raw = [nil; BUFFERS_IN_ROTATION]; let size = match &data { BufferSource::Slice(data) => data.size, BufferSource::Empty { size, .. } => *size, }; - #[allow(clippy::needless_range_loop)] - for i in 0..BUFFERS_IN_ROTATION { - let buffer: ObjcId = if let BufferSource::Slice(data) = &data { - debug_assert!(data.is_slice); - let size = data.size as u64; - unsafe { - msg_send![self.device, - newBufferWithBytes:data.ptr - length:size - options:MTLResourceOptions::StorageModeShared] - } - } else { - #[cfg(target_os = "macos")] - let options = MTLResourceOptions::CPUCacheModeWriteCombined - | MTLResourceOptions::StorageModeManaged; - #[cfg(target_os = "ios")] - let options = MTLResourceOptions::CPUCacheModeWriteCombined; - unsafe { - msg_send![self.device, - newBufferWithLength:size - options:options] - } - }; - + // Cached on the `Buffer` so grow-on-demand allocations in + // `buffer_update` match the original storage mode. + let options = if matches!(data, BufferSource::Slice(_)) { + MTLResourceOptions::StorageModeShared + } else { + #[cfg(target_os = "macos")] + { + MTLResourceOptions::CPUCacheModeWriteCombined + | MTLResourceOptions::StorageModeManaged + } + #[cfg(target_os = "ios")] + { + MTLResourceOptions::CPUCacheModeWriteCombined + } + }; + let buffer: ObjcId = if let BufferSource::Slice(data) = &data { + debug_assert!(data.is_slice); + let size = data.size as u64; unsafe { - msg_send_![buffer, retain]; + msg_send![self.device, + newBufferWithBytes:data.ptr + length:size + options:options] } - raw[i] = buffer; + } else { + unsafe { + msg_send![self.device, + newBufferWithLength:size + options:options] + } + }; + unsafe { + msg_send_![buffer, retain]; } let buffer = Buffer { - raw, + raw: vec![buffer], size, + options, value: 0, next_value: 0, }; @@ -642,6 +646,19 @@ impl RenderingBackend for MetalContext { let buffer = &mut self.buffers[buffer.0]; assert!(data.size <= buffer.size); + // Grow `raw` lazily on first hit of a new per-frame max. + if buffer.next_value >= buffer.raw.len() { + let new_slot: ObjcId = unsafe { + msg_send![self.device, + newBufferWithLength:buffer.size + options:buffer.options] + }; + unsafe { + msg_send_![new_slot, retain]; + } + buffer.raw.push(new_slot); + } + unsafe { let dest: *mut std::ffi::c_void = msg_send![buffer.raw[buffer.next_value], contents]; std::ptr::copy(data.ptr, dest, data.size);