From fb7ef6fd0d4154d00d356044cf49c2266b749a31 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Sun, 4 Oct 2026 21:48:52 -0400 Subject: [PATCH] fix(cutting): retain authored motion metadata in owned programs --- .../CuttingPlanning/CuttingPlanModels.cs | 4 +- .../CuttingPlanning/CuttingPlanService.cs | 53 +---- .../CuttingPlanning/OwnedProgramCopy.cs | 143 ++++++++++++ .../CuttingPlanning/OwnedProgramCopyTests.cs | 213 ++++++++++++++++++ 4 files changed, 362 insertions(+), 51 deletions(-) create mode 100644 OpenNest.Engine/CuttingPlanning/OwnedProgramCopy.cs create mode 100644 OpenNest.Tests/CuttingPlanning/OwnedProgramCopyTests.cs diff --git a/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs b/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs index cbc43a3..81823d3 100644 --- a/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs +++ b/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs @@ -93,9 +93,9 @@ public sealed class FixedProgramPlacement public IReadOnlyList ContourChoices { get; } public bool IsRegenerated => ContourChoices.Count != 0; /// Returns an independent deep copy; never an alias to captured/proposed code. - public Program CopyProgram() => program == null ? null : (Program)program.Clone(); + public Program CopyProgram() => program == null ? null : OwnedProgramCopy.Copy(program); internal FixedProgramPlacement Propose(Program proposed, OwnedExecution execution, IReadOnlyList choices) => - new(SourcePart, SourceOrdinal, Location, Rotation, LeadInsLocked, execution, proposed, Prepared, Material, choices); + new(SourcePart, SourceOrdinal, Location, Rotation, LeadInsLocked, execution, OwnedProgramCopy.Copy(proposed), Prepared, Material, choices); public Part SourcePart { get; } public int SourceOrdinal { get; } diff --git a/OpenNest.Engine/CuttingPlanning/CuttingPlanService.cs b/OpenNest.Engine/CuttingPlanning/CuttingPlanService.cs index 836bc29..4df3631 100644 --- a/OpenNest.Engine/CuttingPlanning/CuttingPlanService.cs +++ b/OpenNest.Engine/CuttingPlanning/CuttingPlanService.cs @@ -48,20 +48,20 @@ public static class CuttingPlanService throw new NotSupportedException("Cutoff dependency ordering is outside the fixed-program route slice."); // Validate both original graphs before Clone or any virtual transform can // erase unsupported runtime semantics, including fixed/ineligible targets. - ValidateCloneGraph(source.BaseDrawing.Program, token); - ValidateCloneGraph(source.Program, token); + OwnedProgramCopy.Validate(source.BaseDrawing.Program, token); + OwnedProgramCopy.Validate(source.Program, token); var clean = ExecutionMotionReader.ReadSupported(source.BaseDrawing.Program, Vector.Zero, null, token); if (!clean.HasCuttingContour) throw new NotSupportedException("Scribe-only or noncutting source drawings are outside this route slice."); var execution = ExecutionMotionReader.ReadSupported(source.Program, source.Location, request.StartPoint, token); if (!execution.HasCuttingContour) throw new ArgumentException("Placed program has no nonzero cutting contour motions."); - var ownedProgram = (Program)source.Program.Clone(); + var ownedProgram = OwnedProgramCopy.Copy(source.Program, token); PreparedContours prepared = null; LeadMaterialSnapshot material = null; if (request.ConfirmedParameters != null) { - var ownedClean = (Program)source.BaseDrawing.Program.Clone(); + var ownedClean = OwnedProgramCopy.Copy(source.BaseDrawing.Program, token); ownedClean.Rotate(source.Rotation - source.BaseDrawing.Program.Rotation); material = LeadMaterialSnapshot.Capture(ownedClean, source.Location, token); if (!material.IsComplete) @@ -272,51 +272,6 @@ public static class CuttingPlanService } } - private static void ValidateCloneGraph(Program program, CancellationToken token) - { - var active = new HashSet(ReferenceEqualityComparer.Instance); - var done = new HashSet(ReferenceEqualityComparer.Instance); - var budget = 1000000; - Visit(program); - void Visit(Program current) - { - token.ThrowIfCancellationRequested(); - if (current == null || active.Contains(current) || active.Count >= 64) - throw new ArgumentException("Missing, recursive or excessively nested clone graph."); - if (done.Contains(current)) return; - // Inactive registered subprograms may be motionless, but Clone still - // traverses them. Refuse unknown semantics without invoking virtual Clone. - if (current.GetType() != typeof(Program) || !Enum.IsDefined(current.Mode)) - throw new NotSupportedException("Unsupported program runtime type or mode."); - if (current.Codes == null) - throw new ArgumentException("Missing clone graph instructions."); - active.Add(current); - foreach (var code in current.Codes) - { - token.ThrowIfCancellationRequested(); - if (--budget < 0) - throw new ArgumentException("Clone graph exceeds the verification limit."); - if (code == null) - throw new ArgumentException("Missing clone graph instruction."); - var type = code.GetType(); - if (type != typeof(RapidMove) && type != typeof(LinearMove) && type != typeof(ArcMove) - && type != typeof(SubProgramCall) && type != typeof(Comment) && type != typeof(Feedrate) && type != typeof(Kerf)) - throw new NotSupportedException("Unsupported instruction runtime type."); - if (code is SubProgramCall call) - Visit(call.Program); - } - foreach (var child in current.SubPrograms.Values) - { - token.ThrowIfCancellationRequested(); - if (--budget < 0) - throw new ArgumentException("Clone graph exceeds the verification limit."); - Visit(child); - } - active.Remove(current); - done.Add(current); - } - } - private static IEnumerable Map(CuttingPlanSnapshot snapshot, IEnumerable findings) => findings.Select(finding => { diff --git a/OpenNest.Engine/CuttingPlanning/OwnedProgramCopy.cs b/OpenNest.Engine/CuttingPlanning/OwnedProgramCopy.cs new file mode 100644 index 0000000..05712a7 --- /dev/null +++ b/OpenNest.Engine/CuttingPlanning/OwnedProgramCopy.cs @@ -0,0 +1,143 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using OpenNest.CNC; + +namespace OpenNest.Engine.CuttingPlanning; + +/// Lossless owned copies at the cutting-plan boundary, not a general Clone change. +internal static class OwnedProgramCopy +{ + private const int Limit = 1000000; + private const int MaxDepth = 64; + + internal static Program Copy(Program original, CancellationToken token = default) + { + Validate(original, token); + // Built-in Clone preserves mode/rotation and binds calls without rotating setters. + // Its per-parent maps can duplicate diamonds; restore one global owned graph below. + var cloned = (Program)original.Clone(); + var programs = new Dictionary(ReferenceEqualityComparer.Instance); + var codes = new Dictionary(ReferenceEqualityComparer.Instance); + var bindings = new Dictionary, Dictionary>(ReferenceEqualityComparer.Instance); + return Restore(original, cloned); + + Program Restore(Program source, Program copy) + { + token.ThrowIfCancellationRequested(); + if (programs.TryGetValue(source, out var existing)) return existing; + programs.Add(source, copy); + for (var i = 0; i < source.Codes.Count; i++) + { + token.ThrowIfCancellationRequested(); + var authored = source.Codes[i]; + if (codes.TryGetValue(authored, out var owned)) + { + copy.Codes[i] = owned; + continue; + } + owned = copy.Codes[i]; + codes.Add(authored, owned); + if (authored is Motion motion) + { + var target = (Motion)owned; + // All other built-in fields are retained by their Clone implementations. + target.UseExactStop = motion.UseExactStop; + target.Feedrate = motion.Feedrate; + if (motion.VariableRefs != null) + { + if (!bindings.TryGetValue(motion.VariableRefs, out var refs)) + { + refs = new Dictionary(motion.VariableRefs, motion.VariableRefs.Comparer); + bindings.Add(motion.VariableRefs, refs); + } + target.VariableRefs = refs; + } + } + if (authored is SubProgramCall call) + { + var target = (SubProgramCall)owned; + target.BindProgram(Restore(call.Program, target.Program)); + } + } + foreach (var (id, child) in source.SubPrograms) + { + token.ThrowIfCancellationRequested(); + copy.SubPrograms[id] = Restore(child, copy.SubPrograms[id]); + } + return copy; + } + } + + // Unlike the execution reader, inactive registered graphs can be motionless or + // contain suppressed motions. They still must be supported, acyclic and bounded. + internal static void Validate(Program program, CancellationToken token) + { + var active = new HashSet(ReferenceEqualityComparer.Instance); + var done = new Dictionary(ReferenceEqualityComparer.Instance); + var budget = Limit; + Visit(program); + (int Cost, int Depth) Visit(Program current) + { + token.ThrowIfCancellationRequested(); + if (current == null || active.Contains(current) || active.Count >= MaxDepth) + throw new ArgumentException("Missing, recursive or excessively nested clone graph."); + if (done.TryGetValue(current, out var previous)) + { + if (active.Count + previous.Depth > MaxDepth) + throw new ArgumentException("Excessively nested clone graph."); + return previous; + } + if (current.GetType() != typeof(Program) || !Enum.IsDefined(current.Mode)) + throw new NotSupportedException("Unsupported program runtime type or mode."); + if (current.Codes == null) + throw new ArgumentException("Missing clone graph instructions."); + active.Add(current); + var children = new HashSet(ReferenceEqualityComparer.Instance); + var cost = 1; + var depth = 1; + foreach (var code in current.Codes) + { + Step(); + cost++; + if (code == null) + throw new ArgumentException("Missing clone graph instruction."); + var type = code.GetType(); + if (type != typeof(RapidMove) && type != typeof(LinearMove) && type != typeof(ArcMove) + && type != typeof(SubProgramCall) && type != typeof(Comment) && type != typeof(Feedrate) && type != typeof(Kerf)) + throw new NotSupportedException("Unsupported instruction runtime type."); + if (code is SubProgramCall call) Child(call.Program); + } + foreach (var child in current.SubPrograms.Values) + { + Step(); + Child(child); + } + if (cost > Limit) + throw new ArgumentException("Clone graph exceeds the verification limit."); + active.Remove(current); + var result = (cost, depth); + done.Add(current, result); + return result; + + void Child(Program child) + { + var summary = Visit(child); + // Program.Clone shares children only within one parent; bound its real + // expanded work as well as the distinct original graph before calling it. + if (!children.Add(child)) return; + if (summary.Cost > Limit - cost) + throw new ArgumentException("Clone expansion exceeds the verification limit."); + cost += summary.Cost; + depth = System.Math.Max(depth, summary.Depth + 1); + } + } + + void Step() + { + token.ThrowIfCancellationRequested(); + if (--budget < 0) + throw new ArgumentException("Clone graph exceeds the verification limit."); + } + } +} diff --git a/OpenNest.Tests/CuttingPlanning/OwnedProgramCopyTests.cs b/OpenNest.Tests/CuttingPlanning/OwnedProgramCopyTests.cs new file mode 100644 index 0000000..451e335 --- /dev/null +++ b/OpenNest.Tests/CuttingPlanning/OwnedProgramCopyTests.cs @@ -0,0 +1,213 @@ +using OpenNest.CNC; +using OpenNest.CNC.CuttingPlanning; +using OpenNest.Engine.CuttingPlanning; +using OpenNest.Geometry; + +namespace OpenNest.Tests.CuttingPlanning; + +public class OwnedProgramCopyTests +{ + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void FixedPayload_PreservesEveryAuthoredFieldAndGraphAlias(bool confirmed, bool locked) + { + var parameters = ExplicitContourTests.Parameters(); + var clean = LeadPathValidationTests.Rectangle(0, 0, 10, 10); + var prepared = PreparedContours.Capture(clean, parameters); + var emitted = prepared.Emit([prepared.ClosestEntry(0, new Vector(-2, 5))]); + var part = new Part(new Drawing("metadata", clean)); + Assert.True(part.RestoreLeadInProgram(emitted, locked)); + part.Rotate(0.2); + AddMetadata(part.Program); + var snapshot = CuttingPlanService.Capture(new CuttingPlanRequest([part], new Vector(-2, 5), + confirmedParameters: confirmed ? parameters : null, eligibleParts: confirmed ? [] : null)); + var captured = Assert.Single(snapshot.Placements); + AssertGraph(part.Program, captured.CopyProgram()); + var result = CuttingPlanService.Plan(snapshot); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + var proposal = Assert.Single(result.ProposedOrder); + AssertGraph(part.Program, proposal.CopyProgram()); + var detached = proposal.CopyProgram(); + var expectedFeed = ((Motion)part.Program.Codes.First(c => c is Motion)).Feedrate; + ((Motion)detached.Codes.First(c => c is Motion)).Feedrate = 999; + detached.SubPrograms[-40].Codes.Clear(); + detached.Variables.Clear(); + AssertGraph(part.Program, proposal.CopyProgram()); + part.Program.Codes.Clear(); + part.Program.SubPrograms[-40].Codes.Clear(); + part.Program.Variables.Clear(); + var retained = proposal.CopyProgram(); + Assert.Equal(expectedFeed, ((Motion)retained.Codes.First(c => c is Motion)).Feedrate); + Assert.NotEmpty(retained.SubPrograms[-40].Codes); + Assert.NotEmpty(retained.Variables); + } + + [Fact] + public void Propose_OwnsLosslessStorageRatherThanRetainingCallerProgram() + { + var part = new Part(new Drawing("proposal", LeadPathValidationTests.Rectangle(0, 0, 10, 10))); + var captured = Assert.Single(CuttingPlanService.Capture(new CuttingPlanRequest([part])).Placements); + var authored = captured.CopyProgram(); + AddMetadata(authored); + var proposed = captured.Propose(authored, ExecutionMotionReader.ReadSupported(authored, Vector.Zero, null), []); + AssertGraph(authored, proposed.CopyProgram()); + authored.Codes.Clear(); authored.SubPrograms[-40].Codes.Clear(); authored.Variables.Clear(); + var retained = proposed.CopyProgram(); + Assert.NotEmpty(retained.Codes); + Assert.NotEmpty(retained.SubPrograms[-40].Codes); + Assert.NotEmpty(retained.Variables); + } + + [Theory] + [InlineData("cycle")] + [InlineData("depth")] + [InlineData("cached-depth")] + [InlineData("expansion")] + public void OwnedCopy_RefusesUnsafeCloneTraversalBeforeInvokingClone(string kind) + { + var root = new Program(); + if (kind == "cycle") root.SubPrograms[-1] = root; + if (kind is "depth" or "cached-depth") + { + var child = new Program(); + if (kind == "cached-depth") root.SubPrograms[-2] = child; + var parent = child; + for (var i = 0; i < 64; i++) + { + var next = new Program(); next.SubPrograms[-1] = parent; parent = next; + } + root.SubPrograms[-1] = parent; + } + if (kind == "expansion") + { + var child = new Program(); + for (var i = 0; i < 20; i++) + { + var left = new Program(); left.SubPrograms[-1] = child; + var right = new Program(); right.SubPrograms[-1] = child; + var parent = new Program(); parent.SubPrograms[-1] = left; parent.SubPrograms[-2] = right; + child = parent; + } + root.SubPrograms[-1] = child; + } + Assert.Throws(() => OwnedProgramCopy.Copy(root)); + } + + [Fact] + public void OwnedCopy_HonorsCancellationBeforeClone() + { + Assert.Throws(() => OwnedProgramCopy.Copy(new Program(), new CancellationToken(true))); + } + + private static void AddMetadata(Program root) + { + var leaf = new Program(); + leaf.MoveTo(-1, 0); + leaf.Codes.Add(new LinearMove(-2, 0) { Layer = LayerType.Scribe }); + leaf.Codes.Add(new ArcMove(-1, 1, -1, 0, RotationType.CW) { Layer = LayerType.Scribe }); + leaf.Rotate(0.37); + leaf.Mode = Mode.Incremental; + var degrees = 0.37 * 180 / System.Math.PI; + var middle = new Program(); + middle.Codes.Add(new SubProgramCall(leaf, degrees) { Id = -7, Offset = new Vector(-1, 0) }); + middle.SubPrograms[-7] = leaf; + root.Codes.Add(new SubProgramCall(middle, 0) { Id = -9, Offset = new Vector(-5, -5) }); + root.Codes.Add(new SubProgramCall(leaf, degrees) { Id = -7, Offset = new Vector(-10, -10) }); + root.SubPrograms[-9] = middle; + root.SubPrograms[-7] = leaf; + var inactive = new Program(Mode.Incremental); + inactive.Codes.Add(new LinearMove(4, 5) { Suppressed = true, Layer = LayerType.Display }); + inactive.SubPrograms[-7] = leaf; + root.SubPrograms[-40] = inactive; + root.SubPrograms[-41] = new Program(); // Accepted motionless inactive registration. + foreach (var p in new[] { root, middle, leaf, inactive }) + { + p.Codes.Insert(0, new Comment("literal, : #value")); + p.Codes.Insert(1, new Feedrate(123.5) { VariableRef = "speed" }); + p.Codes.Insert(2, new Kerf(KerfType.Right)); + p.Variables["value"] = new VariableDefinition("value", "2+3", 5, inline: true, global: true); + foreach (var motion in p.Codes.OfType()) + { + motion.UseExactStop = true; + motion.Feedrate = 123; + motion.VariableRefs = new Dictionary(StringComparer.OrdinalIgnoreCase) { ["X"] = "value" }; + } + } + } + + private static void AssertGraph(Program source, Program copy) + { + var mapped = new Dictionary(ReferenceEqualityComparer.Instance); + var reverse = new HashSet(ReferenceEqualityComparer.Instance); + Visit(source, copy); + void Visit(Program original, Program owned) + { + Assert.NotSame(original, owned); + if (mapped.TryGetValue(original, out var previous)) + { + Assert.Same(previous, owned); + return; + } + Assert.True(reverse.Add(owned)); + mapped.Add(original, owned); + Assert.Equal(original.Mode, owned.Mode); + Bits(original.Rotation, owned.Rotation); + Assert.NotSame(original.Codes, owned.Codes); + Assert.NotSame(original.Variables, owned.Variables); + Assert.NotSame(original.SubPrograms, owned.SubPrograms); + Assert.Equal(original.Variables.Keys, owned.Variables.Keys); + foreach (var (key, value) in original.Variables) + { + var actual = owned.Variables[key]; + Assert.Equal(value.Name, actual.Name); Assert.Equal(value.Expression, actual.Expression); + Bits(value.Value, actual.Value); Assert.Equal(value.Inline, actual.Inline); Assert.Equal(value.Global, actual.Global); + } + Assert.Equal(original.Codes.Count, owned.Codes.Count); + for (var i = 0; i < original.Codes.Count; i++) + { + var code = original.Codes[i]; var actual = owned.Codes[i]; + Assert.NotSame(code, actual); Assert.Equal(code.GetType(), actual.GetType()); + if (code is Motion motion) + { + var m = Assert.IsAssignableFrom(actual); + Point(motion.EndPoint, m.EndPoint); + Assert.Equal(motion.UseExactStop, m.UseExactStop); Assert.Equal(motion.Feedrate, m.Feedrate); + Assert.Equal(motion.Suppressed, m.Suppressed); + if (motion.VariableRefs == null) Assert.Null(m.VariableRefs); + else + { + Assert.NotSame(motion.VariableRefs, m.VariableRefs); + Assert.Equal(motion.VariableRefs, m.VariableRefs); + Assert.Equal(motion.VariableRefs.ContainsKey("x"), m.VariableRefs!.ContainsKey("x")); + } + if (motion is LinearMove line) Assert.Equal(line.Layer, ((LinearMove)m).Layer); + if (motion is ArcMove arc) + { + var a = Assert.IsType(m); + Point(arc.CenterPoint, a.CenterPoint); Assert.Equal(arc.Rotation, a.Rotation); Assert.Equal(arc.Layer, a.Layer); + } + } + if (code is Comment comment) Assert.Equal(comment.Value, Assert.IsType(actual).Value); + if (code is Feedrate feed) + { + var f = Assert.IsType(actual); Bits(feed.Value, f.Value); Assert.Equal(feed.VariableRef, f.VariableRef); + } + if (code is Kerf kerf) Assert.Equal(kerf.Value, Assert.IsType(actual).Value); + if (code is SubProgramCall call) + { + var c = Assert.IsType(actual); + Assert.Equal(call.Id, c.Id); Point(call.Offset, c.Offset); Bits(call.Rotation, c.Rotation); + Visit(call.Program, c.Program); + } + } + Assert.Equal(original.SubPrograms.Keys, owned.SubPrograms.Keys); + foreach (var (key, child) in original.SubPrograms) Visit(child, owned.SubPrograms[key]); + } + } + + private static void Point(Vector expected, Vector actual) { Bits(expected.X, actual.X); Bits(expected.Y, actual.Y); } + private static void Bits(double expected, double actual) => Assert.Equal(BitConverter.DoubleToInt64Bits(expected), BitConverter.DoubleToInt64Bits(actual)); +}