From a9a2c82478e62a822d78ea01536f29f6c1b9614a Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Sat, 5 Sep 2026 16:26:51 +0300 Subject: [PATCH] fix: share block definition geometry across viewport slots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zooming a paper layout out until several viewports fit on screen ran the card out of memory. The log correlates exactly: every error cluster follows a `nav-prepare … space=Paper` with `viewports=3`, `4` or `5`, where earlier sessions had reported `viewports=2` and stayed inside the budget. Each viewport gets its own `Pipeline` slot, and `block_geometry` was a slot field — so every slot uploaded its own copy of the same definition segments. That is the single largest upload this renderer makes: 1 101 definitions, 1 040 267 segments, 59.5 MiB. Three of them land back to back in one burst in the log, each bracketed by its own full arena rebuild. The cache key is `(source_id, mesh_edge, colour, base translation)` — nothing a viewport contributes. Two viewports drawing the same definition draw the same segments, so the cache belongs beside `wire_buffer_cache` on the `MultiPipeline`, which already shares the finished `BlockWireGpu` batches across slots for the same reason. That existing sharing also settles the one question this raises: bind groups built against one slot's layout are already handed to other slots today, so nothing new is being asked of wgpu here. Both GPU regression tests still pass on a real adapter with validation scopes armed. This does not address the wire arena, which stays per slot and rebuilds 52 MiB of instances each time a slot changes hands — `arena-base ok=false` on every single build in the session, never `ok=true`. Co-Authored-By: Claude Opus 5 --- src/scene/pipeline/gpu_tests.rs | 15 +++++++++------ src/scene/pipeline/mod.rs | 22 ++++++++++++++++------ src/scene/view/render.rs | 9 ++++++++- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/scene/pipeline/gpu_tests.rs b/src/scene/pipeline/gpu_tests.rs index def4a18fc..47d4c6fe2 100644 --- a/src/scene/pipeline/gpu_tests.rs +++ b/src/scene/pipeline/gpu_tests.rs @@ -16,6 +16,9 @@ fn block_edits_preserve_cache_coordinates_and_arena_partition() { .expect("GPU device"); let validation = device.push_error_scope(wgpu::ErrorFilter::Validation); let mut pipeline = Pipeline::new(&device, &queue, wgpu::TextureFormat::Bgra8UnormSrgb); + // Definition geometry is shared across viewport slots, so it lives on the + // MultiPipeline; a single-slot test supplies its own. + let mut block_geometry = wire_gpu::BlockGeometryCache::default(); let block = |handle: u64, x: f64| WireModel { name: handle.to_string(), points: vec![[x as f32, 0.0, 0.0], [x as f32 + 1.0, 0.0, 0.0]], @@ -28,25 +31,25 @@ fn block_edits_preserve_cache_coordinates_and_arena_partition() { let first = block(1, 10.0); let second = block(2, 20.0); let depth = rustc_hash::FxHashMap::from_iter([(1, [0.1, 0.01]), (2, [0.2, 0.01])]); - let original = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth); - let unchanged = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth); + let original = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth, &mut block_geometry); + let unchanged = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth, &mut block_geometry); assert_eq!(original[0].geometry_id(), unchanged[0].geometry_id()); assert_eq!(original[0].instance_count, 2); - let removed = pipeline.upload_block_wires(&device, &queue, &[&second], &depth); + let removed = pipeline.upload_block_wires(&device, &queue, &[&second], &depth, &mut block_geometry); assert_ne!( original[0].geometry_id(), removed[0].geometry_id(), "removing the base instance must replace vertices baked at its old position" ); let moved = block(2, 30.0); - let moved_gpu = pipeline.upload_block_wires(&device, &queue, &[&moved], &depth); + let moved_gpu = pipeline.upload_block_wires(&device, &queue, &[&moved], &depth, &mut block_geometry); assert_ne!(removed[0].geometry_id(), moved_gpu[0].geometry_id()); - let restored = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth); + let restored = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth, &mut block_geometry); assert_ne!(moved_gpu[0].geometry_id(), restored[0].geometry_id()); assert_eq!(restored[0].instance_count, 2); assert_eq!( - pipeline.block_geometry.len(), + block_geometry.len(), 1, "discard obsolete geometry" ); diff --git a/src/scene/pipeline/mod.rs b/src/scene/pipeline/mod.rs index 441704005..b2005ba22 100644 --- a/src/scene/pipeline/mod.rs +++ b/src/scene/pipeline/mod.rs @@ -217,9 +217,6 @@ pub struct Pipeline { /// shared resident buffer. pub(crate) gpu_wires: std::sync::Arc>, pub(crate) gpu_block_wires: std::sync::Arc>, - /// Block geometry that survives an edit, keyed by the definition it - /// expands. See `wire_gpu::BlockGeometryCache`. - pub(crate) block_geometry: wire_gpu::BlockGeometryCache, /// Persistent per-entity wire instance arena (capability-selected format). /// When active, `gpu_wires` is a thin wrapper over this arena's buffers and an /// edit patches one entity's slab in place instead of rebuilding every wire. @@ -2300,7 +2297,6 @@ impl Pipeline { surface_format: format, gpu_wires: std::sync::Arc::new(vec![]), gpu_block_wires: std::sync::Arc::new(vec![]), - block_geometry: Default::default(), wire_arena: None, wire_arena_mesh: None, wire_arena_fallback: std::sync::Arc::new(Vec::new()), @@ -2360,6 +2356,7 @@ impl Pipeline { queue: &wgpu::Queue, wires: &[&WireModel], depth_map: &rustc_hash::FxHashMap, + block_geometry: &mut wire_gpu::BlockGeometryCache, ) -> Vec { BlockWireGpu::from_wires( device, @@ -2371,7 +2368,7 @@ impl Pipeline { const_bgl: &self.block_wire_const_bgl, mode: self.block_wire_mode, }, - Some(&mut self.block_geometry), + Some(block_geometry), ) } @@ -2382,6 +2379,7 @@ impl Pipeline { queue: &wgpu::Queue, wires: &[WireModel], depth_map: &rustc_hash::FxHashMap, + block_geometry: &mut wire_gpu::BlockGeometryCache, ) -> ( std::sync::Arc>, std::sync::Arc>, @@ -2440,7 +2438,7 @@ impl Pipeline { const_bgl: &self.block_wire_const_bgl, mode: self.block_wire_mode, }, - Some(&mut self.block_geometry), + Some(block_geometry), ); // Parse handles in parallel, preserving wire order for hover slot lists. @@ -4830,6 +4828,17 @@ pub struct MultiPipeline { /// once between them instead of once per slot. Kept trim by dropping entries /// no slot still references (`Arc::strong_count == 1`) once it grows past a /// small bound. + /// Block geometry that survives an edit, keyed by the definition it + /// expands rather than by anything a viewport contributes. See + /// `wire_gpu::BlockGeometryCache`. + /// + /// Shared across slots for the same reason `wire_buffer_cache` is: a + /// definition's segments are identical in every viewport that draws it, and + /// this is the largest upload the renderer makes — 59.5 MiB on the + /// reproducer. Held per slot, zooming a paper layout out until four or five + /// viewports were on screen uploaded it once each and ran a 2 GB card out + /// of memory. + pub(crate) block_geometry: wire_gpu::BlockGeometryCache, pub(crate) wire_buffer_cache: rustc_hash::FxHashMap< u64, ( @@ -4985,6 +4994,7 @@ impl iced::widget::shader::Pipeline for MultiPipeline { fn new(device: &wgpu::Device, queue: &wgpu::Queue, format: wgpu::TextureFormat) -> Self { install_gpu_error_handler(device); Self { + block_geometry: Default::default(), inners: vec![Pipeline::new(device, queue, format)], format, slot_by_instance: rustc_hash::FxHashMap::default(), diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index 7d5d9eded..60de2fc0a 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -860,6 +860,7 @@ impl shader::Primitive for Primitive { queue, &instanced, &draw_depths, + &mut pipeline.block_geometry, ), ); if _patched { @@ -936,7 +937,13 @@ impl shader::Primitive for Primitive { let errors_before = crate::scene::pipeline::gpu_errors_seen(); let entry = - inner.build_wire_buffers(device, queue, &vp_wires[..], &draw_depths); + inner.build_wire_buffers( + device, + queue, + &vp_wires[..], + &draw_depths, + &mut pipeline.block_geometry, + ); if let Some(start) = t_upload { crate::perf_record!( "[perf] wire-upload {:.1}ms wires={} content_id={}",