refactor: simplify CutOff methods and fix ActionCutOff issues

- Extract MakePoint/AxisBounds/CrossAxisBounds helpers in CutOff to
  eliminate repeated axis-dependent branching
- Simplify BuildProgram loop from 4 code paths to 2
- Use static EmptyExclusions to avoid per-part list allocations
- Fix double event subscription in ActionCutOff constructor
- Dispose debounce timer in DisconnectEvents
- Remove redundant BuildPerimeterCache call in OnMouseDown
- Extract TotalCutLength test helper, remove duplicate test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-03-22 22:51:52 -04:00
parent 17fc9c6cab
commit 1c561d880e
3 changed files with 53 additions and 139 deletions

View File

@@ -104,29 +104,17 @@ namespace OpenNest
return segments;
}
private static readonly List<(double Start, double End)> EmptyExclusions = new();
private List<(double Start, double End)> GetPartExclusions(
Part part, Entity perimeter, double cutPosition, double lineStart, double lineEnd, double clearance)
{
var bb = part.BoundingBox;
double partMin, partMax, partStart, partEnd;
if (Axis == CutOffAxis.Vertical)
{
partMin = bb.X - clearance;
partMax = bb.X + bb.Width + clearance;
partStart = bb.Y - clearance;
partEnd = bb.Y + bb.Length + clearance;
}
else
{
partMin = bb.Y - clearance;
partMax = bb.Y + bb.Length + clearance;
partStart = bb.X - clearance;
partEnd = bb.X + bb.Width + clearance;
}
var (partMin, partMax) = AxisBounds(bb, clearance);
var (partStart, partEnd) = CrossAxisBounds(bb, clearance);
if (cutPosition < partMin || cutPosition > partMax)
return new List<(double Start, double End)>();
return EmptyExclusions;
if (perimeter != null)
{
@@ -141,19 +129,7 @@ namespace OpenNest
private List<(double Start, double End)> IntersectPerimeter(
Entity perimeter, double cutPosition, double lineStart, double lineEnd, double clearance)
{
Vector p1, p2;
if (Axis == CutOffAxis.Vertical)
{
p1 = new Vector(cutPosition, lineStart);
p2 = new Vector(cutPosition, lineEnd);
}
else
{
p1 = new Vector(lineStart, cutPosition);
p2 = new Vector(lineEnd, cutPosition);
}
var cutLine = new Line(p1, p2);
var cutLine = new Line(MakePoint(cutPosition, lineStart), MakePoint(cutPosition, lineEnd));
if (!perimeter.Intersects(cutLine, out var pts) || pts.Count < 2)
return null;
@@ -173,6 +149,21 @@ namespace OpenNest
return result;
}
private Vector MakePoint(double cutCoord, double lineCoord) =>
Axis == CutOffAxis.Vertical
? new Vector(cutCoord, lineCoord)
: new Vector(lineCoord, cutCoord);
private (double Min, double Max) AxisBounds(Box bb, double clearance) =>
Axis == CutOffAxis.Vertical
? (bb.X - clearance, bb.X + bb.Width + clearance)
: (bb.Y - clearance, bb.Y + bb.Length + clearance);
private (double Start, double End) CrossAxisBounds(Box bb, double clearance) =>
Axis == CutOffAxis.Vertical
? (bb.Y - clearance, bb.Y + bb.Length + clearance)
: (bb.X - clearance, bb.X + bb.Width + clearance);
private Program BuildProgram(List<(double Start, double End)> segments, CutOffSettings settings)
{
var program = new Program();
@@ -180,39 +171,18 @@ namespace OpenNest
if (segments.Count == 0)
return program;
if (settings.CutDirection == CutDirection.TowardOrigin)
segments = segments.OrderByDescending(s => s.Start).ToList();
else
segments = segments.OrderBy(s => s.Start).ToList();
var toward = settings.CutDirection == CutDirection.TowardOrigin;
segments = toward
? segments.OrderByDescending(s => s.Start).ToList()
: segments.OrderBy(s => s.Start).ToList();
var cutPos = Axis == CutOffAxis.Vertical ? Position.X : Position.Y;
foreach (var seg in segments)
{
double startVal, endVal;
if (settings.CutDirection == CutDirection.TowardOrigin)
{
startVal = seg.End;
endVal = seg.Start;
}
else
{
startVal = seg.Start;
endVal = seg.End;
}
Vector startPt, endPt;
if (Axis == CutOffAxis.Vertical)
{
startPt = new Vector(Position.X, startVal);
endPt = new Vector(Position.X, endVal);
}
else
{
startPt = new Vector(startVal, Position.Y);
endPt = new Vector(endVal, Position.Y);
}
program.Codes.Add(new RapidMove(startPt));
program.Codes.Add(new LinearMove(endPt));
var (from, to) = toward ? (seg.End, seg.Start) : (seg.Start, seg.End);
program.Codes.Add(new RapidMove(MakePoint(cutPos, from)));
program.Codes.Add(new LinearMove(MakePoint(cutPos, to)));
}
return program;

View File

@@ -9,6 +9,22 @@ public class CutOffGeometryTests
{
private static readonly CutOffSettings ZeroClearance = new() { PartClearance = 0.0 };
private static double TotalCutLength(Program program, CutOffAxis axis = CutOffAxis.Vertical)
{
var total = 0.0;
for (var i = 0; i < program.Codes.Count - 1; i += 2)
{
if (program.Codes[i] is RapidMove rapid &&
program.Codes[i + 1] is LinearMove linear)
{
total += axis == CutOffAxis.Vertical
? System.Math.Abs(rapid.EndPoint.Y - linear.EndPoint.Y)
: System.Math.Abs(rapid.EndPoint.X - linear.EndPoint.X);
}
}
return total;
}
private static Program MakeSquare(double size)
{
var pgm = new Program();
@@ -95,20 +111,8 @@ public class CutOffGeometryTests
// The circle chord at X=2 from center (10,0) is much shorter than 20.
// With geometry, we get a tighter exclusion, so the segments should
// cover more of the plate than with BB.
var segments = cutoff.Drawing.Program.Codes.OfType<LinearMove>().ToList();
Assert.True(segments.Count >= 1);
// Total cut length should be greater than 80 (BB would give 100-20=80)
var totalCutLength = 0.0;
for (var i = 0; i < cutoff.Drawing.Program.Codes.Count - 1; i += 2)
{
if (cutoff.Drawing.Program.Codes[i] is RapidMove rapid &&
cutoff.Drawing.Program.Codes[i + 1] is LinearMove linear)
{
totalCutLength += System.Math.Abs(rapid.EndPoint.Y - linear.EndPoint.Y);
}
}
var totalCutLength = TotalCutLength(cutoff.Drawing.Program);
Assert.True(totalCutLength > 80, $"Geometry should give more cut length than BB. Got {totalCutLength:F2}");
}
@@ -129,18 +133,9 @@ public class CutOffGeometryTests
var cutoff = new CutOff(new Vector(5, 0), CutOffAxis.Vertical);
cutoff.Regenerate(plate, ZeroClearance, cache);
var totalCutLength = 0.0;
for (var i = 0; i < cutoff.Drawing.Program.Codes.Count - 1; i += 2)
{
if (cutoff.Drawing.Program.Codes[i] is RapidMove rapid &&
cutoff.Drawing.Program.Codes[i + 1] is LinearMove linear)
{
totalCutLength += System.Math.Abs(rapid.EndPoint.Y - linear.EndPoint.Y);
}
}
// BB would exclude full 20 → cut length = 80.
// Geometry excludes only 10 → cut length = 90.
var totalCutLength = TotalCutLength(cutoff.Drawing.Program);
Assert.True(totalCutLength > 85, $"Diamond geometry should give more cut than BB. Got {totalCutLength:F2}");
}
@@ -161,18 +156,9 @@ public class CutOffGeometryTests
var cutoff = new CutOff(new Vector(20, 0), CutOffAxis.Vertical);
cutoff.Regenerate(plate, ZeroClearance, cache);
var totalCutLength = 0.0;
for (var i = 0; i < cutoff.Drawing.Program.Codes.Count - 1; i += 2)
{
if (cutoff.Drawing.Program.Codes[i] is RapidMove rapid &&
cutoff.Drawing.Program.Codes[i + 1] is LinearMove linear)
{
totalCutLength += System.Math.Abs(rapid.EndPoint.Y - linear.EndPoint.Y);
}
}
// BB would exclude [10,40] = 30 → cut = 70.
// Geometry excludes [10,30] = 20 → cut = 80.
var totalCutLength = TotalCutLength(cutoff.Drawing.Program);
Assert.True(totalCutLength > 75, $"Triangle geometry should give more cut than BB. Got {totalCutLength:F2}");
}
@@ -208,18 +194,9 @@ public class CutOffGeometryTests
var cutoff = new CutOff(new Vector(0, 2), CutOffAxis.Horizontal);
cutoff.Regenerate(plate, ZeroClearance, cache);
var totalCutLength = 0.0;
for (var i = 0; i < cutoff.Drawing.Program.Codes.Count - 1; i += 2)
{
if (cutoff.Drawing.Program.Codes[i] is RapidMove rapid &&
cutoff.Drawing.Program.Codes[i + 1] is LinearMove linear)
{
totalCutLength += System.Math.Abs(rapid.EndPoint.X - linear.EndPoint.X);
}
}
// BB would exclude X=[0,20] → cut = 80.
// Circle chord at Y=2 is much shorter → cut > 80.
var totalCutLength = TotalCutLength(cutoff.Drawing.Program, CutOffAxis.Horizontal);
Assert.True(totalCutLength > 80, $"Circle horizontal cut should use geometry. Got {totalCutLength:F2}");
}
@@ -338,25 +315,6 @@ public class CutOffGeometryTests
Assert.Single(cache);
}
[Fact]
public void PlatePerimeterCache_OpenContourUsesConvexHull()
{
var pgm = new Program();
pgm.Codes.Add(new RapidMove(new Vector(0, 0)));
pgm.Codes.Add(new LinearMove(new Vector(10, 0)));
pgm.Codes.Add(new LinearMove(new Vector(10, 10)));
var plate = new Plate(100, 100);
plate.Parts.Add(new Part(new Drawing("open", pgm)));
var cache = Plate.BuildPerimeterCache(plate);
Assert.Single(cache);
var part = plate.Parts[0];
Assert.True(cache.ContainsKey(part));
Assert.NotNull(cache[part]);
}
[Fact]
public void RegenerateCutOffs_UsesGeometryExclusions()
{
@@ -373,17 +331,8 @@ public class CutOffGeometryTests
// Find the materialized cut-off part
var cutPart = plate.Parts.First(p => p.BaseDrawing.IsCutOff);
var totalCutLength = 0.0;
for (var i = 0; i < cutPart.BaseDrawing.Program.Codes.Count - 1; i += 2)
{
if (cutPart.BaseDrawing.Program.Codes[i] is RapidMove rapid &&
cutPart.BaseDrawing.Program.Codes[i + 1] is LinearMove linear)
{
totalCutLength += System.Math.Abs(rapid.EndPoint.Y - linear.EndPoint.Y);
}
}
// BB would give 80 (100 - 20). Geometry should give more.
var totalCutLength = TotalCutLength(cutPart.BaseDrawing.Program);
Assert.True(totalCutLength > 80, $"RegenerateCutOffs should use geometry. Got {totalCutLength:F2}");
}

View File

@@ -23,14 +23,9 @@ namespace OpenNest.Actions
: base(plateView)
{
settings = plateView.CutOffSettings;
perimeterCache = Plate.BuildPerimeterCache(plateView.Plate);
debounceTimer = new Timer { Interval = 16 };
debounceTimer.Tick += OnDebounce;
plateView.MouseMove += OnMouseMove;
plateView.MouseDown += OnMouseDown;
plateView.KeyDown += OnKeyDown;
plateView.Paint += OnPaint;
ConnectEvents();
}
public override void ConnectEvents()
@@ -46,6 +41,7 @@ namespace OpenNest.Actions
public override void DisconnectEvents()
{
debounceTimer.Stop();
debounceTimer.Dispose();
plateView.MouseMove -= OnMouseMove;
plateView.MouseDown -= OnMouseDown;
plateView.KeyDown -= OnKeyDown;
@@ -90,7 +86,6 @@ namespace OpenNest.Actions
plateView.Plate.CutOffs.Add(cutoff);
plateView.Plate.RegenerateCutOffs(settings);
perimeterCache = Plate.BuildPerimeterCache(plateView.Plate);
plateView.Invalidate();
}