forked from 1b-agent/opencadstudio-upstream
fix: share block definition geometry across viewport slots
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
d9c998fa17
commit
a9a2c82478
3 files changed
+33
-13
No files matched your search
@@ -16,6 +16,9 @@ fn block_edits_preserve_cache_coordinates_and_arena_partition() {
|
|||||||
.expect("GPU device");
|
.expect("GPU device");
|
||||||
let validation = device.push_error_scope(wgpu::ErrorFilter::Validation);
|
let validation = device.push_error_scope(wgpu::ErrorFilter::Validation);
|
||||||
let mut pipeline = Pipeline::new(&device, &queue, wgpu::TextureFormat::Bgra8UnormSrgb);
|
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 {
|
let block = |handle: u64, x: f64| WireModel {
|
||||||
name: handle.to_string(),
|
name: handle.to_string(),
|
||||||
points: vec![[x as f32, 0.0, 0.0], [x as f32 + 1.0, 0.0, 0.0]],
|
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 first = block(1, 10.0);
|
||||||
let second = block(2, 20.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 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 original = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth, &mut block_geometry);
|
||||||
let unchanged = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth);
|
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].geometry_id(), unchanged[0].geometry_id());
|
||||||
assert_eq!(original[0].instance_count, 2);
|
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!(
|
assert_ne!(
|
||||||
original[0].geometry_id(),
|
original[0].geometry_id(),
|
||||||
removed[0].geometry_id(),
|
removed[0].geometry_id(),
|
||||||
"removing the base instance must replace vertices baked at its old position"
|
"removing the base instance must replace vertices baked at its old position"
|
||||||
);
|
);
|
||||||
let moved = block(2, 30.0);
|
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());
|
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_ne!(moved_gpu[0].geometry_id(), restored[0].geometry_id());
|
||||||
assert_eq!(restored[0].instance_count, 2);
|
assert_eq!(restored[0].instance_count, 2);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
pipeline.block_geometry.len(),
|
block_geometry.len(),
|
||||||
1,
|
1,
|
||||||
"discard obsolete geometry"
|
"discard obsolete geometry"
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -217,9 +217,6 @@ pub struct Pipeline {
|
|||||||
/// shared resident buffer.
|
/// shared resident buffer.
|
||||||
pub(crate) gpu_wires: std::sync::Arc<Vec<WireGpu>>,
|
pub(crate) gpu_wires: std::sync::Arc<Vec<WireGpu>>,
|
||||||
pub(crate) gpu_block_wires: std::sync::Arc<Vec<BlockWireGpu>>,
|
pub(crate) gpu_block_wires: std::sync::Arc<Vec<BlockWireGpu>>,
|
||||||
/// 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).
|
/// Persistent per-entity wire instance arena (capability-selected format).
|
||||||
/// When active, `gpu_wires` is a thin wrapper over this arena's buffers and an
|
/// 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.
|
/// edit patches one entity's slab in place instead of rebuilding every wire.
|
||||||
@@ -2300,7 +2297,6 @@ impl Pipeline {
|
|||||||
surface_format: format,
|
surface_format: format,
|
||||||
gpu_wires: std::sync::Arc::new(vec![]),
|
gpu_wires: std::sync::Arc::new(vec![]),
|
||||||
gpu_block_wires: std::sync::Arc::new(vec![]),
|
gpu_block_wires: std::sync::Arc::new(vec![]),
|
||||||
block_geometry: Default::default(),
|
|
||||||
wire_arena: None,
|
wire_arena: None,
|
||||||
wire_arena_mesh: None,
|
wire_arena_mesh: None,
|
||||||
wire_arena_fallback: std::sync::Arc::new(Vec::new()),
|
wire_arena_fallback: std::sync::Arc::new(Vec::new()),
|
||||||
@@ -2360,6 +2356,7 @@ impl Pipeline {
|
|||||||
queue: &wgpu::Queue,
|
queue: &wgpu::Queue,
|
||||||
wires: &[&WireModel],
|
wires: &[&WireModel],
|
||||||
depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
|
depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
|
||||||
|
block_geometry: &mut wire_gpu::BlockGeometryCache,
|
||||||
) -> Vec<BlockWireGpu> {
|
) -> Vec<BlockWireGpu> {
|
||||||
BlockWireGpu::from_wires(
|
BlockWireGpu::from_wires(
|
||||||
device,
|
device,
|
||||||
@@ -2371,7 +2368,7 @@ impl Pipeline {
|
|||||||
const_bgl: &self.block_wire_const_bgl,
|
const_bgl: &self.block_wire_const_bgl,
|
||||||
mode: self.block_wire_mode,
|
mode: self.block_wire_mode,
|
||||||
},
|
},
|
||||||
Some(&mut self.block_geometry),
|
Some(block_geometry),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2382,6 +2379,7 @@ impl Pipeline {
|
|||||||
queue: &wgpu::Queue,
|
queue: &wgpu::Queue,
|
||||||
wires: &[WireModel],
|
wires: &[WireModel],
|
||||||
depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
|
depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
|
||||||
|
block_geometry: &mut wire_gpu::BlockGeometryCache,
|
||||||
) -> (
|
) -> (
|
||||||
std::sync::Arc<Vec<WireGpu>>,
|
std::sync::Arc<Vec<WireGpu>>,
|
||||||
std::sync::Arc<Vec<BlockWireGpu>>,
|
std::sync::Arc<Vec<BlockWireGpu>>,
|
||||||
@@ -2440,7 +2438,7 @@ impl Pipeline {
|
|||||||
const_bgl: &self.block_wire_const_bgl,
|
const_bgl: &self.block_wire_const_bgl,
|
||||||
mode: self.block_wire_mode,
|
mode: self.block_wire_mode,
|
||||||
},
|
},
|
||||||
Some(&mut self.block_geometry),
|
Some(block_geometry),
|
||||||
);
|
);
|
||||||
|
|
||||||
// Parse handles in parallel, preserving wire order for hover slot lists.
|
// 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
|
/// 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
|
/// no slot still references (`Arc::strong_count == 1`) once it grows past a
|
||||||
/// small bound.
|
/// 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<
|
pub(crate) wire_buffer_cache: rustc_hash::FxHashMap<
|
||||||
u64,
|
u64,
|
||||||
(
|
(
|
||||||
@@ -4985,6 +4994,7 @@ impl iced::widget::shader::Pipeline for MultiPipeline {
|
|||||||
fn new(device: &wgpu::Device, queue: &wgpu::Queue, format: wgpu::TextureFormat) -> Self {
|
fn new(device: &wgpu::Device, queue: &wgpu::Queue, format: wgpu::TextureFormat) -> Self {
|
||||||
install_gpu_error_handler(device);
|
install_gpu_error_handler(device);
|
||||||
Self {
|
Self {
|
||||||
|
block_geometry: Default::default(),
|
||||||
inners: vec![Pipeline::new(device, queue, format)],
|
inners: vec![Pipeline::new(device, queue, format)],
|
||||||
format,
|
format,
|
||||||
slot_by_instance: rustc_hash::FxHashMap::default(),
|
slot_by_instance: rustc_hash::FxHashMap::default(),
|
||||||
|
|||||||
@@ -860,6 +860,7 @@ impl shader::Primitive for Primitive {
|
|||||||
queue,
|
queue,
|
||||||
&instanced,
|
&instanced,
|
||||||
&draw_depths,
|
&draw_depths,
|
||||||
|
&mut pipeline.block_geometry,
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
if _patched {
|
if _patched {
|
||||||
@@ -936,7 +937,13 @@ impl shader::Primitive for Primitive {
|
|||||||
let errors_before =
|
let errors_before =
|
||||||
crate::scene::pipeline::gpu_errors_seen();
|
crate::scene::pipeline::gpu_errors_seen();
|
||||||
let entry =
|
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 {
|
if let Some(start) = t_upload {
|
||||||
crate::perf_record!(
|
crate::perf_record!(
|
||||||
"[perf] wire-upload {:.1}ms wires={} content_id={}",
|
"[perf] wire-upload {:.1}ms wires={} content_id={}",
|
||||||
|
|||||||
Reference in new issue
Block a user