From 1f89b78a7e4ae0ba53f179aa9aa70f6fb90c7852 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Sat, 11 Jul 2026 21:55:10 +0300 Subject: [PATCH] fix(precision): keep typed hatch boundaries exact in f64 (#311) The HATCH/GRADIENT/BOUNDARY commands built the persisted hatch through the render-side boundary (f32 offsets from world_origin), and with a zero world_origin that meant absolute f32 - so a typed boundary vertex of 2000.8 was stored as 2000.8020, and at UTM scale the fill landed far from its boundary. HatchModel.boundary stays f32 (the GPU render contract). Added HatchModel.boundary_wcs: Option>>, an exact absolute-WCS boundary set only by the draw commands. Scene::add_hatch persists from boundary_wcs when present (exact f64 -> DxfHatch), else falls back to the f32 + world_origin reconstruction used by DXF-rebuilt hatches. manual_pts is now Vec; make_hatch takes the f64 boundary, stores the exact copy and derives the RTE render rep. The committed render model is rebuilt from the DXF entity, so nothing renders the command model's f32 boundary. This also fixes command hatches mis-placing at large coordinates (world_origin was [0,0]). Co-Authored-By: Claude Opus 4.8 --- src/modules/draw/draw/hatch.rs | 103 +++++++++++++++++++++++---------- src/scene/entity.rs | 37 ++++++++---- src/scene/mod.rs | 1 + src/scene/model/hatch_model.rs | 6 ++ src/scene/paper.rs | 1 + tests/block_hatch_export.rs | 1 + 6 files changed, 106 insertions(+), 43 deletions(-) diff --git a/src/modules/draw/draw/hatch.rs b/src/modules/draw/draw/hatch.rs index db010c9e3..dfed623db 100644 --- a/src/modules/draw/draw/hatch.rs +++ b/src/modules/draw/draw/hatch.rs @@ -13,7 +13,7 @@ use crate::command::{CadCommand, CmdResult}; use crate::modules::IconKind; use crate::scene::model::hatch_model::{HatchModel, HatchPattern, PatFamily}; use crate::scene::model::wire_model::WireModel; -use glam::{DVec3, Vec3}; +use glam::DVec3; // ── Icons ────────────────────────────────────────────────────────────────── @@ -70,12 +70,30 @@ fn point_in_polygon(p: [f32; 2], poly: &[[f32; 2]]) -> bool { inside } +/// Split an absolute boundary into the `(f32 offsets, f64 origin)` pair that +/// `HatchModel` expects: the origin anchors on the first vertex in full f64 so a +/// typed coordinate (issue #311) and large/UTM positions keep their precision, +/// and `add_hatch` reconstructs each WCS vertex as `origin + offset`. A zero +/// origin with absolute f32 offsets — the previous command output — quantized +/// typed points and mis-placed the fill at large coordinates. +fn rte_boundary(pts: impl Iterator) -> (Vec<[f32; 2]>, [f64; 2]) { + let pts: Vec<(f64, f64)> = pts.collect(); + let Some(&(ox, oy)) = pts.first() else { + return (vec![], [0.0; 2]); + }; + let rel = pts + .iter() + .map(|&(x, y)| [(x - ox) as f32, (y - oy) as f32]) + .collect(); + (rel, [ox, oy]) +} + // ── HATCH command ────────────────────────────────────────────────────────── pub struct HatchCommand { outlines: Vec>, mode: Mode, - manual_pts: Vec, + manual_pts: Vec, missed: bool, } @@ -89,7 +107,8 @@ impl HatchCommand { } } - fn make_hatch(&self, boundary: Vec<[f32; 2]>) -> HatchModel { + fn make_hatch(&self, boundary_wcs: Vec<[f64; 2]>) -> HatchModel { + let (rel, origin) = rte_boundary(boundary_wcs.iter().map(|&[x, y]| (x, y))); // Default: ANSI31 from catalog; fallback to a single 45° family. let pat_name = "ANSI31"; let families = crate::scene::model::hatch_patterns::find(pat_name) @@ -113,13 +132,14 @@ impl HatchCommand { }] }); HatchModel { - boundary: std::sync::Arc::new(boundary), + boundary: std::sync::Arc::new(rel), pattern: HatchPattern::Pattern(families), name: pat_name.into(), color: [0.75, 0.75, 0.75, 0.85], angle_offset: 0.0, scale: 1.0, - world_origin: [0.0; 2], + world_origin: origin, + boundary_wcs: Some(std::sync::Arc::new(boundary_wcs)), vp_scissor: None, draw_depth: 0.0, } @@ -166,20 +186,23 @@ impl CadCommand for HatchCommand { } } - fn on_point(&mut self, pt: DVec3) -> CmdResult { let pt = pt.as_vec3(); + fn on_point(&mut self, pt: DVec3) -> CmdResult { match &self.mode { Mode::PickInside => { - let xy = [pt.x, pt.y]; + // Hit-test against the f32 outline catalog (screen-space). + let xy = [pt.x as f32, pt.y as f32]; for outline in &self.outlines { if point_in_polygon(xy, outline) { self.missed = false; - return CmdResult::CommitHatch(self.make_hatch(outline.clone())); + let wcs = outline.iter().map(|&[x, y]| [x as f64, y as f64]).collect(); + return CmdResult::CommitHatch(self.make_hatch(wcs)); } } self.missed = true; CmdResult::NeedPoint } Mode::Manual => { + // Keep the typed/snapped point exact (issue #311). self.manual_pts.push(pt); CmdResult::NeedPoint } @@ -193,8 +216,8 @@ impl CadCommand for HatchCommand { if self.manual_pts.len() < 3 { return CmdResult::Cancel; } - let boundary = self.manual_pts.iter().map(|p| [p.x, p.y]).collect(); - CmdResult::CommitHatch(self.make_hatch(boundary)) + let wcs = self.manual_pts.iter().map(|p| [p.x, p.y]).collect(); + CmdResult::CommitHatch(self.make_hatch(wcs)) } } } @@ -221,12 +244,16 @@ impl CadCommand for HatchCommand { if self.manual_pts.is_empty() { return None; } - let mut pts: Vec<[f32; 3]> = self.manual_pts.iter().map(|p| [p.x, p.y, p.z]).collect(); + let mut pts: Vec<[f32; 3]> = self + .manual_pts + .iter() + .map(|p| [p.x as f32, p.y as f32, p.z as f32]) + .collect(); pts.push([pt.x, pt.y, pt.z]); pts.push([ - self.manual_pts[0].x, - self.manual_pts[0].y, - self.manual_pts[0].z, + self.manual_pts[0].x as f32, + self.manual_pts[0].y as f32, + self.manual_pts[0].z as f32, ]); return Some(WireModel::solid( "rubber_band".into(), @@ -244,7 +271,7 @@ impl CadCommand for HatchCommand { pub struct GradientCommand { outlines: Vec>, mode: Mode, - manual_pts: Vec, + manual_pts: Vec, missed: bool, } @@ -258,9 +285,10 @@ impl GradientCommand { } } - fn make_hatch(&self, boundary: Vec<[f32; 2]>) -> HatchModel { + fn make_hatch(&self, boundary_wcs: Vec<[f64; 2]>) -> HatchModel { + let (rel, origin) = rte_boundary(boundary_wcs.iter().map(|&[x, y]| (x, y))); HatchModel { - boundary: std::sync::Arc::new(boundary), + boundary: std::sync::Arc::new(rel), pattern: HatchPattern::Gradient { angle_deg: 0.0, color2: [0.18, 0.18, 0.18, 0.0], @@ -269,7 +297,8 @@ impl GradientCommand { color: [0.30, 0.60, 0.95, 0.80], angle_offset: 0.0, scale: 1.0, - world_origin: [0.0; 2], + world_origin: origin, + boundary_wcs: Some(std::sync::Arc::new(boundary_wcs)), vp_scissor: None, draw_depth: 0.0, } @@ -316,20 +345,23 @@ impl CadCommand for GradientCommand { } } - fn on_point(&mut self, pt: DVec3) -> CmdResult { let pt = pt.as_vec3(); + fn on_point(&mut self, pt: DVec3) -> CmdResult { match &self.mode { Mode::PickInside => { - let xy = [pt.x, pt.y]; + // Hit-test against the f32 outline catalog (screen-space). + let xy = [pt.x as f32, pt.y as f32]; for outline in &self.outlines { if point_in_polygon(xy, outline) { self.missed = false; - return CmdResult::CommitHatch(self.make_hatch(outline.clone())); + let wcs = outline.iter().map(|&[x, y]| [x as f64, y as f64]).collect(); + return CmdResult::CommitHatch(self.make_hatch(wcs)); } } self.missed = true; CmdResult::NeedPoint } Mode::Manual => { + // Keep the typed/snapped point exact (issue #311). self.manual_pts.push(pt); CmdResult::NeedPoint } @@ -343,8 +375,8 @@ impl CadCommand for GradientCommand { if self.manual_pts.len() < 3 { return CmdResult::Cancel; } - let boundary = self.manual_pts.iter().map(|p| [p.x, p.y]).collect(); - CmdResult::CommitHatch(self.make_hatch(boundary)) + let wcs = self.manual_pts.iter().map(|p| [p.x, p.y]).collect(); + CmdResult::CommitHatch(self.make_hatch(wcs)) } } } @@ -371,12 +403,16 @@ impl CadCommand for GradientCommand { if self.manual_pts.is_empty() { return None; } - let mut pts: Vec<[f32; 3]> = self.manual_pts.iter().map(|p| [p.x, p.y, p.z]).collect(); + let mut pts: Vec<[f32; 3]> = self + .manual_pts + .iter() + .map(|p| [p.x as f32, p.y as f32, p.z as f32]) + .collect(); pts.push([pt.x, pt.y, pt.z]); pts.push([ - self.manual_pts[0].x, - self.manual_pts[0].y, - self.manual_pts[0].z, + self.manual_pts[0].x as f32, + self.manual_pts[0].y as f32, + self.manual_pts[0].z as f32, ]); return Some(WireModel::solid( "rubber_band".into(), @@ -419,20 +455,25 @@ impl CadCommand for BoundaryCommand { format!("BOUNDARY Pick internal point:{miss}") } - fn on_point(&mut self, pt: DVec3) -> CmdResult { let pt = pt.as_vec3(); - let xy = [pt.x, pt.y]; + fn on_point(&mut self, pt: DVec3) -> CmdResult { + // Hit-test against the f32 outline catalog (screen-space). + let xy = [pt.x as f32, pt.y as f32]; for outline in &self.outlines { if point_in_polygon(xy, outline) { self.missed = false; // Store as a Hatch entity (solid fill) so it is selectable. + let wcs: Vec<[f64; 2]> = + outline.iter().map(|&[x, y]| [x as f64, y as f64]).collect(); + let (rel, origin) = rte_boundary(wcs.iter().map(|&[x, y]| (x, y))); let model = HatchModel { - boundary: std::sync::Arc::new(outline.clone()), + boundary: std::sync::Arc::new(rel), pattern: HatchPattern::Solid, name: "SOLID".into(), color: [0.45, 0.45, 0.45, 0.60], angle_offset: 0.0, scale: 1.0, - world_origin: [0.0; 2], + world_origin: origin, + boundary_wcs: Some(std::sync::Arc::new(wcs)), vp_scissor: None, draw_depth: 0.0, }; diff --git a/src/scene/entity.rs b/src/scene/entity.rs index 19fbd4d0b..f5d9935cd 100644 --- a/src/scene/entity.rs +++ b/src/scene/entity.rs @@ -580,6 +580,7 @@ impl Scene { for boundary in fills { models.push(HatchModel { boundary: Arc::new(boundary), + boundary_wcs: None, pattern: model::hatch_model::HatchPattern::Solid, name: "SOLID".into(), color, @@ -854,6 +855,7 @@ impl Scene { }; models.push(HatchModel { boundary: Arc::new(ring), + boundary_wcs: None, pattern: model::hatch_model::HatchPattern::Solid, name: "SOLID".into(), color, @@ -909,6 +911,7 @@ impl Scene { } models.push(HatchModel { boundary: Arc::new(boundary), + boundary_wcs: None, pattern: model::hatch_model::HatchPattern::Solid, name: "WIPEOUT_FILL".into(), color: fill_color, @@ -1430,6 +1433,7 @@ impl Scene { Some(HatchModel { boundary: std::sync::Arc::new(boundary_f32), + boundary_wcs: None, pattern, name, color, @@ -1624,6 +1628,7 @@ impl Scene { ]; HatchModel { boundary: std::sync::Arc::new(boundary), + boundary_wcs: None, pattern: model::hatch_model::HatchPattern::Solid, name: "SOLID".into(), color, @@ -1641,18 +1646,26 @@ impl Scene { model.pattern, crate::scene::model::hatch_model::HatchPattern::Solid ); - // The boundary points arrive in local render space (world_offset - // already subtracted). The stored DXF entity must hold WCS, so add the - // offset back — otherwise the boundary wire, re-projected through the - // normal entity path, lands `world_offset` away from the fill. - let wx = model.world_origin[0]; - let wy = model.world_origin[1]; - let verts: Vec = model - .boundary - .iter() - .filter(|v| v[0].is_finite() && v[1].is_finite()) - .map(|&[x, y]| Vector2::new(x as f64 + wx, y as f64 + wy)) - .collect(); + // Prefer the command-supplied exact f64 boundary so a typed vertex is + // persisted without f32 quantization (issue #311). Falling back to the + // render-side `boundary`, the points arrive in local render space + // (world_offset already subtracted), so add `world_origin` back — + // otherwise the boundary wire lands `world_offset` away from the fill. + let verts: Vec = if let Some(wcs) = &model.boundary_wcs { + wcs.iter() + .filter(|v| v[0].is_finite() && v[1].is_finite()) + .map(|&[x, y]| Vector2::new(x, y)) + .collect() + } else { + let wx = model.world_origin[0]; + let wy = model.world_origin[1]; + model + .boundary + .iter() + .filter(|v| v[0].is_finite() && v[1].is_finite()) + .map(|&[x, y]| Vector2::new(x as f64 + wx, y as f64 + wy)) + .collect() + }; let edge = PolylineEdge::new(verts, true); let mut path = BoundaryPath::external(); path.add_edge(BoundaryEdge::Polyline(edge)); diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 20474934b..b69c745ad 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -1604,6 +1604,7 @@ impl Scene { Some(HatchModel { world_origin: [0.0, 0.0], boundary: Arc::new(vec![[x0, y0], [x1, y0], [x1, y1], [x0, y1], [x0, y0]]), + boundary_wcs: None, pattern: crate::scene::model::hatch_model::HatchPattern::Solid, name: "SOLID".to_string(), color: self.paper_bg_color, diff --git a/src/scene/model/hatch_model.rs b/src/scene/model/hatch_model.rs index 97fa8080e..4ea8b0b72 100644 --- a/src/scene/model/hatch_model.rs +++ b/src/scene/model/hatch_model.rs @@ -60,6 +60,12 @@ pub struct HatchModel { /// f32 offsets from `world_origin`. NaN-NaN sentinels separate /// disconnected paths and must be preserved un-shifted by consumers. pub boundary: Arc>, + /// Exact absolute-WCS boundary in f64, set only by the draw commands so a + /// typed boundary vertex is persisted without the f32 quantization the + /// render-side `boundary` would incur (issue #311). `None` for hatches + /// rebuilt from a DXF entity — `add_hatch` then reconstructs the persisted + /// vertices from `boundary` + `world_origin` instead. + pub boundary_wcs: Option>>, /// Fill pattern. pub pattern: HatchPattern, /// Catalog name for this pattern (e.g. "ANSI31", "SOLID", "LINEAR"). diff --git a/src/scene/paper.rs b/src/scene/paper.rs index 3bb499fd6..48e856be0 100644 --- a/src/scene/paper.rs +++ b/src/scene/paper.rs @@ -283,6 +283,7 @@ impl Scene { } models.push(HatchModel { boundary: Arc::new(boundary), + boundary_wcs: None, pattern: model::hatch_model::HatchPattern::Solid, name: "WIPEOUT_FILL".into(), color: fill_color, diff --git a/tests/block_hatch_export.rs b/tests/block_hatch_export.rs index 40fdd27fb..eb46cde42 100644 --- a/tests/block_hatch_export.rs +++ b/tests/block_hatch_export.rs @@ -241,6 +241,7 @@ fn app_created_hatch_roundtrips_catalog_spacing() { let model = HatchModel { world_origin: [0.0, 0.0], boundary: Arc::new(boundary), + boundary_wcs: None, pattern: entry.gpu.clone(), name: "ANSI31".into(), color: [0.75, 0.75, 0.75, 0.85],