From 4f1e710b352544990635fc5ca893b4506a470f1a Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Fri, 17 Jul 2026 15:22:56 +0300 Subject: [PATCH] fix(pick): catch a wide polyline on its band, not just its centreline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A wide polyline draws a solid band, but hit-testing only ever saw the centreline 8 px out, so a donut could only be selected by clicking the phantom circle inside the disc it paints. `self.hatches` — the map the hatch pick channel walks — is filled from HATCH and SOLID alone, so the band `wide_fills` produces was drawn (via `synced_hatch_models`) and never picked. The band rides in on `pick_tris`, the same channel a thickness wall uses: `wide_fills` already computes it, so it costs one tessellation and inherits the block transform. It ear clips rather than fans — an arc band is an annular sector, concave on its inner edge — reusing `triangulate_planar`. Wiring `pick_tris` through `Lines` alone was not enough: a plain wide polyline leaves `to_truck` as `Contour`, and a PLINEGEN=0 one as `SegmentedLines`. Both now carry it. This needs the wire's cullable box to cover the band, and it did not. `bounding_box()` boxes a polyline's stored vertices, knowing nothing of the width painted around them: a donut's two vertices sit on one horizontal line, so the box had zero height and rejected every click on the disc before any triangle was tested. Every site that stamps the entity box onto a wire now goes through `set_wire_aabb`, which widens it over `pick_tris` (a no-op for the entities that have none, which is nearly all), and the block cache chains `pick_tris` into its own box for the same reason. That box was already too small for the centreline itself — an arc leaves the vertex box — so a donut was unselectable at its top and bottom at enough zoom. Widening fixes that too. Verified end-to-end: tessellate a width-0.5 donut on a 0.05 centreline radius, then `click_hit` five points 20 px clear of the centreline — all land, and a click outside the disc still misses. Co-Authored-By: Claude Opus 4.8 --- src/entities/common.rs | 24 ++++++++++++++ src/entities/lwpolyline.rs | 6 ++-- src/entities/mesh.rs | 2 +- src/entities/polyline.rs | 3 +- src/scene/cache/block_cache.rs | 5 +++ src/scene/convert/tess.rs | 55 ++++++++++++++++++++++++++------- src/scene/convert/tessellate.rs | 12 ++++--- src/scene/model/wire_model.rs | 23 +++++++++----- src/scene/pick/hit_test.rs | 15 ++++----- 9 files changed, 111 insertions(+), 34 deletions(-) diff --git a/src/entities/common.rs b/src/entities/common.rs index e4e42812d..381c360c1 100644 --- a/src/entities/common.rs +++ b/src/entities/common.rs @@ -311,6 +311,29 @@ impl BulgeArc { } } +/// Triangulate the solid bands a `wide_fills` returns into the flat WCS f64 +/// triangle list `TruckEntity::pick_tris` carries, so a wide polyline is +/// selectable across the band it draws and not just along its centreline. +/// +/// `origin` and `fills` are that function's own pair: 2-D offsets from the +/// first vertex, which is the exact frame the band's `HatchModel` renders in +/// (`world_origin` + boundary, no elevation). Building the pick geometry from +/// the same numbers keeps hit-testing on whatever the fill actually drew. +/// +/// An arc band is an annular sector — concave on its inner edge — so this ear +/// clips rather than fans. +pub(crate) fn wide_band_tris(origin: [f64; 2], fills: &[Vec<[f32; 2]>]) -> Vec<[f64; 3]> { + let mut out = Vec::new(); + for poly in fills { + let ring: Vec<[f64; 3]> = poly + .iter() + .map(|&[x, y]| [origin[0] + x as f64, origin[1] + y as f64, 0.0]) + .collect(); + out.extend(crate::entities::mesh::triangulate_planar(&ring)); + } + out +} + /// Compute the filled boundary polygon for one polyline segment. /// For straight segments: a rectangle/trapezoid. /// For arc segments: an arc band (outer arc + reversed inner arc). @@ -392,3 +415,4 @@ pub(crate) fn polyline_segment_fill( Some(boundary) } } + diff --git a/src/entities/lwpolyline.rs b/src/entities/lwpolyline.rs index c1a5a3b26..b326cf1ac 100644 --- a/src/entities/lwpolyline.rs +++ b/src/entities/lwpolyline.rs @@ -222,8 +222,9 @@ fn to_truck(pline: &LwPolyline) -> TruckEntity { pts.push([f64::NAN; 3]); } } + let (fill_origin, fills) = wide_fills(pline); return TruckEntity { - pick_tris: Vec::new(), + pick_tris: crate::entities::common::wide_band_tris(fill_origin, &fills), object: TruckObject::SegmentedLines(pts), snap_pts: vec![], tangent_geoms: tgs, @@ -271,8 +272,9 @@ fn to_truck(pline: &LwPolyline) -> TruckEntity { key_verts.push([p1.x, p1.y, p1.z]); } + let (fill_origin, fills) = wide_fills(pline); TruckEntity { - pick_tris: Vec::new(), + pick_tris: crate::entities::common::wide_band_tris(fill_origin, &fills), object: TruckObject::Contour(edges.into_iter().collect::()), snap_pts: vec![], tangent_geoms: tangents, diff --git a/src/entities/mesh.rs b/src/entities/mesh.rs index 49d325077..9a1c74df4 100644 --- a/src/entities/mesh.rs +++ b/src/entities/mesh.rs @@ -13,7 +13,7 @@ use crate::scene::model::wire_model::SnapHint; /// from vertex 0 is only valid for convex faces — a concave face (e.g. an /// L-shaped mesh face) fans into triangles that spill outside the outline. Ear /// clipping handles both. Falls back to a fan when the polygon is degenerate. -fn triangulate_planar(poly: &[[f64; 3]]) -> Vec<[f64; 3]> { +pub(crate) fn triangulate_planar(poly: &[[f64; 3]]) -> Vec<[f64; 3]> { let n = poly.len(); if n < 3 { return Vec::new(); diff --git a/src/entities/polyline.rs b/src/entities/polyline.rs index 48ab13c1c..7be9af884 100644 --- a/src/entities/polyline.rs +++ b/src/entities/polyline.rs @@ -303,8 +303,9 @@ fn tessellate_polyline2d(pl: &Polyline2D) -> TruckEntity { key_verts.push([p1.x, p1.y, p1.z]); } + let (fill_origin, fills) = wide_fills(pl); TruckEntity { - pick_tris: Vec::new(), + pick_tris: crate::entities::common::wide_band_tris(fill_origin, &fills), object: TruckObject::Contour(edges.into_iter().collect::()), snap_pts: vec![], tangent_geoms: tangents, diff --git a/src/scene/cache/block_cache.rs b/src/scene/cache/block_cache.rs index d3ea1df68..77f5a74c2 100644 --- a/src/scene/cache/block_cache.rs +++ b/src/scene/cache/block_cache.rs @@ -444,11 +444,16 @@ fn tessellate_sub_local( // SDF text wires have no points/fills — fold in the glyph-quad positions // so the view-frustum cull uses the text's real box, not a degenerate // point at the block origin (which would drop the text entirely). + // `pick_tris` is in here for the same reason `fill_tris` is: hit-testing + // rejects on this box before it looks at the triangles, so a box drawn + // only around `points` would make a block child's thickness wall or wide + // polyline band unpickable — `points` merely bounds those. let aabb_local = aabb_from_points_iter( wire.points .iter() .copied() .chain(wire.fill_tris.iter().copied()) + .chain(wire.pick_tris.iter().copied()) .chain(wire.text_verts.iter().map(|v| v.pos)), ); let is_fill_only = wire.points.is_empty() && !wire.fill_tris.is_empty(); diff --git a/src/scene/convert/tess.rs b/src/scene/convert/tess.rs index 7c0f0f53f..a8ddafc14 100644 --- a/src/scene/convert/tess.rs +++ b/src/scene/convert/tess.rs @@ -208,7 +208,7 @@ pub(crate) fn tessellate_entity( ); let ab = entity_aabb(e); for w in &mut wires { - w.aabb = ab; + set_wire_aabb(w, ab); } return wires; } @@ -321,7 +321,7 @@ pub(crate) fn tessellate_entity( // AABB; only stroke/fill wires take the whole-block // box as a broad-phase pick hint. if !w.points.is_empty() || !w.fill_tris.is_empty() { - w.aabb = aabb; + set_wire_aabb(w, aabb); } } return wires; @@ -356,7 +356,7 @@ pub(crate) fn tessellate_entity( // text — clicking empty space inside the dimension selects nothing, // only the lines or the text do. if !w.points.is_empty() || !w.fill_tris.is_empty() { - w.aabb = aabb; + set_wire_aabb(w, aabb); } } return wires; @@ -380,7 +380,7 @@ pub(crate) fn tessellate_entity( // As with dimensions: keep the whole-leader box only on stroke/fill // wires; empty SDF-text wires keep their tight glyph-box AABB. if !w.points.is_empty() || !w.fill_tris.is_empty() { - w.aabb = aabb; + set_wire_aabb(w, aabb); } } return wires; @@ -448,7 +448,7 @@ pub(crate) fn tessellate_entity( // AABB; only stroke/fill wires take the whole-block // box as a broad-phase pick hint. if !w.points.is_empty() || !w.fill_tris.is_empty() { - w.aabb = aabb; + set_wire_aabb(w, aabb); } } return wires; @@ -478,7 +478,7 @@ pub(crate) fn tessellate_entity( // stroke/fill wires take the whole-table box as a broad-phase // pick hint (matches the dim / mleader / baked-block paths). if !w.points.is_empty() || !w.fill_tris.is_empty() { - w.aabb = aabb; + set_wire_aabb(w, aabb); } } return wires; @@ -642,11 +642,12 @@ pub(crate) fn tessellate_entity( // (UNBOUNDED) — otherwise it never culls and stalls snapping on // block-heavy drawings. if w.text_verts.is_empty() { - w.aabb = if sub_aabb == WireModel::UNBOUNDED_AABB { + let box_ = if sub_aabb == WireModel::UNBOUNDED_AABB { wire_points_aabb(w) } else { sub_aabb }; + set_wire_aabb(w, box_); } } wires @@ -698,7 +699,7 @@ pub(crate) fn tessellate_entity( // in the Text arm; entity_aabb would mis-place the pick box for MTEXT, // so don't clobber it. Every other wire takes the entity AABB. if b.text_verts.is_empty() { - b.aabb = aabb; + set_wire_aabb(b, aabb); } } @@ -723,7 +724,7 @@ pub(crate) fn tessellate_entity( ); if !wires.is_empty() { for w in &mut wires { - w.aabb = aabb; + set_wire_aabb(w, aabb); } return wires; } @@ -796,7 +797,7 @@ pub(crate) fn tessellate_entity( ); for x in &mut w { x.aci = aci; - x.aabb = aabb; + set_wire_aabb(x, aabb); if wall_pat_len > 1e-6 { x.pattern = wall_pat; x.pattern_length = wall_pat_len; @@ -835,7 +836,7 @@ pub(crate) fn tessellate_entity( for w in &mut wires { w.name = host_name.clone(); w.aci = aci; - w.aabb = aabb; + set_wire_aabb(w, aabb); } bases.extend(wires); } @@ -1009,6 +1010,37 @@ pub(crate) fn wire_points_aabb(w: &WireModel) -> [f32; 4] { } } +/// Assign `entity_box` as `w`'s cullable box, widened to cover the wire's +/// pick-only geometry. +/// +/// `entity_aabb`'s box comes from acadrust's `bounding_box()`, which for a +/// polyline is the box of its stored vertices — it knows nothing about the band +/// a width paints around them, nor the wall a thickness extrudes. Hit-testing +/// rejects on this box before it looks at `pick_tris`, so a box that stops short +/// of them makes them silently unpickable: a donut's vertices are two points on +/// one horizontal line, giving a zero-height box that rejects every click on the +/// disc it draws. +/// +/// A no-op for the entities that have no `pick_tris`, which is nearly all. +pub(crate) fn set_wire_aabb(w: &mut WireModel, entity_box: [f32; 4]) { + if w.pick_tris.is_empty() || entity_box == WireModel::UNBOUNDED_AABB { + w.aabb = entity_box; + return; + } + let [mut x0, mut y0, mut x1, mut y1] = entity_box; + for (i, p) in w.pick_tris.iter().enumerate() { + let lo = w.pick_tris_low.get(i).copied().unwrap_or([0.0; 3]); + let (x, y) = (p[0] + lo[0], p[1] + lo[1]); + if x.is_finite() && y.is_finite() { + x0 = x0.min(x); + y0 = y0.min(y); + x1 = x1.max(x); + y1 = y1.max(y); + } + } + w.aabb = [x0, y0, x1, y1]; +} + pub(crate) fn entity_aabb(e: &acadrust::EntityType) -> [f32; 4] { let bbox = e.as_entity().bounding_box(); let min_x = (bbox.min.x) as f32; @@ -1054,3 +1086,4 @@ pub(crate) fn is_unindexable_entity(e: &acadrust::EntityType) -> bool { E::Insert(_) | E::Viewport(_) | E::Block(_) | E::BlockEnd(_) ) } + diff --git a/src/scene/convert/tessellate.rs b/src/scene/convert/tessellate.rs index 233934105..d8db06525 100644 --- a/src/scene/convert/tessellate.rs +++ b/src/scene/convert/tessellate.rs @@ -1044,9 +1044,11 @@ pub fn tessellate( .into_iter() .map(|[x, y, z]| [x, y, z]) .collect(); + // A wide polyline arrives here: its band is pick geometry. + let (pick_tris, pick_tris_low) = points_to_ds(te.pick_tris); return vec![WireModel { - pick_tris: Vec::new(), - pick_tris_low: Vec::new(), + pick_tris, + pick_tris_low, dash_from_start: false, dash_align_end: None, text_verts: Vec::new(), @@ -1205,9 +1207,11 @@ pub fn tessellate( .into_iter() .map(|[x, y, z]| [x, y, z]) .collect(); + // A wide polyline with PLINEGEN=0 arrives here. + let (pick_tris, pick_tris_low) = points_to_ds(te.pick_tris); return vec![WireModel { - pick_tris: Vec::new(), - pick_tris_low: Vec::new(), + pick_tris, + pick_tris_low, dash_from_start: false, dash_align_end: None, text_verts: Vec::new(), diff --git a/src/scene/model/wire_model.rs b/src/scene/model/wire_model.rs index e7fb92ee9..a0f51fd35 100644 --- a/src/scene/model/wire_model.rs +++ b/src/scene/model/wire_model.rs @@ -96,15 +96,22 @@ pub struct WireModel { /// tessellation from CAD f64 fills it so fills stay precise at UTM scale. pub fill_tris_low: Vec<[f32; 3]>, /// Pre-triangulated pick-only geometry: flat vertex list, 3 per triangle. - /// Hit-testing treats these as solid, the renderer never uploads them. + /// Hit-testing treats these as solid; this wire's own draw never uses them. /// - /// Non-empty only for entities extruded by a DXF thickness (code 39): the - /// swept wall between each base segment and its extruded copy. The wall is - /// drawn as its four wireframe edges (which live in [`points`]), so it has - /// no fill geometry to pick — without this the interior is a hole the - /// cursor falls through. Separate from [`fill_tris`] because that channel - /// reaches the GPU, and a wall that renders shaded would change how every - /// thickness drawing looks. + /// Carries the interior of things whose surface the cursor would otherwise + /// fall through, because [`points`] only bounds them: + /// + /// - An entity extruded by a DXF thickness (code 39) — the swept wall + /// between each base segment and its extruded copy. Drawn as its four + /// wireframe edges, with nothing in between to pick. + /// - A wide polyline (codes 43 / 40 / 41) — the solid band. It *is* drawn, + /// but by the hatch pipeline off `wide_fills`, so the wire that carries + /// its centreline has no fill of its own to hit-test. + /// + /// Separate from [`fill_tris`] because that channel reaches the GPU: a + /// thickness wall put there would render shaded and change how every such + /// drawing looks, and a polyline band would be drawn a second time on top + /// of the hatch that already draws it. pub pick_tris: Vec<[f32; 3]>, /// Low residual paired with [`pick_tris`] (double-single). Empty = all-zero. pub pick_tris_low: Vec<[f32; 3]>, diff --git a/src/scene/pick/hit_test.rs b/src/scene/pick/hit_test.rs index 05821c27f..48902648c 100644 --- a/src/scene/pick/hit_test.rs +++ b/src/scene/pick/hit_test.rs @@ -212,14 +212,15 @@ pub fn click_hit<'a>( return Some(n); } - // No drawn surface either. Entities extruded by a thickness draw their wall - // as four edges and nothing in between, so the interior is a hole the cursor - // falls straight through even though the wall reads as a face. Test the - // pick-only wall geometry to close it, front-most first. + // No fill of this wire's own either. `pick_tris` closes the surfaces that + // `points` only bounds: a thickness wall (drawn as four edges with nothing + // between them) and a wide polyline's band (drawn, but by the hatch + // pipeline, so no fill hangs off this wire). Without them the cursor falls + // through what plainly reads as solid. Front-most wins. // - // Ranked below `fill_tris` on purpose: those triangles are actually - // rendered, these are invisible, and where an opaque face overlaps a wall - // the user is looking at the face — so the face is what they mean to click. + // Ranked below `fill_tris` because that geometry is this wire's own drawn + // surface — where the two overlap, the nearer thing to the eye is decided + // by depth, but a wire that has a real fill should win on it first. let mut best_wall: Option<(f32, &str)> = None; for wire in wires { if wire.pick_tris.is_empty() {