Merge pull request #1006 with block rendering fixes

Invalidate cached world-space block vertices when the base translation changes. Share the regular/mesh partition between full uploads and patches so instanced blocks are uploaded only once. Cover both paths with CPU and GPU regression checks.

src/scene/pipeline/gpu_tests.rs

@@ -3,6 +3,95 @@ use crate::scene::model::{mesh_model::MeshLodSet, mesh_model::MeshModel, wire_mo
 use acadrust::{types::Transform, Handle};
 use iced::futures::executor::block_on;
 
+#[test]
+#[ignore = "requires a GPU adapter"]
+fn block_edits_preserve_cache_coordinates_and_arena_partition() {
+    let instance = wgpu::Instance::new(wgpu::InstanceDescriptor::new_without_display_handle());
+    let adapter = block_on(instance.request_adapter(&wgpu::RequestAdapterOptions::default()))
+        .expect("GPU adapter");
+    let (device, queue) = block_on(adapter.request_device(&wgpu::DeviceDescriptor {
+        required_limits: adapter.limits(),
+        ..Default::default()
+    }))
+    .expect("GPU device");
+    let validation = device.push_error_scope(wgpu::ErrorFilter::Validation);
+    let mut pipeline = Pipeline::new(&device, &queue, wgpu::TextureFormat::Bgra8UnormSrgb);
+    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]],
+        render_instance: Some(crate::scene::model::instance_model::RenderInstance {
+            source_id: 1,
+            translation: [x, 0.0, 0.0],
+        }),
+        ..Default::default()
+    };
+    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);
+    assert_eq!(original[0].vertex_buffer, unchanged[0].vertex_buffer);
+    assert_eq!(original[0].instance_count, 2);
+
+    let removed = pipeline.upload_block_wires(&device, &queue, &[&second], &depth);
+    assert_ne!(
+        original[0].vertex_buffer, removed[0].vertex_buffer,
+        "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);
+    assert_ne!(removed[0].vertex_buffer, moved_gpu[0].vertex_buffer);
+    let restored = pipeline.upload_block_wires(&device, &queue, &[&first, &second], &depth);
+    assert_ne!(moved_gpu[0].vertex_buffer, restored[0].vertex_buffer);
+    assert_eq!(restored[0].instance_count, 2);
+    assert_eq!(
+        pipeline.block_geometry.len(),
+        1,
+        "discard obsolete geometry"
+    );
+
+    let line = WireModel {
+        render_instance: None,
+        ..first.clone()
+    };
+    let changed = [second];
+    let (regular, _) = wire_arena::split_wires(&changed);
+    let runs = rustc_hash::FxHashMap::from_iter([(Handle::new(2), regular)]);
+    for layout in [pipeline.wire_const_bgl.as_ref(), None] {
+        let mut arena = wire_arena::PersistentWireArena::build(
+            &device,
+            &queue,
+            &[&line],
+            &depth,
+            layout,
+            false,
+        )
+        .unwrap();
+        assert!(arena.patch(
+            &queue,
+            &[(Handle::new(2), crate::scene::ChangeKind::Added)],
+            &runs,
+            true,
+            &depth
+        ));
+        assert_eq!(
+            arena
+                .wire_gpus()
+                .iter()
+                .map(|gpu| gpu.instance_count)
+                .sum::<u32>(),
+            1,
+            "the block must not also enter the regular arena"
+        );
+    }
+    queue.submit([]);
+    device.poll(wgpu::PollType::wait_indefinitely()).unwrap();
+    assert!(
+        block_on(validation.pop()).is_none(),
+        "GPU validation failed"
+    );
+}
+
 // OCS_GPU_CHUNK_MIB=1 cargo test --lib bounded_gpu_uploads -- --ignored --nocapture
 #[test]
 #[ignore = "requires a GPU adapter and OCS_GPU_CHUNK_MIB=1"]

src/scene/pipeline/mod.rs

@@ -213,6 +213,9 @@ pub struct Pipeline {
     /// shared resident buffer.
     pub(crate) gpu_wires: std::sync::Arc<Vec<WireGpu>>,
     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).
     /// 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.
@@ -2279,6 +2282,7 @@ 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()),
@@ -2331,13 +2335,28 @@ impl Pipeline {
         }
     }
 
