diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 0faa7ab2d..9d62dd7f6 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1715,6 +1715,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>, @@ -2075,6 +2081,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()), @@ -6030,6 +6037,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(); @@ -6159,6 +6171,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(); @@ -10675,6 +10692,90 @@ 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 + /// 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 362f045ab..9b3b45b99 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), @@ -4980,6 +4989,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. @@ -4993,6 +5008,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; @@ -5024,6 +5041,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 bbaddb773..322dea9f0 100644 --- a/src/scene/pipeline/wire_arena.rs +++ b/src/scene/pipeline/wire_arena.rs @@ -174,6 +174,33 @@ 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. +/// +/// 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 { + 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. @@ -190,26 +217,39 @@ 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; } - 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) { + 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; } @@ -229,6 +269,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 93c8047de..026fa517b 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>, @@ -839,31 +842,96 @@ impl shader::Primitive for Primitive { gpus.extend(arena.wire_gpus()); } inner.gpu_wires = std::sync::Arc::new(gpus); - let partitioned = - wire_arena::partition_wires(&vp_wires, &draw_depths); - inner.gpu_block_wires = std::sync::Arc::new( - inner.upload_block_wires( - device, - queue, - &partitioned.instanced, - &draw_depths, - &mut pipeline.block_geometry, - ), - ); - 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, - ), - ); + // 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 = (!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); + 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 { + // 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( &mut inner.wire_handle_index, @@ -4239,6 +4307,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,