perf(render): arena handles add/remove too, not just in-place edits

The in-place-only arena still fell back to a full batched rebuild on add/remove —
re-uploading the entire wire instance buffer, which on a drawing whose wires
expand to millions of segments (~265 MB) stalls ~1s per add/remove on a
shared-memory GPU. Only move/edit avoided it.

Add append/tombstone so add/remove patch the instance buffer too: a new or
segment-count-changed entity bump-allocates a slab at the tail (buffers carry
1.5x headroom); an erased entity is tombstoned (blanked at const slot 0). The
huge instance buffer only grows/blanks by the changed entity.

Three correctness points make add/remove safe (each verified by an adversarial
review, findings folded in):
- draw_depth_map normalises each entity`s z-bias by the block entity count, so a
  structural change re-scales every entity`s bias. Keep a CPU mirror of the const
  array; on any structural change refresh every slab`s draw_depth and re-upload
  the small (~1 MB) const buffer, leaving the instance buffer untouched.
- a tail-appended entity draws last, which only mis-orders wires whose overlap
  resolves by submission order rather than the z-bias: transparency, and 3D-solid
  fallback edges (Solid3D/Region/Body/Surface are excluded from draw_depth_map, so
  their opaque edges share depth 0). `append_unsafe` bails to a full rebuild when
  the set contains either.
- an in-place edit also flushes its consts to the GPU (a colour change is not
  structural, so the whole-buffer refresh would otherwise skip it).

Still behind OCS_WIRE_GPU_PATCH (default off); every uncertain case returns false
and the caller does the identical full rebuild, so correctness never rides on the
fast path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/scene/pipeline/wire_arena.rs

@@ -1,20 +1,39 @@
 // Persistent per-entity wire instance arena (native, behind OCS_WIRE_GPU_PATCH).
 //
-// The normal wire path re-emits EVERY wire into a fresh, tightly-packed instance
+// The normal wire path re-emits EVERY wire into a fresh instance buffer whenever
-// buffer whenever the resident set's content id changes — so moving one line on a
+// the resident set's content id changes — so any edit on a drawing whose wires
-// million-wire drawing re-uploads all million instances. This arena instead keeps
+// expand to millions of segments re-uploads the whole (hundreds-of-MB) buffer,
-// one persistent instance buffer (plus its shared WireConst storage) with per-
+// which stalls for ~1s on a shared-memory GPU. This arena instead keeps one
-// entity *slabs*, so an in-place edit is a `queue.write_buffer` of just that
+// persistent instance buffer (plus its shared WireConst storage) laid out as
-// entity's slab.
+// per-entity *slabs*, so an edit only writes what changed:
 //
-// Scope (v1): IN-PLACE patches only. A move / rotate / scale / colour change keeps
+//   * Modify in place — a move / rotate / scale / colour change keeps the
-// an entity's slab, its instance count, and — crucially — the block's entity count,
+//     entity's segment count, so its slab is overwritten where it sits.
-// so `draw_depth_map`'s per-entity z-bias (normalised by that count) is unchanged
+//   * Add / a Modify whose segment count changed — bump-allocate a fresh slab at
-// and every other slab stays correct. `patch` returns false (⇒ the caller does a
+//     the tail (tombstone the old one); the instance buffer only grows by that
-// full batched rebuild) for anything else: an Add or Remove (which re-scales every
+//     entity.
-// entity's z-bias), a segment-count change, or a set that isn't a single scissor-
+//   * Erase — tombstone the slab (blank instances that render nothing).
-// free batch (mesh-edge fills, paper viewport scissor). Because a full rebuild is
+//
-// always the fallback, correctness never rides on the fast path.
+// Two correctness points make add/remove safe:
+//   * draw_depth_map normalises each entity's draw-order z-bias by the block's
+//     entity count, so ANY add/remove re-scales EVERY entity's bias. We keep a
+//     CPU mirror of the WireConst array and, on a structural change, refresh every
+//     slab's draw_depth and re-upload the (small, ~1 MB) const buffer — the huge
+//     instance buffer is untouched.
+//   * A tail-appended entity draws last, which only mis-orders alpha-blended /
+//     coincident wires. So when the set contains ANY transparent wire we bail to a
+//     full rebuild instead of appending. Opaque overlap resolves by the z-bias, so
+//     it is order-independent and safe to relocate.
+//
+// A tombstoned instance points at const slot 0 (a blank WireConst, half_width 0),
+// so the shader expands it to a zero-area quad — no pixels. When tombstone waste
+// or capacity is exceeded, `patch` returns false and the caller compacts via a
+// full rebuild. Because a full rebuild is always the fallback, correctness never
+// rides on the fast path.
+//
+// Scope: a SINGLE batch — the set must have no mesh-edge fills (which force the
+// draw-order-preserving multi-batch split) and no per-wire scissor (paper content
+// viewports). Mixed 2D/3D or scissored sets fall back to the batched path.
 
 #![cfg(not(target_arch = "wasm32"))]
 
@@ -25,6 +44,13 @@ use acadrust::Handle;
 use iced::wgpu;
 use rustc_hash::FxHashMap;
 
+/// Spare capacity multiplier when (re)allocating, so a run of adds appends
+/// without reallocating each time.
+const HEADROOM_NUM: u64 = 3;
+const HEADROOM_DEN: u64 = 2;
+const MIN_INST_CAP: u64 = 4096;
+const MIN_CONST_CAP: u64 = 1024;
+
 struct Slab {
     inst_off: u32,
     inst_len: u32,
@@ -34,10 +60,19 @@ struct Slab {
 
 pub struct WireArena {
     inst_buf: wgpu::Buffer,
+    inst_cap: u32,
     inst_tail: u32,
     const_buf: wgpu::Buffer,
     const_bind_group: std::sync::Arc<wgpu::BindGroup>,
+    const_cap: u32,
+    const_tail: u32,
+    /// CPU mirror of the const buffer so a structural edit can refresh every
+    /// slab's draw_depth (denominator change) without re-emitting geometry.
+    consts_cpu: Vec<WireConst>,
     slabs: FxHashMap<Handle, Slab>,
+    /// Tombstoned instances (blanked, not reclaimed) — past half the tail a patch
+    /// bails so the caller compacts with a full rebuild.
+    tombstoned: u32,
 }
 
 fn handle_of(w: &WireModel) -> Option<Handle> {
@@ -45,19 +80,30 @@ fn handle_of(w: &WireModel) -> Option<Handle> {
 }
 
 /// True if the whole set is arena-eligible: a single batch that draws with no
-/// viewport scissor — no mesh/solid fill (which forces the multi-batch draw-order
+/// viewport scissor — no mesh/solid fill and no per-wire scissor.
-/// split) and no per-wire scissor (paper content viewports, which the arena's
-/// scissor-free wrapper would fail to clip). Model tiles qualify; scissored paper
-/// viewports fall back to the normal batched path.
 pub fn is_arena_eligible(wires: &[WireModel]) -> bool {
     wires
         .iter()
         .all(|w| w.fill_tris.is_empty() && w.vp_scissor.is_none())
 }
 
+/// True when appending a new entity at the tail could change the image, so the
+/// arena must fall back to a full rebuild instead of relocating a slab. Two
+/// cases where draw order (not the z-bias) decides the winning pixel:
+///   * transparency — alpha blends in submission order;
+///   * a wire with NO draw-order depth — 3D solids (Solid3D / Region / Body /
+///     Surface) are excluded from `draw_depth_map`, so their fallback edge wires
+///     get draw_depth 0.0. Two coincident opaque such wires share a z-bias and
+///     resolve by submission order, which a tail relocation would flip.
+fn append_unsafe(wires: &[WireModel], depth_map: &FxHashMap<u64, f32>) -> bool {
+    wires.iter().any(|w| {
+        w.color[3] < 0.999
+            || handle_of(w).map_or(true, |h| !depth_map.contains_key(&h.value()))
+    })
+}
+
 /// handle → wire-slot index for the selection / text-highlight overlays, built
-/// from the resident Vec (independent of the arena's slab layout, exactly like
+/// from the resident Vec (independent of the arena's slab layout).
-/// the batched path's index).
 pub fn build_handle_index(wires: &[WireModel]) -> std::sync::Arc<FxHashMap<u64, Vec<u32>>> {
     let mut index: FxHashMap<u64, Vec<u32>> = FxHashMap::default();
     index.reserve(wires.len());
@@ -85,7 +131,7 @@ fn handle_ranges(wires: &[WireModel]) -> Option<Vec<(Handle, usize, usize)>> {
     Some(out)
 }
 
-fn build_const_bind_group(
+fn make_const_bg(
     device: &wgpu::Device,
     bgl: &wgpu::BindGroupLayout,
     buf: &wgpu::Buffer,
@@ -100,28 +146,46 @@ fn build_const_bind_group(
     }))
 }
 
-fn alloc_inst(device: &wgpu::Device, cap: u32) -> wgpu::Buffer {
+fn alloc_inst(device: &wgpu::Device, cap: u64) -> wgpu::Buffer {
     device.create_buffer(&wgpu::BufferDescriptor {
         label: Some("wire_arena.ibuf"),
-        size: (cap as u64) * std::mem::size_of::<WireInstance>() as u64,
+        size: cap * std::mem::size_of::<WireInstance>() as u64,
         usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
         mapped_at_creation: false,
     })
 }
 
-fn alloc_const(device: &wgpu::Device, cap: u32) -> wgpu::Buffer {
+fn alloc_const(device: &wgpu::Device, cap: u64) -> wgpu::Buffer {
     device.create_buffer(&wgpu::BufferDescriptor {
         label: Some("wire_arena.cbuf"),
-        size: (cap as u64) * std::mem::size_of::<WireConst>() as u64,
+        size: cap * std::mem::size_of::<WireConst>() as u64,
         usage: wgpu::BufferUsages::STORAGE | wgpu::BufferUsages::COPY_DST,
         mapped_at_creation: false,
     })
 }
 
+fn blank_const() -> WireConst {
+    <WireConst as bytemuck::Zeroable>::zeroed()
+}
+
+/// A blank instance: zero-length segment at const slot 0 (half_width 0) — the
+/// shader expands it to a zero-area quad, so it rasterises nothing.
+fn blank_instance() -> WireInstance {
+    WireInstance {
+        pos_a: [0.0; 3],
+        pos_a_low: [0.0; 3],
+        pos_b: [0.0; 3],
+        pos_b_low: [0.0; 3],
+        distance_a: 0.0,
+        distance_b: 0.0,
+        wire_id: 0,
+    }
+}
+
 impl WireArena {
-    /// Build a fresh arena from the full resident set, or `None` if it isn't
+    /// Build a fresh arena from the full resident set, or `None` if it isn't a
-    /// single-batch (has mesh-edge fills) or a wire is unnamed — in which case
+    /// single scissor-free batch or a wire is unnamed (caller keeps the batched
-    /// the caller keeps using the normal batched path.
+    /// path).
     pub fn build(
         device: &wgpu::Device,
         queue: &wgpu::Queue,
@@ -134,18 +198,19 @@ impl WireArena {
         }
         let ranges = handle_ranges(wires)?;
 
+        // const slot 0 = blank tombstone target.
         let mut instances: Vec<WireInstance> = Vec::new();
-        let mut consts: Vec<WireConst> = Vec::new();
+        let mut consts_cpu: Vec<WireConst> = vec![blank_const()];
         let mut slabs: FxHashMap<Handle, Slab> = FxHashMap::default();
         for (h, i, j) in ranges {
             let inst_off = instances.len() as u32;
-            let const_off = consts.len() as u32;
+            let const_off = consts_cpu.len() as u32;
             for w in &wires[i..j] {
-                let wire_id = consts.len() as u32;
+                let wire_id = consts_cpu.len() as u32;
                 let dd = wire_draw_depth(w, depth_map);
                 let (mut insts, cst) = emit_wire_native(w, wire_id, w.color, dd);
                 instances.append(&mut insts);
-                consts.push(cst);
+                consts_cpu.push(cst);
             }
             slabs.insert(
                 h,
@@ -153,33 +218,49 @@ impl WireArena {
                     inst_off,
                     inst_len: instances.len() as u32 - inst_off,
                     const_off,
-                    const_len: consts.len() as u32 - const_off,
+                    const_len: consts_cpu.len() as u32 - const_off,
                 },
             );
         }
 
-        // In-place-only: allocate exactly the set's size (no append headroom).
         let inst_tail = instances.len() as u32;
-        let inst_buf = alloc_inst(device, inst_tail.max(1));
+        let const_tail = consts_cpu.len() as u32;
-        let const_buf = alloc_const(device, (consts.len() as u32).max(1));
+        let inst_cap = ((inst_tail as u64 * HEADROOM_NUM / HEADROOM_DEN).max(MIN_INST_CAP)) as u32;
+        let const_cap =
+            ((const_tail as u64 * HEADROOM_NUM / HEADROOM_DEN).max(MIN_CONST_CAP)) as u32;
+        let inst_buf = alloc_inst(device, inst_cap as u64);
+        let const_buf = alloc_const(device, const_cap as u64);
         if inst_tail > 0 {
             queue.write_buffer(&inst_buf, 0, bytemuck::cast_slice(&instances));
         }
-        queue.write_buffer(&const_buf, 0, bytemuck::cast_slice(&consts));
+        queue.write_buffer(&const_buf, 0, bytemuck::cast_slice(&consts_cpu));
-        let const_bind_group = build_const_bind_group(device, const_bgl, &const_buf);
+        let const_bind_group = make_const_bg(device, const_bgl, &const_buf);
 
         Some(Self {
             inst_buf,
+            inst_cap,
             inst_tail,
             const_buf,
             const_bind_group,
+            const_cap,
+            const_tail,
+            consts_cpu,
             slabs,
+            tombstoned: 0,
         })
     }
 
-    /// Apply the changed handles to the arena in place. Returns false (⇒ the
+    fn write_insts(&self, queue: &wgpu::Queue, off: u32, data: &[WireInstance]) {
-    /// caller does a full batched rebuild) for any change that isn't an in-place
+        if data.is_empty() {
-    /// slab overwrite — see the module header.
+            return;
+        }
+        let sz = std::mem::size_of::<WireInstance>() as u64;
+        queue.write_buffer(&self.inst_buf, off as u64 * sz, bytemuck::cast_slice(data));
+    }
+
+    /// Apply the changed handles in place; returns false (⇒ full rebuild) when the
+    /// arena can't absorb the change: not eligible, a transparent append, a
+    /// capacity overflow, or too much tombstone waste.
     pub fn patch(
         &mut self,
         queue: &wgpu::Queue,
@@ -195,60 +276,122 @@ impl WireArena {
         };
         let range_of: FxHashMap<Handle, (usize, usize)> =
             ranges.into_iter().map(|(h, i, j)| (h, (i, j))).collect();
+        let append_unsafe = append_unsafe(wires, depth_map);
 
-        let inst_sz = std::mem::size_of::<WireInstance>() as u64;
+        let mut structural = false;
-        let const_sz = std::mem::size_of::<WireConst>() as u64;
-
-        // Only IN-PLACE updates are safe to patch: an entity keeps its slab and
-        // its instance/const count (a move / rotate / scale / colour change).
-        // Add / Remove / any count-changed edit alter the layout AND — because
-        // draw_depth_map normalises each entity's z-bias by the block's entity
-        // count — re-scale EVERY entity's draw order, which we can't patch slab-
-        // by-slab. Those fall back to a full rebuild (return false), where the
-        // batched path re-emits everything with the current depth map.
         for &(h, kind) in changes {
-            if matches!(kind, ChangeKind::Added | ChangeKind::Removed) {
+            let new_range = range_of.get(&h).copied();
-                return false;
+
+            // Removed / now-hidden ⇒ tombstone the slab.
+            if matches!(kind, ChangeKind::Removed) || new_range.is_none() {
+                if let Some(slab) = self.slabs.remove(&h) {
+                    let blanks = vec![blank_instance(); slab.inst_len as usize];
+                    self.write_insts(queue, slab.inst_off, &blanks);
+                    self.tombstoned += slab.inst_len;
+                    structural = true;
                 }
-            let Some(&(i, j)) = range_of.get(&h) else {
+                continue;
-                return false; // Modified but now absent (hidden / not in set)
+            }
-            };
+            let (i, j) = new_range.unwrap();
-            let Some(&Slab {
-                inst_off,
-                inst_len,
-                const_off,
-                const_len,
-            }) = self.slabs.get(&h)
-            else {
-                return false; // no existing slab to overwrite
-            };
             let run = &wires[i..j];
 
-            // Emit into the entity's existing const slots (wire_id = const_off + k).
+            // Emit into fresh, run-local const slots (patched to absolute below).
             let mut insts: Vec<WireInstance> = Vec::new();
             let mut csts: Vec<WireConst> = Vec::new();
             for w in run {
-                let wire_id = const_off + csts.len() as u32;
+                let wire_id = csts.len() as u32;
                 let dd = wire_draw_depth(w, depth_map);
                 let (mut wi, c) = emit_wire_native(w, wire_id, w.color, dd);
                 insts.append(&mut wi);
                 csts.push(c);
             }
-            if insts.len() as u32 != inst_len || csts.len() as u32 != const_len {
+            let inst_len = insts.len() as u32;
-                return false; // segment count changed ⇒ not in place ⇒ rebuild
+            let const_len = csts.len() as u32;
+
+            let in_place = self
+                .slabs
+                .get(&h)
+                .map(|s| s.inst_len == inst_len && s.const_len == const_len)
+                .unwrap_or(false);
+
+            if in_place {
+                let (inst_off, const_off) = {
+                    let s = self.slabs.get(&h).unwrap();
+                    (s.inst_off, s.const_off)
+                };
+                for w in insts.iter_mut() {
+                    w.wire_id += const_off;
                 }
-            if inst_len > 0 {
+                self.write_insts(queue, inst_off, &insts);
-                queue.write_buffer(
+                for (k, c) in csts.iter().enumerate() {
-                    &self.inst_buf,
+                    self.consts_cpu[const_off as usize + k] = *c;
-                    inst_off as u64 * inst_sz,
-                    bytemuck::cast_slice(&insts),
-                );
                 }
+                // Push the entity's consts to the GPU too — an in-place edit is
+                // NOT structural, so the whole-buffer refresh below won't run and
+                // a colour change would otherwise never reach the shader.
+                let csz = std::mem::size_of::<WireConst>() as u64;
                 queue.write_buffer(
                     &self.const_buf,
-                const_off as u64 * const_sz,
+                    const_off as u64 * csz,
                     bytemuck::cast_slice(&csts),
                 );
+                continue;
+            }
+
+            // Layout changed ⇒ append at the tail. Unsafe to relocate when the set
+            // resolves overlap by submission order (transparency, or a wire with
+            // no draw-order depth); fall back to a full rebuild instead.
+            if append_unsafe {
+                return false;
+            }
+            if self.inst_tail + inst_len > self.inst_cap
+                || self.const_tail + const_len > self.const_cap
+            {
+                return false;
+            }
+            structural = true;
+            if let Some(s) = self.slabs.remove(&h) {
+                let blanks = vec![blank_instance(); s.inst_len as usize];
+                self.write_insts(queue, s.inst_off, &blanks);
+                self.tombstoned += s.inst_len;
+            }
+            let inst_off = self.inst_tail;
+            let const_off = self.const_tail;
+            for w in insts.iter_mut() {
+                w.wire_id += const_off;
+            }
+            self.write_insts(queue, inst_off, &insts);
+            for c in &csts {
+                self.consts_cpu.push(*c);
+            }
+            self.inst_tail += inst_len;
+            self.const_tail += const_len;
+            self.slabs.insert(
+                h,
+                Slab {
+                    inst_off,
+                    inst_len,
+                    const_off,
+                    const_len,
+                },
+            );
+        }
+
+        if structural {
+            // The entity count changed ⇒ draw_depth_map re-normalised every
+            // entity's z-bias. Refresh each live slab's draw_depth from the new
+            // depth map and re-upload the whole (small) const buffer; the instance
+            // buffer is untouched.
+            for (h, slab) in &self.slabs {
+                let dd = depth_map.get(&h.value()).copied().unwrap_or(0.0);
+                for k in 0..slab.const_len {
+                    self.consts_cpu[(slab.const_off + k) as usize].draw_depth = dd;
+                }
+            }
+            queue.write_buffer(&self.const_buf, 0, bytemuck::cast_slice(&self.consts_cpu));
+            if self.tombstoned > self.inst_tail / 2 {
+                return false;
+            }
         }
         true
     }