-    /// Build the resident wire batches + handle index for `wires`, wrapped in
+    /// Upload block placements while reusing unchanged geometry.
-    /// `Arc` so the caller can cache them by `wire_content_id` and share one
+    pub fn upload_block_wires(
-    /// copy across every slot that renders that content (see
+        &mut self,
-    /// `MultiPipeline::wire_buffer_cache`). Takes `&self` (reads only the const
+        device: &wgpu::Device,
-    /// bind-group layout) so the shared cache — not the slot — owns the result.
+        queue: &wgpu::Queue,
+        wires: &[&WireModel],
+        depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
+    ) -> Vec<BlockWireGpu> {
+        BlockWireGpu::from_wires(
+            device,
+            queue,
+            wires,
+            depth_map,
+            None,
+            &self.block_wire_const_bgl,
+            Some(&mut self.block_geometry),
+        )
+    }
+
+    /// Build resident batches and a handle index shared across viewport slots.
     pub fn build_wire_buffers(
-        &self,
+        &mut self,
         device: &wgpu::Device,
         queue: &wgpu::Queue,
         wires: &[WireModel],
@@ -2397,6 +2416,7 @@ impl Pipeline {
             depth_map,
             None,
             &self.block_wire_const_bgl,
+            Some(&mut self.block_geometry),
         );
 
         // Index handle → wire slots once, here, so the per-hover selection
@@ -2523,6 +2543,7 @@ impl Pipeline {
                 depth_map,
                 Some(tint),
                 &self.block_wire_const_bgl,
+                None,
             )
         } else {
             Vec::new()
@@ -2534,6 +2555,7 @@ impl Pipeline {
             depth_map,
             Some(WireModel::HOVER),
             &self.block_wire_const_bgl,
+            None,
         ));
         self.gpu_selected_block_wires = block_gpu;
         if let Some(started) = perf_started {

src/scene/pipeline/wire_arena.rs

@@ -120,9 +120,15 @@ fn handle_of(w: &WireModel) -> Option<Handle> {
     crate::scene::Scene::handle_from_wire_name(&w.name)
 }
 
-/// True when `w` needs the shaded-mode edge treatment.
+/// Use the same regular/mesh partition for full uploads and changed runs.
-pub fn is_mesh_edge(w: &WireModel, _mesh_names: &rustc_hash::FxHashSet<u64>) -> bool {
+/// Instanced blocks are uploaded separately by `BlockWireGpu`.
-    !w.points.is_empty() && w.fill_is_3d
+pub fn split_wires(wires: &[WireModel]) -> (Vec<&WireModel>, Vec<&WireModel>) {
+    wires
+        .iter()
+        .filter(|wire| {
+            wire.display_visible && !wire.points.is_empty() && wire.render_instance.is_none()
+        })
+        .partition(|wire| !wire.fill_is_3d)
 }
 
 /// True when appending a new entity at the tail could change the image, so the
@@ -1385,6 +1391,45 @@ impl PersistentWireArena {
 mod tests {
     use super::*;
 
+    #[test]
+    fn wire_partition_excludes_blocks_and_preserves_mixed_run_edges() {
+        let line = WireModel {
+            name: "line".into(),
+            points: vec![[0.0; 3], [1.0, 0.0, 0.0]],
+            ..Default::default()
+        };
+        let mesh = WireModel {
+            name: "mesh".into(),
+            fill_is_3d: true,
+            ..line.clone()
+        };
+        let block = WireModel {
+            render_instance: Some(crate::scene::model::instance_model::RenderInstance {
+                source_id: 1,
+                translation: [0.0; 3],
+            }),
+            ..line.clone()
+        };
+        let block_mesh = WireModel {
+            fill_is_3d: true,
+            ..block.clone()
+        };
+        let hidden = WireModel {
+            display_visible: false,
+            ..line.clone()
+        };
+        let wires = [block, line, block_mesh, mesh, hidden, WireModel::default()];
+        let (regular, edges) = split_wires(&wires);
+        assert_eq!(
+            regular.iter().map(|w| w.name.as_str()).collect::<Vec<_>>(),
+            ["line"]
+        );
+        assert_eq!(
+            edges.iter().map(|w| w.name.as_str()).collect::<Vec<_>>(),
+            ["mesh"]
+        );
+    }
+
     fn slab() -> Slab {
         Slab {
             inst_off: 100,

src/scene/pipeline/wire_gpu.rs

@@ -665,6 +665,17 @@ pub(crate) fn wire_draw_depth(
     }
 }
 
+/// Cached block geometry keyed by source, edge mode, colour and base translation.
+/// Vertices use the first instance's world coordinates, so changing that base
+/// must invalidate the geometry as well as rebuild the relative placements.
+pub type BlockGeometryCache = rustc_hash::FxHashMap<
+    (u64, bool, [u32; 4], [u64; 3]),
+    (
+        std::sync::Arc<Vec<(wgpu::Buffer, u32)>>,
+        std::sync::Arc<wgpu::BindGroup>,
+    ),
+>;
+
 impl BlockWireGpu {
     pub fn from_wires(
         device: &wgpu::Device,
@@ -673,8 +684,8 @@ impl BlockWireGpu {
         depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
         color_override: Option<[f32; 4]>,
         const_bgl: &wgpu::BindGroupLayout,
+        mut cache: Option<&mut BlockGeometryCache>,
     ) -> Vec<Self> {
-
         let mut slots: rustc_hash::FxHashMap<(u64, bool), usize> =
             rustc_hash::FxHashMap::default();
         let mut groups: Vec<(bool, Vec<&WireModel>)> = Vec::new();
@@ -699,6 +710,7 @@ impl BlockWireGpu {
         }
 
         let mut out = Vec::new();
+        let mut live_keys = rustc_hash::FxHashSet::default();
         for (mesh_edge, group) in groups {
             let Some(&source) = group.first() else {
                 continue;
@@ -707,6 +719,31 @@ impl BlockWireGpu {
                 continue;
             };
             let color = color_override.unwrap_or(source.color);
+            let cache_key = (
+                base.source_id,
+                mesh_edge,
+                color.map(f32::to_bits),
+                base.translation.map(f64::to_bits),
+            );
+            live_keys.insert(cache_key);
+            // Reuse this definition's geometry when it is already on the GPU.
+            if let Some((chunks, bind_group)) = cache
+                .as_deref()
+                .and_then(|c| c.get(&cache_key))
+                .map(|(v, b)| (std::sync::Arc::clone(v), std::sync::Arc::clone(b)))
+            {
+                let instances = block_instances(&group, base, mesh_edge, depth_map);
+                push_block_batches(
+                    &mut out,
+                    device,
+                    queue,
+                    &chunks,
+                    &instances,
+                    mesh_edge,
+                    &bind_group,
+                );
+                continue;
+            }
             let (segments, mut constant) = emit_wire_native(source, 0, color, 0.0);
             if segments.is_empty() {
                 continue;
@@ -724,13 +761,7 @@ impl BlockWireGpu {
                 };
                 vertices.extend_from_slice(&[vertex; 6]);
             }
-            // Split vertex data into buffers small enough to allocate on
+            // Bound uploads without splitting a segment's six vertices.
-            // low-VRAM devices. Same scheme as the solid-mesh chunking in
-            // `mesh_gpu.rs` (#203) and the fill chunking in `face3d_gpu.rs`
-            // (#358); the block-wire path was never converted, so a
-            // block-heavy drawing produced one multi-hundred-MB buffer.
-            // Round down to a multiple of 6: each segment expands to six
-            // vertices and must not straddle two chunks.
             let max_verts =
                 super::gpu_budget::max_elements_grouped::<BlockWireVertex>(device, 6);
             let vertex_chunks: Vec<(wgpu::Buffer, u32)> = vertices
@@ -748,11 +779,7 @@ impl BlockWireGpu {
                     )
                 })
                 .collect();
-            // Tiny, but size is irrelevant here: once the device is out of
+            // Queue uploads avoid mapping a failed allocation.
-            // memory every allocation fails, including a one-struct uniform,
-            // and create_buffer_init would map the invalid buffer and panic.
-            // This is the allocation that killed the process after the vertex
-            // buffers had already degraded gracefully.
             let const_buffer = super::gpu_upload::upload_buffer(
                 device,
                 queue,
@@ -770,63 +797,107 @@ impl BlockWireGpu {
                     }],
                 },
             ));
-            let instances: Vec<BlockWireInstance> = group
+            let vertex_chunks = std::sync::Arc::new(vertex_chunks);
-                .iter()
+            if let Some(cache) = cache.as_deref_mut() {
-                .filter_map(|wire| {
+                cache.insert(
-                    let instance = wire.render_instance?;
+                    cache_key,
-                    let delta = [
-                        instance.translation[0] - base.translation[0],
-                        instance.translation[1] - base.translation[1],
-                        instance.translation[2] - base.translation[2],
-                    ];
-                    let high = delta.map(|value| value as f32);
-                    Some(BlockWireInstance {
-                        translation: high,
-                        translation_low: [
-                            (delta[0] - high[0] as f64) as f32,
-                            (delta[1] - high[1] as f64) as f32,
-                            (delta[2] - high[2] as f64) as f32,
-                        ],
-                        depth: [
-                            if mesh_edge {
-                                0.0
-                            } else {
-                                wire_draw_depth(wire, depth_map)
-                            },
-                            0.0,
-                        ],
-                    })
-                })
-                .collect();
-            let max_instances = super::gpu_budget::max_elements::<BlockWireInstance>(device);
-            // Build each instance buffer once and reuse it across vertex
-            // chunks, rather than re-uploading it per chunk.
-            let instance_chunks: Vec<(wgpu::Buffer, u32)> = instances
-                .chunks(max_instances)
-                .map(|chunk| {
                     (
-                        instance_buffer(device, queue, "block_wire.instances", chunk),
+                        std::sync::Arc::clone(&vertex_chunks),
-                        chunk.len() as u32,
+                        std::sync::Arc::clone(&const_bind_group),
-                    )
+                    ),
-                })
+                );
-                .collect();
-            for (vertex_buffer, vertex_count) in &vertex_chunks {
-                for (instance_buffer, instance_count) in &instance_chunks {
-                    out.push(Self {
-                        vertex_buffer: vertex_buffer.clone(),
-                        instance_buffer: instance_buffer.clone(),
-                        vertex_count: *vertex_count,
-                        instance_count: *instance_count,
-                        is_3d_mesh_edge: mesh_edge,
-                        const_bind_group: const_bind_group.clone(),
-                    });
-                }
             }
+            let instances = block_instances(&group, base, mesh_edge, depth_map);
+            push_block_batches(
+                &mut out,
+                device,
+                queue,
+                &vertex_chunks,
+                &instances,
+                mesh_edge,
+                &const_bind_group,
+            );
+        }
+        // Release geometry that this build no longer uses.
+        if let Some(cache) = cache {
+            cache.retain(|key, _| live_keys.contains(key));
         }
         out
     }
 }
 
+/// Placements and draw depths relative to the group's first instance.
+fn block_instances(
+    group: &[&WireModel],
+    base: crate::scene::model::instance_model::RenderInstance,
+    mesh_edge: bool,
+    depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
+) -> Vec<BlockWireInstance> {
+    group
+        .iter()
+        .filter_map(|wire| {
+            let instance = wire.render_instance?;
+            let delta = [
+                instance.translation[0] - base.translation[0],
+                instance.translation[1] - base.translation[1],
+                instance.translation[2] - base.translation[2],
+            ];
+            let high = delta.map(|value| value as f32);
+            Some(BlockWireInstance {
+                translation: high,
+                translation_low: [
+                    (delta[0] - high[0] as f64) as f32,
+                    (delta[1] - high[1] as f64) as f32,
+                    (delta[2] - high[2] as f64) as f32,
+                ],
+                depth: [
+                    if mesh_edge {
+                        0.0
+                    } else {
+                        wire_draw_depth(wire, depth_map)
+                    },
+                    0.0,
+                ],
+            })
+        })
+        .collect()
+}
+
+/// One batch per (vertex chunk × instance chunk). Instance buffers are built
+/// once and shared across vertex chunks rather than re-uploaded per chunk.
+fn push_block_batches(
+    out: &mut Vec<BlockWireGpu>,
+    device: &wgpu::Device,
+    queue: &wgpu::Queue,
+    vertex_chunks: &[(wgpu::Buffer, u32)],
+    instances: &[BlockWireInstance],
+    mesh_edge: bool,
+    const_bind_group: &std::sync::Arc<wgpu::BindGroup>,
+) {
+    let max_instances = super::gpu_budget::max_elements::<BlockWireInstance>(device);
+    let instance_chunks: Vec<(wgpu::Buffer, u32)> = instances
+        .chunks(max_instances)
+        .map(|chunk| {
+            (
+                instance_buffer(device, queue, "block_wire.instances", chunk),
+                chunk.len() as u32,
+            )
+        })
+        .collect();
+    for (vertex_buffer, vertex_count) in vertex_chunks {
+        for (instance_buffer, instance_count) in &instance_chunks {
+            out.push(BlockWireGpu {
+                vertex_buffer: vertex_buffer.clone(),
+                instance_buffer: instance_buffer.clone(),
+                vertex_count: *vertex_count,
+                instance_count: *instance_count,
+                is_3d_mesh_edge: mesh_edge,
+                const_bind_group: const_bind_group.clone(),
+            });
+        }
+    }
+}
+
 /// Upload one constant chunk; its instances use chunk-local wire IDs.
 fn build_const_bind_group(
     device: &wgpu::Device,

src/scene/view/render.rs

@@ -654,8 +654,7 @@ impl shader::Primitive for Primitive {
                     && (inner.wire_const_bgl.is_some()
                         || ((vp.wire_patch.is_some()
                             || inner.wire_arena_id != u64::MAX)
-                            && packed_arena_owner))
+                            && packed_arena_owner));
-                    && !vp_wires.iter().any(|wire| wire.render_instance.is_some());
                 if use_wire_arena {
                     use crate::scene::pipeline::wire_arena::{
                         self, PersistentWireArena as WireArena,
@@ -696,21 +695,9 @@ impl shader::Primitive for Primitive {
                                 .get(&handle)
                                 .map(|wires| wires.as_slice())
                                 .unwrap_or(&[]);
-                            let mesh_entity = run
+                            let (regular, mesh) = wire_arena::split_wires(run);
-                                .iter()
+                            regular_changed.insert(handle, regular);
-                                .any(|w| !w.fill_tris.is_empty() && !w.fill_tris_low.is_empty());
+                            mesh_changed.insert(handle, mesh);
-                            regular_changed.insert(
-                                handle,
-                                run.iter()
-                                    .filter(|w| !w.points.is_empty() && !mesh_entity)
-                                    .collect(),
-                            );
-                            mesh_changed.insert(
-                                handle,
-                                run.iter()
-                                    .filter(|w| !w.points.is_empty() && mesh_entity)
-                                    .collect(),
-                            );
                         }
                     }
                     let fallback_touched = |mesh_edge: bool| {
@@ -758,22 +745,7 @@ impl shader::Primitive for Primitive {
                     if !reg_ok || !mesh_ok {
                         // Initial upload or a patch that outgrew arena capacity:
                         // only then pay the full regular/mesh split.
-                        let mesh_names: rustc_hash::FxHashSet<u64> = vp_wires
+                        let (regular, mesh) = wire_arena::split_wires(&vp_wires);
-                            .iter()
-                            .filter(|w| !w.fill_tris.is_empty() && !w.fill_tris_low.is_empty())
-                            .filter_map(|w| w.name.parse::<u64>().ok())
-                            .collect();
-                        let regular: Vec<&crate::scene::WireModel> = vp_wires
-                            .iter()
-                            .filter(|w| {
-                                !w.points.is_empty()
-                                    && !wire_arena::is_mesh_edge(w, &mesh_names)
-                            })
-                            .collect();
-                        let mesh: Vec<&crate::scene::WireModel> = vp_wires
-                            .iter()
-                            .filter(|w| wire_arena::is_mesh_edge(w, &mesh_names))
-                            .collect();
                         if !reg_ok {
                             inner.wire_arena = WireArena::build(
                                 device,
@@ -874,7 +846,19 @@ impl shader::Primitive for Primitive {
                             gpus.extend(arena.wire_gpus());
                         }
                         inner.gpu_wires = std::sync::Arc::new(gpus);
-                        inner.gpu_block_wires = std::sync::Arc::new(Vec::new());
+                        // Blocks keep their own cached geometry and placements.
+                        let instanced: Vec<&crate::scene::WireModel> = vp_wires
+                            .iter()
+                            .filter(|w| w.render_instance.is_some())
+                            .collect();
+                        inner.gpu_block_wires = std::sync::Arc::new(
+                            inner.upload_block_wires(
+                                device,
+                                queue,
+                                &instanced,
+                                &draw_depths,
+                            ),
+                        );
                         if _patched {
                             wire_arena::patch_handle_index(
                                 &mut inner.wire_handle_index,