Merge PR #1102: measure selection phases

src/app/update/viewport.rs

@@ -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(),
                         );

src/scene/pick/hit_test.rs

@@ -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;
         }