From fdbbcc419eb0678e7023670799b612feb6c9de85 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Mon, 28 Sep 2026 19:50:38 +0100 Subject: [PATCH] perf(flatten): pre-sized collect/plan and batched entity publish flatten_plan 2,289 -> 1,098-1,144 us (-50..-52%); flatten_scene_apply 1,213 -> 789-813 us (-33..-35% vs baseline; -28.5% same-session O2-vs-O1, drift-corrected). Pre-sizes collect/plan vectors and adds Scene::update_entities so FLATTEN issues one geometry bump per batch instead of per entity; single update_entity keeps its exact bump-per-call semantics. --- src/app/commands/inquiry.rs | 10 +++--- src/modules/draw/modify/flatten.rs | 52 ++++++++++++++---------------- src/scene/entity.rs | 52 ++++++++++++++++++++++++++++-- 3 files changed, 77 insertions(+), 37 deletions(-) diff --git a/src/app/commands/inquiry.rs b/src/app/commands/inquiry.rs index ab3e64ff4..e13a454d4 100644 --- a/src/app/commands/inquiry.rs +++ b/src/app/commands/inquiry.rs @@ -1146,12 +1146,10 @@ impl OpenCADStudio { 0 } else { self.push_undo_snapshot(i, "FLATTEN"); - let mut moved = 0usize; - for entity in updates { - if self.tabs[i].scene.update_entity(entity) { - moved += 1; - } - } + let moved = crate::modules::draw::modify::flatten::apply_flatten_updates( + &mut self.tabs[i].scene, + updates, + ); if moved == 0 { self.discard_last_undo_entry(i); } diff --git a/src/modules/draw/modify/flatten.rs b/src/modules/draw/modify/flatten.rs index 1c398e0ad..18abad951 100644 --- a/src/modules/draw/modify/flatten.rs +++ b/src/modules/draw/modify/flatten.rs @@ -289,45 +289,41 @@ fn z_axis_normal(normal: codec::types::Vector3) -> bool { pub fn collect_flatten_handles(scene: &Scene) -> Vec { let sel = scene.selected_entities(); if sel.is_empty() { - scene - .document - .entities() - .map(|e| e.common().handle) - .filter(|handle| { - scene.entity_belongs_to_active_space(*handle) - && !scene.is_layer_locked(*handle) - }) - .collect() + let mut out = Vec::with_capacity(scene.document.entities().size_hint().0); + for handle in scene.document.entities().map(|e| e.common().handle) { + if scene.entity_belongs_to_active_space(handle) && !scene.is_layer_locked(handle) { + out.push(handle); + } + } + out } else { - sel.into_iter() - .map(|(h, _)| h) - .filter(|handle| { - scene.entity_belongs_to_active_space(*handle) - && !scene.is_layer_locked(*handle) - }) - .collect() + let mut out = Vec::with_capacity(sel.len()); + for (handle, _) in sel { + if scene.entity_belongs_to_active_space(handle) && !scene.is_layer_locked(handle) { + out.push(handle); + } + } + out } } /// The replacement entities for candidates that actually move (None for /// unsupported / already-flat entities, exactly as the arm filtered them). pub fn plan_flatten(scene: &Scene, handles: &[Handle]) -> Vec { - handles - .iter() - .filter_map(|&handle| scene.document.get_entity(handle)) - .filter_map(flatten_entity_z) - .collect() + let mut out = Vec::with_capacity(handles.len()); + for &handle in handles { + if let Some(entity) = scene.document.get_entity(handle) { + if let Some(flattened) = flatten_entity_z(entity) { + out.push(flattened); + } + } + } + out } /// Publish flattened replacements; returns how many entities moved. pub fn apply_flatten_updates(scene: &mut Scene, updates: Vec) -> usize { - let mut moved = 0usize; - for entity in updates { - if scene.update_entity(entity) { - moved += 1; - } - } - moved + scene.update_entities(updates) } #[cfg(test)] diff --git a/src/scene/entity.rs b/src/scene/entity.rs index b73f344d8..d16269d3a 100644 --- a/src/scene/entity.rs +++ b/src/scene/entity.rs @@ -514,7 +514,53 @@ impl Scene { /// [`add_entity`](Self::add_entity) used to commit a plugin's edit of an /// existing entity. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] - pub fn update_entity(&mut self, mut entity: EntityType) -> bool { + pub fn update_entity(&mut self, entity: EntityType) -> bool { + let mut changed = Vec::with_capacity(1); + let mut block_change = false; + if !self.update_entity_impl(entity, &mut changed, &mut block_change) { + return false; + } + self.publish_entity_updates(changed, block_change); + true + } + + /// Batched variant of [`update_entity`]: writes every replacement, then + /// issues a single geometry bump for the whole set instead of one bump per + /// entity. + pub fn update_entities(&mut self, updates: Vec) -> usize { + let mut moved = 0usize; + let mut changed = Vec::with_capacity(updates.len()); + let mut block_change = false; + for entity in updates { + if self.update_entity_impl(entity, &mut changed, &mut block_change) { + moved += 1; + } + } + if !changed.is_empty() || block_change { + self.publish_entity_updates(changed, block_change); + } + moved + } + + /// One geometry bump for a set of slot writes already reflected in + /// `changed` / `block_change`. + fn publish_entity_updates(&mut self, changed: Vec<(Handle, ChangeKind)>, block_change: bool) { + if block_change { + self.bump_geometry(); + } else { + self.bump_entities(&changed); + } + } + + /// Shared write path for `update_entity` / `update_entities`: validates, + /// records undo, writes the slot, and reseeds derived caches, recording + /// the change instead of bumping geometry. + fn update_entity_impl( + &mut self, + mut entity: EntityType, + changed: &mut Vec<(Handle, ChangeKind)>, + block_change: &mut bool, + ) -> bool { let handle = entity.common().handle; if self.is_layer_locked(handle) { return false; @@ -637,12 +683,12 @@ impl Scene { if affects_blocks { self.mark_entity_dirty(handle); - self.bump_geometry(); + *block_change = true; } else { // One entity changed in place: report just this handle so every // derived cache patches it instead of rebuilding (bump_entities also // drops it from the tessellation memos). - self.bump_entities(&[(handle, ChangeKind::Modified)]); + changed.push((handle, ChangeKind::Modified)); } true }