forked from 1b-agent/opencadstudio-upstream
fix(ellipse,mirror): WCS ellipse rendering + delegate hatch MIRROR upstream
Two follow-ups from the boundary-arc convention audit: - ELLIPSE is a WCS entity in DXF (codes 10/11 are world coordinates, unlike ARC/CIRCLE). The renderer ran center and major_axis through the arbitrary-axis OCS, which misplaced any ellipse whose normal isn't Z-up — e.g. the (0,0,-1) result of a mirrored-block explode. Read them as WCS; pairs with acadrust 569bd4d, which stores WCS on its explode paths. Z-up ellipses are unaffected (the conversion was an identity). - The MIRROR command's hand-rolled hatch closure swapped boundary-arc angles itself — only valid for ccw arcs and ignorant of the stored mirrored-angle / wrap-encoded-sweep conventions. It now builds a reflection Transform (new reflection_about_xy_line helper) and delegates to acadrust's transform_hatch, the single source of truth for those conventions. Covered by a Line→Arc→Line continuity + sweep regression test. Note: full effect of the ellipse change lands once the acadrust pin is bumped to 569bd4d; until then only the (previously broken) non-Z-up corner differs. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
80059922c5
commit
d09614be39
3 files changed
+110
-62
No files matched your search
+9
-12
@@ -12,22 +12,19 @@ use crate::scene::wire_model::SnapHint;
|
||||
const TAU: f64 = std::f64::consts::TAU;
|
||||
|
||||
fn to_truck(ell: &Ellipse) -> TruckEntity {
|
||||
let cx = ell.center.x;
|
||||
let cy = ell.center.y;
|
||||
let cz = ell.center.z;
|
||||
let normal = (ell.normal.x, ell.normal.y, ell.normal.z);
|
||||
let (nx, ny, nz) = normal;
|
||||
|
||||
// Center in WCS.
|
||||
let (cwx, cwy, cwz) = crate::scene::transform::ocs_point_to_wcs((cx, cy, cz), normal);
|
||||
|
||||
// Major axis vector rotated into WCS: maj_ocs.x*Ax + maj_ocs.y*Ay + maj_ocs.z*N
|
||||
let (ax_basis, ay_basis) = crate::scene::transform::ocs_axes(normal);
|
||||
let (mx, my, mz) = (ell.major_axis.x, ell.major_axis.y, ell.major_axis.z);
|
||||
// ELLIPSE is one of the few WCS entities in DXF: `center` (code 10) and
|
||||
// `major_axis` (code 11) are world coordinates already — unlike ARC /
|
||||
// CIRCLE, whose centers are OCS. (This used to run both through the
|
||||
// arbitrary-axis OCS, which misplaced any ellipse whose normal isn't
|
||||
// Z-up — e.g. the (0,0,-1) result of a mirrored-block explode.)
|
||||
let (cwx, cwy, cwz) = (ell.center.x, ell.center.y, ell.center.z);
|
||||
let wcs_maj = glam::Vec3::new(
|
||||
(mx * ax_basis.0 + my * ay_basis.0 + mz * nx) as f32,
|
||||
(mx * ax_basis.1 + my * ay_basis.1 + mz * ny) as f32,
|
||||
(mx * ax_basis.2 + my * ay_basis.2 + mz * nz) as f32,
|
||||
ell.major_axis.x as f32,
|
||||
ell.major_axis.y as f32,
|
||||
ell.major_axis.z as f32,
|
||||
);
|
||||
let r_major = wcs_maj.length() as f64;
|
||||
let r_minor = r_major * ell.minor_axis_ratio;
|
||||
|
||||
+9
-50
@@ -89,57 +89,16 @@ fn apply_geom_prop(h: &mut Hatch, field: &str, value: &str) {
|
||||
}
|
||||
|
||||
fn apply_transform(h: &mut Hatch, t: &EntityTransform) {
|
||||
use crate::scene::transform::reflect_xy_point as reflect;
|
||||
crate::scene::transform::apply_standard_entity_transform(h, t, |entity, p1, p2| {
|
||||
let dx = (p2.x - p1.x) as f64;
|
||||
let dy = (p2.y - p1.y) as f64;
|
||||
let len2 = dx * dx + dy * dy;
|
||||
if len2 < 1e-12 {
|
||||
return;
|
||||
}
|
||||
let line_angle = dy.atan2(dx);
|
||||
for path in &mut entity.paths {
|
||||
for edge in &mut path.edges {
|
||||
match edge {
|
||||
BoundaryEdge::Line(l) => {
|
||||
reflect(&mut l.start.x, &mut l.start.y, p1, p2);
|
||||
reflect(&mut l.end.x, &mut l.end.y, p1, p2);
|
||||
}
|
||||
BoundaryEdge::CircularArc(a) => {
|
||||
reflect(&mut a.center.x, &mut a.center.y, p1, p2);
|
||||
let tmp = a.start_angle;
|
||||
a.start_angle = 2.0 * line_angle - a.end_angle;
|
||||
a.end_angle = 2.0 * line_angle - tmp;
|
||||
}
|
||||
BoundaryEdge::EllipticArc(e) => {
|
||||
reflect(&mut e.center.x, &mut e.center.y, p1, p2);
|
||||
let ax = dx;
|
||||
let ay = dy;
|
||||
let rx = e.major_axis_endpoint.x;
|
||||
let ry = e.major_axis_endpoint.y;
|
||||
let dot = rx * ax + ry * ay;
|
||||
e.major_axis_endpoint.x = 2.0 * dot * ax / len2 - rx;
|
||||
e.major_axis_endpoint.y = 2.0 * dot * ay / len2 - ry;
|
||||
let tmp = e.start_angle;
|
||||
e.start_angle = 2.0 * line_angle - e.end_angle;
|
||||
e.end_angle = 2.0 * line_angle - tmp;
|
||||
}
|
||||
BoundaryEdge::Spline(s) => {
|
||||
for cp in &mut s.control_points {
|
||||
reflect(&mut cp.x, &mut cp.y, p1, p2);
|
||||
}
|
||||
for fp in &mut s.fit_points {
|
||||
reflect(&mut fp.x, &mut fp.y, p1, p2);
|
||||
}
|
||||
}
|
||||
BoundaryEdge::Polyline(p) => {
|
||||
for v in &mut p.vertices {
|
||||
reflect(&mut v.x, &mut v.y, p1, p2);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Delegate the mirror to acadrust's transform_hatch (via the Entity
|
||||
// trait): it flips the boundary-arc direction flags, re-mirrors the
|
||||
// stored angles and preserves the stored sweep — including the
|
||||
// wrap-encoded end angles above 2π that AutoCAD writes. The old
|
||||
// hand-rolled angle-swap here was only valid for ccw boundary arcs on
|
||||
// an axis-aligned mirror line and went stale the moment those
|
||||
// conventions were fixed upstream.
|
||||
let t = crate::scene::transform::reflection_about_xy_line(p1, p2);
|
||||
acadrust::entities::Entity::apply_transform(entity, &t);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -43,6 +43,26 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
/// Reflection across the world-XY line through `p1`→`p2` as an acadrust
|
||||
/// `Transform`, for delegating MIRROR to acadrust's entity-aware
|
||||
/// `apply_transform` paths (which handle direction flags, stored-angle
|
||||
/// conventions and bulges themselves). Degenerate line → identity.
|
||||
pub fn reflection_about_xy_line(p1: Vec3, p2: Vec3) -> acadrust::types::Transform {
|
||||
use acadrust::types::{Matrix4, Transform};
|
||||
let dx = (p2.x - p1.x) as f64;
|
||||
let dy = (p2.y - p1.y) as f64;
|
||||
if dx * dx + dy * dy < 1e-12 {
|
||||
return Transform::identity();
|
||||
}
|
||||
let ang = dy.atan2(dx);
|
||||
let m = Matrix4::translation(p1.x as f64, p1.y as f64, 0.0)
|
||||
* Matrix4::rotation_z(ang)
|
||||
* Matrix4::scaling(1.0, -1.0, 1.0)
|
||||
* Matrix4::rotation_z(-ang)
|
||||
* Matrix4::translation(-(p1.x as f64), -(p1.y as f64), 0.0);
|
||||
Transform::from_matrix(m)
|
||||
}
|
||||
|
||||
pub fn reflect_xy_point(x: &mut f64, y: &mut f64, p1: Vec3, p2: Vec3) {
|
||||
let ax = (p2.x - p1.x) as f64;
|
||||
let ay = (p2.y - p1.y) as f64;
|
||||
@@ -108,3 +128,75 @@ pub fn ocs_point_to_wcs(ocs: (f64, f64, f64), normal: (f64, f64, f64)) -> (f64,
|
||||
ox * ax.2 + oy * ay.2 + oz * nz,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod mirror_delegation_tests {
|
||||
use super::*;
|
||||
use crate::command::EntityTransform;
|
||||
use crate::entities::traits::Transformable;
|
||||
use acadrust::entities::hatch::{BoundaryEdge, BoundaryPath, CircularArcEdge, LineEdge};
|
||||
use acadrust::entities::Hatch;
|
||||
use acadrust::types::Vector2;
|
||||
|
||||
// MIRROR on a hatch goes through reflection_about_xy_line + acadrust's
|
||||
// transform_hatch. Mirror a Line→Arc→Line path across a vertical line and
|
||||
// assert the arc stays endpoint-continuous, flips its direction flag, and
|
||||
// keeps its sweep magnitude (the stored-angle conventions the old
|
||||
// hand-rolled closure violated).
|
||||
#[test]
|
||||
fn hatch_mirror_keeps_arc_continuous_and_sweep() {
|
||||
let mut path = BoundaryPath::new();
|
||||
path.edges.push(BoundaryEdge::Line(LineEdge {
|
||||
start: Vector2::new(0.0, -1.0),
|
||||
end: Vector2::new(2.0, 0.0),
|
||||
}));
|
||||
path.edges.push(BoundaryEdge::CircularArc(CircularArcEdge {
|
||||
center: Vector2::new(1.0, 0.0),
|
||||
radius: 1.0,
|
||||
start_angle: 0.0,
|
||||
end_angle: std::f64::consts::PI,
|
||||
counter_clockwise: true,
|
||||
}));
|
||||
path.edges.push(BoundaryEdge::Line(LineEdge {
|
||||
start: Vector2::new(0.0, 0.0),
|
||||
end: Vector2::new(0.0, -1.0),
|
||||
}));
|
||||
let mut h = Hatch::new();
|
||||
h.paths.push(path);
|
||||
|
||||
// Mirror across the vertical line x = 5.
|
||||
h.apply_transform(&EntityTransform::Mirror {
|
||||
p1: Vec3::new(5.0, 0.0, 0.0),
|
||||
p2: Vec3::new(5.0, 1.0, 0.0),
|
||||
});
|
||||
|
||||
let edges = &h.paths[0].edges;
|
||||
let (l1, arc) = match (&edges[0], &edges[1]) {
|
||||
(BoundaryEdge::Line(a), BoundaryEdge::CircularArc(b)) => (a, b),
|
||||
_ => panic!("edge kinds changed"),
|
||||
};
|
||||
assert!(!arc.counter_clockwise, "mirror must flip the flag");
|
||||
let sweep = arc.end_angle - arc.start_angle;
|
||||
assert!(
|
||||
(sweep - std::f64::consts::PI).abs() < 1e-9,
|
||||
"stored sweep must stay π, got {sweep}"
|
||||
);
|
||||
// Stored-angle convention: true point of a CW edge is at -θ.
|
||||
let pt = |theta: f64| {
|
||||
let a = if arc.counter_clockwise { theta } else { -theta };
|
||||
(
|
||||
arc.center.x + arc.radius * a.cos(),
|
||||
arc.center.y + arc.radius * a.sin(),
|
||||
)
|
||||
};
|
||||
let (sx, sy) = pt(arc.start_angle);
|
||||
assert!(
|
||||
(sx - l1.end.x).abs() < 1e-9 && (sy - l1.end.y).abs() < 1e-9,
|
||||
"arc start {:?} must meet previous line end {:?}",
|
||||
(sx, sy),
|
||||
(l1.end.x, l1.end.y)
|
||||
);
|
||||
// Mirror of (2,0) across x=5 is (8,0).
|
||||
assert!((sx - 8.0).abs() < 1e-9 && sy.abs() < 1e-9);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user