From a88673504060fe981819499b5d9eac8edad319fa Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Mon, 28 Sep 2026 18:14:19 -0400 Subject: [PATCH] fix(cnc): keep hole sub-programs private to each program copy Program.Clone deep-copied the SubPrograms dictionary but left every SubProgramCall pointing at the source's sub-program, and SubProgramCall.Clone went through the Rotation setter, which re-rotated that shared program to the call's stale angle. Copying a program with hole lead-ins therefore rotated the source's holes, and rotating the copy rotated the source again. Program.Rotate also rotated a shared sub-program once per call, so two identical holes (one deduplicated sub-program) turned twice. Clone now binds calls to one private copy per shared sub-program without re-aligning it, and Rotate turns each distinct sub-program once. --- OpenNest.Core/CNC/Program.cs | 30 ++++- OpenNest.Core/CNC/SubProgramCall.cs | 20 +++- .../CNC/ProgramSubProgramTransformTests.cs | 111 ++++++++++++++++++ 3 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 OpenNest.Tests/CNC/ProgramSubProgramTransformTests.cs diff --git a/OpenNest.Core/CNC/Program.cs b/OpenNest.Core/CNC/Program.cs index 078a025..4fd490b 100644 --- a/OpenNest.Core/CNC/Program.cs +++ b/OpenNest.Core/CNC/Program.cs @@ -90,6 +90,9 @@ namespace OpenNest.CNC SetModeAbs(); + // Several calls can share one sub-program (identical holes); rotate each once. + var rotatedSubPrograms = new HashSet(ReferenceEqualityComparer.Instance); + for (int i = 0; i < Codes.Count; ++i) { var code = Codes[i]; @@ -110,7 +113,7 @@ namespace OpenNest.CNC ); } - if (subpgm.Program != null) + if (subpgm.Program != null && rotatedSubPrograms.Add(subpgm.Program)) subpgm.Program.Rotate(angle, origin); } @@ -519,8 +522,31 @@ namespace OpenNest.CNC foreach (var kvp in Variables) pgm.Variables[kvp.Key] = kvp.Value; + // The copy owns its sub-programs: rotating it must never turn the source's holes. + // Calls that shared one sub-program keep sharing one copy. + Dictionary subCopies = null; + + Program CopyOf(Program sub) + { + subCopies ??= new Dictionary(ReferenceEqualityComparer.Instance); + + if (!subCopies.TryGetValue(sub, out var copy)) + { + copy = (Program)sub.Clone(); + subCopies[sub] = copy; + } + + return copy; + } + foreach (var kvp in SubPrograms) - pgm.SubPrograms[kvp.Key] = (Program)kvp.Value.Clone(); + pgm.SubPrograms[kvp.Key] = CopyOf(kvp.Value); + + foreach (var code in codes) + { + if (code is SubProgramCall call && call.Program != null) + call.BindProgram(CopyOf(call.Program)); + } return pgm; } diff --git a/OpenNest.Core/CNC/SubProgramCall.cs b/OpenNest.Core/CNC/SubProgramCall.cs index fe3eb2c..fe1c425 100644 --- a/OpenNest.Core/CNC/SubProgramCall.cs +++ b/OpenNest.Core/CNC/SubProgramCall.cs @@ -79,12 +79,28 @@ namespace OpenNest.CNC } /// - /// Gets a shallow copy. + /// Gets a shallow copy that references the same program. Copies the fields + /// directly: going through the setters would re-align (rotate) the shared program. /// /// public ICode Clone() { - return new SubProgramCall(program, Rotation) { Id = Id, Offset = Offset }; + return new SubProgramCall + { + program = program, + rotation = rotation, + Id = Id, + Offset = Offset, + }; + } + + /// + /// Points the call at , a copy of its current program, without + /// re-aligning its rotation: the copy already has the geometry the call executes. + /// + internal void BindProgram(Program copy) + { + program = copy; } public override string ToString() diff --git a/OpenNest.Tests/CNC/ProgramSubProgramTransformTests.cs b/OpenNest.Tests/CNC/ProgramSubProgramTransformTests.cs new file mode 100644 index 0000000..37c4d62 --- /dev/null +++ b/OpenNest.Tests/CNC/ProgramSubProgramTransformTests.cs @@ -0,0 +1,111 @@ +using OpenNest.CNC; +using OpenNest.Converters; +using OpenNest.Geometry; + +namespace OpenNest.Tests.CNC; + +/// +/// Hole sub-programs are shared by reference: identical holes call one sub-program, +/// and program copies must not reach back into the source's sub-programs. +/// +public class ProgramSubProgramTransformTests +{ + private const double QuarterTurn = System.Math.PI / 2; + + /// Incremental hole sub-program: a lead-in from the hole centre to + /// (1, 0), then a full radius-1 circle. + private static Program MakeHoleSub() + { + var sub = new Program(Mode.Absolute); + sub.Codes.Add(new LinearMove(new Vector(1, 0)) { Layer = LayerType.Leadin }); + sub.Codes.Add(new ArcMove(new Vector(1, 0), new Vector(0, 0), RotationType.CW)); + sub.Mode = Mode.Incremental; + return sub; + } + + /// Program with two identical holes, at (5, 5) and (15, 5), calling one + /// shared sub-program, as ContourCuttingStrategy emits them. + private static Program MakeTwoHoleProgram() + { + var sub = MakeHoleSub(); + var pgm = new Program(Mode.Absolute); + pgm.SubPrograms[7] = sub; + pgm.Codes.Add(new SubProgramCall { Id = 7, Program = sub, Offset = new Vector(5, 5) }); + pgm.Codes.Add(new SubProgramCall { Id = 7, Program = sub, Offset = new Vector(15, 5) }); + return pgm; + } + + /// End points of the lead-in lines in the program's own frame. + private static List LeadInEnds(Program pgm) => + ConvertProgram + .ToGeometry(pgm) + .OfType() + .Where(l => l.Layer == SpecialLayers.Leadin) + .Select(l => l.EndPoint) + .ToList(); + + private static void AssertPoint(double x, double y, Vector actual) + { + Assert.Equal(x, actual.X, 6); + Assert.Equal(y, actual.Y, 6); + } + + [Fact] + public void Rotate_SharedSubProgram_TurnsEachHoleOnce() + { + var pgm = MakeTwoHoleProgram(); + + pgm.Rotate(QuarterTurn); + + // (5, 5) -> (-5, 5) and (15, 5) -> (-5, 15); each lead-in ends one unit along + // the rotated +X axis, i.e. at +Y from its centre. + var ends = LeadInEnds(pgm); + Assert.Equal(2, ends.Count); + AssertPoint(-5, 6, ends[0]); + AssertPoint(-5, 16, ends[1]); + } + + [Fact] + public void Clone_DoesNotRealignSourceSubProgram() + { + var pgm = MakeTwoHoleProgram(); + pgm.Rotate(QuarterTurn); + var before = LeadInEnds(pgm); + + _ = pgm.Clone(); + + var after = LeadInEnds(pgm); + AssertPoint(before[0].X, before[0].Y, after[0]); + AssertPoint(before[1].X, before[1].Y, after[1]); + } + + [Fact] + public void Clone_OwnsItsSubPrograms() + { + var pgm = MakeTwoHoleProgram(); + + var copy = (Program)pgm.Clone(); + copy.Rotate(QuarterTurn); + + var sourceEnds = LeadInEnds(pgm); + AssertPoint(6, 5, sourceEnds[0]); + AssertPoint(16, 5, sourceEnds[1]); + + var copyEnds = LeadInEnds(copy); + AssertPoint(-5, 6, copyEnds[0]); + AssertPoint(-5, 16, copyEnds[1]); + } + + [Fact] + public void Clone_KeepsCallsAndDictionaryOnOneSharedCopy() + { + var pgm = MakeTwoHoleProgram(); + + var copy = (Program)pgm.Clone(); + + var calls = copy.Codes.OfType().ToList(); + Assert.NotSame(pgm.SubPrograms[7], copy.SubPrograms[7]); + Assert.Same(copy.SubPrograms[7], calls[0].Program); + Assert.Same(copy.SubPrograms[7], calls[1].Program); + } +}