From 01789c592967ba082bfef2a750889b503374afe5 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Wed, 23 Sep 2026 09:43:02 -0400 Subject: [PATCH] fix(geometry): harden the arc-preserving per-entity offset GetOffsetPerimeterEntities/GetOffsetPartEntities feed directional-distance loops (FillLinear, Compactor, RotationSlideStrategy) that handle arcs natively. Switching them to Clipper line output (plan option B) made OpenNest.Tests run 48s -> 8m19s, Fill tests ~3x slower, and broke 20 exact-fit tests through tessellation and conservative padding, so they keep the per-entity offset (option A), hardened: - Arc, Circle and Line offsets are now side-symmetric. Right on a CCW arc shrank instead of growing, Right on a CW circle grew, and Right on a line offset to the left and reversed it. Only Left was used on hot paths, so this was latent (SimplifierViewer drew both tolerance bands on one side). - Shape.OffsetEntity closes every gap between consecutive offset pieces: convex non-tangent line/arc corners get a round join about the original corner, lines across a collapsed fillet are mitered, and any other gap (concave arc corner, collapsed entity) is bridged with a line. Before, only line-line corners were joined, so a vertex could slip through. - Zero-area spikes are left in place and documented: they lie inside the offset envelope, which is harmless for directional distance. - OffsetOutward/OffsetInward become internal; PartGeometry is their only caller. Co-Authored-By: Claude Opus 5.5 --- OpenNest.Core/Geometry/Arc.cs | 24 +- OpenNest.Core/Geometry/Circle.cs | 7 +- OpenNest.Core/Geometry/Line.cs | 6 +- OpenNest.Core/Geometry/Shape.cs | 243 +++++++++++++++----- OpenNest.Tests/Geometry/ShapeOffsetTests.cs | 155 +++++++++++++ 5 files changed, 360 insertions(+), 75 deletions(-) create mode 100644 OpenNest.Tests/Geometry/ShapeOffsetTests.cs diff --git a/OpenNest.Core/Geometry/Arc.cs b/OpenNest.Core/Geometry/Arc.cs index b361763..5a3c4a7 100644 --- a/OpenNest.Core/Geometry/Arc.cs +++ b/OpenNest.Core/Geometry/Arc.cs @@ -443,19 +443,23 @@ namespace OpenNest.Geometry boundingBox.Width = maxY - minY; } + /// + /// Offsets the arc to the given side of its travel direction. The center lies to + /// the left of a CCW arc and to the right of a CW (reversed) one, so the arc grows + /// on the other side and shrinks toward its center. Returns null when it shrinks + /// to nothing. + /// public override Entity OffsetEntity(double distance, OffsetSide side) { - if (side == OffsetSide.Left && reversed) - { - return new Arc(center, radius + distance, startAngle, endAngle, reversed); - } - else - { - if (distance >= radius) - return null; + var grows = (side == OffsetSide.Left) == reversed; - return new Arc(center, radius - distance, startAngle, endAngle, reversed); - } + if (grows) + return new Arc(center, radius + distance, startAngle, endAngle, reversed); + + if (distance >= radius) + return null; + + return new Arc(center, radius - distance, startAngle, endAngle, reversed); } public override Entity OffsetEntity(double distance, Vector pt) diff --git a/OpenNest.Core/Geometry/Circle.cs b/OpenNest.Core/Geometry/Circle.cs index e89a62b..7c890f0 100644 --- a/OpenNest.Core/Geometry/Circle.cs +++ b/OpenNest.Core/Geometry/Circle.cs @@ -273,7 +273,10 @@ namespace OpenNest.Geometry public override Entity OffsetEntity(double distance, OffsetSide side) { - if (side == OffsetSide.Left && Rotation == RotationType.CCW) + // The center lies to the left of a CCW circle and to the right of a CW one. + var shrinks = (side == OffsetSide.Left) == (Rotation == RotationType.CCW); + + if (shrinks) { return Radius <= distance ? null @@ -281,7 +284,7 @@ namespace OpenNest.Geometry } else { - return new Circle(center, Radius + distance) { Layer = Layer }; + return new Circle(center, Radius + distance) { Layer = Layer, Rotation = Rotation }; } } diff --git a/OpenNest.Core/Geometry/Line.cs b/OpenNest.Core/Geometry/Line.cs index 5477cec..1fb4ef8 100644 --- a/OpenNest.Core/Geometry/Line.cs +++ b/OpenNest.Core/Geometry/Line.cs @@ -398,11 +398,9 @@ namespace OpenNest.Geometry var x = System.Math.Cos(angle) * distance; var y = System.Math.Sin(angle) * distance; - var pt = new Vector(x, y); + var pt = side == OffsetSide.Left ? new Vector(x, y) : new Vector(-x, -y); - return side == OffsetSide.Left - ? new Line(StartPoint + pt, EndPoint + pt) - : new Line(EndPoint + pt, StartPoint + pt); + return new Line(StartPoint + pt, EndPoint + pt); } public override Entity OffsetEntity(double distance, Vector pt) diff --git a/OpenNest.Core/Geometry/Shape.cs b/OpenNest.Core/Geometry/Shape.cs index ab32d66..2146351 100644 --- a/OpenNest.Core/Geometry/Shape.cs +++ b/OpenNest.Core/Geometry/Shape.cs @@ -463,80 +463,60 @@ namespace OpenNest.Geometry boundingBox = Entities.Select(geo => geo.BoundingBox).ToList().GetBoundingBox(); } + /// + /// Offsets each perimeter entity to the given side and joins the pieces into a + /// closed chain: line-line corners get a round join (convex) or a miter (concave), + /// other convex corners get a round join, and any remaining gap (a concave corner + /// involving an arc, or an entity that collapsed under the offset) is bridged + /// with a line. Cutouts are offset the same way. + /// + /// Where a feature is narrower than twice the distance, the result keeps zero-area + /// spikes and inverted loops. They lie inside the true offset envelope, so they are + /// harmless to directional-distance queries, which only need a closed boundary + /// that never falls inside the envelope. Use when a + /// clean region is needed. + /// + /// public override Entity OffsetEntity(double distance, OffsetSide side) { var offsetShape = new Shape(); var definedShape = new ShapeProfile(this); - Entity firstEntity = null; - Entity firstOffsetEntity = null; - Entity lastEntity = null; - Entity lastOffsetEntity = null; + var pieces = new List(); + var collapsed = false; foreach (var entity in definedShape.Perimeter.Entities) { var offsetEntity = entity.OffsetEntity(distance, side); if (offsetEntity == null) + { + collapsed = true; continue; - - if (firstEntity == null) - { - firstEntity = entity; - firstOffsetEntity = offsetEntity; } - switch (entity.Type) - { - case EntityType.Line: - { - var line = (Line)entity; - var offsetLine = (Line)offsetEntity; - - if (lastOffsetEntity != null && lastOffsetEntity.Type == EntityType.Line) - { - JoinOffsetLines( - (Line)lastEntity, - (Line)lastOffsetEntity, - line, - offsetLine, - distance, - side, - offsetShape - ); - } - - offsetShape.Entities.Add(offsetLine); - break; - } - - default: - offsetShape.Entities.Add(offsetEntity); - break; - } - - lastOffsetEntity = offsetEntity; - lastEntity = entity; + pieces.Add(new OffsetPiece(entity, offsetEntity, collapsed)); + collapsed = false; } - // Close the shape: join last offset entity back to first - if ( - lastOffsetEntity != null - && firstOffsetEntity != null - && lastOffsetEntity != firstOffsetEntity - && lastOffsetEntity.Type == EntityType.Line - && firstOffsetEntity.Type == EntityType.Line - ) + // Entities that collapsed at the end of the loop sit before the first piece. + if (collapsed && pieces.Count > 0) + pieces[0] = pieces[0] with { CollapsedBefore = true }; + + for (var i = 0; i < pieces.Count; i++) { - JoinOffsetLines( - (Line)lastEntity, - (Line)lastOffsetEntity, - (Line)firstEntity, - (Line)firstOffsetEntity, - distance, - side, - offsetShape - ); + offsetShape.Entities.Add(pieces[i].Offset); + + if (pieces.Count > 1) + { + JoinOffsetPieces( + pieces[i], + pieces[(i + 1) % pieces.Count], + distance, + side, + offsetShape + ); + } } foreach (var cutout in definedShape.Cutouts) @@ -547,6 +527,151 @@ namespace OpenNest.Geometry return offsetShape; } + private readonly record struct OffsetPiece( + Entity Source, + Entity Offset, + bool CollapsedBefore + ); + + private static void JoinOffsetPieces( + OffsetPiece last, + OffsetPiece next, + double distance, + OffsetSide side, + Shape offsetShape + ) + { + // Lines meeting across a collapsed fillet are concave, so a miter trims both at + // their intersection. Parallel ones (a round-bottomed slot) fall through to + // the bridge below. + if ( + next.CollapsedBefore + && last.Offset is Line lastOffsetLine + && next.Offset is Line nextOffsetLine + && Intersect.IntersectsUnbounded(nextOffsetLine, lastOffsetLine, out var miter) + ) + { + lastOffsetLine.EndPoint = miter; + nextOffsetLine.StartPoint = miter; + return; + } + + if (!next.CollapsedBefore && last.Source is Line lastLine && next.Source is Line nextLine) + { + JoinOffsetLines( + lastLine, + (Line)last.Offset, + nextLine, + (Line)next.Offset, + distance, + side, + offsetShape + ); + return; + } + + if ( + !TryGetEnds(last.Offset, out _, out var gapStart) + || !TryGetEnds(next.Offset, out var gapEnd, out _) + ) + return; + + if (gapStart.DistanceTo(gapEnd) <= OpenNest.Math.Tolerance.Epsilon) + return; + + if ( + !next.CollapsedBefore + && IsConvexCorner(last.Source, next.Source, side, out var corner) + ) + { + offsetShape.Entities.Add( + new Arc( + corner, + distance, + corner.AngleTo(gapStart), + corner.AngleTo(gapEnd), + side == OffsetSide.Left + ) + ); + return; + } + + // Concave corner or collapsed entity: the neighbors' offsets overlap, so a + // straight bridge stays inside the offset envelope and closes the chain. + offsetShape.Entities.Add(new Line(gapStart, gapEnd)); + } + + private static bool IsConvexCorner( + Entity last, + Entity next, + OffsetSide side, + out Vector corner + ) + { + corner = default; + + if ( + !TryGetEnds(last, out _, out corner) + || !TryGetTangents(last, out _, out var d1) + || !TryGetTangents(next, out var d2, out _) + ) + return false; + + var cross = d1.X * d2.Y - d1.Y * d2.X; + + return (side == OffsetSide.Left && cross < -OpenNest.Math.Tolerance.Epsilon) + || (side == OffsetSide.Right && cross > OpenNest.Math.Tolerance.Epsilon); + } + + private static bool TryGetEnds(Entity entity, out Vector start, out Vector end) + { + switch (entity) + { + case Line line: + start = line.StartPoint; + end = line.EndPoint; + return true; + + case Arc arc: + start = arc.StartPoint(); + end = arc.EndPoint(); + return true; + + default: + start = end = default; + return false; + } + } + + /// + /// Direction of travel at the start and end of a line or arc. + /// + private static bool TryGetTangents(Entity entity, out Vector start, out Vector end) + { + switch (entity) + { + case Line line: + start = end = line.EndPoint - line.StartPoint; + return true; + + case Arc arc: + start = ArcTangent(arc, arc.StartAngle); + end = ArcTangent(arc, arc.EndAngle); + return true; + + default: + start = end = default; + return false; + } + } + + private static Vector ArcTangent(Arc arc, double angle) + { + var sin = System.Math.Sin(angle); + var cos = System.Math.Cos(angle); + return arc.IsReversed ? new Vector(sin, -cos) : new Vector(-sin, cos); + } + private static void JoinOffsetLines( Line lastLine, Line lastOffsetLine, @@ -611,7 +736,7 @@ namespace OpenNest.Geometry /// Normalizes to CW winding before offsetting Left (which is outward for CW), /// making the method independent of the original contour winding direction. /// - public Shape OffsetOutward(double distance) + internal Shape OffsetOutward(double distance) { var poly = ToPolygon(); @@ -660,7 +785,7 @@ namespace OpenNest.Geometry /// Normalizes to CCW winding before offsetting Left (which is inward for CCW), /// making the method independent of the original contour winding direction. /// - public Shape OffsetInward(double distance) + internal Shape OffsetInward(double distance) { var poly = ToPolygon(); diff --git a/OpenNest.Tests/Geometry/ShapeOffsetTests.cs b/OpenNest.Tests/Geometry/ShapeOffsetTests.cs new file mode 100644 index 0000000..ce5ce53 --- /dev/null +++ b/OpenNest.Tests/Geometry/ShapeOffsetTests.cs @@ -0,0 +1,155 @@ +using System.Collections.Generic; +using System.Linq; +using OpenNest.Geometry; +using OpenNest.Math; + +namespace OpenNest.Tests.Geometry; + +public class ShapeOffsetTests +{ + [Theory] + [InlineData(false, OffsetSide.Left, 4)] // CCW: center on the left, shrinks. + [InlineData(false, OffsetSide.Right, 6)] + [InlineData(true, OffsetSide.Left, 6)] // CW: center on the right, grows. + [InlineData(true, OffsetSide.Right, 4)] + public void ArcOffset_GrowsAwayFromCenter(bool reversed, OffsetSide side, double radius) + { + var arc = new Arc(0, 0, 5, 0, Angle.HalfPI, reversed); + + var offset = (Arc)arc.OffsetEntity(1, side); + + Assert.Equal(radius, offset.Radius, 9); + Assert.Equal(reversed, offset.IsReversed); + } + + [Theory] + [InlineData(RotationType.CCW, OffsetSide.Left, 4)] + [InlineData(RotationType.CCW, OffsetSide.Right, 6)] + [InlineData(RotationType.CW, OffsetSide.Left, 6)] + [InlineData(RotationType.CW, OffsetSide.Right, 4)] + public void CircleOffset_GrowsAwayFromCenter(RotationType rotation, OffsetSide side, double radius) + { + var circle = new Circle(0, 0, 5) { Rotation = rotation }; + + var offset = (Circle)circle.OffsetEntity(1, side); + + Assert.Equal(radius, offset.Radius, 9); + Assert.Equal(rotation, offset.Rotation); + } + + [Theory] + [InlineData(OffsetSide.Left, 1)] + [InlineData(OffsetSide.Right, -1)] + public void LineOffset_MovesToSideAndKeepsDirection(OffsetSide side, double y) + { + var line = new Line(0, 0, 10, 0); + + var offset = (Line)line.OffsetEntity(1, side); + + Assert.True(offset.StartPoint.DistanceTo(new Vector(0, y)) < 1e-9); + Assert.True(offset.EndPoint.DistanceTo(new Vector(10, y)) < 1e-9); + } + + [Fact] + public void OffsetOutward_NonTangentLineArcCorners_GetRoundJoins() + { + // D shape: right half of an r=5 circle closed by the Y axis. Both corners are + // convex and not tangent, so the offset needs a round join at each. + var shape = new Shape(); + shape.Entities.Add(new Arc(0, 0, 5, -Angle.HalfPI, Angle.HalfPI)); + shape.Entities.Add(new Line(0, 5, 0, -5)); + + var offset = shape.OffsetOutward(1); + + AssertClosedChain(offset.Entities); + Assert.Equal(2, offset.Entities.OfType().Count(a => a.Radius.IsEqualTo(1))); + Assert.All(Samples(offset.Entities), p => Assert.True(DistanceTo(shape, p) > 1 - 1e-6)); + } + + [Fact] + public void OffsetOutward_CollapsedFillet_ClosesTheChain() + { + // 10x4 part with a 0.2-wide slot down from the top, with a round (r=0.1) bottom. + // Offsetting outward by 0.25 collapses the slot's end arc. + var shape = new Shape(); + shape.Entities.Add(new Line(0, 0, 10, 0)); + shape.Entities.Add(new Line(10, 0, 10, 4)); + shape.Entities.Add(new Line(10, 4, 5.1, 4)); + shape.Entities.Add(new Line(5.1, 4, 5.1, 2)); + shape.Entities.Add(new Arc(5, 2, 0.1, 0, System.Math.PI, reversed: true)); + shape.Entities.Add(new Line(4.9, 2, 4.9, 4)); + shape.Entities.Add(new Line(4.9, 4, 0, 4)); + shape.Entities.Add(new Line(0, 4, 0, 0)); + + var offset = shape.OffsetOutward(0.25); + + AssertClosedChain(offset.Entities); + Assert.All( + Samples(offset.Entities), + p => Assert.True(DistanceTo(shape, p) > 0.25 - 1e-6 || IsInsideSlot(p)) + ); + } + + // The collapsed slot leaves a line bridging its walls' offsets, which lies inside + // the offset envelope (closer than the spacing) by design. + private static bool IsInsideSlot(Vector p) => p.X > 4.8 && p.X < 5.2 && p.Y > 1.7; + + private static void AssertClosedChain(List entities) + { + for (var i = 0; i < entities.Count; i++) + { + var end = End(entities[i]); + var start = Start(entities[(i + 1) % entities.Count]); + + Assert.True( + end.DistanceTo(start) < 1e-6, + $"Gap of {end.DistanceTo(start)} after entity {i} ({entities[i].Type})." + ); + } + } + + private static IEnumerable Samples(List entities) + { + foreach (var entity in entities) + { + for (var t = 0.0; t <= 1.0; t += 0.1) + { + yield return entity switch + { + Line l => l.StartPoint + (l.EndPoint - l.StartPoint) * t, + Arc a => ArcPoint(a, t), + _ => Start(entity), + }; + } + } + } + + private static Vector ArcPoint(Arc arc, double t) + { + var sweep = arc.SweepAngle(); + var angle = arc.StartAngle + (arc.IsReversed ? -sweep : sweep) * t; + return new Vector( + arc.Center.X + arc.Radius * System.Math.Cos(angle), + arc.Center.Y + arc.Radius * System.Math.Sin(angle) + ); + } + + private static double DistanceTo(Shape shape, Vector p) => + shape.Entities.Min(e => e.ClosestPointTo(p).DistanceTo(p)); + + private static Vector Start(Entity e) => + e switch + { + Line l => l.StartPoint, + Arc a => a.StartPoint(), + _ => default, + }; + + private static Vector End(Entity e) => + e switch + { + Line l => l.EndPoint, + Arc a => a.EndPoint(), + _ => default, + }; +}