From feb3ca4d28c753c89c5a83d2b3fb5f5f4cac1350 Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:58:33 +0300 Subject: [PATCH 1/6] perf: split the arena patch's post-work into its three steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the selection-filter rebuild gone, a drawn point costs 98 ms and `prepare` is 93 % of it: wire 73.0ms arena-patch wires=236961 gpu_instances=405551 wire-cull 5.1ms face3d-upload 3.9ms text-upload 3.0ms resident-patch 1.0ms changes=1 draw-depth-patch 0.0ms `arena-base ok=true patch=Some(16) changes=1` — the arena patches incrementally, one entity, and still takes 73 ms. The patch itself is careful; the three steps that follow it are not, and all three are O(resident set) rather than O(changes): partition_wires(&vp_wires, …) 236 961 wires, every frame upload_block_wires(…instanced…) 10 719 placements rebuilt upload_circles/ellipses_from_instances `arena-post partition= blocks= curves=` says which dominates. Guessing between three plausible candidates is what the earlier rounds of this work paid for repeatedly. Co-Authored-By: Claude Opus 5 --- src/scene/view/render.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index b25fb4810..faf1a165b 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -839,8 +839,15 @@ impl shader::Primitive for Primitive { gpus.extend(arena.wire_gpus()); } inner.gpu_wires = std::sync::Arc::new(gpus); + // A one-entity patch costs 73 ms in this block, and the + // three steps below are all O(resident set) rather than + // O(changes). Which of them dominates decides the fix. + let t_part = _perf.then(iced::time::Instant::now); let partitioned = wire_arena::partition_wires(&vp_wires, &draw_depths); + let part_ms = t_part + .map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); + let t_blk = _perf.then(iced::time::Instant::now); inner.gpu_block_wires = std::sync::Arc::new( inner.upload_block_wires( device, @@ -850,6 +857,9 @@ impl shader::Primitive for Primitive { &mut pipeline.block_geometry, ), ); + let blk_ms = t_blk + .map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); + let t_curve = _perf.then(iced::time::Instant::now); inner.gpu_circles = std::sync::Arc::new( inner.upload_circles_from_instances( device, @@ -864,6 +874,17 @@ impl shader::Primitive for Primitive { &partitioned.ellipse_instances, ), ); + if _perf { + crate::perf_record!( + "[perf] arena-post partition={part_ms:.1}ms blocks={blk_ms:.1}ms \ +curves={:.1}ms wires={} instanced={} circles={} ellipses={}", + t_curve.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0), + vp_wires.len(), + partitioned.instanced.len(), + partitioned.circle_instances.len(), + partitioned.ellipse_instances.len(), + ); + } if _patched { wire_arena::patch_handle_index( &mut inner.wire_handle_index, From b68989768daa9db1610f109d5423c89f6bae76b8 Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Tue, 8 Sep 2026 13:08:35 +0300 Subject: [PATCH 2/6] perf: look the draw depth up only where it is used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `arena-post partition=55.2ms blocks=5.9ms curves=2.5ms` — the partition is the whole of the 73 ms an arena patch spends after patching, and it runs on every drawn point over the entire resident set. Inside it, `wire_draw_depth` is a hash lookup whose result feeds only the analytical circle and ellipse extraction, and it was computed before the test that decides whether either will run. So every patch paid 236 956 lookups to serve the ~56 000 wires that are circles or ellipses. Moving it inside that branch is behaviour-preserving: the value was unused on every path that skipped it. This does not make the partition incremental — it still walks the resident set for a one-entity change — but that walk is memory-bound and a structural fix needs the cached partition to survive a patch, which the borrows in `PartitionedWires` make its own piece of work. Co-Authored-By: Claude Opus 5 --- src/scene/pipeline/wire_arena.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/scene/pipeline/wire_arena.rs b/src/scene/pipeline/wire_arena.rs index bbaddb773..954e2fd8e 100644 --- a/src/scene/pipeline/wire_arena.rs +++ b/src/scene/pipeline/wire_arena.rs @@ -199,12 +199,16 @@ pub fn partition_wires<'a>( instanced.push(wire); continue; } - let depth = super::wire_gpu::wire_draw_depth(wire, depth_map); + // `wire_draw_depth` is a hash lookup and its result is used only by the + // analytical extractions below, which most wires never reach. Computing + // it first cost one lookup per resident wire on every patch — 236 956 + // of them for the ~56 000 that are circles or ellipses. if !wire.tangent_geoms.is_empty() && wire.fill_tris.is_empty() && wire.pick_tris.is_empty() && wire.text_verts.is_empty() { + let depth = super::wire_gpu::wire_draw_depth(wire, depth_map); if let Some(insts) = super::circle_gpu::extract_circle_instances(wire, depth) { circle_instances.extend(insts); continue; From 03c64e3d383e37075c405071485ff03cb80b569b Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:34:25 +0300 Subject: [PATCH 3/6] perf: skip the wire partition when a patch cannot change its output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `arena-post partition=55.2ms blocks=5.9ms curves=2.5ms` — the partition is almost the whole of the 73 ms an arena patch spends after patching, and it walks all 236 956 resident wires to serve a one-entity change. Its three consumers here — the instanced, circle and ellipse uploads — depend only on wires that are block references or analytical curves. A patch that touches none of those cannot change any of them, so the walk is skipped and the uploads already on the slot are kept. Two things make that sound, and both are checked rather than assumed: **Depths hold still.** The instances bake a draw depth. `inserted_draw_depth_label` allocates into a reserved gap instead of renumbering, so adding an entity leaves every other label alone — the existing doc comment says so and there is now a test. A rebuild from scratch *can* move them, so `draw_depth_generation` bumps only there, and a slot reuses its uploads only while that generation is unchanged. **Contributors are remembered.** `partition_wires` records which handles fed those three outputs, and the skip requires that no changed handle is among them and that no changed handle's new wires would join them. That catches an entity entering the set and an entity leaving it. The membership test is shared with the walk it decides to skip, so the two cannot drift, and it is deliberately conservative: a wire that looks analytical but fails extraction forces a needless re-partition, never a stale upload. `arena-post` gains `skipped=` so a run says how often this actually fires rather than leaving it to be inferred from the timing. Not verified on a device yet — the headless harness never reaches `prepare`, so this needs a GUI run on the reproducer to confirm both the saving and that nothing goes missing on screen. Co-Authored-By: Claude Opus 5 --- src/scene/mod.rs | 63 +++++++++++++++++++ src/scene/pipeline/mod.rs | 13 ++++ src/scene/pipeline/wire_arena.rs | 31 ++++++++++ src/scene/view/render.rs | 102 +++++++++++++++++++++---------- 4 files changed, 176 insertions(+), 33 deletions(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 187b0d7a5..d6a269690 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1706,6 +1706,12 @@ pub struct Scene { /// constants. `[depth, half]` retains a fixed child sub-range for block /// composition. Full sort/layout/block changes rebuild the labels. draw_depth_cache: RefCell>, + /// Bumped only when the draw-order map is rebuilt from scratch, which is + /// the one case that can change an existing entity's depth — an + /// incremental insert allocates into the reserved label gap and leaves + /// every other label alone. Consumers that cached anything carrying a baked + /// depth compare this before reusing it. + draw_depth_generation: std::cell::Cell, /// Shared empty map for 3-D wireframe, where true depth wins instead of /// the entity submission order used by the optimized 2-D style. no_draw_depths: Arc>, @@ -2053,6 +2059,7 @@ impl Scene { interaction_handle_index_cache: RefCell::new(None), sort_cache: RefCell::new(None), draw_depth_cache: RefCell::new(None), + draw_depth_generation: std::cell::Cell::new(0), no_draw_depths: Arc::new(HashMap::default()), hatch_cache: RefCell::new(HashMap::default()), wipeout_cache: RefCell::new(HashMap::default()), @@ -5998,6 +6005,11 @@ impl Scene { /// A full build assigns sparse labels in effective draw order. Incremental /// Add/Remove then changes only the named handle: existing siblings retain /// their depth, avoiding an O(all entities) map rewrite and GPU const upload. + /// See [`Self::draw_depth_generation`]. + pub(in crate::scene) fn draw_depth_generation(&self) -> u64 { + self.draw_depth_generation.get() + } + pub(super) fn draw_depth_map(&self) -> Arc> { { let cache = self.draw_depth_cache.borrow(); @@ -6127,6 +6139,11 @@ impl Scene { } } + // From here the labels are assigned afresh, so an existing entity's + // depth can move. Anything holding a baked depth has to rebuild. + self.draw_depth_generation + .set(self.draw_depth_generation.get().wrapping_add(1)); + use acadrust::objects::ObjectType; // Per-block SortEntitiesTable overrides: block -> (entity_val -> sort_val). let mut overrides: HashMap> = HashMap::default(); @@ -10643,6 +10660,52 @@ mod journal_tests { // Differential oracle: the incrementally-patched entity index must always // equal a from-scratch rebuild after any add / move / erase. #[test] + /// The partition skip reuses uploads that bake a draw depth, and it is only + /// sound while existing depths hold still. An incremental add must leave + /// them alone and must not bump the generation; a rebuild from scratch may + /// move them and must bump it. + #[test] + fn adding_an_entity_leaves_existing_draw_depths_alone() { + use acadrust::entities::Line; + use acadrust::types::Vector3; + + fn add(scene: &mut Scene, x: f64) -> Handle { + scene.add_entity(EntityType::Line(Line::from_points( + Vector3::new(x, 0.0, 0.0), + Vector3::new(x + 1.0, 0.0, 0.0), + ))) + } + let mut scene = Scene::new(); + let first = add(&mut scene, 0.0); + let second = add(&mut scene, 1.0); + let before = scene.draw_depth_map(); + let generation = scene.draw_depth_generation(); + + add(&mut scene, 2.0); + let after = scene.draw_depth_map(); + assert_eq!( + scene.draw_depth_generation(), + generation, + "an incremental add must not invalidate baked depths", + ); + for handle in [first, second] { + assert_eq!( + after.get(&handle.value()), + before.get(&handle.value()), + "an existing entity's depth must not move when another is added", + ); + } + + // A rebuild from scratch reassigns labels, so it has to say so. + scene.draw_depth_cache.borrow_mut().take(); + let _ = scene.draw_depth_map(); + assert_ne!( + scene.draw_depth_generation(), + generation, + "a full rebuild must invalidate anything holding a baked depth", + ); + } + /// Folding a lone addition must land the handle where the full walk would /// have put it — at the end of its block's list, because that list is in /// document order and document order is draw order. diff --git a/src/scene/pipeline/mod.rs b/src/scene/pipeline/mod.rs index c77fa6d2d..3c7bcff3c 100644 --- a/src/scene/pipeline/mod.rs +++ b/src/scene/pipeline/mod.rs @@ -251,6 +251,13 @@ pub struct Pipeline { pub(crate) wire_arena_fallback_handles: rustc_hash::FxHashSet, /// The Model content id both arenas currently mirror (`u64::MAX` = none). pub(crate) wire_arena_id: u64, + /// Entities that fed the instanced / circle / ellipse uploads this slot is + /// holding. A patch touching none of them cannot change those uploads, so + /// the partition walk over the whole resident set can be skipped. + pub(crate) partition_contributors: rustc_hash::FxHashSet, + /// The draw-depth generation those uploads baked. A full depth rebuild + /// reassigns every label, so they stop being reusable when it moves. + pub(crate) partition_depth_generation: u64, /// Last content/camera/viewport tuple used to derive visible instance /// ranges from the resident arena. pub(crate) wire_cull_key: (u64, u64, u32, u32), @@ -2338,6 +2345,8 @@ impl Pipeline { wire_arena_fallback_kind: None, wire_arena_fallback_handles: rustc_hash::FxHashSet::default(), wire_arena_id: u64::MAX, + partition_contributors: rustc_hash::FxHashSet::default(), + partition_depth_generation: u64::MAX, wire_cull_key: (u64::MAX, u64::MAX, 0, 0), hatch_lod_key: (usize::MAX, u64::MAX, 0, 0, false), wipeout_lod_key: (usize::MAX, u64::MAX, 0, 0, false), @@ -4991,6 +5000,8 @@ impl Pipeline { self.wire_arena = None; self.wire_arena_mesh = None; self.wire_arena_id = u64::MAX; + self.partition_contributors.clear(); + self.partition_depth_generation = u64::MAX; self.alloc_size = Size::new(0, 0); self.shadow_full = None; self.background_source_id = usize::MAX; @@ -5022,6 +5033,8 @@ impl Pipeline { self.wire_arena_fallback_kind = None; self.wire_arena_fallback_handles.clear(); self.wire_arena_id = u64::MAX; + self.partition_contributors.clear(); + self.partition_depth_generation = u64::MAX; self.gpu_selected_wires.clear(); self.gpu_selected_block_wires.clear(); diff --git a/src/scene/pipeline/wire_arena.rs b/src/scene/pipeline/wire_arena.rs index 954e2fd8e..7dcd29954 100644 --- a/src/scene/pipeline/wire_arena.rs +++ b/src/scene/pipeline/wire_arena.rs @@ -174,6 +174,27 @@ pub struct PartitionedWires<'a> { pub instanced: Vec<&'a WireModel>, pub circle_instances: Vec, pub ellipse_instances: Vec, + /// Entities that fed `instanced` / `circle_instances` / `ellipse_instances`. + /// + /// A patch that touches none of these cannot change those three uploads, so + /// the caller can keep the ones it already has and skip the walk entirely. + pub contributors: rustc_hash::FxHashSet, +} + +/// Whether a change to this wire can alter what `partition_wires` produces for +/// the instanced, circle and ellipse uploads. +/// +/// Deliberately conservative: a wire that looks analytical here may still fail +/// extraction and end up in `regular`, which costs an unnecessary re-partition +/// and never a stale upload. Shared with `partition_wires` so the test that +/// decides to skip cannot drift from the walk it is skipping. +pub fn feeds_analytical_uploads(wire: &WireModel) -> bool { + wire.display_visible + && (wire.render_instance.is_some() + || (!wire.tangent_geoms.is_empty() + && wire.fill_tris.is_empty() + && wire.pick_tris.is_empty() + && wire.text_verts.is_empty())) } /// Single-pass classification and extraction of all viewport wire categories. @@ -190,12 +211,19 @@ pub fn partition_wires<'a>( let mut instanced = Vec::new(); let mut circle_instances = Vec::new(); let mut ellipse_instances = Vec::new(); + let mut contributors: rustc_hash::FxHashSet = rustc_hash::FxHashSet::default(); + let note = |wire: &WireModel, set: &mut rustc_hash::FxHashSet| { + if let Some(handle) = handle_of(wire) { + set.insert(handle); + } + }; for wire in wires { if !wire.display_visible { continue; } if wire.render_instance.is_some() { + note(wire, &mut contributors); instanced.push(wire); continue; } @@ -210,10 +238,12 @@ pub fn partition_wires<'a>( { let depth = super::wire_gpu::wire_draw_depth(wire, depth_map); if let Some(insts) = super::circle_gpu::extract_circle_instances(wire, depth) { + note(wire, &mut contributors); circle_instances.extend(insts); continue; } if let Some(insts) = super::ellipse_gpu::extract_ellipse_instances(wire, depth) { + note(wire, &mut contributors); ellipse_instances.extend(insts); continue; } @@ -233,6 +263,7 @@ pub fn partition_wires<'a>( instanced, circle_instances, ellipse_instances, + contributors, } } diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index faf1a165b..cb01fac4d 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -149,6 +149,9 @@ pub struct ViewportData { /// by the wire / face3d pipelines as a clip-z bias. WireModels carry no /// depth field (84 construction sites); the bias is looked up by handle /// at GPU-upload time from this map instead. + /// See `Scene::draw_depth_generation`. Cached uploads that bake a depth are + /// only reusable while this is unchanged. + pub(in crate::scene) draw_depth_generation: u64, pub(in crate::scene) draw_depths: std::sync::Weak>, pub(in crate::scene) hatches: Arc>, @@ -842,47 +845,79 @@ impl shader::Primitive for Primitive { // A one-entity patch costs 73 ms in this block, and the // three steps below are all O(resident set) rather than // O(changes). Which of them dominates decides the fix. + // The partition walks the whole resident set, and on + // a patch it almost always produces the three uploads it + // produced last frame. Skip it when no changed entity + // feeds them and none did before. + // + // Safe because adding an entity does not disturb any + // other entity's draw depth: `inserted_draw_depth_label` + // allocates into the reserved label gap rather than + // renumbering, so instances already uploaded stay right. + let analytical_untouched = _patched + && inner.partition_depth_generation == vp.draw_depth_generation + && patch.is_some_and(|patch| { + patch.changes.iter().all(|(handle, _)| { + !inner.partition_contributors.contains(handle) + && !patch.runs.get(handle).is_some_and(|run| { + run.iter() + .any(wire_arena::feeds_analytical_uploads) + }) + }) + }); let t_part = _perf.then(iced::time::Instant::now); - let partitioned = - wire_arena::partition_wires(&vp_wires, &draw_depths); + let partitioned = (!analytical_untouched) + .then(|| wire_arena::partition_wires(&vp_wires, &draw_depths)); let part_ms = t_part .map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); let t_blk = _perf.then(iced::time::Instant::now); - inner.gpu_block_wires = std::sync::Arc::new( - inner.upload_block_wires( - device, - queue, - &partitioned.instanced, - &draw_depths, - &mut pipeline.block_geometry, - ), - ); - let blk_ms = t_blk - .map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); - let t_curve = _perf.then(iced::time::Instant::now); - inner.gpu_circles = std::sync::Arc::new( - inner.upload_circles_from_instances( - device, - queue, - &partitioned.circle_instances, - ), - ); - inner.gpu_ellipses = std::sync::Arc::new( - inner.upload_ellipses_from_instances( - device, - queue, - &partitioned.ellipse_instances, - ), - ); + let mut blk_ms = 0.0f64; + let mut curve_ms = 0.0f64; + // `None` means the three uploads this slot holds are + // still the answer, so they are left alone. + if let Some(partitioned) = &partitioned { + inner.gpu_block_wires = std::sync::Arc::new( + inner.upload_block_wires( + device, + queue, + &partitioned.instanced, + &draw_depths, + &mut pipeline.block_geometry, + ), + ); + blk_ms = t_blk + .map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); + let t_curve = _perf.then(iced::time::Instant::now); + inner.gpu_circles = std::sync::Arc::new( + inner.upload_circles_from_instances( + device, + queue, + &partitioned.circle_instances, + ), + ); + inner.gpu_ellipses = std::sync::Arc::new( + inner.upload_ellipses_from_instances( + device, + queue, + &partitioned.ellipse_instances, + ), + ); + inner + .partition_contributors + .clone_from(&partitioned.contributors); + inner.partition_depth_generation = vp.draw_depth_generation; + curve_ms = t_curve + .map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); + } if _perf { crate::perf_record!( "[perf] arena-post partition={part_ms:.1}ms blocks={blk_ms:.1}ms \ -curves={:.1}ms wires={} instanced={} circles={} ellipses={}", - t_curve.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0), +curves={curve_ms:.1}ms skipped={} wires={} instanced={} circles={} ellipses={}", + partitioned.is_none(), vp_wires.len(), - partitioned.instanced.len(), - partitioned.circle_instances.len(), - partitioned.ellipse_instances.len(), + partitioned.as_ref().map_or(0, |p| p.instanced.len()), + partitioned.as_ref().map_or(0, |p| p.circle_instances.len()), + partitioned.as_ref().map_or(0, |p| p.ellipse_instances.len()), ); } if _patched { @@ -4259,6 +4294,7 @@ impl Scene { face3d_wires, text_verts, preview_text_verts, + draw_depth_generation: self.draw_depth_generation(), draw_depths: Arc::downgrade(&draw_depths), hatches, wipeout_hatches, From d46c00281f85263422b24d0f12c016db8d12aa5c Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:45:16 +0300 Subject: [PATCH 4/6] fix: decide partition membership by extraction, not by shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `skipped=false` on every frame of a real session — the skip added in the previous commit never fired once. The membership test used the same cheap structural filter that `partition_wires` uses before attempting extraction: a tangent geom, no fill, pick or text triangles. That filter is a pre-check, not a decision. An ordinary drawn line carries a tangent geom for snapping and no triangles, so it passed, and every drawn line was counted as feeding the analytical uploads. Nothing was ever skippable. It now decides the way the walk decides, by attempting the extraction. `draw_depth` only rides along in the extracted instance, so the value passed for the test cannot change the answer, and it runs only for the handful of wires a patch changed. Drawing a circle or an ellipse still forces the walk, correctly — those are contributors, and the session that exposed this contained both. What changes is that the lines drawn afterwards no longer do. Tested on real tessellated geometry: a line must not force the walk, a circle must. Co-Authored-By: Claude Opus 5 --- src/scene/mod.rs | 38 ++++++++++++++++++++++++++++++++ src/scene/pipeline/wire_arena.rs | 26 +++++++++++++--------- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index d6a269690..0fc58caa2 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -10660,6 +10660,44 @@ mod journal_tests { // Differential oracle: the incrementally-patched entity index must always // equal a from-scratch rebuild after any add / move / erase. #[test] + /// The partition skip turns on a membership test, and the test has to tell + /// a drawn line from a drawn circle. A line carries a tangent geom for + /// snapping and no triangles, so the cheap structural filter admits it — + /// deciding on that alone called every line a contributor and the skip + /// never fired once in a real session. + #[test] + fn the_partition_membership_test_separates_lines_from_curves() { + use acadrust::entities::{Circle, Line}; + use acadrust::types::Vector3; + use crate::scene::pipeline::wire_arena::feeds_analytical_uploads; + + let mut scene = Scene::new(); + let feeds = |scene: &Scene, handle: Handle| { + let entity = scene.document.get_entity(handle).unwrap().clone(); + scene + .tessellate_one(&entity) + .iter() + .any(feeds_analytical_uploads) + }; + + let line = scene.add_entity(EntityType::Line(Line::from_points( + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(10.0, 5.0, 0.0), + ))); + assert!( + !feeds(&scene, line), + "a drawn line must not force the partition walk", + ); + + let mut circle = Circle::new(); + circle.radius = 4.0; + let circle = scene.add_entity(EntityType::Circle(circle)); + assert!( + feeds(&scene, circle), + "a drawn circle feeds the analytical uploads and must force the walk", + ); + } + /// The partition skip reuses uploads that bake a draw depth, and it is only /// sound while existing depths hold still. An incremental add must leave /// them alone and must not bump the generation; a rebuild from scratch may diff --git a/src/scene/pipeline/wire_arena.rs b/src/scene/pipeline/wire_arena.rs index 7dcd29954..322dea9f0 100644 --- a/src/scene/pipeline/wire_arena.rs +++ b/src/scene/pipeline/wire_arena.rs @@ -184,17 +184,23 @@ pub struct PartitionedWires<'a> { /// Whether a change to this wire can alter what `partition_wires` produces for /// the instanced, circle and ellipse uploads. /// -/// Deliberately conservative: a wire that looks analytical here may still fail -/// extraction and end up in `regular`, which costs an unnecessary re-partition -/// and never a stale upload. Shared with `partition_wires` so the test that -/// decides to skip cannot drift from the walk it is skipping. +/// Decided by attempting the extraction, exactly as the walk decides it. The +/// cheap structural test alone is not enough: an ordinary drawn line carries a +/// tangent geom for snapping and no triangles, so it passes that test, fails +/// extraction, and lands in `regular` — a membership test that stopped there +/// would call every line a contributor and never skip anything. +/// +/// `draw_depth` only rides along in the extracted instance, so the value passed +/// here cannot change the answer. pub fn feeds_analytical_uploads(wire: &WireModel) -> bool { - wire.display_visible - && (wire.render_instance.is_some() - || (!wire.tangent_geoms.is_empty() - && wire.fill_tris.is_empty() - && wire.pick_tris.is_empty() - && wire.text_verts.is_empty())) + if !wire.display_visible { + return false; + } + if wire.render_instance.is_some() { + return true; + } + super::circle_gpu::extract_circle_instances(wire, 0.0).is_some() + || super::ellipse_gpu::extract_ellipse_instances(wire, 0.0).is_some() } /// Single-pass classification and extraction of all viewport wire categories. From 5eb762e88dcfa88325b8d6b7fe80647780e442e1 Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:51:23 +0300 Subject: [PATCH 5/6] fix: forget the partition record with the keys that describe it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `forget_cached_keys` is the one place that means "this slot holds nothing you can trust", and it left the partition record behind. A reused slot rebuilds regardless — its content id changed, so the arena takes the build path and the record is overwritten — but that made the safety an accident of another code path rather than something this function guarantees. Co-Authored-By: Claude Opus 5 --- src/scene/pipeline/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/scene/pipeline/mod.rs b/src/scene/pipeline/mod.rs index 3c7bcff3c..0c3c5f1ef 100644 --- a/src/scene/pipeline/mod.rs +++ b/src/scene/pipeline/mod.rs @@ -4987,6 +4987,12 @@ impl Pipeline { self.silhouette_key = (usize::MAX, u64::MAX, [u32::MAX; 3], false); self.silhouette_source_key = (usize::MAX, usize::MAX, u64::MAX); self.render_sig = u64::MAX; + // The partition record describes uploads this slot is no longer + // entitled to reuse. A reused slot rebuilds anyway, because its content + // id changed, but leaving a stale record here would make that an + // accident rather than a guarantee. + self.partition_contributors.clear(); + self.partition_depth_generation = u64::MAX; } /// Catch errors delivered after the previous upload/submit completed. From eef007d08038b84b48117952cf4a39ad9b80d67f Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:37:09 +0300 Subject: [PATCH 6/6] perf: report a skipped partition without printing zero counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skip branch printed `circles=0 ellipses=0`, which reads as "there are no circles" when it means "not recomputed this frame" — and those counts were the evidence that confirmed the skip works, so the wording nearly cost a misreading of it. The skip now prints its own line with what it retained. Measured effect of the skip itself, on the reproducer: a drawn point goes from 98 ms to 17-39 ms, `prepare` from 89 ms to 11-23 ms. Frames that draw a circle still re-partition, and the instance counts show it — 40868, 40869 … 40874, one per circle, with the line frames skipped in between. Co-Authored-By: Claude Opus 5 --- src/scene/view/render.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index cb01fac4d..8dced63d5 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -910,15 +910,27 @@ impl shader::Primitive for Primitive { .map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); } if _perf { - crate::perf_record!( - "[perf] arena-post partition={part_ms:.1}ms blocks={blk_ms:.1}ms \ -curves={curve_ms:.1}ms skipped={} wires={} instanced={} circles={} ellipses={}", - partitioned.is_none(), - vp_wires.len(), - partitioned.as_ref().map_or(0, |p| p.instanced.len()), - partitioned.as_ref().map_or(0, |p| p.circle_instances.len()), - partitioned.as_ref().map_or(0, |p| p.ellipse_instances.len()), - ); + // Counts only when a partition produced them. + // Printing zeros on a skip reads as "no circles" + // rather than "not recomputed", which is the + // opposite of what the line is reporting. + match &partitioned { + Some(partitioned) => crate::perf_record!( + "[perf] arena-post partition={part_ms:.1}ms \ +blocks={blk_ms:.1}ms curves={curve_ms:.1}ms skipped=false wires={} instanced={} \ +circles={} ellipses={}", + vp_wires.len(), + partitioned.instanced.len(), + partitioned.circle_instances.len(), + partitioned.ellipse_instances.len(), + ), + None => crate::perf_record!( + "[perf] arena-post skipped=true wires={} \ +retained_contributors={}", + vp_wires.len(), + inner.partition_contributors.len(), + ), + } } if _patched { wire_arena::patch_handle_index(