Build the selection highlight in one pass instead of four

Highlighting a whole-drawing selection took 299 ms, and it runs when the
selection changes — directly between the click that completes a selection and
seeing it.

Most of that was asking the same question twice. Each wire went through the
circle and ellipse extractors at its draw depth to build the analytic
instances, then through both again at depth 0.0 purely to ask *whether* it is
a circle or an ellipse, allocating a vector each time and dropping it.
Neither extractor's `Some`/`None` answer depends on the depth: every
`return None` in them turns on the geometry — which `TangentGeom` variant,
the radius, the axis lengths, the parameters — and the depth is only written
into the instance, never read to decide. So the extraction that builds the
instances already answers the classification. One pass now does both, and
fans out across cores, since it reads a wire and writes nothing shared. That
took 173 ms of shape extraction and splitting down to 19 ms, and the whole
highlight from 299 ms to 150 ms.

Gathering the highlighted wires also cloned each handle's slot vector and
sorted the copy — one allocation and one sort per selected entity. The slots
are already ascending: both builders walk the wires in order and push as they
go, and `patch_handle_index` shifts a handle's slots by a constant, which
preserves it. A `debug_assert` on the slots actually read holds that in
place. The compat path of `from_highlight_refs` emitted its instances
sequentially while the native path beside it fans out; it now matches.

Nothing renders in a test, so a wrong bucket in the classification would
surface only as circles drawn wrong when selected. Two tests cover it: each
wire lands where the old predicate put it, and the colour override — always
for hover, and the configured tint or `SELECTED` for a selection — reaches
the instances.

`wire-highlight-detail` reports the phases: gathering, classifying,
uploading the analytic instances, the regular wires and the block ones.

src/scene/pipeline/mod.rs

