From fd4b7a7bc0e5d2de4eadf682eb2aee9510a5fc66 Mon Sep 17 00:00:00 2001 From: CamWight <91156642+CamWight@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:59:59 +0000 Subject: [PATCH] Seg2: one rule for an arc under a shape-keeping map (#4816) (#7316) `Seg2::mapped(f, arc)` is a segment under a map of the plane that keeps shapes. Points go through `f` itself (an arc's centre, every control point). `ArcMap { angle, mirrored, scale }` says how the map moves an arc: it turns it by `angle`, after a mirror across the x axis when `mirrored` (start `angle - start`, sweep reversed), and scales its radius by `scale`. `ArcMap::turn(angle, mirrored)` keeps lengths; `ArcMap::rigid(f)` measures a map of points that keeps lengths, from where it puts the origin and the unit axes, with a scale of exactly 1. `Seg2::similar` and the three arc maps written out by hand use it: - a sheet-metal region onto a panel's flat (`region_to_flat`, which measured its map by hand and now asks `ArcMap::rigid`); - the mirrored flat (`flip_x_loop`); - a sketch region mirrored in its x axis (`mirror_y`). The points go through the same functions as before, so every result is the same, bit for bit. This comes from the architecture review of the drawing work (2026-10-25): four places wrote the arc rule out, each its own way, and one measured its map by hand. `map_controls` is private to geom now: on its own it hands an arc back unmapped, which is what the three hand-written arms were there to avoid. Tests: every kind of segment under a mirrored map with a turn, a scale and a shift, point by point, its length scaled and an arc's sweep reversed; the three old formulas, bit for bit (the sketch mirror keeps a +0.0 start's -0.0); a rigid map's turn and mirror measured whatever its shift, and the identity; the sheet-metal and mirror tests unchanged. Co-authored-by: Claude Opus 5.5 --- crates/doc/src/eval_more.rs | 34 +++++++++---- crates/doc/src/sheet.rs | 78 ++++++++++++++++++------------ crates/engine/src/drawing_scene.rs | 10 ++-- crates/geom/src/lib.rs | 2 +- crates/geom/src/profile.rs | 47 +++++++++++++++--- crates/geom/src/tests.rs | 69 +++++++++++++++++++++++--- 6 files changed, 179 insertions(+), 61 deletions(-) diff --git a/crates/doc/src/eval_more.rs b/crates/doc/src/eval_more.rs index 2e94d4a6e3..d6e3e5e740 100644 --- a/crates/doc/src/eval_more.rs +++ b/crates/doc/src/eval_more.rs @@ -5,7 +5,7 @@ use super::*; use crate::EmbossFace; use crate::apply_point; use crate::sheet; -use solidcraft_geom::{Aabb3, Loop2, Ring, Seg2}; +use solidcraft_geom::{Aabb3, ArcMap, Loop2, Ring, Seg2}; pub(super) fn eval(doc: &Document, vals: &BTreeMap, f: &Feature, st: &mut ModelState) -> Result<()> { match &f.kind { @@ -480,14 +480,8 @@ fn on_sheet_material(st: &ModelState, i: usize, p: Vec3) -> bool { fn mirror_y(r: &Region2) -> Region2 { let f = |p: Vec2| Vec2::new(p.x, -p.y); let m = |l: &Loop2| { - let segs: Vec = l - .segs - .iter() - .map(|s| match *s { - Seg2::Arc { center, radius, start, sweep } => Seg2::Arc { center: f(center), radius, start: -start, sweep: -sweep }, - other => other.map_controls(f), - }) - .collect(); + // -0.0: a start of +0.0 mirrors to -0.0, as `-start` does. + let segs: Vec = l.segs.iter().map(|s| s.mapped(f, ArcMap::turn(-0.0, true))).collect(); Loop2 { segs }.ccw() }; Region2 { outer: m(&r.outer), holes: r.holes.iter().map(|h| m(h).reversed()).collect() } @@ -2215,3 +2209,25 @@ fn tag_flanges(sh: &mut crate::sheet::SheetBody, from: usize, feature: u64) { fl.tag = format!("F{feature}.{}", k - from); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// A flipped base's sketch region mirrors in the sketch's x axis exactly: its arc stays an arc + /// and bulges the mirrored way, and the outline keeps its sense. + #[test] + fn a_flipped_region_mirrors_its_arcs_exactly() { + let c = Vec2::new(40.0, 30.0); + let r = Vec2::new(10.0, 0.0); + let half = + Loop2 { segs: vec![Seg2::Arc { center: c, radius: 10.0, start: 0.0, sweep: std::f64::consts::PI }, Seg2::Line { a: c - r, b: c + r }] }; + let mirrored = mirror_y(&Region2 { outer: half, holes: vec![] }).outer; + let arc = mirrored.segs.iter().find(|s| matches!(s, Seg2::Arc { .. })).unwrap(); + assert!( + mirrored.is_ccw() + && matches!(arc, Seg2::Arc { center, radius, .. } if (*center - Vec2::new(40.0, -30.0)).len() < 1e-12 && *radius == 10.0) + ); + assert!((arc.mid() - Vec2::new(40.0, -40.0)).len() < 1e-9, "{:?}", arc.mid()); + } +} diff --git a/crates/doc/src/sheet.rs b/crates/doc/src/sheet.rs index d529700f8a..67802bafae 100644 --- a/crates/doc/src/sheet.rs +++ b/crates/doc/src/sheet.rs @@ -10,7 +10,7 @@ //! together; the flat solid is the outline extruded through the thickness. use serde::{Deserialize, Serialize}; -use solidcraft_geom::{Aabb2, LINEAR_TOL, Loop2, Plane, Region2, Ring, Seg2, Vec2, Vec3}; +use solidcraft_geom::{Aabb2, ArcMap, LINEAR_TOL, Loop2, Plane, Region2, Ring, Seg2, Vec2, Vec3}; use solidcraft_kernel::{self as kernel, Body, EdgeSpec, FaceSpec, SurfSpec}; use crate::expr::{self, Kind, Value}; @@ -4285,24 +4285,8 @@ impl SheetBody { Vec2::new(q.x, q.y) }; // A rigid map of the plane: lines and arcs stay lines and arcs (mirrored or not). - let (o, ex, ey) = (map(Vec2::default()), map(Vec2::new(1.0, 0.0)), map(Vec2::new(0.0, 1.0))); - let (ax, ay) = (ex - o, ey - o); - let mirrored = ax.cross(ay) < 0.0; - let rot = ax.y.atan2(ax.x); - let seg = |sg: &Seg2| -> Seg2 { - match *sg { - Seg2::Line { a, b } => Seg2::Line { a: map(a), b: map(b) }, - Seg2::Arc { center, radius, start, sweep } => { - if mirrored { - Seg2::Arc { center: map(center), radius, start: rot - start, sweep: -sweep } - } else { - Seg2::Arc { center: map(center), radius, start: rot + start, sweep } - } - } - // Free-form segments map exactly, through their control points. - other => other.map_controls(map), - } - }; + let arc = ArcMap::rigid(map); + let seg = |sg: &Seg2| sg.mapped(map, arc); let map_loop = |lp: &Loop2| Loop2 { segs: lp.segs.iter().map(seg).collect() }; Ok(Region2 { outer: map_loop(&r.outer).ccw(), holes: r.holes.iter().map(|h| map_loop(h).ccw().reversed()).collect() }) } @@ -4654,16 +4638,7 @@ fn flip_x(p: Vec2) -> Vec2 { /// follow their control points), and run backwards so it keeps its sense: an outer loop stays /// counter-clockwise, a hole clockwise. fn flip_x_loop(l: &Loop2) -> Loop2 { - let segs = l - .segs - .iter() - .map(|s| match *s { - Seg2::Arc { center, radius, start, sweep } => { - Seg2::Arc { center: flip_x(center), radius, start: std::f64::consts::PI - start, sweep: -sweep } - } - other => other.map_controls(flip_x), - }) - .collect(); + let segs = l.segs.iter().map(|s| s.mapped(flip_x, ArcMap::turn(std::f64::consts::PI, true))).collect(); Loop2 { segs }.reversed() } @@ -5310,7 +5285,8 @@ mod tests { // right: the same ellipse on the sheet, every curve through its mirrored controls. let below = Plane { origin: Vec3::ZERO, x: Vec3::X, y: Vec3::Y * -1.0 }; let mirror = |p: Vec2| Vec2::new(p.x, -p.y); - let seen = Region2 { outer: Loop2 { segs: hole.outer.segs.iter().map(|x| x.map_controls(mirror)).collect() }, holes: vec![] }; + let seen = + Region2 { outer: Loop2 { segs: hole.outer.segs.iter().map(|x| x.mapped(mirror, ArcMap::turn(-0.0, true))).collect() }, holes: vec![] }; let mapped = s.region_to_flat(None, &below, &seen).unwrap(); assert!(mapped.outer.is_ccw() && mapped.outer.segs.len() == 4 && mapped.outer.segs.iter().all(|x| hole.outer.segs.contains(x)), "{mapped:?}"); // A cubic and a quartic closed by a line, clockwise in that plane: counter-clockwise on @@ -5333,6 +5309,48 @@ mod tests { assert!(mapped.holes.is_empty() && (mapped.area() + drawn.signed_area()).abs() < 1e-9 && mapped.area() > 100.0, "{}", mapped.area()); } + /// A half disc: an arc over the top about `c` (radius 10), closed by a line, counter-clockwise. + fn half_disc(c: Vec2) -> Loop2 { + let r = Vec2::new(10.0, 0.0); + Loop2 { segs: vec![Seg2::Arc { center: c, radius: 10.0, start: 0.0, sweep: std::f64::consts::PI }, Seg2::Line { a: c - r, b: c + r }] } + } + + /// Where `lp`'s arc lands: its centre, radius and middle point. + fn arc_of(lp: &Loop2) -> (Vec2, f64, Vec2) { + let arc = |s: &Seg2| match s { + Seg2::Arc { center, radius, .. } => Some((*center, *radius, s.mid())), + _ => None, + }; + lp.segs.iter().find_map(arc).unwrap() + } + + /// Arcs map into the flat frame exactly: drawn from below (a mirror) and on a plane turned a + /// quarter, a half disc's arc lands where its points do and bulges the way they go. + #[test] + fn arcs_map_into_the_flat_frame_mirrored_and_turned() { + let s = plate(100.0, 60.0); + let below = Plane { origin: Vec3::ZERO, x: Vec3::X, y: Vec3::Y * -1.0 }; + let turned = Plane { origin: Vec3::ZERO, x: Vec3::Y, y: Vec3::X * -1.0 }; + let (c, top) = (Vec2::new(40.0, 30.0), Vec2::new(40.0, 40.0)); + let flat_of_below = |p: Vec2| Vec2::new(p.x, -p.y); + let flat_of_turned = |p: Vec2| Vec2::new(-p.y, p.x); + for (plane, to_flat) in [(below, &flat_of_below as &dyn Fn(Vec2) -> Vec2), (turned, &flat_of_turned)] { + let mapped = s.region_to_flat(None, &plane, &Region2 { outer: half_disc(c), holes: vec![] }).unwrap(); + let (center, radius, mid) = arc_of(&mapped.outer); + assert!(mapped.outer.is_ccw() && radius == 10.0, "{mapped:?}"); + assert!((center - to_flat(c)).len() < 1e-9 && (mid - to_flat(top)).len() < 1e-9, "{center:?} {mid:?}"); + } + } + + /// A flat loop mirrored across its y axis keeps its arcs exact and its sense. + #[test] + fn a_flat_loop_mirrors_its_arcs_exactly() { + let mirrored = flip_x_loop(&half_disc(Vec2::new(40.0, 30.0))); + let (center, radius, mid) = arc_of(&mirrored); + assert!(mirrored.is_ccw() && radius == 10.0); + assert!((center - Vec2::new(-40.0, 30.0)).len() < 1e-12 && (mid - Vec2::new(-40.0, 40.0)).len() < 1e-9, "{center:?} {mid:?}"); + } + #[test] fn pockets_with_free_form_outlines_are_machined_beside_free_form_cut_outs() { let mut s = plate(100.0, 60.0); diff --git a/crates/engine/src/drawing_scene.rs b/crates/engine/src/drawing_scene.rs index 67ba333535..5583425d68 100644 --- a/crates/engine/src/drawing_scene.rs +++ b/crates/engine/src/drawing_scene.rs @@ -34,7 +34,7 @@ use solidcraft_doc::{DocError, ModelState, ids}; use solidcraft_geom::cell::Projection; use solidcraft_geom::paper::text::TextRun; use solidcraft_geom::paper::{Group, Item, MAX_RECORDS, ObjectKind, Picture, Scene}; -use solidcraft_geom::{Aabb2, Loop2, Region2, Seg2, Vec2}; +use solidcraft_geom::{Aabb2, ArcMap, Loop2, Region2, Seg2, Vec2}; use crate::drawing_picture::{picture_items, view_parts}; use crate::drawing_view::{CHORD_MM, LineRequest, MIN_MODEL_CHORD_MM, ShownSet, Stats, Styles, Unmeasured, paper_group}; @@ -294,12 +294,8 @@ fn group_bounds(group: &Group) -> Aabb2 { /// centre are the same bytes however they got there. fn moved(group: &Group, by: Vec2) -> Group { let segs = |path: &[Seg2]| -> Vec { - let shift = |seg: &Seg2| match *seg { - // An arc's centre moves; its radius and its angles stay. - Seg2::Arc { center, radius, start, sweep } => Seg2::Arc { center: center + by, radius, start, sweep }, - _ => seg.map_controls(|p| p + by), - }; - path.iter().map(shift).collect() + // A shift keeps an arc's radius and angles: no turn, no mirror. + path.iter().map(|seg| seg.mapped(|p| p + by, ArcMap::turn(0.0, false))).collect() }; let ring = |l: &Loop2| Loop2 { segs: segs(&l.segs) }; let items = group.items.iter().map(|item| match item { diff --git a/crates/geom/src/lib.rs b/crates/geom/src/lib.rs index e7fba784db..b114b0ebfb 100644 --- a/crates/geom/src/lib.rs +++ b/crates/geom/src/lib.rs @@ -55,7 +55,7 @@ pub use chain::{Chain, Closed, Ends, Open, Path, Planar, Ring}; pub use grid::Grid; pub use mesh::{Aabb3, BoxedTriangle, InsideIndex, MassProperties, Mesh, MeshMeasure, find_close_pair}; pub use plane::Plane; -pub use profile::{Loop2, MAX_BEZIER_CTRL, MAX_BEZIER_DEGREE, MAX_DERIVATIVE_ORDER, Region2, Seg2, Similarity2, bezier, regions_apart}; +pub use profile::{ArcMap, Loop2, MAX_BEZIER_CTRL, MAX_BEZIER_DEGREE, MAX_DERIVATIVE_ORDER, Region2, Seg2, Similarity2, bezier, regions_apart}; pub use unit_vec3::UnitVec3; pub use vec::{Vec2, Vec3}; diff --git a/crates/geom/src/profile.rs b/crates/geom/src/profile.rs index 5602d6f831..420ca106a1 100644 --- a/crates/geom/src/profile.rs +++ b/crates/geom/src/profile.rs @@ -58,6 +58,31 @@ pub const MAX_BEZIER_CTRL: usize = MAX_BEZIER_DEGREE + 1; /// continuity order a blend curve can be built for (4 = G4). pub const MAX_DERIVATIVE_ORDER: usize = 4; +/// How a map of the plane that keeps shapes moves an arc ([`Seg2::mapped`]): it turns it by `angle` +/// after mirroring it across the x axis when `mirrored`, and scales its radius by `scale`. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct ArcMap { + pub angle: f64, + pub mirrored: bool, + pub scale: f64, +} + +impl ArcMap { + /// A turn by `angle`, after a mirror when `mirrored`, that keeps lengths. + pub const fn turn(angle: f64, mirrored: bool) -> ArcMap { + ArcMap { angle, mirrored, scale: 1.0 } + } + + /// The arc map of `f`, a map of points that keeps lengths (a turn, a mirror and a shift), as + /// its images of the origin and the unit axes give it: the x axis's angle, and whether the + /// axes come out reversed. Its scale is 1, never a measured length. + pub fn rigid(f: impl Fn(Vec2) -> Vec2) -> ArcMap { + let o = f(Vec2::ZERO); + let (ax, ay) = (f(Vec2::new(1.0, 0.0)) - o, f(Vec2::new(0.0, 1.0)) - o); + ArcMap::turn(ax.y.atan2(ax.x), ax.cross(ay) < 0.0) + } +} + /// A similarity of the plane: a point is scaled by `scale` (above 0) about the origin, turned /// `angle` radians counter-clockwise, then moved by `shift`. It keeps shapes and the direction /// curves turn in, so an arc stays an arc ([`Seg2::similar`]). @@ -411,9 +436,9 @@ impl Seg2 { } } /// The segment with each control point mapped by `f`: exact for any affine map of lines, - /// Béziers and conics. An arc is returned as it is: only a similarity keeps it an arc - /// ([`Seg2::similar`]), and callers that mirror one handle it themselves. - pub fn map_controls(&self, f: impl Fn(Vec2) -> Vec2) -> Seg2 { + /// Béziers and conics. An arc is returned as it is, so callers go through [`Seg2::mapped`]; only + /// a segment that holds no arc, such as a glyph's outline, takes a map that doesn't keep shapes. + pub(crate) fn map_controls(&self, f: impl Fn(Vec2) -> Vec2) -> Seg2 { match *self { Seg2::Line { a, b } => Seg2::Line { a: f(a), b: f(b) }, Seg2::Cubic { p0, p1, p2, p3 } => Seg2::Cubic { p0: f(p0), p1: f(p1), p2: f(p2), p3: f(p3) }, @@ -425,14 +450,22 @@ impl Seg2 { arc @ Seg2::Arc { .. } => arc, } } - /// The segment moved by the similarity `m`, exactly: an arc's centre maps, its radius scales - /// and its start turns; every other kind maps its control points. + /// The segment moved by the similarity `m`, exactly ([`Seg2::mapped`]). pub fn similar(&self, m: &Similarity2) -> Seg2 { + self.mapped(|p| m.apply(p), ArcMap { angle: m.angle(), mirrored: false, scale: m.scale() }) + } + /// The segment under a map of the plane that keeps shapes: `f` moves its points (an arc's centre + /// and every control point), and `arc` says how it turns an arc (start `angle - start` and the + /// sweep reversed when mirrored, `angle + start` otherwise; the radius scaled). Points go + /// through `f` itself, so a map exact on points stays exact. + pub fn mapped(&self, f: impl Fn(Vec2) -> Vec2, arc: ArcMap) -> Seg2 { + let ArcMap { angle, mirrored, scale } = arc; match *self { Seg2::Arc { center, radius, start, sweep } => { - Seg2::Arc { center: m.apply(center), radius: radius * m.scale(), start: start + m.angle(), sweep } + let (start, sweep) = if mirrored { (angle - start, -sweep) } else { (angle + start, sweep) }; + Seg2::Arc { center: f(center), radius: radius * scale, start, sweep } } - _ => self.map_controls(|p| m.apply(p)), + _ => self.map_controls(f), } } /// The smallest box round the piece: exact for a line and an arc, and for a curve within a diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs index ab0a5ae52e..e99cdba5b8 100644 --- a/crates/geom/src/tests.rs +++ b/crates/geom/src/tests.rs @@ -952,22 +952,77 @@ fn a_box_from_points_holds_them_and_grows() { assert!(b.intersection(&far).is_empty()); } -/// A similarity moves every kind of segment exactly: the moved segment's points are the original's -/// points moved, its length scales, and an arc stays an arc turning the same way. -#[test] -fn a_similarity_moves_each_kind_of_segment_exactly() { - let m = Similarity2::new(2.5, 0.7, Vec2::new(-3.0, 11.0)).unwrap(); +/// One segment of each kind: a line, an arc, a cubic, a conic and a quintic Bézier. +fn every_kind_of_segment() -> [Seg2; 5] { let mut p = [Vec2::ZERO; MAX_BEZIER_CTRL]; for (i, q) in p.iter_mut().take(6).enumerate() { *q = Vec2::new(i as f64, (i * i) as f64 * 0.3); } - let segments = [ + [ Seg2::Line { a: Vec2::new(1.0, 2.0), b: Vec2::new(4.0, -1.0) }, Seg2::Arc { center: Vec2::new(2.0, 1.0), radius: 3.0, start: 0.4, sweep: -2.1 }, Seg2::Cubic { p0: Vec2::ZERO, p1: Vec2::new(1.0, 2.0), p2: Vec2::new(3.0, 2.0), p3: Vec2::new(4.0, 0.0) }, Seg2::Conic { a: Vec2::new(10.0, 0.0), apex: Vec2::new(10.0, 10.0), b: Vec2::new(0.0, 10.0), w: std::f64::consts::FRAC_1_SQRT_2 }, Seg2::Bezier { n: 5, p }, - ]; + ] +} + +/// A mirrored map moves every kind of segment exactly too: after a mirror in the x axis, a turn of +/// 0.7, a scale of 2.5 and a shift, each segment's points are the original's mapped, its length +/// scales and an arc's sweep reverses. And the arc numbers the sheet-metal flat and the sketch +/// mirror wrote out by hand come out bit for bit. +#[test] +fn a_mirrored_map_moves_each_kind_of_segment_exactly() { + let (angle, scale, shift) = (0.7, 2.5, Vec2::new(-3.0, 11.0)); + let (sin, cos) = f64::sin_cos(angle); + let f = |p: Vec2| Vec2::new(cos * p.x + sin * p.y, sin * p.x - cos * p.y) * scale + shift; + for seg in every_kind_of_segment() { + let moved = seg.mapped(f, ArcMap { angle, mirrored: true, scale }); + for i in 0..=8 { + let t = f64::from(i) / 8.0; + assert!((moved.point_at(t) - f(seg.point_at(t))).len() < 1e-9, "{seg:?} at {t}"); + } + assert!((moved.length() - seg.length() * scale).abs() < 1e-9 * seg.length().max(1.0), "{seg:?}"); + if let (Seg2::Arc { sweep: s, .. }, Seg2::Arc { sweep: m, .. }) = (seg, moved) { + assert_eq!(m, -s); + } + } + // The sketch mirror's -start (a +0.0 start to -0.0), the mirrored flat's π - start, and the + // flat's rot - start mirrored or rot + start not. + let start_of = |s: Seg2| if let Seg2::Arc { start, .. } = s { start } else { f64::NAN }; + let arc = |start: f64| Seg2::Arc { center: Vec2::ZERO, radius: 1.0, start, sweep: 1.0 }; + let same = |p: Vec2| p; + for (start, angle, mirrored, want) in [ + (0.4, -0.0, true, -0.4), + (0.0, -0.0, true, -0.0), + (0.4, std::f64::consts::PI, true, std::f64::consts::PI - 0.4), + (0.4, 1.1, true, 1.1 - 0.4), + (0.4, 1.1, false, 1.1 + 0.4), + ] { + assert_eq!(start_of(arc(start).mapped(same, ArcMap::turn(angle, mirrored))).to_bits(), f64::to_bits(want), "{start} {angle} {mirrored}"); + } +} + +/// A rigid map's arc map comes from where it puts the origin and the unit axes: the x axis's turn, +/// whether the axes come out reversed, and a scale of exactly 1, whatever the shift. +#[test] +fn a_rigid_maps_arc_map_is_its_turn_and_its_mirror() { + let (sin, cos) = f64::sin_cos(0.7); + let shift = Vec2::new(-3.0, 11.0); + let mirrored = |p: Vec2| Vec2::new(cos * p.x + sin * p.y, sin * p.x - cos * p.y) + shift; + let turned = |p: Vec2| Vec2::new(cos * p.x - sin * p.y, sin * p.x + cos * p.y) + shift; + for (arc, mirror) in [(ArcMap::rigid(mirrored), true), (ArcMap::rigid(turned), false)] { + assert!((arc.angle - 0.7).abs() < 1e-12 && arc.mirrored == mirror && arc.scale == 1.0, "{arc:?}"); + } + assert_eq!(ArcMap::rigid(|p| p), ArcMap::turn(0.0, false)); +} + +/// A similarity moves every kind of segment exactly: the moved segment's points are the original's +/// points moved, its length scales, and an arc stays an arc turning the same way. +#[test] +fn a_similarity_moves_each_kind_of_segment_exactly() { + let m = Similarity2::new(2.5, 0.7, Vec2::new(-3.0, 11.0)).unwrap(); + let segments = every_kind_of_segment(); for seg in segments { let moved = seg.similar(&m); for i in 0..=8 {