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,