From fa7ef87f00d32bf4fce3d925e4d49060d34f0740 Mon Sep 17 00:00:00 2001 From: Afonsop Date: Thu, 17 Sep 2026 19:22:11 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20final-cycle=20review=20minors=20?= =?UTF-8?q?=E2=80=94=20decimation=20duplicate,=20endpoint-map=20acceptance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last review pass over the hardening commit found no blockers; two minors are fixed here. Decimation's force-retain used index arithmetic that the seam entry pushed right after a stride doubling invalidates, duplicating the last segment at exact halving boundaries — it now compares against the last kept entry. The f64 endpoint map's first-wins buckets are 4 f32 ulp wide, so two distinct near-coincident segments can collide and hand a geom the WRONG endpoints; lookups now verify the hit against the geom's own f32 pair (within an ulp) and fall back to the widened pair on mismatch, restoring the closest-match semantics of the scan the map replaced. --- src/scene/model/mesh_model.rs | 7 ++++--- src/snap.rs | 13 ++++++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/scene/model/mesh_model.rs b/src/scene/model/mesh_model.rs index 13624f742..b1f829676 100644 --- a/src/scene/model/mesh_model.rs +++ b/src/scene/model/mesh_model.rs @@ -80,10 +80,11 @@ pub fn edge_snap_wire( total += 1; } // Force-retain the final segment so decimation never removes the far end - // of the mesh: `(total - 1) % stride != 0` proves it is not already kept - // (every kept entry sits at an index that is a multiple of `stride`). + // of the mesh. The seam entry pushed right after a stride doubling is + // NOT on the stride grid, so index arithmetic cannot prove membership — + // compare against the last kept entry instead. if let Some(last) = last_pair { - if (total - 1) % stride != 0 { + if kept.last() != Some(&last) { if kept.len() >= MAX_EDGE_SEGMENTS { halve_kept_segments(&mut kept); } diff --git a/src/snap.rs b/src/snap.rs index ba2999f07..31d1cae10 100644 --- a/src/snap.rs +++ b/src/snap.rs @@ -2510,9 +2510,11 @@ fn line_geom_key(p1: [f32; 3], p2: [f32; 3]) -> LineGeomKey { /// the wire's full-precision f64 pair behind it, so every Line geom of the /// wire resolves with an O(1) lookup instead of rescanning the whole /// tessellation per geom (which made `curves_in_frame` O(points × geoms) for -/// every wire pair on every snap). First-wins on quantisation collisions: -/// adjacent vertices closer than the grid are indistinguishable at snap -/// scale anyway. +/// every wire pair on every snap). First-wins on quantisation collisions — +/// a bucket is 4 f32 ulp wide, so two DISTINCT near-coincident segments can +/// share one; lookups therefore verify the hit against the geom's own f32 +/// endpoints and fall back to the widened pair on mismatch, restoring the +/// closest-match semantics of the scan this replaced. fn line_endpoint_map(wire: &WireModel) -> HashMap { let mut map = HashMap::with_capacity_and_hasher(wire.points.len() / 2 + 1, Default::default()); @@ -2545,8 +2547,13 @@ fn line_geom_endpoints_f64( if wire.points.len() == 2 { return (wp_f64(wire, 0), wp_f64(wire, 1)); } + let near = |v: Vec3, p: [f32; 3]| { + let ulp = |x: f32, y: f32| (x - y).abs() <= y.abs().max(1.0) * f32::EPSILON; + ulp(v.x, p[0]) && ulp(v.y, p[1]) && ulp(v.z, p[2]) + }; endpoints .get(&line_geom_key(p1, p2)) + .filter(|(a, b)| near(a.as_vec3(), p1) && near(b.as_vec3(), p2)) .copied() .unwrap_or((Vec3::from_array(p1).as_dvec3(), Vec3::from_array(p2).as_dvec3())) }