@@ -2612,6 +2612,72 @@ impl Pipeline {
     /// refreshes just this overlay instead of re-tessellating the model. The
     /// xray pass applies neither scissor nor mesh-edge skip, so everything
     /// merges into one order-preserving run.
+    /// Split highlighted wires into analytic instances and the wires that need
+    /// real geometry, in one extraction pass per wire.
+    #[allow(clippy::type_complexity)]
+    fn classify_highlight_wires<'a>(
+        wires: &[&'a WireModel],
+        color: Option<[f32; 4]>,
+        depth_map: &rustc_hash::FxHashMap<u64, [f32; 2]>,
+    ) -> (
+        Vec<CircleInstance>,
+        Vec<EllipseInstance>,
+        Vec<&'a WireModel>,
+        Vec<&'a WireModel>,
+    ) {
+        use crate::par::prelude::*;
+        // Pure per-wire work, so it fans out; `collect` on an indexed
+        // parallel iterator keeps the order, and the order is what decides
+        // the instance layout.
+        let per: Vec<(Option<Vec<CircleInstance>>, Option<Vec<EllipseInstance>>, bool)> =
+            wires
+                .par_iter()
+                .map(|&wire| {
+                    if wire.render_instance.is_some() {
+                        return (None, None, true);
+                    }
+                    let depth = wire_gpu::wire_draw_depth(wire, depth_map);
+                    let mut circles = circle_gpu::extract_circle_instances(wire, depth);
+                    let mut ellipses = ellipse_gpu::extract_ellipse_instances(wire, depth);
+                    if let Some(color) = color {
+                        if let Some(instances) = circles.as_mut() {
+                            for instance in instances {
+                                instance.color = color;
+                            }
+                        }
+                        if let Some(instances) = ellipses.as_mut() {
+                            for instance in instances {
+                                instance.color = color;
+                            }
+                        }
+                    }
+                    (circles, ellipses, false)
+                })
+                .collect();
+
+        let mut circles: Vec<CircleInstance> = Vec::new();
+        let mut ellipses: Vec<EllipseInstance> = Vec::new();
+        let mut regular: Vec<&'a WireModel> = Vec::new();
+        let mut blocks: Vec<&'a WireModel> = Vec::new();
+        for (&wire, (wire_circles, wire_ellipses, is_block)) in wires.iter().zip(per) {
+            if is_block {
+                blocks.push(wire);
+                continue;
+            }
+            let shaped = wire_circles.is_some() || wire_ellipses.is_some();
+            if let Some(instances) = wire_circles {
+                circles.extend(instances);
+            }
+            if let Some(instances) = wire_ellipses {
+                ellipses.extend(instances);
+            }
+            if !shaped {
+                regular.push(wire);
+            }
+        }
+        (circles, ellipses, regular, blocks)
+    }
+
     pub fn upload_selected_wires(
         &mut self,
         device: &wgpu::Device,
@@ -2635,11 +2701,20 @@ impl Pipeline {
         // O(highlighted), no per-wire string parse or deep geometry clone.
         let mut selected_wires: Vec<&WireModel> = Vec::new();
         let mut hover_wires: Vec<&WireModel> = Vec::new();
+        // The index is built by walking the wires in order, so each handle's
+        // slots come out ascending, and `patch_handle_index` only shifts them
+        // by a constant, which keeps that. Cloning and re-sorting per handle
+        // was one allocation and one sort for every selected entity — 186 468
+        // of each on a whole-drawing selection. The assert holds the invariant
+        // in place: if it ever breaks, a debug build says so here rather than
+        // drawing the highlight in the wrong order.
         for h in selected {
             if let Some(idxs) = self.wire_handle_index.get(&h.value()) {
-                let mut slots = idxs.clone();
-                slots.sort_unstable();
-                for &i in &slots {
+                debug_assert!(
+                    idxs.windows(2).all(|pair| pair[0] <= pair[1]),
+                    "wire_handle_index slots must stay ascending",
+                );
+                for &i in idxs {
                     if let Some(w) = wires.get(i as usize) {
                         if !w.display_visible {
                             continue;
@@ -2651,9 +2726,7 @@ impl Pipeline {
         }
         for h in hovered.iter().filter(|handle| !selected.contains(handle)) {
             if let Some(idxs) = self.wire_handle_index.get(&h.value()) {
-                let mut slots = idxs.clone();
-                slots.sort_unstable();
-                for &i in &slots {
+                for &i in idxs {
                     if let Some(w) = wires.get(i as usize) {
                         if !w.display_visible {
                             continue;
@@ -2673,26 +2746,42 @@ impl Pipeline {
                 hover_wires.push(wire);
             }
         }
-        let mut selected_circles: Vec<CircleInstance> = Vec::new();
-        let circle_sel_tint = selected_tint.unwrap_or(WireModel::SELECTED);
-        for &wire in &selected_wires {
-            let depth = wire_gpu::wire_draw_depth(wire, depth_map);
-            if let Some(insts) = circle_gpu::extract_circle_instances(wire, depth) {
-                for mut inst in insts {
-                    inst.color = circle_sel_tint;
-                    selected_circles.push(inst);
-                }
-            }
-        }
-        for &wire in &hover_wires {
-            let depth = wire_gpu::wire_draw_depth(wire, depth_map);
-            if let Some(insts) = circle_gpu::extract_circle_instances(wire, depth) {
-                for mut inst in insts {
-                    inst.color = WireModel::HOVER;
-                    selected_circles.push(inst);
-                }
-            }
-        }
+        let t_gather = perf_started.map(|t| t.elapsed().as_secs_f64() * 1000.0);
+        // One extraction pass per wire, where there used to be four.
+        //
+        // The old shape asked each extractor twice: once at the draw depth to
+        // build the instances, then once more at depth 0.0 purely to ask
+        // *whether* the wire is a circle or an ellipse — allocating a vector
+        // each time and dropping it. Neither extractor's `Some`/`None` answer
+        // depends on the depth: every `return None` in them turns on the
+        // geometry (which `TangentGeom` variant, the radius, the axis lengths,
+        // the parameters), and the depth is only written into the instance,
+        // never read to decide. So the extraction that builds the instances
+        // already answers the classification, and the second pair is pure
+        // waste — 116 ms of shapes plus 57 ms of splitting, out of a 299 ms
+        // highlight at 186 468 entities.
+        //
+        // A wire with a `render_instance` is a block wire, and both extractors
+        // reject those outright, so it skips extraction altogether.
+        // Selected keeps its colour unless a tint is configured; hover is
+        // always recoloured. Both vectors carry selected first, then hover,
+        // exactly as the four separate loops produced them.
+        let (mut selected_circles, mut selected_ellipses, selected_regular, selected_blocks) =
+            Self::classify_highlight_wires(
+                &selected_wires,
+                Some(selected_tint.unwrap_or(WireModel::SELECTED)),
+                depth_map,
+            );
+        let (hover_circles, hover_ellipses, hover_regular, hover_blocks) =
+            Self::classify_highlight_wires(
+                &hover_wires,
+                Some(WireModel::HOVER),
+                depth_map,
+            );
+        selected_circles.extend(hover_circles);
+        selected_ellipses.extend(hover_ellipses);
+
+        let t_shapes = perf_started.map(|t| t.elapsed().as_secs_f64() * 1000.0);
         self.gpu_selected_circles = if selected_circles.is_empty() {
             vec![]
         } else {
@@ -2703,27 +2792,6 @@ impl Pipeline {
                 &selected_circles,
             )]
         };
-
-        let mut selected_ellipses: Vec<EllipseInstance> = Vec::new();
-        let ellipse_sel_tint = selected_tint.unwrap_or(WireModel::SELECTED);
-        for &wire in &selected_wires {
-            let depth = wire_gpu::wire_draw_depth(wire, depth_map);
-            if let Some(insts) = ellipse_gpu::extract_ellipse_instances(wire, depth) {
-                for mut inst in insts {
-                    inst.color = ellipse_sel_tint;
-                    selected_ellipses.push(inst);
-                }
-            }
-        }
-        for &wire in &hover_wires {
-            let depth = wire_gpu::wire_draw_depth(wire, depth_map);
-            if let Some(insts) = ellipse_gpu::extract_ellipse_instances(wire, depth) {
-                for mut inst in insts {
-                    inst.color = WireModel::HOVER;
-                    selected_ellipses.push(inst);
-                }
-            }
-        }
         self.gpu_selected_ellipses = if selected_ellipses.is_empty() {
             vec![]
         } else {
@@ -2734,35 +2802,7 @@ impl Pipeline {
                 &selected_ellipses,
             )]
         };
-
-        let selected_regular: Vec<&WireModel> = selected_wires
-            .iter()
-            .copied()
-            .filter(|wire| {
-                wire.render_instance.is_none()
-                    && circle_gpu::extract_circle_instances(wire, 0.0).is_none()
-                    && ellipse_gpu::extract_ellipse_instances(wire, 0.0).is_none()
-            })
-            .collect();
-        let hover_regular: Vec<&WireModel> = hover_wires
-            .iter()
-            .copied()
-            .filter(|wire| {
-                wire.render_instance.is_none()
-                    && circle_gpu::extract_circle_instances(wire, 0.0).is_none()
-                    && ellipse_gpu::extract_ellipse_instances(wire, 0.0).is_none()
-            })
-            .collect();
-        let selected_blocks: Vec<&WireModel> = selected_wires
-            .iter()
-            .copied()
-            .filter(|wire| wire.render_instance.is_some())
-            .collect();
-        let hover_blocks: Vec<&WireModel> = hover_wires
-            .iter()
-            .copied()
-            .filter(|wire| wire.render_instance.is_some())
-            .collect();
+        let t_split = perf_started.map(|t| t.elapsed().as_secs_f64() * 1000.0);
         let mut gpu = if let Some(tint) = selected_tint {
             WireGpu::from_highlight_refs(
                 device,
@@ -2783,6 +2823,7 @@ impl Pipeline {
             depth_map,
             self.wire_const_bgl.as_ref(),
         ));
+        let t_regular = perf_started.map(|t| t.elapsed().as_secs_f64() * 1000.0);
         self.gpu_selected_wires = gpu;
         let mut block_gpu = if let Some(tint) = selected_tint {
             BlockWireGpu::from_wires(
@@ -2816,6 +2857,24 @@ impl Pipeline {
         if let Some(started) = perf_started {
             let elapsed_ms = started.elapsed().as_secs_f64() * 1000.0;
             if elapsed_ms >= 1.0 {
+                // Cumulative marks, differenced here: gathering the wires
+                // from the handle index, classifying them and extracting their
+                // analytic instances, uploading those, building the regular
+                // wire instances, and the block ones. A one-time cost when the
+                // selection changes, which puts it directly between the user's
+                // second click and seeing the selection.
+                let gather = t_gather.unwrap_or(0.0);
+                let classify = t_shapes.unwrap_or(0.0);
+                let analytic = t_split.unwrap_or(0.0);
+                let regular = t_regular.unwrap_or(0.0);
+                crate::perf_record!(
+                    "[perf] wire-highlight-detail gather={gather:.1} classify={:.1} \
+analytic={:.1} regular={:.1} blocks={:.1}",
+                    classify - gather,
+                    analytic - classify,
+                    regular - analytic,
+                    elapsed_ms - regular,
+                );
                 crate::perf_record!(
                     "[perf] wire-highlight {:>7.1}ms handles={} wires={}",
                     elapsed_ms,
@@ -5627,3 +5686,97 @@ impl iced::widget::shader::Pipeline for MultiPipeline {
         }
     }
 }
+
+#[cfg(test)]
+mod highlight_classification_tests {
+    use super::*;
+    use crate::scene::model::wire_model::TangentGeom;
+
+    fn plain(name: &str) -> WireModel {
+        WireModel::solid(
+            name.to_string(),
+            vec![[0.0, 0.0, 0.0], [1.0, 1.0, 0.0]],
+            [1.0; 4],
+            false,
+        )
+    }
+
+    fn circle(name: &str) -> WireModel {
+        let mut wire = plain(name);
+        wire.tangent_geoms.push(TangentGeom::PlanarCircle {
+            center: [0.0, 0.0, 0.0],
+            axis_x: [1.0, 0.0, 0.0],
+            axis_y: [0.0, 1.0, 0.0],
+            radius: 2.0,
+        });
+        wire
+    }
+
+    fn ellipse(name: &str) -> WireModel {
+        let mut wire = plain(name);
+        wire.tangent_geoms.push(TangentGeom::PlanarEllipse {
+            center: [0.0, 0.0, 0.0],
+            major_axis: [2.0, 0.0, 0.0],
+            normal: [0.0, 0.0, 1.0],
+            minor_axis_ratio: 0.5,
+            start_param: 0.0,
+            end_param: std::f64::consts::TAU,
+        });
+        wire
+    }
+
+    // The classification used to be a second pair of extractions at depth 0.0,
+    // run purely to ask which bucket a wire belongs in. It now rides the
+    // extraction that builds the instances. Nothing renders in a test, so a
+    // wrong answer here would show up only as circles drawn wrong when
+    // selected — this asserts the buckets directly, against the predicate the
+    // old code used.
+    #[test]
+    fn each_wire_lands_in_the_bucket_the_old_predicate_chose() {
+        let wires = vec![plain("1"), circle("2"), ellipse("3"), plain("4")];
+        let refs: Vec<&WireModel> = wires.iter().collect();
+        let depth_map = rustc_hash::FxHashMap::default();
+
+        let (circles, ellipses, regular, blocks) =
+            Pipeline::classify_highlight_wires(&refs, None, &depth_map);
+
+        for wire in &refs {
+            let was_regular = wire.render_instance.is_none()
+                && circle_gpu::extract_circle_instances(wire, 0.0).is_none()
+                && ellipse_gpu::extract_ellipse_instances(wire, 0.0).is_none();
+            assert_eq!(
+                was_regular,
+                regular.iter().any(|kept| std::ptr::eq(*kept, *wire)),
+                "wire {} disagrees with the old predicate",
+                wire.name,
+            );
+        }
+
+        assert_eq!(regular.len(), 2, "two plain lines");
+        assert_eq!(circles.len(), 1, "one circle instance");
+        assert_eq!(ellipses.len(), 1, "one ellipse instance");
+        assert!(blocks.is_empty(), "no block wires in this sample");
+    }
+
+    // Hover recolours every instance; a selection only recolours when a tint is
+    // configured. Both go through the same argument, so it has to actually
+    // reach the instances.
+    #[test]
+    fn the_colour_override_reaches_the_instances() {
+        let wires = vec![circle("2"), ellipse("3")];
+        let refs: Vec<&WireModel> = wires.iter().collect();
+        let depth_map = rustc_hash::FxHashMap::default();
+
+        let (tinted_circles, tinted_ellipses, _, _) =
+            Pipeline::classify_highlight_wires(&refs, Some(WireModel::HOVER), &depth_map);
+        assert_eq!(tinted_circles[0].color, WireModel::HOVER);
+        assert_eq!(tinted_ellipses[0].color, WireModel::HOVER);
+
+        let (plain_circles, _, _, _) =
+            Pipeline::classify_highlight_wires(&refs, None, &depth_map);
+        assert_ne!(
+            plain_circles[0].color, WireModel::HOVER,
+            "without a tint the extractor's own colour stands",
+        );
+    }
+}

src/scene/pipeline/wire_gpu.rs

@@ -1148,8 +1148,14 @@ impl WireGpu {
 
         let _ = const_bgl;
         let max_packed_instances = super::gpu_budget::max_elements::<PackedWireInstance>(device);
+        // Same fan-out the native path above uses: emitting a wire reads that
+        // wire and the depth map and writes nothing shared, and a selection
+        // large enough to be slow is exactly where it pays. `collect` on an
+        // indexed parallel iterator keeps the order, which the instance indices
+        // depend on.
+        use crate::par::prelude::*;
         let per: Vec<Vec<PackedWireInstance>> = wires
-            .iter()
+            .par_iter()
             .map(|wire| {
                 emit_wire_packed(wire, color, wire_draw_depth(wire, depth_map))
             })