diff --git a/src/scene/pipeline/mod.rs b/src/scene/pipeline/mod.rs index 880464b12..422c8f305 100644 --- a/src/scene/pipeline/mod.rs +++ b/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, + ) -> ( + Vec, + Vec, + 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>, Option>, 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 = Vec::new(); + let mut ellipses: Vec = 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 = 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 = 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", + ); + } +} diff --git a/src/scene/pipeline/wire_gpu.rs b/src/scene/pipeline/wire_gpu.rs index ab1fd455b..80761302d 100644 --- a/src/scene/pipeline/wire_gpu.rs +++ b/src/scene/pipeline/wire_gpu.rs @@ -1148,8 +1148,14 @@ impl WireGpu { let _ = const_bgl; let max_packed_instances = super::gpu_budget::max_elements::(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> = wires - .iter() + .par_iter() .map(|wire| { emit_wire_packed(wire, color, wire_draw_depth(wire, depth_map)) })