Stop the properties panel walking the whole selection

Rebuilding the properties panel for a 186 468-entity selection took ~112 ms,
and four things in it scaled with the selection for no reason.

The panel filtered the selection down to the active group with
`group.handles.contains(handle)`, a linear scan of a `Vec`, once per selected
entity. The default group is "All", which holds every selected handle, so
both factors are the whole selection: 186 468 squared. Hashing the group once
makes it linear. The measured cost never showed this — 55 ms at 1 993
entities against 112 ms at 186 468 is not a quadratic curve — but the scan is
still wrong, and it is the shape of thing that turns a large selection into
an unresponsive window.

`entity_in_working_plane` clones the entity before it checks whether the
plane is the identity, so with no UCS in play the panel deep-cloned every
selected entity to get back what it was handed. It now borrows, and clones
only where a transform is applied.

`aggregate_sections` built every entity's property list, collected them all,
then folded them together and read each exactly once. A property list is
sections of labels and formatted values, every one an allocation, so a
whole-drawing selection held 186 468 of them in memory at the same time. It
now builds one, folds it, and drops it, and stops once nothing is left in
common. The fold order is unchanged, so the result is too.

The ribbon sync cloned each entity's linetype into a `String` only to compare
it, and kept walking after every field had been found mixed. Once all four
are mixed nothing further can change, so it stops there.

`aggregate_sections` and `merge_sections` had no tests, so the fold that
drives the whole multi-selection panel was resting on the compiler. Two now
cover it: a field the entities agree on keeps its value, one they differ on
reads as varying, and a single entity folds to its own rows.

`properties-detail` and `properties-panel` report the phases, which is how
the panel arm was ruled out as the remaining cost — it does not reach its
5 ms floor at 186 468 entities.
This commit is contained in:
Panos committed 2026-09-09 10:38:12 +03:00
1 parent f128ed3eaf
commit 273821658a
1 file changed
+180 -22
+180 -22
View File
@@ -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<acadrust::Handle> = 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<Handle> =
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 != &lt => 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<Vec<crate::scene::model::object::PropSection>> = 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, &sections);
}
// 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(&sections, "layer").expect("a layer row");
assert!(
format!("{:?}", layer.value).contains("WALLS"),
"one layer across all three: {:?}",
layer.value,
);
let color = row(&sections, "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(&sections, "layer").expect("a layer row");
assert!(format!("{:?}", layer.value).contains("WALLS"));
}
}