1b-agent/OneCAD-Draft
1
0 Fork 0
forked from 1b-agent/opencadstudio-upstream
Code Pull Requests Activity

Merge PR #1101: reduce selection property work

Browse Source
This commit is contained in:
Hakan Seven committed 2026-09-09 13:09:18 +03:00
2 parents 7510cc76bf 273821658a
commit 55908fefce
1 file changed
+180 -22
Download Patch File Download Diff File Expand all files Collapse all files
src/app/properties.rs
+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"));
}
}