Merge PR #1098: scale bulk selections

This commit is contained in:
Hakan Seven committed 2026-09-09 13:09:18 +03:00
commit bba46f1c9c
5 files changed
+424 -42

No files matched your search

+69
View File
@@ -2426,6 +2426,11 @@ impl OpenCADStudio {
}
/// Rebuild the cached selected_grips from the current entity selection.
/// Selections larger than this get no grips, matching AutoCAD's
/// `GRIPOBJLIMIT` default. The DWG header does not carry the variable, so
/// the default stands in for it.
const GRIP_OBJECT_LIMIT: usize = 100;
pub(super) fn refresh_selected_grips(&mut self) {
let i = self.active_tab;
let locked_active_grip = self.tabs[i].active_grip.as_ref().is_some_and(|grip| {
@@ -2453,6 +2458,22 @@ impl OpenCADStudio {
.then(|| selected[0].0);
let mut grips = Vec::new();
let mut handles = Vec::new();
// Above the limit, no grips at all — the same rule AutoCAD applies
// through GRIPOBJLIMIT, and for the same reason. Every selected
// entity contributes several grips, the whole list is projected and
// rebuilt into markers on **every frame**, and a 185 096-entity
// selection put ~3.9 s in this function and then held the view at
// ~10 fps for as long as the selection stood.
//
// Downstream this reads as "this selection has no grips": the
// consumers all iterate the list, so they simply find nothing, and
// grip editing is unavailable for a selection far past the size
// where dragging one is a sensible thing to do.
let selected = if selected.len() > Self::GRIP_OBJECT_LIMIT {
Vec::new()
} else {
selected
};
for (handle, entity) in selected {
if self.tabs[i].scene.is_layer_locked(handle) {
continue;
@@ -4027,3 +4048,51 @@ mod chprop_integration_tests {
assert_eq!(app.ribbon.active_lineweight, new_lw);
}
}
#[cfg(test)]
mod grip_limit_tests {
use super::*;
/// Every selected entity contributes grips, and the whole list is projected
/// and rebuilt into markers on every frame. A 185 096-entity selection put
/// ~3.9 s in `refresh_selected_grips` and then held the view at ~10 fps for
/// as long as the selection stood, which is what AutoCAD's `GRIPOBJLIMIT`
/// exists to prevent.
#[test]
fn a_selection_past_the_limit_gets_no_grips() {
use acadrust::entities::{EntityType, Line};
use acadrust::types::Vector3;
let select_n = |n: usize| -> usize {
let mut app = OpenCADStudio::new_for_test();
app.automation_op(r#"{"op":"new"}"#);
let i = app.active_tab;
let handles: Vec<_> = (0..n)
.map(|k| {
let x = k as f64;
app.tabs[i].scene.add_entity(EntityType::Line(
Line::from_points(
Vector3::new(x, 0.0, 0.0),
Vector3::new(x + 1.0, 1.0, 0.0),
),
))
})
.collect();
for handle in handles {
app.tabs[i].scene.select_entity(handle, false);
}
app.refresh_selected_grips();
app.tabs[i].selected_grips.len()
};
assert!(
select_n(OpenCADStudio::GRIP_OBJECT_LIMIT) > 0,
"a selection at the limit must still show its grips",
);
assert_eq!(
select_n(OpenCADStudio::GRIP_OBJECT_LIMIT + 1),
0,
"one past the limit must show none at all",
);
}
}
+50 -18
View File
@@ -4093,6 +4093,10 @@ impl OpenCADStudio {
aabb
},
);
// A 186 k-entity box selection sits at ~800 ms in this
// handler and two guesses about which step owns it have
// both been wrong. Split it.
let t_sel = crate::perf::enabled().then(Instant::now);
let area_candidates = self.tabs[i].scene.interaction_candidates_in_aabb(
all_wires,
world_aabb,
@@ -4101,6 +4105,9 @@ impl OpenCADStudio {
eye,
bounds,
);
let cand_ms = t_sel
.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0);
let t_hit = crate::perf::enabled().then(Instant::now);
let candidate_handles = self.tabs[i]
.scene
.interaction_candidate_handles(&area_candidates);
@@ -4161,20 +4168,30 @@ impl OpenCADStudio {
// selection, Shift+box removes the boxed
// entities. Esc / empty-space click still clears.
// PICKADD 0 (#226): a plain box REPLACES.
let hit_ms = t_hit
.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0);
let t_apply = crate::perf::enabled().then(Instant::now);
if self.shift_down || self.select_remove_mode {
for h in &handles {
self.tabs[i].scene.deselect_entity(*h);
}
self.tabs[i].scene.deselect_entities(&handles);
} else {
if !selection_pick_add && !handles.is_empty() {
self.tabs[i].scene.deselect_all();
}
for h in &handles {
self.tabs[i].scene.select_entity(*h, false);
}
self.tabs[i].scene.select_entities(&handles);
self.tabs[i].scene.expand_selection_for_groups(&handles);
}
let apply_ms = t_apply
.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0);
let t_props = crate::perf::enabled().then(Instant::now);
self.refresh_properties();
if crate::perf::enabled() {
crate::perf_record!(
"[perf] select-commit kind=drag-box candidates={cand_ms:.1}ms \
hit={hit_ms:.1}ms apply={apply_ms:.1}ms properties={:.1}ms picked={}",
t_props.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0),
handles.len(),
);
}
selection_just_completed = true;
}
} else {
@@ -4208,9 +4225,7 @@ impl OpenCADStudio {
// leaves the current selection untouched so a stray
// drag never discards hard-won picks.
if self.shift_down || self.select_remove_mode {
for h in &handles {
self.tabs[i].scene.deselect_entity(*h);
}
self.tabs[i].scene.deselect_entities(&handles);
} else {
// PICKADD 0 (#226): a plain marquee REPLACES
// the selection (empty results still leave it
@@ -4218,9 +4233,7 @@ impl OpenCADStudio {
if !selection_pick_add && !handles.is_empty() {
self.tabs[i].scene.deselect_all();
}
for h in &handles {
self.tabs[i].scene.select_entity(*h, false);
}
self.tabs[i].scene.select_entities(&handles);
self.tabs[i].scene.expand_selection_for_groups(&handles);
}
self.refresh_properties();
@@ -4454,6 +4467,10 @@ impl OpenCADStudio {
aabb
},
);
// This is the path a click-move-click window takes, as
// opposed to a press-drag; it is the one a large selection
// actually goes through.
let t_sel = crate::perf::enabled().then(Instant::now);
let area_candidates = self.tabs[i].scene.interaction_candidates_in_aabb(
all_wires,
world_aabb,
@@ -4462,6 +4479,9 @@ impl OpenCADStudio {
eye,
bounds,
);
let cand_ms =
t_sel.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0);
let t_hit = crate::perf::enabled().then(Instant::now);
let candidate_handles = self.tabs[i]
.scene
.interaction_candidate_handles(&area_candidates);
@@ -4517,27 +4537,39 @@ impl OpenCADStudio {
bounds,
candidate_handles.as_ref(),
));
let hit_ms = t_hit.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0);
let t_filter = crate::perf::enabled().then(Instant::now);
// Selection filter: keep only allowed types.
handles.retain(|&h| self.tabs[i].scene.passes_selection_filter(h));
let filter_ms =
t_filter.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0);
let t_apply = crate::perf::enabled().then(Instant::now);
// Accumulate (issue #83): a plain box adds to the
// current selection, Shift+box removes the boxed
// entities. An empty box leaves the selection alone
// so an accidental empty drag never discards it.
// PICKADD 0 (#226): a plain box REPLACES instead.
if self.shift_down || self.select_remove_mode {
for h in &handles {
self.tabs[i].scene.deselect_entity(*h);
}
self.tabs[i].scene.deselect_entities(&handles);
} else {
if !selection_pick_add && !handles.is_empty() {
self.tabs[i].scene.deselect_all();
}
for h in &handles {
self.tabs[i].scene.select_entity(*h, false);
}
self.tabs[i].scene.select_entities(&handles);
self.tabs[i].scene.expand_selection_for_groups(&handles);
}
let apply_ms =
t_apply.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0);
let t_props = crate::perf::enabled().then(Instant::now);
self.refresh_properties();
if crate::perf::enabled() {
crate::perf_record!(
"[perf] select-commit kind=window candidates={cand_ms:.1}ms \
hit={hit_ms:.1}ms filter={filter_ms:.1}ms apply={apply_ms:.1}ms properties={:.1}ms picked={}",
t_props.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0),
handles.len(),
);
}
let mut sel = self.tabs[i].scene.selection.borrow_mut();
sel.box_last = Some((a, p));
sel.box_last_crossing = crossing;
+64 -15
View File
@@ -183,21 +183,22 @@ impl Scene {
handles: &[Handle],
) -> HashSet<Handle> {
let mut expanded: HashSet<Handle> = handles.iter().copied().collect();
expanded.extend(
self.document
.objects
.values()
.filter_map(|obj| match obj {
ObjectType::Group(g)
if g.selectable
&& handles.iter().any(|handle| g.contains(*handle)) =>
{
Some(g.entities.clone())
}
_ => None,
})
.flatten(),
);
// Asked the other way round, this was quadratic: for every group, scan
// every selected handle, and `Group::contains` is a linear scan of the
// group's own entity list. A box selection of 185 096 entities spent
// seconds here.
//
// The membership set is the *input* handles, not `expanded` — a group
// must match because something the user selected is in it, not because
// an earlier group already contributed one of its members.
let wanted: HashSet<Handle> = handles.iter().copied().collect();
for obj in self.document.objects.values() {
if let ObjectType::Group(g) = obj {
if g.selectable && g.entities.iter().any(|e| wanted.contains(e)) {
expanded.extend(g.entities.iter().copied());
}
}
}
expanded
}
@@ -211,3 +212,51 @@ impl Scene {
}
}
}
#[cfg(test)]
mod group_expansion_tests {
use super::*;
use crate::scene::Scene;
/// Selecting one member of a selectable group selects the whole group, and
/// a group nobody touched stays out. The lookup was inverted for speed —
/// it asked every group about every selected handle, and `Group::contains`
/// scans the group's own list — so the answer has to be shown unchanged.
#[test]
fn group_expansion_pulls_in_siblings_and_nothing_else() {
use acadrust::entities::{EntityType, Line};
use acadrust::types::Vector3;
let mut scene = Scene::new();
let mut line = |x: f64| {
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 (a, b, c) = (line(0.0), line(1.0), line(2.0));
let (d, e) = (line(3.0), line(4.0));
let lonely = line(9.0);
scene.create_group("GRUPPO".to_string(), vec![a, b, c]);
scene.create_group("ALTRO".to_string(), vec![d, e]);
// One member pulls in its siblings, and only its own group's.
let from_a = scene.handles_expanded_for_selectable_groups(&[a]);
assert_eq!(
from_a,
[a, b, c].into_iter().collect::<HashSet<_>>(),
"selecting a member must select that group and no other",
);
// An entity in no group expands to itself.
assert_eq!(
scene.handles_expanded_for_selectable_groups(&[lonely]),
[lonely].into_iter().collect::<HashSet<_>>(),
);
// Touching both groups pulls in both, and nothing outside them.
let both = scene.handles_expanded_for_selectable_groups(&[a, e]);
assert_eq!(both, [a, b, c, d, e].into_iter().collect::<HashSet<_>>());
assert!(!both.contains(&lonely));
}
}
+10
View File
@@ -1877,6 +1877,13 @@ pub struct Scene {
entity_block_map_cache: RefCell<Option<(u64, HashMap<Handle, Handle>)>>,
/// Candidate handles per block, in document order and keyed by geometry epoch.
block_members_cache: RefCell<Option<BlockMembers>>,
/// Annotation handle -> the LEADERs that point at it, resolved once per
/// `geometry_epoch`.
///
/// Selecting an entity has to pull in a LEADER whose annotation it is, and
/// that used to be answered by walking the whole document — per selected
/// handle. Selecting N entities cost N document walks.
leaders_by_annotation_cache: RefCell<Option<(u64, HashMap<Handle, Vec<Handle>>)>>,
/// Insert/Viewport/Block/BlockEnd handles omitted by the spatial index.
unindexable_cache: RefCell<Option<(u64, Vec<Handle>)>>,
/// `(epoch, layout block, present type names, list exposed to the UI)`.
@@ -2128,6 +2135,7 @@ impl Scene {
model_extents_cache: RefCell::new(None),
entity_block_map_cache: RefCell::new(None),
block_members_cache: RefCell::new(None),
leaders_by_annotation_cache: RefCell::new(None),
unindexable_cache: RefCell::new(None),
layout_type_names_cache: RefCell::new(None),
dependency_index_cache: RefCell::new(None),
@@ -10797,6 +10805,8 @@ mod journal_tests {
assert_eq!(folded.len(), before.len() + 1);
}
// Differential oracle: the incrementally-patched entity index must always
// equal a from-scratch rebuild after any add / move / erase.
#[test]
fn block_member_index_matches_the_full_scan() {
use acadrust::entities::Line;
+231 -9
View File
@@ -4,11 +4,110 @@ impl Scene {
// ── Selection ─────────────────────────────────────────────────────────
/// Treat a classic LEADER and its attached annotation as one logical object.
/// Clicking/copying/deleting either side expands to the complete pair.
/// Every LEADER that points at `annotation`, resolved once per
/// `geometry_epoch` rather than by walking the document per handle.
fn leaders_by_annotation(
&self,
) -> std::cell::Ref<'_, (u64, HashMap<Handle, Vec<Handle>>)> {
{
let cache = self.leaders_by_annotation_cache.borrow();
if cache.as_ref().is_some_and(|(epoch, _)| *epoch == self.geometry_epoch) {
drop(cache);
return std::cell::Ref::map(
self.leaders_by_annotation_cache.borrow(),
|c| c.as_ref().unwrap(),
);
}
}
let mut by_annotation: HashMap<Handle, Vec<Handle>> = HashMap::default();
for entity in self.document.entities() {
if let EntityType::Leader(leader) = entity {
if !leader.annotation_handle.is_null() {
by_annotation
.entry(leader.annotation_handle)
.or_default()
.push(entity.common().handle);
}
}
}
*self.leaders_by_annotation_cache.borrow_mut() =
Some((self.geometry_epoch, by_annotation));
std::cell::Ref::map(self.leaders_by_annotation_cache.borrow(), |c| {
c.as_ref().unwrap()
})
}
/// The handles plus their leader/annotation partners, in the order given.
///
/// Kept separate from the sorted, deduplicated form because a bulk
/// selection has to preserve pick order — `selected_handles_in_order`
/// reports it — and because sorting per handle is what made selecting
/// 186 000 entities one call at a time expensive.
fn expanded_with_leaders(&self, handles: &[Handle]) -> Vec<Handle> {
let leaders = self.leaders_by_annotation();
let mut expanded = Vec::with_capacity(handles.len());
for &handle in handles {
expanded.push(handle);
if let Some(EntityType::Leader(leader)) = self.document.get_entity(handle) {
if !leader.annotation_handle.is_null() {
expanded.push(leader.annotation_handle);
}
}
if let Some(pointing) = leaders.1.get(&handle) {
expanded.extend(pointing.iter().copied());
}
}
expanded
}
/// Select many handles in one pass.
///
/// The single-handle path allocates, sorts and deduplicates for every
/// entity, and a box selection called it once per entity — most of the
/// remaining 786 ms of a 186 460-entity selection.
pub fn select_entities(&mut self, handles: &[Handle]) {
if handles.is_empty() {
return;
}
let expanded = self.expanded_with_leaders(handles);
let mut changed = false;
for handle in expanded {
if self.selected.insert(handle) {
self.selected_order.push(handle);
changed = true;
}
}
if changed {
self.bump_selection_set();
}
}
/// Deselect many handles in one pass.
///
/// `deselect_entity` rebuilds `selected_order` with a `retain` per handle,
/// so removing N of M selected entities walked the order list N times.
pub fn deselect_entities(&mut self, handles: &[Handle]) {
if handles.is_empty() {
return;
}
let doomed: HashSet<Handle> =
self.expanded_with_leaders(handles).into_iter().collect();
let mut changed = false;
for handle in &doomed {
changed |= self.selected.remove(handle);
}
if changed {
self.selected_order.retain(|handle| !doomed.contains(handle));
self.bump_selection_set();
}
}
pub(crate) fn handles_expanded_for_leader_annotations(
&self,
handles: &[Handle],
) -> Vec<Handle> {
let mut expanded = handles.to_vec();
let leaders = self.leaders_by_annotation();
for &handle in handles {
// LEADER -> annotation.
@@ -19,15 +118,9 @@ impl Scene {
}
// Annotation -> LEADER.
expanded.extend(self.document.entities().filter_map(|entity| match entity {
EntityType::Leader(leader)
if !leader.annotation_handle.is_null()
&& leader.annotation_handle == handle =>
{
Some(entity.common().handle)
}
_ => None,
}));
if let Some(pointing) = leaders.1.get(&handle) {
expanded.extend(pointing.iter().copied());
}
}
expanded.sort_unstable_by_key(|handle| handle.value());
@@ -1003,6 +1096,135 @@ impl Scene {
mod tests {
use super::*;
/// A box selection used to call `select_entity` once per entity. The bulk
/// path has to land on the same selection, in the same pick order —
/// `selected_handles_in_order` is read by commands such as DIM, where the
/// order the user picked things in decides what they mean.
#[test]
fn bulk_selection_matches_selecting_one_at_a_time() {
use acadrust::entities::Line;
use acadrust::types::Vector3;
let build = || {
let mut scene = Scene::new();
let handles: Vec<_> = (0..40)
.map(|k| {
let x = k as f64;
scene.add_entity(EntityType::Line(Line::from_points(
Vector3::new(x, 0.0, 0.0),
Vector3::new(x + 1.0, 0.0, 0.0),
)))
})
.collect();
(scene, handles)
};
// Picked in an order that is neither creation nor handle order.
let picked = |handles: &[Handle]| -> Vec<Handle> {
let mut order: Vec<_> = handles.iter().copied().collect();
order.reverse();
order.retain(|h| h.value() % 3 != 0);
order
};
let (mut one_at_a_time, handles) = build();
let order = picked(&handles);
for &handle in &order {
one_at_a_time.select_entity(handle, false);
}
let (mut in_bulk, handles) = build();
in_bulk.select_entities(&picked(&handles));
assert_eq!(
in_bulk.selected_handles_in_order(),
one_at_a_time.selected_handles_in_order(),
"the bulk path must select the same entities in the same order",
);
// And removing a subset must agree too.
let drop: Vec<_> = order.iter().copied().take(7).collect();
for &handle in &drop {
one_at_a_time.deselect_entity(handle);
}
in_bulk.deselect_entities(&drop);
assert_eq!(
in_bulk.selected_handles_in_order(),
one_at_a_time.selected_handles_in_order(),
"the bulk removal must leave the same selection in the same order",
);
}
/// Selecting an entity pulls in any LEADER pointing at it, and that was
/// answered by walking the whole document once per selected handle —
/// selecting 471 583 entities in a 585 786-entity drawing did not finish
/// inside eight minutes. The reverse index has to give exactly what the
/// walk gave.
#[test]
fn the_leader_index_matches_a_document_walk() {
use acadrust::entities::Leader;
use acadrust::types::Vector3;
let mut scene = Scene::new();
let line = |x: f64| {
EntityType::Line(acadrust::entities::Line::from_points(
Vector3::new(x, 0.0, 0.0),
Vector3::new(x + 1.0, 0.0, 0.0),
))
};
let annotation = scene.add_entity(line(0.0));
let unrelated = scene.add_entity(line(5.0));
let mut leader_on = |target: Handle| {
let mut leader = Leader::default();
leader.annotation_handle = target;
scene.add_entity(EntityType::Leader(leader))
};
// Two leaders share one annotation; a third points nowhere.
let first = leader_on(annotation);
let second = leader_on(annotation);
let dangling = leader_on(Handle::NULL);
// The walk this replaced, written out so the index is compared against
// behaviour rather than against itself.
let by_walk = |handle: Handle| -> Vec<Handle> {
let mut out = vec![handle];
if let Some(EntityType::Leader(leader)) = scene.document.get_entity(handle) {
if !leader.annotation_handle.is_null() {
out.push(leader.annotation_handle);
}
}
out.extend(scene.document.entities().filter_map(|entity| match entity {
EntityType::Leader(leader)
if !leader.annotation_handle.is_null()
&& leader.annotation_handle == handle =>
{
Some(entity.common().handle)
}
_ => None,
}));
out.sort_unstable_by_key(Handle::value);
out.dedup();
out
};
for handle in [annotation, unrelated, first, second, dangling] {
assert_eq!(
scene.handles_expanded_for_leader_annotations(&[handle]),
by_walk(handle),
"the index must answer exactly what the walk answered",
);
}
let expanded = scene.handles_expanded_for_leader_annotations(&[annotation]);
assert!(
expanded.contains(&first) && expanded.contains(&second),
"both leaders on one annotation must come back, not just one",
);
}
/// The reason the cache carries a set as well as a list: drawing into a
/// drawing that already has that type must not rebuild either, because the
/// rebuild walks every entity the layout owns — 444 937 of them on the
/// reproducer, ~100 ms, on every single drawn point.
#[test]
fn adding_a_type_already_present_reuses_the_list() {
use acadrust::entities::{Circle, EntityType, Line};