mirror of
https://github.com/ajisaacs/OpenNest.git
synced 2026-10-01 08:34:15 -04:00
refactor(geometry): share signed-angle accumulation through ArcFit
EllipseConverter, SplineConverter and GeometrySimplifier each carried an
identical private SumSignedAngles. Move the unchanged body to
ArcFit.SumSignedAngles and call it from all three, keeping the ordered
accumulation, strict half-turn comparisons and empty/single-point result.
ArcFitTests compares the shared method bit-for-bit with a separate
test-local accumulator across half turns, the atan2 seam, multiple
turns, translated centres and NaN inputs, and checks that inputs are not
mutated. The converter winding characterization from 8664656 still
passes unchanged.
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
using System.Collections.Generic;
|
||||
using OpenNest.Math;
|
||||
|
||||
namespace OpenNest.Geometry
|
||||
{
|
||||
@@ -118,6 +119,27 @@ namespace OpenNest.Geometry
|
||||
return System.Math.Atan2(ux * to.Y - uy * to.X, ux * to.X + uy * to.Y);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Sums signed angular change traversing consecutive points around a center.
|
||||
/// Positive = CCW, negative = CW.
|
||||
/// </summary>
|
||||
public static double SumSignedAngles(Vector center, List<Vector> points)
|
||||
{
|
||||
var total = 0.0;
|
||||
for (var i = 0; i < points.Count - 1; i++)
|
||||
{
|
||||
var a1 = System.Math.Atan2(points[i].Y - center.Y, points[i].X - center.X);
|
||||
var a2 = System.Math.Atan2(points[i + 1].Y - center.Y, points[i + 1].X - center.X);
|
||||
var da = a2 - a1;
|
||||
while (da > System.Math.PI)
|
||||
da -= Angle.TwoPI;
|
||||
while (da < -System.Math.PI)
|
||||
da += Angle.TwoPI;
|
||||
total += da;
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Computes the maximum radial deviation of interior points from a circle.
|
||||
/// </summary>
|
||||
|
||||
@@ -330,7 +330,7 @@ namespace OpenNest.Geometry
|
||||
var endAngle = System.Math.Atan2(p1.Y - arcCenter.Y, p1.X - arcCenter.X);
|
||||
|
||||
var points = new List<Vector> { p0, pMid, p1 };
|
||||
var isReversed = SumSignedAngles(arcCenter, points) < 0;
|
||||
var isReversed = ArcFit.SumSignedAngles(arcCenter, points) < 0;
|
||||
|
||||
if (startAngle < 0)
|
||||
startAngle += Angle.TwoPI;
|
||||
@@ -339,22 +339,5 @@ namespace OpenNest.Geometry
|
||||
|
||||
return new Arc(arcCenter, radius, startAngle, endAngle, isReversed);
|
||||
}
|
||||
|
||||
private static double SumSignedAngles(Vector center, List<Vector> points)
|
||||
{
|
||||
var total = 0.0;
|
||||
for (var i = 0; i < points.Count - 1; i++)
|
||||
{
|
||||
var a1 = System.Math.Atan2(points[i].Y - center.Y, points[i].X - center.X);
|
||||
var a2 = System.Math.Atan2(points[i + 1].Y - center.Y, points[i + 1].X - center.X);
|
||||
var da = a2 - a1;
|
||||
while (da > System.Math.PI)
|
||||
da -= Angle.TwoPI;
|
||||
while (da < -System.Math.PI)
|
||||
da += Angle.TwoPI;
|
||||
total += da;
|
||||
}
|
||||
return total;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -435,7 +435,7 @@ public class GeometrySimplifier
|
||||
// Reject arcs that subtend a tiny angle — these are nearly-straight lines
|
||||
// that happen to fit a huge circle. Applied after extension so that many small
|
||||
// segments can accumulate enough sweep to qualify.
|
||||
var sweep = System.Math.Abs(SumSignedAngles(center, points));
|
||||
var sweep = System.Math.Abs(ArcFit.SumSignedAngles(center, points));
|
||||
if (sweep < Angle.ToRadians(5))
|
||||
return null;
|
||||
|
||||
@@ -454,7 +454,7 @@ public class GeometrySimplifier
|
||||
continue;
|
||||
|
||||
// Check that the arc doesn't bulge away from the original line segments
|
||||
var isReversed = SumSignedAngles(center, points) < 0;
|
||||
var isReversed = ArcFit.SumSignedAngles(center, points) < 0;
|
||||
var arcDev = MaxArcToSegmentDeviation(points, center, radius, isReversed);
|
||||
if (arcDev > Tolerance)
|
||||
continue;
|
||||
@@ -673,7 +673,7 @@ public class GeometrySimplifier
|
||||
var lastPt = points[^1];
|
||||
var rx = lastPt.X - center.X;
|
||||
var ry = lastPt.Y - center.Y;
|
||||
var sign = SumSignedAngles(center, points) >= 0 ? 1 : -1;
|
||||
var sign = ArcFit.SumSignedAngles(center, points) >= 0 ? 1 : -1;
|
||||
return new Vector(-sign * ry, sign * rx);
|
||||
}
|
||||
|
||||
@@ -792,7 +792,7 @@ public class GeometrySimplifier
|
||||
var endAngle = NormalizeAngle(
|
||||
System.Math.Atan2(lastPoint.Y - center.Y, lastPoint.X - center.X)
|
||||
);
|
||||
var isReversed = SumSignedAngles(center, points) < 0;
|
||||
var isReversed = ArcFit.SumSignedAngles(center, points) < 0;
|
||||
|
||||
var arc = new Arc(center, radius, startAngle, endAngle, isReversed);
|
||||
arc.Layer = sourceEntity.Layer;
|
||||
@@ -816,27 +816,6 @@ public class GeometrySimplifier
|
||||
_ => Vector.Invalid,
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Sums signed angular change traversing consecutive points around a center.
|
||||
/// Positive = CCW, negative = CW.
|
||||
/// </summary>
|
||||
private static double SumSignedAngles(Vector center, List<Vector> points)
|
||||
{
|
||||
var total = 0.0;
|
||||
for (var i = 0; i < points.Count - 1; i++)
|
||||
{
|
||||
var a1 = System.Math.Atan2(points[i].Y - center.Y, points[i].X - center.X);
|
||||
var a2 = System.Math.Atan2(points[i + 1].Y - center.Y, points[i + 1].X - center.X);
|
||||
var da = a2 - a1;
|
||||
while (da > System.Math.PI)
|
||||
da -= Angle.TwoPI;
|
||||
while (da < -System.Math.PI)
|
||||
da += Angle.TwoPI;
|
||||
total += da;
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Measures the maximum distance from sampled points along the fitted arc
|
||||
/// back to the original line segments. This catches cases where points lie
|
||||
|
||||
@@ -80,7 +80,7 @@ namespace OpenNest.Geometry
|
||||
}
|
||||
|
||||
var finalPoints = points.GetRange(start, endIdx - start + 1);
|
||||
var sweep = System.Math.Abs(SumSignedAngles(center, finalPoints));
|
||||
var sweep = System.Math.Abs(ArcFit.SumSignedAngles(center, finalPoints));
|
||||
if (sweep < Angle.ToRadians(5))
|
||||
return null;
|
||||
|
||||
@@ -151,27 +151,10 @@ namespace OpenNest.Geometry
|
||||
double radius
|
||||
) => ArcFit.MaxRadialDeviation(points, cx, cy, radius);
|
||||
|
||||
private static double SumSignedAngles(Vector center, List<Vector> points)
|
||||
{
|
||||
var total = 0.0;
|
||||
for (var i = 0; i < points.Count - 1; i++)
|
||||
{
|
||||
var a1 = System.Math.Atan2(points[i].Y - center.Y, points[i].X - center.X);
|
||||
var a2 = System.Math.Atan2(points[i + 1].Y - center.Y, points[i + 1].X - center.X);
|
||||
var da = a2 - a1;
|
||||
while (da > System.Math.PI)
|
||||
da -= Angle.TwoPI;
|
||||
while (da < -System.Math.PI)
|
||||
da += Angle.TwoPI;
|
||||
total += da;
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
private static Vector ComputeEndTangent(Vector center, List<Vector> points)
|
||||
{
|
||||
var lastPt = points[^1];
|
||||
var totalAngle = SumSignedAngles(center, points);
|
||||
var totalAngle = ArcFit.SumSignedAngles(center, points);
|
||||
|
||||
var rx = lastPt.X - center.X;
|
||||
var ry = lastPt.Y - center.Y;
|
||||
@@ -186,7 +169,7 @@ namespace OpenNest.Geometry
|
||||
|
||||
var startAngle = System.Math.Atan2(firstPoint.Y - center.Y, firstPoint.X - center.X);
|
||||
var endAngle = System.Math.Atan2(lastPoint.Y - center.Y, lastPoint.X - center.X);
|
||||
var isReversed = SumSignedAngles(center, points) < 0;
|
||||
var isReversed = ArcFit.SumSignedAngles(center, points) < 0;
|
||||
|
||||
if (startAngle < 0)
|
||||
startAngle += Angle.TwoPI;
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
using OpenNest.Geometry;
|
||||
|
||||
namespace OpenNest.Tests.Geometry;
|
||||
|
||||
public class ArcFitTests
|
||||
{
|
||||
[Theory]
|
||||
[InlineData("empty")]
|
||||
[InlineData("single")]
|
||||
[InlineData("positive-half-turn")]
|
||||
[InlineData("negative-half-turn")]
|
||||
[InlineData("repeated")]
|
||||
[InlineData("translated")]
|
||||
[InlineData("closed-ccw")]
|
||||
[InlineData("closed-cw")]
|
||||
[InlineData("multiple-turns-ccw")]
|
||||
[InlineData("multiple-turns-cw")]
|
||||
[InlineData("irregular-multiple-turns")]
|
||||
[InlineData("two-points")]
|
||||
[InlineData("atan-seam-ccw")]
|
||||
[InlineData("atan-seam-cw")]
|
||||
[InlineData("positive-x-seam")]
|
||||
[InlineData("collinear-through-center")]
|
||||
[InlineData("nan-point-x")]
|
||||
[InlineData("nan-point-y")]
|
||||
[InlineData("nan-center")]
|
||||
[InlineData("nan-empty-center")]
|
||||
[InlineData("nan-single-center")]
|
||||
[InlineData("nan-single-point")]
|
||||
public void SumSignedAngles_MatchesFrozenAccumulatorBitExactlyWithoutMutation(string scenario)
|
||||
{
|
||||
var (center, points) = Corpus(scenario);
|
||||
var centerBefore = Snapshot(center);
|
||||
var pointsBefore = points.Select(Snapshot).ToArray();
|
||||
var expected = FrozenSumSignedAngles(center, points);
|
||||
|
||||
var actual = ArcFit.SumSignedAngles(center, points);
|
||||
|
||||
Assert.Equal(BitConverter.DoubleToInt64Bits(expected), BitConverter.DoubleToInt64Bits(actual));
|
||||
Assert.Equal(centerBefore, Snapshot(center));
|
||||
Assert.Equal(pointsBefore, points.Select(Snapshot).ToArray());
|
||||
|
||||
switch (scenario)
|
||||
{
|
||||
// Strict > / < comparisons must leave exact +PI and -PI unreduced.
|
||||
case "positive-half-turn":
|
||||
Assert.Equal(BitConverter.DoubleToInt64Bits(System.Math.PI), BitConverter.DoubleToInt64Bits(actual));
|
||||
break;
|
||||
case "negative-half-turn":
|
||||
Assert.Equal(BitConverter.DoubleToInt64Bits(-System.Math.PI), BitConverter.DoubleToInt64Bits(actual));
|
||||
break;
|
||||
case "empty":
|
||||
case "single":
|
||||
case "repeated":
|
||||
case "collinear-through-center":
|
||||
case "nan-empty-center":
|
||||
case "nan-single-center":
|
||||
case "nan-single-point":
|
||||
Assert.Equal(BitConverter.DoubleToInt64Bits(0.0), BitConverter.DoubleToInt64Bits(actual));
|
||||
break;
|
||||
case "multiple-turns-ccw":
|
||||
case "irregular-multiple-turns":
|
||||
Assert.True(actual > 2 * System.Math.PI);
|
||||
break;
|
||||
case "multiple-turns-cw":
|
||||
Assert.True(actual < -2 * System.Math.PI);
|
||||
break;
|
||||
case "nan-point-x":
|
||||
case "nan-point-y":
|
||||
case "nan-center":
|
||||
// Characterized before extraction: a visited NaN propagates into the
|
||||
// total; neither normalization loop runs because comparisons are false.
|
||||
// Empty/single lists never evaluate Atan2, even with a NaN center/point.
|
||||
Assert.True(double.IsNaN(actual));
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
private static (long x, long y) Snapshot(Vector point) =>
|
||||
(BitConverter.DoubleToInt64Bits(point.X), BitConverter.DoubleToInt64Bits(point.Y));
|
||||
|
||||
private static (Vector center, List<Vector> points) Corpus(string scenario)
|
||||
{
|
||||
var center = new Vector(0, 0);
|
||||
var points = scenario switch
|
||||
{
|
||||
"empty" => new List<Vector>(),
|
||||
"single" => new List<Vector> { new Vector(2, -3) },
|
||||
"positive-half-turn" => new List<Vector> { new Vector(1, 0), new Vector(-1, 0) },
|
||||
"negative-half-turn" => new List<Vector> { new Vector(-1, 0), new Vector(1, 0) },
|
||||
"repeated" => new List<Vector> { new Vector(1, 2), new Vector(1, 2), new Vector(1, 2) },
|
||||
"translated" => new List<Vector>
|
||||
{
|
||||
new Vector(9, -11), new Vector(7, -9), new Vector(5, -11),
|
||||
new Vector(7, -13), new Vector(9, -11),
|
||||
},
|
||||
"closed-ccw" or "closed-cw" => CardinalLoop(1),
|
||||
"multiple-turns-ccw" or "multiple-turns-cw" => CardinalLoop(3),
|
||||
"irregular-multiple-turns" => IrregularTurns(),
|
||||
"two-points" => new List<Vector> { new Vector(1, 1), new Vector(-1, 1) },
|
||||
"atan-seam-ccw" => new List<Vector> { new Vector(-1, 0.001), new Vector(-1, -0.001) },
|
||||
"atan-seam-cw" => new List<Vector> { new Vector(-1, -0.001), new Vector(-1, 0.001) },
|
||||
"positive-x-seam" => new List<Vector> { new Vector(1, -0.001), new Vector(1, 0.001) },
|
||||
"collinear-through-center" => new List<Vector>
|
||||
{
|
||||
new Vector(1, 0), new Vector(0, 0), new Vector(-1, 0),
|
||||
new Vector(0, 0), new Vector(1, 0),
|
||||
},
|
||||
"nan-point-x" => new List<Vector>
|
||||
{
|
||||
new Vector(1, 0), new Vector(double.NaN, 1), new Vector(0, 1),
|
||||
},
|
||||
"nan-point-y" => new List<Vector>
|
||||
{
|
||||
new Vector(1, 0), new Vector(1, double.NaN), new Vector(0, 1),
|
||||
},
|
||||
"nan-center" => new List<Vector> { new Vector(1, 0), new Vector(0, 1) },
|
||||
"nan-empty-center" => new List<Vector>(),
|
||||
"nan-single-center" => new List<Vector> { new Vector(1, 0) },
|
||||
"nan-single-point" => new List<Vector> { new Vector(double.NaN, double.NaN) },
|
||||
_ => throw new ArgumentOutOfRangeException(nameof(scenario)),
|
||||
};
|
||||
if (scenario == "translated")
|
||||
center = new Vector(7, -11);
|
||||
else if (scenario is "nan-center" or "nan-empty-center" or "nan-single-center")
|
||||
center = new Vector(double.NaN, 0);
|
||||
if (scenario is "closed-cw" or "multiple-turns-cw")
|
||||
points.Reverse();
|
||||
return (center, points);
|
||||
}
|
||||
|
||||
private static List<Vector> CardinalLoop(int turns)
|
||||
{
|
||||
var cycle = new[] { new Vector(1, 0), new Vector(0, 1), new Vector(-1, 0), new Vector(0, -1) };
|
||||
var points = new List<Vector>();
|
||||
for (var i = 0; i <= turns * 4; i++)
|
||||
points.Add(cycle[i % 4]);
|
||||
return points;
|
||||
}
|
||||
|
||||
private static List<Vector> IrregularTurns()
|
||||
{
|
||||
var steps = new[] { 0.13, 0.81, -0.27, 1.01, 0.34, 0.0 };
|
||||
var points = new List<Vector>();
|
||||
var angle = 2.9;
|
||||
for (var i = 0; i < 80; i++)
|
||||
{
|
||||
points.Add(new Vector(System.Math.Cos(angle), System.Math.Sin(angle)));
|
||||
angle += steps[i % steps.Length];
|
||||
}
|
||||
return points;
|
||||
}
|
||||
|
||||
// Independent frozen test oracle, written before the production extraction.
|
||||
// Do not route this through ArcFit or any other production angle helper.
|
||||
private static double FrozenSumSignedAngles(Vector center, List<Vector> points)
|
||||
{
|
||||
const double fullTurn = 2.0 * System.Math.PI;
|
||||
var sum = 0.0;
|
||||
for (var index = 1; index < points.Count; index++)
|
||||
{
|
||||
var previous = points[index - 1];
|
||||
var current = points[index];
|
||||
var previousAngle = System.Math.Atan2(previous.Y - center.Y, previous.X - center.X);
|
||||
var currentAngle = System.Math.Atan2(current.Y - center.Y, current.X - center.X);
|
||||
var change = currentAngle - previousAngle;
|
||||
while (change > System.Math.PI)
|
||||
change -= fullTurn;
|
||||
while (change < -System.Math.PI)
|
||||
change += fullTurn;
|
||||
sum += change;
|
||||
}
|
||||
return sum;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user