forked from 1b-agent/opencadstudio-upstream
Say where a selection's hit-testing time goes, and stop hashing to skip wires
`hit` was one number covering five things: resolving candidate handles, the wire box test, two hatch tests and two mesh tests. `select-commit` now reports them apart, which immediately reframed the problem — at 186 468 entities the wire box test is 55 ms of 206, the hatch tests are 103, and handle resolution is 47. Two changes alongside it, both in the crossing scan. Skipping a wire already in the selection was keyed by hashing its name, once per triangle and once per glyph inside the box. A bitmap over wire indices answers the same question with an array index. The name set stays behind it, because several wires can share one entity's name and the output dedups by name; a wire whose name is already in that set now gets tested once more before being dropped, which costs one projection and changes no output. The degenerate-wire loop that ends both the box and the lasso scans tested `seen.contains(wire.name)` before the structural tests beside it. `||` short-circuits, so with the name lookup last a wire with two or more points — very nearly all of them — is dropped without hashing anything. That loop walks every candidate, so it was one string hash per wire in the box.
This commit is contained in:
1 parent
273821658a
commit
f9cacaec2b
2 files changed
+67
-13
No files matched your search
@@ -4251,6 +4251,12 @@ properties={:.1}ms picked={}",
|
||||
if box_anchor.is_none() {
|
||||
let (view_rot, eye, all_wires) = self.pick_view(i, &edit_cam, bounds);
|
||||
let click_world = self.cursor_model_point(i, &edit_cam, p, bounds);
|
||||
// The click that *starts* a window: the pick under the cursor decides
|
||||
// whether this selects an entity or arms a box, and it runs before the
|
||||
// rubber band can appear. Timed separately from the commit, which is
|
||||
// what `select-commit` covers.
|
||||
let t_arm = crate::perf::enabled().then(Instant::now);
|
||||
let prior_selection = self.tabs[i].scene.selected.len();
|
||||
let click_candidates = self.tabs[i].scene.interaction_pick_candidates_near(
|
||||
all_wires,
|
||||
click_world,
|
||||
@@ -4385,6 +4391,16 @@ properties={:.1}ms picked={}",
|
||||
// (#234). Computed before the selection
|
||||
// borrow so the &self projection can't clash.
|
||||
let anchor_world = self.cursor_model_point(i, &edit_cam, p, bounds);
|
||||
if let Some(t) = t_arm {
|
||||
let arm_ms = t.elapsed().as_secs_f64() * 1000.0;
|
||||
if arm_ms >= 5.0 {
|
||||
crate::perf_record!(
|
||||
"[perf] select-arm {arm_ms:>7.1}ms pick+clear, \
|
||||
was_selected={}",
|
||||
prior_selection,
|
||||
);
|
||||
}
|
||||
}
|
||||
let mut sel = self.tabs[i].scene.selection.borrow_mut();
|
||||
// Full-canvas space: ViewportMove updates
|
||||
// box_current in canvas coords and the overlay
|
||||
@@ -4486,6 +4502,10 @@ properties={:.1}ms picked={}",
|
||||
let candidate_handles = self.tabs[i]
|
||||
.scene
|
||||
.interaction_candidate_handles(&area_candidates);
|
||||
// `hit` covers five things, and which of them owns it has
|
||||
// never been separated: resolving candidate handles, the wire
|
||||
// box test, the two hatch tests and the two mesh tests.
|
||||
let m_handles = t_hit.map(|t| t.elapsed().as_secs_f64() * 1000.0);
|
||||
let mut handles: Vec<Handle> = scene::pick::hit_test::box_hit(
|
||||
a,
|
||||
p,
|
||||
@@ -4498,6 +4518,7 @@ properties={:.1}ms picked={}",
|
||||
.into_iter()
|
||||
.filter_map(|s| Scene::handle_from_wire_name(s))
|
||||
.collect();
|
||||
let m_wires = t_hit.map(|t| t.elapsed().as_secs_f64() * 1000.0);
|
||||
handles.extend(scene::pick::hit_test::box_hit_hatch(
|
||||
a,
|
||||
p,
|
||||
@@ -4520,6 +4541,7 @@ properties={:.1}ms picked={}",
|
||||
bounds,
|
||||
candidate_handles.as_ref(),
|
||||
));
|
||||
let m_hatch = t_hit.map(|t| t.elapsed().as_secs_f64() * 1000.0);
|
||||
handles.extend(self.tabs[i].scene.mesh_box_hit(
|
||||
a,
|
||||
p,
|
||||
@@ -4566,8 +4588,13 @@ properties={:.1}ms picked={}",
|
||||
if crate::perf::enabled() {
|
||||
crate::perf_record!(
|
||||
"[perf] select-commit kind=window crossing={crossing} \
|
||||
candidates={cand_ms:.1}ms hit={hit_ms:.1}ms filter={filter_ms:.1}ms \
|
||||
apply={apply_ms:.1}ms properties={:.1}ms picked={}",
|
||||
candidates={cand_ms:.1}ms hit={hit_ms:.1}ms [handles={:.1} wires={:.1} \
|
||||
hatch={:.1} mesh={:.1}] filter={filter_ms:.1}ms apply={apply_ms:.1}ms \
|
||||
properties={:.1}ms picked={}",
|
||||
m_handles.unwrap_or(0.0),
|
||||
m_wires.unwrap_or(0.0) - m_handles.unwrap_or(0.0),
|
||||
m_hatch.unwrap_or(0.0) - m_wires.unwrap_or(0.0),
|
||||
hit_ms - m_hatch.unwrap_or(0.0),
|
||||
t_props.map_or(0.0, |t| t.elapsed().as_secs_f64() * 1000.0),
|
||||
handles.len(),
|
||||
);
|
||||
|
||||
+38
-11
@@ -1287,13 +1287,30 @@ fn indexed_box_crossing_hits<'a, W: WireSource + ?Sized>(
|
||||
// the remaining work is small enough that per-chunk sets, which cannot see
|
||||
// wires already hit in another chunk, measured *slower* than one pass
|
||||
// (45.6 ms against 42.4 ms at 186 468 wires).
|
||||
let mut wire_hit: HashSet<u32> = HashSet::default();
|
||||
// A bitmap over wire indices rather than a hash set: this is consulted once
|
||||
// per segment, triangle and glyph inside the box — millions of times on a
|
||||
// whole-drawing selection — and an array index beats hashing a `u32`, let
|
||||
// alone hashing the wire's name, which is what the primitive loops below
|
||||
// used to do. `seen` stays for the names, because several wires can share
|
||||
// one entity's name and the output dedups by name.
|
||||
let mut wire_hit: Vec<bool> = Vec::new();
|
||||
fn mark(wire_hit: &mut Vec<bool>, wire: u32) {
|
||||
let index = wire as usize;
|
||||
if index >= wire_hit.len() {
|
||||
wire_hit.resize(index + 1, false);
|
||||
}
|
||||
wire_hit[index] = true;
|
||||
}
|
||||
fn already(wire_hit: &[bool], wire: u32) -> bool {
|
||||
wire_hit.get(wire as usize).copied().unwrap_or(false)
|
||||
}
|
||||
|
||||
for segment in wires.segments().unwrap_or_default() {
|
||||
if wire_hit.contains(&segment.wire) {
|
||||
if already(&wire_hit, segment.wire) {
|
||||
continue;
|
||||
}
|
||||
if let Some(name) = hit_name(segment) {
|
||||
wire_hit.insert(segment.wire);
|
||||
mark(&mut wire_hit, segment.wire);
|
||||
if seen.insert(name) {
|
||||
out.push(name);
|
||||
}
|
||||
@@ -1315,7 +1332,7 @@ fn indexed_box_crossing_hits<'a, W: WireSource + ?Sized>(
|
||||
let Some(wire) = wires.source_wire(triangle.wire) else {
|
||||
continue;
|
||||
};
|
||||
if seen.contains(wire.name.as_str()) {
|
||||
if already(&wire_hit, triangle.wire) {
|
||||
continue;
|
||||
}
|
||||
if projected_wire_triangle(
|
||||
@@ -1327,9 +1344,11 @@ fn indexed_box_crossing_hits<'a, W: WireSource + ?Sized>(
|
||||
bounds,
|
||||
)
|
||||
.is_some_and(|triangle| triangle_crosses_box(triangle, corners))
|
||||
&& seen.insert(wire.name.as_str())
|
||||
{
|
||||
out.push(wire.name.as_str());
|
||||
mark(&mut wire_hit, triangle.wire);
|
||||
if seen.insert(wire.name.as_str()) {
|
||||
out.push(wire.name.as_str());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1337,7 +1356,7 @@ fn indexed_box_crossing_hits<'a, W: WireSource + ?Sized>(
|
||||
let Some(wire) = wires.source_wire(glyph.wire) else {
|
||||
continue;
|
||||
};
|
||||
if seen.contains(wire.name.as_str()) {
|
||||
if already(&wire_hit, glyph.wire) {
|
||||
continue;
|
||||
}
|
||||
let Some(screen) =
|
||||
@@ -1358,11 +1377,15 @@ fn indexed_box_crossing_hits<'a, W: WireSource + ?Sized>(
|
||||
|
||||
// Degenerate point-only wires have no indexed segment or surface primitive.
|
||||
for wire in wires.iter() {
|
||||
if seen.contains(wire.name.as_str())
|
||||
|| wire.points.len() >= 2
|
||||
// Structural tests first: `||` short-circuits, so putting the name
|
||||
// lookup last means a wire with two or more points — very nearly all
|
||||
// of them — is dropped without hashing its name. This loop walks every
|
||||
// candidate, so that was one string hash per wire in the box.
|
||||
if wire.points.len() >= 2
|
||||
|| !wire.fill_tris.is_empty()
|
||||
|| !wire.pick_tris.is_empty()
|
||||
|| !wire.text_verts.is_empty()
|
||||
|| seen.contains(wire.name.as_str())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
@@ -1568,11 +1591,15 @@ fn indexed_polygon_crossing_hits<'a, W: WireSource + ?Sized>(
|
||||
}
|
||||
}
|
||||
for wire in wires.iter() {
|
||||
if seen.contains(wire.name.as_str())
|
||||
|| wire.points.len() >= 2
|
||||
// Structural tests first: `||` short-circuits, so putting the name
|
||||
// lookup last means a wire with two or more points — very nearly all
|
||||
// of them — is dropped without hashing its name. This loop walks every
|
||||
// candidate, so that was one string hash per wire in the box.
|
||||
if wire.points.len() >= 2
|
||||
|| !wire.fill_tris.is_empty()
|
||||
|| !wire.pick_tris.is_empty()
|
||||
|| !wire.text_verts.is_empty()
|
||||
|| seen.contains(wire.name.as_str())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user