diff --git a/src/app/properties.rs b/src/app/properties.rs index 39d6e29d6..c2df6d4b3 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -32,6 +32,10 @@ impl OpenCADStudio { /// Rebuild the PropertiesPanel from the current entity selection. /// Preserves UI state (open pickers, edit buffer) across refreshes. pub(super) fn refresh_properties(&mut self) { + // A 186 468-entity selection spends ~112 ms in here. Split it into the + // three phases that can own that: resolving the selected handles, + // building the panel, and syncing the ribbon. + let t_all = crate::perf::enabled().then(std::time::Instant::now); let i = self.active_tab; if !crate::entities::object_data::cache_is_prepared( &self.tabs[i].scene.object_data_cache, @@ -105,12 +109,21 @@ impl OpenCADStudio { // Current-Vertex focus survives only while the same object stays // selected; a changed selection resets to the first vertex. Seed the // per-thread focus so the polyline property builder / editor targets it. + // Everything before this is the prelude: the object-data cache, the + // unit context, the layer and linetype lists. + let m_prelude = t_all.map(|t| t.elapsed().as_secs_f64() * 1000.0); + let t_handles = crate::perf::enabled().then(std::time::Instant::now); + // `selected_entities` resolves every handle to an entity and builds a + // vector of pairs; only the handles are wanted here, but the resolution + // is what decides membership — a handle whose entity is gone is not in + // the list — so it is kept, and only the pair vector avoided. let cur_handles: Vec = self.tabs[i] .scene - .selected_entities() - .iter() - .map(|(h, _)| *h) + .selected_handles_in_order() + .into_iter() + .filter(|h| self.tabs[i].scene.document.get_entity(*h).is_some()) .collect(); + let handles_ms = t_handles.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0); let prop_vertex = if cur_handles == prev_handles { self.tabs[i].properties.prop_vertex } else { @@ -2179,38 +2192,81 @@ impl OpenCADStudio { ..Default::default() }, _ => { + // The panel for a multi-selection: grouping, filtering to + // the active group, moving into the working plane, and + // aggregating into "varies" rows. `properties-detail` + // reports this whole arm as `rest`; this splits it. + let t_arm = crate::perf::enabled().then(std::time::Instant::now); let groups = build_selection_groups(&selected); + let t_groups = t_arm.map(|t| t.elapsed().as_secs_f64() * 1000.0); let active_group = selected_group .and_then(|group| groups.iter().find(|g| g.label == group.label).cloned()) .or_else(|| groups.first().cloned()); + // `group.handles` is a `Vec`, so `contains` scans it. Run + // per selected entity that is O(selected x group), which on + // a whole-drawing selection is 186 468 squared — the "All" + // group holds every handle, so both factors are the full + // selection. Hashing the group once makes it linear. let filtered: Vec<(Handle, &EntityType)> = active_group .as_ref() .map(|group| { + let wanted: rustc_hash::FxHashSet = + group.handles.iter().copied().collect(); selected .iter() - .filter(|(handle, _)| group.handles.contains(handle)) + .filter(|(handle, _)| wanted.contains(handle)) .copied() .collect() }) .unwrap_or_default(); + let t_filter = t_arm.map(|t| t.elapsed().as_secs_f64() * 1000.0); let plane = if self.tabs[i].editing_model_space() { self.tabs[i].ucs_xform().working_plane() } else { crate::command::WorkingPlane::default() }; - let local_entities: Vec<(Handle, EntityType)> = filtered - .iter() - .map(|(handle, entity)| { - (*handle, dispatch::entity_in_working_plane(entity, plane)) - }) - .collect(); + // `entity_in_working_plane` clones before it checks + // whether the plane is the identity, so with no UCS in + // play — the ordinary case — this deep-cloned every + // selected entity to hand back what it was given. Borrow + // instead, and clone only where a transform is applied. + let local_entities: Vec<(Handle, std::borrow::Cow<'_, EntityType>)> = + filtered + .iter() + .map(|(handle, entity)| { + let local = if plane.is_identity() { + std::borrow::Cow::Borrowed(*entity) + } else { + std::borrow::Cow::Owned( + dispatch::entity_in_working_plane(entity, plane), + ) + }; + (*handle, local) + }) + .collect(); let local_refs: Vec<(Handle, &EntityType)> = local_entities .iter() - .map(|(handle, entity)| (*handle, entity)) + .map(|(handle, entity)| (*handle, entity.as_ref())) .collect(); + let t_local = t_arm.map(|t| t.elapsed().as_secs_f64() * 1000.0); let mut sections = aggregate_sections(&local_refs, &text_style_names); + if let (Some(t), Some(groups_ms), Some(filter_ms), Some(local_ms)) = + (t_arm, t_groups, t_filter, t_local) + { + let total = t.elapsed().as_secs_f64() * 1000.0; + if total >= 5.0 { + crate::perf_record!( + "[perf] properties-panel {total:>7.1}ms groups={groups_ms:.1} \ +filter={:.1} local={:.1} aggregate={:.1} entities={}", + filter_ms - groups_ms, + local_ms - filter_ms, + total - local_ms, + local_refs.len(), + ); + } + } if local_refs.iter().all(|(handle, _)| { crate::scene::model::solid_history::has_compact_solid_properties( &self.tabs[i].scene.document, @@ -2315,7 +2371,24 @@ impl OpenCADStudio { self.tabs[i].properties = new_panel; self.refresh_selected_grips(); + let m_panel = t_all.map(|t| t.elapsed().as_secs_f64() * 1000.0); + let t_ribbon = crate::perf::enabled().then(std::time::Instant::now); self.sync_ribbon_from_selection(); + if let Some(t) = t_all { + let ribbon_ms = t_ribbon.map_or(0.0, |r| r.elapsed().as_secs_f64() * 1000.0); + let total_ms = t.elapsed().as_secs_f64() * 1000.0; + if total_ms >= 5.0 { + let prelude = m_prelude.unwrap_or(0.0); + let panel = m_panel.unwrap_or(0.0); + crate::perf_record!( + "[perf] properties-detail {total_ms:>7.1}ms prelude={prelude:.1} \ +handles={handles_ms:.1} panel={:.1} ribbon={ribbon_ms:.1} tail={:.1} selected={}", + panel - prelude - handles_ms, + total_ms - panel - ribbon_ms, + cur_handles.len(), + ); + } + } } /// Drive the Home-ribbon Layer / Color / Linetype / Lineweight dropdowns @@ -2376,11 +2449,20 @@ impl OpenCADStudio { let mut lineweight_mixed = false; for (_h, e) in &selected { + // Once every field is mixed nothing further can change: a mixed + // field is already `Some`, so the remaining entities would only + // re-set flags that are set. Selecting a whole drawing reaches this + // within the first few entities instead of walking 186 468 of them. + if layer_mixed && color_mixed && linetype_mixed && lineweight_mixed { + break; + } let c = e.common(); + // Borrowed, not cloned: this only ever gets compared, and cloning + // it here was one heap allocation per selected entity. let lt = if c.linetype.is_empty() { - "ByLayer".to_string() + "ByLayer" } else { - c.linetype.clone() + c.linetype.as_str() }; match &layer { None => layer = Some(c.layer.clone()), @@ -2393,8 +2475,8 @@ impl OpenCADStudio { _ => {} } match &linetype { - None => linetype = Some(lt), - Some(prev) if prev != < => linetype_mixed = true, + None => linetype = Some(lt.to_string()), + Some(prev) if prev.as_str() != lt => linetype_mixed = true, _ => {} } match &lineweight { @@ -3094,13 +3176,23 @@ pub(super) fn aggregate_sections( return vec![]; } - let mut all_sections: Vec> = selected - .iter() - .map(|(handle, entity)| dispatch::properties_sectioned(*handle, entity, text_style_names)) - .collect(); - - let mut result = all_sections.remove(0); - for sections in all_sections { + // Built one at a time and folded as they come. Collecting them first held + // a property list — sections, labels, formatted values, every one of them + // an allocation — for every selected entity at once, so selecting a whole + // drawing put 186 468 of them in memory simultaneously to read each one + // exactly once. The fold order is unchanged, so the result is too. + let mut entities = selected.iter(); + let Some((handle, entity)) = entities.next() else { + return vec![]; + }; + let mut result = dispatch::properties_sectioned(*handle, entity, text_style_names); + for (handle, entity) in entities { + // Nothing in common left to narrow: every later entity can only + // intersect against an empty set. + if result.is_empty() { + break; + } + let sections = dispatch::properties_sectioned(*handle, entity, text_style_names); result = merge_sections(&result, §ions); } // Sum the filled area while individual Area rows may still vary. @@ -4096,3 +4188,69 @@ mod grip_limit_tests { ); } } + +#[cfg(test)] +mod aggregation_tests { + use super::*; + use acadrust::EntityType; + + fn line(layer: &str, color: i16) -> EntityType { + let mut line = acadrust::entities::line::Line::default(); + line.common.layer = layer.to_string(); + line.common.color = acadrust::types::Color::from_index(color); + EntityType::Line(line) + } + + fn row<'a>( + sections: &'a [crate::scene::model::object::PropSection], + field: &str, + ) -> Option<&'a crate::scene::model::object::Property> { + sections + .iter() + .flat_map(|section| section.props.iter()) + .find(|property| property.field == field) + } + + // The aggregation folds one entity at a time rather than building every + // entity's property list first. This pins the behaviour that fold produces: + // a field all the entities agree on keeps its value, one they differ on + // reads as varying. + #[test] + fn shared_values_survive_the_fold_and_differing_ones_do_not() { + let entities = [line("WALLS", 1), line("WALLS", 3), line("WALLS", 1)]; + let selected: Vec<(Handle, &EntityType)> = entities + .iter() + .enumerate() + .map(|(i, entity)| (Handle::new(i as u64 + 1), entity)) + .collect(); + + let sections = aggregate_sections(&selected, &[]); + assert!(!sections.is_empty(), "three lines share their layer rows"); + + // A colour the entities disagree on has its own variant rather than + // the text label, so the two rows are checked on their own terms. + let layer = row(§ions, "layer").expect("a layer row"); + assert!( + format!("{:?}", layer.value).contains("WALLS"), + "one layer across all three: {:?}", + layer.value, + ); + let color = row(§ions, "color").expect("a color row"); + assert!( + format!("{:?}", color.value).contains("Varies"), + "two colours across three lines: {:?}", + color.value, + ); + } + + // One entity folds to its own properties, and the fold must not depend on + // there being a second one to merge against. + #[test] + fn a_single_entity_aggregates_to_its_own_rows() { + let entity = line("WALLS", 1); + let selected = [(Handle::new(1), &entity)]; + let sections = aggregate_sections(&selected, &[]); + let layer = row(§ions, "layer").expect("a layer row"); + assert!(format!("{:?}", layer.value).contains("WALLS")); + } +}