From 92461deb98d6374f9c2c8f301c6c961ec7b12ac4 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Thu, 9 Apr 2026 17:40:05 -0400 Subject: [PATCH] fix: apply SubProgramCall offset additively and restore curpos after expansion ConvertMode.ToIncremental skips SubProgramCalls when computing deltas, so all code paths that expand SubProgramCalls must: (1) set curpos to savedPos + Offset before expanding, and (2) restore curpos afterward so subsequent incremental codes get correct deltas. Fixed in ConvertProgram, GraphicsHelper (AddProgram, AddProgramSplit), PlateRenderer (DrawRapids, DrawProgramPiercePoints, GetFirstPiercePoint), and CutDirectionArrows. Co-Authored-By: Claude Opus 4.6 (1M context) --- OpenNest.Core/Converters/ConvertProgram.cs | 13 +- .../CuttingStrategy/HoleSubProgramTests.cs | 122 ++++++++++++++++++ OpenNest/Controls/CutDirectionArrows.cs | 5 + OpenNest/Controls/PlateRenderer.cs | 13 ++ OpenNest/GraphicsHelper.cs | 6 + 5 files changed, 156 insertions(+), 3 deletions(-) diff --git a/OpenNest.Core/Converters/ConvertProgram.cs b/OpenNest.Core/Converters/ConvertProgram.cs index be261a3..7553390 100644 --- a/OpenNest.Core/Converters/ConvertProgram.cs +++ b/OpenNest.Core/Converters/ConvertProgram.cs @@ -43,13 +43,20 @@ namespace OpenNest.Converters case CodeType.SubProgramCall: var subpgm = (SubProgramCall)code; var savedMode = mode; + var savedPos = curpos; - // Apply offset: sub-program executes at the call's offset position - if (subpgm.Offset.X != 0 || subpgm.Offset.Y != 0) - curpos = subpgm.Offset; + // Position the sub-program at savedPos + Offset. + // savedPos is the base position ((0,0) here, Part.Location in rendering). + // Offset is the hole center in drawing-local coordinates. + curpos = new Vector(savedPos.X + subpgm.Offset.X, savedPos.Y + subpgm.Offset.Y); AddProgram(subpgm.Program, ref mode, ref curpos, ref geometry); mode = savedMode; + + // Restore curpos: ConvertMode.ToIncremental skips SubProgramCalls + // when computing deltas, so subsequent incremental codes expect + // curpos to be where it was before the call. + curpos = savedPos; break; } } diff --git a/OpenNest.Tests/CuttingStrategy/HoleSubProgramTests.cs b/OpenNest.Tests/CuttingStrategy/HoleSubProgramTests.cs index 16203c8..6ade4cd 100644 --- a/OpenNest.Tests/CuttingStrategy/HoleSubProgramTests.cs +++ b/OpenNest.Tests/CuttingStrategy/HoleSubProgramTests.cs @@ -1,5 +1,6 @@ using OpenNest.CNC; using OpenNest.CNC.CuttingStrategy; +using OpenNest.Converters; using OpenNest.Geometry; using System.Linq; @@ -159,6 +160,127 @@ public class HoleSubProgramTests Assert.NotEqual(calls[0].Offset.X, calls[1].Offset.X); } + [Fact] + public void Apply_HoleCenters_PreservedInGeometry() + { + // Square perimeter 10x10 with two circle holes at known positions + var holeCenter1 = new Vector(3, 3); + var holeCenter2 = new Vector(7, 5); + var holeRadius = 0.5; + + var pgm = new Program(Mode.Absolute); + // Perimeter + pgm.Codes.Add(new RapidMove(0, 0)); + pgm.Codes.Add(new LinearMove(10, 0)); + pgm.Codes.Add(new LinearMove(10, 10)); + pgm.Codes.Add(new LinearMove(0, 10)); + pgm.Codes.Add(new LinearMove(0, 0)); + // Hole 1 at (3, 3) + pgm.Codes.Add(new RapidMove(holeCenter1.X + holeRadius, holeCenter1.Y)); + pgm.Codes.Add(new ArcMove( + new Vector(holeCenter1.X + holeRadius, holeCenter1.Y), + holeCenter1, RotationType.CW)); + // Hole 2 at (7, 5) + pgm.Codes.Add(new RapidMove(holeCenter2.X + holeRadius, holeCenter2.Y)); + pgm.Codes.Add(new ArcMove( + new Vector(holeCenter2.X + holeRadius, holeCenter2.Y), + holeCenter2, RotationType.CW)); + + var strategy = new ContourCuttingStrategy + { + Parameters = new CuttingParameters + { + ArcCircleLeadIn = new LineLeadIn { Length = 0.125, ApproachAngle = 90 }, + ArcCircleLeadOut = new NoLeadOut() + } + }; + + var result = strategy.Apply(pgm, new Vector(10, 10)); + + // Convert to geometry — this is what PlateView renders + var geometry = ConvertProgram.ToGeometry(result.Program); + var circles = geometry.OfType().ToList(); + + Assert.Equal(2, circles.Count); + + // Circle centers must match the original hole positions + var center1 = circles[0].Center; + var center2 = circles[1].Center; + + Assert.Equal(holeCenter1.X, center1.X, 2); + Assert.Equal(holeCenter1.Y, center1.Y, 2); + Assert.Equal(holeCenter2.X, center2.X, 2); + Assert.Equal(holeCenter2.Y, center2.Y, 2); + } + + [Fact] + public void Part_ApplyLeadIns_HolesAndPerimeter_CorrectPositions() + { + // Build a drawing with a square and two holes + var holeCenter1 = new Vector(3, 3); + var holeCenter2 = new Vector(7, 5); + var holeRadius = 0.5; + + var pgm = new Program(Mode.Absolute); + pgm.Codes.Add(new RapidMove(0, 0)); + pgm.Codes.Add(new LinearMove(10, 0)); + pgm.Codes.Add(new LinearMove(10, 10)); + pgm.Codes.Add(new LinearMove(0, 10)); + pgm.Codes.Add(new LinearMove(0, 0)); + pgm.Codes.Add(new RapidMove(holeCenter1.X + holeRadius, holeCenter1.Y)); + pgm.Codes.Add(new ArcMove( + new Vector(holeCenter1.X + holeRadius, holeCenter1.Y), + holeCenter1, RotationType.CW)); + pgm.Codes.Add(new RapidMove(holeCenter2.X + holeRadius, holeCenter2.Y)); + pgm.Codes.Add(new ArcMove( + new Vector(holeCenter2.X + holeRadius, holeCenter2.Y), + holeCenter2, RotationType.CW)); + + var drawing = new Drawing("TestPart") { Program = pgm }; + var part = new Part(drawing); + + var parameters = new CuttingParameters + { + RoundLeadInAngles = true, + LeadInAngleIncrement = 5.0, + ArcCircleLeadIn = new LineLeadIn { Length = 0.125, ApproachAngle = 90 }, + ArcCircleLeadOut = new NoLeadOut(), + ExternalLeadIn = new LineLeadIn { Length = 0.25, ApproachAngle = 90 }, + ExternalLeadOut = new NoLeadOut() + }; + + part.ApplyLeadIns(parameters, new Vector(10, 10)); + + // Convert to geometry — this is what PlateView renders + var geometry = ConvertProgram.ToGeometry(part.Program); + var circles = geometry.OfType().ToList(); + var lines = geometry.OfType().Where(l => l.Layer != SpecialLayers.Rapid).ToList(); + + // Hole circles must be at correct positions + Assert.Equal(2, circles.Count); + Assert.Equal(holeCenter1.X, circles[0].Center.X, 2); + Assert.Equal(holeCenter1.Y, circles[0].Center.Y, 2); + Assert.Equal(holeCenter2.X, circles[1].Center.X, 2); + Assert.Equal(holeCenter2.Y, circles[1].Center.Y, 2); + Assert.Equal(holeRadius, circles[0].Radius, 2); + Assert.Equal(holeRadius, circles[1].Radius, 2); + + // Perimeter lines must stay within the original 10x10 bounding box. + // This catches the mode conversion bug where perimeter gets shifted + // by the last hole's position. + foreach (var line in lines) + { + Assert.True(line.StartPoint.X >= -1 && line.StartPoint.X <= 11, + $"Perimeter line start X={line.StartPoint.X} is outside the 10x10 part bounds"); + Assert.True(line.StartPoint.Y >= -1 && line.StartPoint.Y <= 11, + $"Perimeter line start Y={line.StartPoint.Y} is outside the 10x10 part bounds"); + Assert.True(line.EndPoint.X >= -1 && line.EndPoint.X <= 11, + $"Perimeter line end X={line.EndPoint.X} is outside the 10x10 part bounds"); + Assert.True(line.EndPoint.Y >= -1 && line.EndPoint.Y <= 11, + $"Perimeter line end Y={line.EndPoint.Y} is outside the 10x10 part bounds"); + } + } + [Fact] public void Program_BoundingBox_IncludesSubProgramOffset() { diff --git a/OpenNest/Controls/CutDirectionArrows.cs b/OpenNest/Controls/CutDirectionArrows.cs index 5cc74f1..b5393e0 100644 --- a/OpenNest/Controls/CutDirectionArrows.cs +++ b/OpenNest/Controls/CutDirectionArrows.cs @@ -18,7 +18,12 @@ namespace OpenNest.Controls { var subpgm = (SubProgramCall)code; if (subpgm.Program != null) + { + var savedPos = pos; + pos = new Vector(savedPos.X + subpgm.Offset.X, savedPos.Y + subpgm.Offset.Y); DrawProgram(g, view, subpgm.Program, ref pos, pen, spacing, arrowSize); + pos = savedPos; + } continue; } diff --git a/OpenNest/Controls/PlateRenderer.cs b/OpenNest/Controls/PlateRenderer.cs index 14f67f9..d4b5a83 100644 --- a/OpenNest/Controls/PlateRenderer.cs +++ b/OpenNest/Controls/PlateRenderer.cs @@ -404,6 +404,9 @@ namespace OpenNest.Controls { for (var i = 0; i < pgm.Length; i++) { + if (pgm[i] is SubProgramCall call && call.Program != null) + return GetFirstPiercePoint(call.Program, partLocation + call.Offset); + if (pgm[i] is Motion motion) { if (pgm.Mode == Mode.Incremental) @@ -428,7 +431,12 @@ namespace OpenNest.Controls var program = subpgm.Program; if (program != null) + { + var savedPos = pos; + pos = new Vector(savedPos.X + subpgm.Offset.X, savedPos.Y + subpgm.Offset.Y); DrawRapids(g, program, ref pos); + pos = savedPos; + } } else { @@ -489,7 +497,12 @@ namespace OpenNest.Controls { var subpgm = (SubProgramCall)code; if (subpgm.Program != null) + { + var savedPos = pos; + pos = new Vector(savedPos.X + subpgm.Offset.X, savedPos.Y + subpgm.Offset.Y); DrawProgramPiercePoints(g, subpgm.Program, ref pos, brush, pen); + pos = savedPos; + } } else { diff --git a/OpenNest/GraphicsHelper.cs b/OpenNest/GraphicsHelper.cs index 0d499d9..740ed69 100644 --- a/OpenNest/GraphicsHelper.cs +++ b/OpenNest/GraphicsHelper.cs @@ -147,7 +147,10 @@ namespace OpenNest { cutPath.StartFigure(); leadPath.StartFigure(); + var savedPos = curpos; + curpos = new Vector(savedPos.X + subpgm.Offset.X, savedPos.Y + subpgm.Offset.Y); AddProgramSplit(cutPath, leadPath, subpgm.Program, mode, ref curpos); + curpos = savedPos; } mode = tmpmode; break; @@ -305,7 +308,10 @@ namespace OpenNest if (subpgm.Program != null) { + var savedPos = curpos; + curpos = new Vector(savedPos.X + subpgm.Offset.X, savedPos.Y + subpgm.Offset.Y); AddProgram(path, subpgm.Program, mode, ref curpos); + curpos = savedPos; } mode = tmpmode;