From 0f40cfa0547f320ad6b9e8e68c76681283f9619a Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Fri, 5 Jun 2026 09:16:44 +0300 Subject: [PATCH] perf(scene): skip wire GPU re-upload when only the camera moved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3.2 (wire pan path). Wire vertex buffers are world-space, so a camera move changes only the view_proj uniform (uploaded every frame), never the wire data — yet upload_wires re-ran on every camera_generation tick, re-packing vertices and re-writing the GPU buffer on each pan. Stamp every Model-tile re-tessellation with a monotonic content id (WIRE_CONTENT_GEN), reused when a pan reuses the tessellation. The pipeline records the resident buffer's id and skips upload_wires when the incoming id matches. The gate is pulled out of the (epoch, camera_gen) block so a preview/interim wire change still uploads even with a static camera; non-tile paths and overlay frames force a fresh id, preserving current behaviour. Monotonic id sidesteps the ABA hazard a freed/realloc'd Arc pointer would carry. Pairs with the pan tessellation reuse: a pan now skips both the CPU re-tessellation and the GPU wire upload. Co-Authored-By: Claude Opus 4.8 --- ROADMAP.md | 16 ++++++++++++- src/scene/mod.rs | 34 +++++++++++++++++++++++--- src/scene/pipeline/mod.rs | 7 ++++++ src/scene/render.rs | 50 +++++++++++++++++++++++++++++++-------- 4 files changed, 93 insertions(+), 14 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index a47ceebd3..9e6bb3afc 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -184,7 +184,21 @@ is now free. The full camera/selection-from-tessellation split is still open and needs running-app verification (highlight colour is baked into `WireModel.color` across several tessellation sites). -### 3.2 Persistent GPU buffer pool — diff upload +### 3.2 Persistent GPU buffer pool — diff upload ✅ DONE (wire pan path) + +Wire vertex buffers are world-space, so a camera move alone never changes +them — only the `view_proj` uniform (already uploaded per frame). The wire +upload was gated on `(geometry_epoch, camera_generation)`, re-sending every +pan. Now each Model-tile tessellation is stamped with a monotonic content id +(`WIRE_CONTENT_GEN`), reused when a pan reuses the tessellation; the pipeline +holds the resident buffer's id and `upload_wires` is skipped when it matches. +Gate is independent of the camera tick so a preview/interim change still +uploads. Non-tile paths and overlay frames force a fresh id (unchanged +behaviour). Monotonic id avoids the ABA hazard of a raw `Arc` pointer. + +Still open: a true `HashMap` per-entity pool for partial +edits (re-upload only the changed slots); this covers the whole-buffer +pan/idle case, the dominant one. Today every wire GPU buffer is re-uploaded when [`cached_epoch`](src/scene/pipeline/mod.rs#L101) changes. A persistent diff --git a/src/scene/mod.rs b/src/scene/mod.rs index d5f007a02..2985bf340 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -73,6 +73,15 @@ use std::sync::Arc; /// GPU Pipeline to skip re-uploading geometry when switching tabs. static GEOMETRY_EPOCH: AtomicU64 = AtomicU64::new(1); +/// Process-wide monotonic id stamped on each Model-tile wire re-tessellation. +/// Reused (not re-stamped) when a pan reuses the cached tessellation, so it +/// uniquely identifies a wire-buffer's *content* across frames. The GPU +/// pipeline gates wire re-upload on it (Phase 3.2): a pan that reuses the +/// tessellation keeps the same id, so the world-space wire buffer is not +/// re-sent. Monotonic (never reused) → free of the ABA hazard a raw `Arc` +/// pointer would carry when an address is freed and reallocated. +static WIRE_CONTENT_GEN: AtomicU64 = AtomicU64::new(1); + /// Resolve a viewport's paper-to-model scale ratio from its two /// DXF-derived sources. /// @@ -624,7 +633,7 @@ pub struct Scene { /// the tessellation instead of rebuilding it. Zoom / orbit / edits change /// the epoch or signature and rebuild as before. model_tile_wire_cache: - RefCell>)>>, + RefCell>)>>, /// Index built from every SortEntitiesTable in the document. /// Maps block_handle → (entity_handle.value() → sort_handle.value()). /// Replaces the O(objects) linear scan inside `wires_for_block()` with an O(1) lookup. @@ -732,6 +741,17 @@ pub struct Scene { pub(crate) last_tess_ms: std::cell::Cell, /// Wire count produced by that most recent re-tessellation. pub(crate) last_tess_wires: std::cell::Cell, + /// Content id ([`WIRE_CONTENT_GEN`]) of the Model-tile wire set returned by + /// the most recent `model_tile_wires_arc` call — stamped on a miss, reused + /// on a pan-hit. `build_primitive` reads it right after the call to gate + /// GPU wire re-upload (Phase 3.2). 0 = none yet. + pub(crate) last_model_wire_gen: std::cell::Cell, + /// Monotonic per-build nonce for wire sources that must NOT be skipped by + /// the Phase 3.2 upload gate — the paper / per-viewport wire paths and any + /// frame carrying live preview / interim wires. High bit set so it can + /// never collide with a real [`WIRE_CONTENT_GEN`] id; incremented every + /// use so the GPU always sees a fresh id and re-uploads. + pub(crate) wire_force_nonce: std::cell::Cell, } impl Scene { @@ -791,6 +811,8 @@ impl Scene { viewcube_hover: std::cell::Cell::new(None), last_tess_ms: std::cell::Cell::new(0.0), last_tess_wires: std::cell::Cell::new(0), + last_model_wire_gen: std::cell::Cell::new(0), + wire_force_nonce: std::cell::Cell::new(0), } } @@ -1586,11 +1608,14 @@ impl Scene { }; { let cache = self.model_tile_wire_cache.borrow(); - if let Some(((epoch, sig, region), arc)) = cache.get(&tile_idx) { + if let Some(((epoch, sig, region), gen, arc)) = cache.get(&tile_idx) { if *epoch == self.geometry_epoch && *sig == pan_sig && aabb_contains(*region, need_region) { + // Pan-hit: reuse the tessellation AND its content id, so + // the GPU wire upload is skipped too (Phase 3.2). + self.last_model_wire_gen.set(*gen); return Arc::clone(arc); } } @@ -1613,9 +1638,12 @@ impl Scene { self.last_tess_wires.set(arc.len()); let stored_region = tess_region .unwrap_or([f32::NEG_INFINITY, f32::NEG_INFINITY, f32::INFINITY, f32::INFINITY]); + // New tessellation → fresh content id; the GPU will re-upload. + let gen = WIRE_CONTENT_GEN.fetch_add(1, Ordering::Relaxed); + self.last_model_wire_gen.set(gen); self.model_tile_wire_cache.borrow_mut().insert( tile_idx, - ((self.geometry_epoch, pan_sig, stored_region), Arc::clone(&arc)), + ((self.geometry_epoch, pan_sig, stored_region), gen, Arc::clone(&arc)), ); arc } diff --git a/src/scene/pipeline/mod.rs b/src/scene/pipeline/mod.rs index 5d68adbb5..d4e65d1a9 100644 --- a/src/scene/pipeline/mod.rs +++ b/src/scene/pipeline/mod.rs @@ -116,6 +116,12 @@ pub struct Pipeline { /// were uploaded. We re-upload when either side changes — pan/zoom bumps /// camera_generation, which triggers re-culling and a fresh upload. pub cached_epoch: (u64, u64), + /// Content id of the wire buffer currently resident on the GPU (Phase + /// 3.2). When the incoming `ViewportData.wire_content_id` matches, the + /// world-space wire vertices are unchanged (e.g. a pure pan reused the + /// Model-tile tessellation) and `upload_wires` is skipped. `u64::MAX` = + /// nothing uploaded yet. + pub cached_wire_id: u64, } impl Pipeline { @@ -884,6 +890,7 @@ impl Pipeline { gpu_face3d_edges: vec![], viewcube, cached_epoch: (u64::MAX, u64::MAX), + cached_wire_id: u64::MAX, } } diff --git a/src/scene/render.rs b/src/scene/render.rs index 649c36731..734594258 100644 --- a/src/scene/render.rs +++ b/src/scene/render.rs @@ -74,9 +74,13 @@ pub struct ViewportData { pub(super) hidden_line: bool, pub(super) geometry_epoch: u64, /// Camera generation captured when this Primitive was assembled. Paired - /// with `geometry_epoch` so the wire buffers re-upload when the view - /// changes (frustum culling produces a different wire list). + /// with `geometry_epoch` so the per-frame scissor / LOD recompute runs. pub(super) camera_generation: u64, + /// Content id of `wires` (Phase 3.2). Stable across a pure pan that reuses + /// the Model-tile tessellation, so `prepare` skips re-uploading the + /// world-space wire buffer when only the camera moved. Non-tile and + /// preview/interim frames carry a fresh id each time → always re-upload. + pub(super) wire_content_id: u64, /// Screen rectangle this viewport fills, **normalized** to the widget /// bounds (each component in 0..1). A single full-widget view is /// `(0, 0, 1, 1)`; tiled / floating viewports are sub-rectangles. @@ -227,9 +231,6 @@ impl shader::Primitive for Primitive { inner.upload_images(device, queue, &vp.images[..]); inner.upload_meshes(device, &vp.meshes[..]); } - // Wires re-upload on every camera change because the visible - // subset shifts under frustum culling. - inner.upload_wires(device, &vp.wires[..], &vp.draw_depths); inner.upload_face3d( device, &vp.face3d_wires[..], @@ -239,6 +240,17 @@ impl shader::Primitive for Primitive { ); inner.cached_epoch = cur_key; } + // Wire buffers are world-space, so a camera move alone doesn't + // change them — only the view_proj uniform (uploaded every frame). + // Gate the upload on the wire content id instead of the camera tick + // (Phase 3.2): a pan that reused the Model-tile tessellation keeps + // the id, skipping the vertex re-pack + GPU write. Kept independent + // of the `cur_key` block so a preview/interim wire change still + // uploads even when the camera didn't move. + if vp.wire_content_id != inner.cached_wire_id { + inner.upload_wires(device, &vp.wires[..], &vp.draw_depths); + inner.cached_wire_id = vp.wire_content_id; + } let vproj = vp.uniforms.view_proj; inner.compute_wire_scissors(vproj, clip_size.width, clip_size.height); inner.compute_wipeout_scissors(vproj, clip_size.width, clip_size.height); @@ -587,22 +599,39 @@ impl Scene { // tile's camera — culling/LOD depend on per-tile zoom, not the // active tile's. Paper-space content viewports already had a // per-viewport cache. - let base_arc = if let Some(tile_idx) = inst.tile_idx { + // `tile_wire_gen` is the Model-tile content id (Phase 3.2): present + // only on the tiled Model path, where a pan reuses the tessellation + // and its id so the GPU wire upload can be skipped. The other wire + // sources don't participate and force a re-upload below. + let (base_arc, tile_wire_gen) = if let Some(tile_idx) = inst.tile_idx { let aspect = if full.height > 0.0 { full.width / full.height } else { 1.0 }; - self.model_tile_wires_arc(tile_idx, &inst.camera, aspect, full.height) + let arc = self.model_tile_wires_arc(tile_idx, &inst.camera, aspect, full.height); + (arc, Some(self.last_model_wire_gen.get())) } else if inst.paper_sheet { // The sheet renders the paper block's own entities + viewport // borders — NOT the projected viewport content (the GPU content // viewports draw that themselves). - self.paper_sheet_wires_arc() + (self.paper_sheet_wires_arc(), None) } else if inst.handle == acadrust::Handle::NULL { - self.entity_wires_arc() + (self.entity_wires_arc(), None) } else { - self.model_wires_for_viewport_arc(inst.handle, full.height) + (self.model_wires_for_viewport_arc(inst.handle, full.height), None) + }; + // Wire-buffer content id for the upload gate. Live preview / interim + // wires are appended below and vary frame-to-frame, so any frame + // carrying them forces a fresh upload, as do the non-tile paths. + let has_overlay = self.interim_wire.is_some() || !self.preview_wires.is_empty(); + let wire_content_id = match tile_wire_gen { + Some(g) if !has_overlay => g, + _ => { + let n = self.wire_force_nonce.get().wrapping_add(1); + self.wire_force_nonce.set(n); + n | (1u64 << 63) + } }; let (face3d_wires, other_wires) = split_face3d_wires(&base_arc, &self.document); let all_wires = if self.interim_wire.is_none() && self.preview_wires.is_empty() { @@ -694,6 +723,7 @@ impl Scene { hidden_line: flags.hidden_line, geometry_epoch: self.geometry_epoch, camera_generation: self.camera_generation, + wire_content_id, screen_rect, }) }