From 48367da8206b85c4fd3a4a61c0f8a1af770b03b0 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Mon, 5 Oct 2026 22:04:47 -0400 Subject: [PATCH] fix(diagnostics): refuse absolute subprograms instead of normalizing 1b422ef read absolute-mode hole subprograms by converting an incremental-mode copy of every clean program. Rebuilding absolute endpoints from incremental deltas is not exact: after a rapid at 1e12 a 1x10 rectangle moved by about 2.4e-5 and a real 2e-5 overlap was reported clear, in the overlap overlay and pre-post verification as well as Plan Cutting. Convert programs directly again, which reads absolute coordinates exactly, and refuse an absolute-mode subprogram as an incomplete check instead: the converter adds a call's frame offset to incremental moves only, so it would read such a hole at its frame origin. OpenNest writes hole subprograms in incremental mode. The null-list and unknown-instruction refusals from 1b422ef stay, and CopyForGeometry is private to the planner again. --- .../CNC/CuttingPlanning/PreparedContours.cs | 7 +--- .../Diagnostics/PlateOverlapAnalyzer.cs | 21 +++++++----- .../CuttingPlanning/CuttingPlanBatchTests.cs | 15 +++++++-- .../Diagnostics/PlateOverlapAnalyzerTests.cs | 33 +++++++++++++++++-- docs/cutting-planner.md | 3 +- docs/post-verification.md | 5 ++- 6 files changed, 62 insertions(+), 22 deletions(-) diff --git a/OpenNest.Core/CNC/CuttingPlanning/PreparedContours.cs b/OpenNest.Core/CNC/CuttingPlanning/PreparedContours.cs index e43b24c..377ba77 100644 --- a/OpenNest.Core/CNC/CuttingPlanning/PreparedContours.cs +++ b/OpenNest.Core/CNC/CuttingPlanning/PreparedContours.cs @@ -215,12 +215,7 @@ public sealed class PreparedContours private static Vector Start(Entity entity) => entity is Line line ? line.StartPoint : ((Arc)entity).StartPoint(); private static Vector End(Entity entity) => entity is Line line ? line.EndPoint : ((Arc)entity).EndPoint(); - /// - /// Owned copy of a validated graph with every program in incremental mode, so - /// keeps absolute subprogram frame offsets. Shared - /// subprograms stay shared. Validate exact instruction types first: this clones every code. - /// - internal static Program CopyForGeometry(Program source, CancellationToken token) + private static Program CopyForGeometry(Program source, CancellationToken token) { var copies = new Dictionary(ReferenceEqualityComparer.Instance); return Copy(source); diff --git a/OpenNest.Core/Diagnostics/PlateOverlapAnalyzer.cs b/OpenNest.Core/Diagnostics/PlateOverlapAnalyzer.cs index eb191ec..e329249 100644 --- a/OpenNest.Core/Diagnostics/PlateOverlapAnalyzer.cs +++ b/OpenNest.Core/Diagnostics/PlateOverlapAnalyzer.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; using System.Threading; using OpenNest.CNC; -using OpenNest.CNC.CuttingPlanning; using OpenNest.Converters; using OpenNest.Geometry; @@ -72,12 +71,10 @@ public static class PlateOverlapAnalyzer { try { - ValidateProgram(program, new HashSet(ReferenceEqualityComparer.Instance)); - // Convert an incremental-mode copy: the converter adds call offsets to incremental moves - // only, so absolute subprogram holes would otherwise land at their frame origin. The - // copy is owned; the live program is neither converted in place nor rotated. - var geometry = PreparedContours.CopyForGeometry(program, CancellationToken.None); - return new OverlapSource(program, ConvertProgram.ToGeometry(geometry) + ValidateProgram(program, new HashSet(ReferenceEqualityComparer.Instance), false); + // Conversion creates fresh geometry, including expanded shared hole calls; + // no cloning/rotation of a live program or subprogram is necessary. + return new OverlapSource(program, ConvertProgram.ToGeometry(program) .Where(entity => SpecialLayers.IsMaterial(entity.Layer) && entity.Layer != SpecialLayers.Leadin && entity.Layer != SpecialLayers.Leadout).ToList(), null); @@ -278,12 +275,18 @@ public static class PlateOverlapAnalyzer typeof(Comment), typeof(Feedrate), typeof(Kerf), ]; - private static void ValidateProgram(Program program, HashSet visiting) + private static void ValidateProgram(Program program, HashSet visiting, bool subprogram) { if (program == null || !visiting.Add(program) || visiting.Count > 64) throw new ArgumentException("Missing, recursive, or excessively nested subprogram."); if (program.Codes == null) throw new ArgumentException("Program has no instruction list."); + // The converter adds a call's frame offset to incremental moves only, so an absolute + // subprogram would be read at its frame origin. Converting it exactly needs a lossless + // frame transform; until then it is refused rather than misread (OpenNest writes hole + // subprograms in incremental mode). + if (subprogram && program.Mode == Mode.Absolute) + throw new NotSupportedException("Absolute-mode subprograms are not supported by the overlap check."); foreach (var code in program.Codes) { if (code == null) @@ -297,7 +300,7 @@ public static class PlateOverlapAnalyzer { if (!OverlapMaterial.IsFinite(call.Offset) || !double.IsFinite(call.Rotation)) throw new ArgumentException("Subprogram pose must be finite."); - ValidateProgram(call.Program, visiting); + ValidateProgram(call.Program, visiting, true); } } visiting.Remove(program); diff --git a/OpenNest.Tests/CuttingPlanning/CuttingPlanBatchTests.cs b/OpenNest.Tests/CuttingPlanning/CuttingPlanBatchTests.cs index b5568e6..cc8188a 100644 --- a/OpenNest.Tests/CuttingPlanning/CuttingPlanBatchTests.cs +++ b/OpenNest.Tests/CuttingPlanning/CuttingPlanBatchTests.cs @@ -98,7 +98,7 @@ public class CuttingPlanBatchTests [Theory] [InlineData(Mode.Absolute)] [InlineData(Mode.Incremental)] - public void Plan_SubprogramHolesInEitherMode_AreNotFalseOverlaps(Mode mode) + public void Plan_SubprogramHoles_AreNeverAFalseOverlap(Mode mode) { var hole = new Program(); hole.MoveTo(1, 0); @@ -115,8 +115,17 @@ public class CuttingPlanBatchTests var proposal = CuttingPlanBatch.Capture([plate], ExplicitContourTests.Parameters(), false).Plan(); var planned = Assert.Single(proposal.Plates); - Assert.True(planned.IsOverlapClear, string.Join("; ", planned.Overlap.Issues.Select(i => i.Message))); - Assert.True(planned.IsReady); + Assert.True(planned.IsRouteReady); + Assert.Empty(planned.Overlap.Pairs); + if (mode == Mode.Incremental) + Assert.True(planned.IsReady, string.Join("; ", planned.Overlap.Issues.Select(i => i.Message))); + else + { + // The overlap check cannot read absolute subprograms yet: blocked as unchecked, not as overlapping. + Assert.False(planned.IsReady); + Assert.Contains("Overlap check incomplete for part 1: Absolute-mode subprograms", + string.Join("\n", proposal.Describe("in"))); + } } [Fact] diff --git a/OpenNest.Tests/Diagnostics/PlateOverlapAnalyzerTests.cs b/OpenNest.Tests/Diagnostics/PlateOverlapAnalyzerTests.cs index 82c53fa..390b02b 100644 --- a/OpenNest.Tests/Diagnostics/PlateOverlapAnalyzerTests.cs +++ b/OpenNest.Tests/Diagnostics/PlateOverlapAnalyzerTests.cs @@ -504,7 +504,7 @@ public class PlateOverlapAnalyzerTests [Theory] [InlineData(Mode.Absolute)] [InlineData(Mode.Incremental)] - public void Analyze_SubprogramHolesKeepTheirCallOffsetsInEitherMode(Mode mode) + public void Analyze_IncrementalHolesKeepTheirOffsetsAndAbsoluteOnesAreRefused(Mode mode) { var hole = new Program(); hole.MoveTo(1, 0); @@ -525,8 +525,37 @@ public class PlateOverlapAnalyzerTests var report = PlateOverlapAnalyzer.Analyze(new[] { host, insert }); - Assert.True(report.IsComplete, string.Join("; ", report.Issues.Select(issue => issue.Message))); + // Never a false overlap: incremental holes are read at their call offsets, and an absolute + // subprogram, which the converter would read at its frame origin, is an incomplete check. Assert.Empty(report.Pairs); + if (mode == Mode.Incremental) + Assert.True(report.IsComplete, string.Join("; ", report.Issues.Select(issue => issue.Message))); + else + { + var issue = Assert.Single(report.Issues); + Assert.Equal((0, (int?)null), (issue.PartAId, issue.PartBId)); + Assert.Contains("Absolute-mode subprograms", issue.Message); + } + } + + [Fact] + public void Analyze_AbsoluteCoordinatesFarFromTheMaterialAreReadExactly() + { + // Rebuilding absolute endpoints from incremental deltas after a rapid at 1e12 moves the + // contour by about 2.4e-5, enough to hide this 2e-5 wide overlap. + var far = new Program(Mode.Absolute); + far.Codes.Add(new RapidMove(new Vector(1e12, 0))); + far.Codes.Add(new RapidMove(new Vector(0.1, 0))); + foreach (var point in new[] { new Vector(1.1, 0), new Vector(1.1, 10), new Vector(0.1, 10), new Vector(0.1, 0) }) + far.Codes.Add(new LinearMove(point)); + var first = new Part(new Drawing("far rapid", far)); + var second = Rectangle(1.09998, 0, 1, 10); + + var report = PlateOverlapAnalyzer.Analyze(new[] { first, second }); + + Assert.True(report.IsComplete, string.Join("; ", report.Issues.Select(issue => issue.Message))); + var pair = Assert.Single(report.Pairs); + Assert.Equal(2e-4, pair.Area, 6); } [Fact] diff --git a/docs/cutting-planner.md b/docs/cutting-planner.md index 7a27468..73a6c04 100644 --- a/docs/cutting-planner.md +++ b/docs/cutting-planner.md @@ -202,7 +202,8 @@ plans every plate that has parts. Both open one dialog built on already pass the checks. - Every plate is captured on the UI thread and checked and planned on a worker. Clean part material is checked for overlaps with the pre-post overlap analyzer; overlapping parts or - an incomplete check block that plate whatever its route. A free-order search that ends + an incomplete check (see [pre-post verification](post-verification.md)) block that plate + whatever its route. A free-order search that ends `NoSolutionWithinBudget` is retried once with the current part order, and the summary says the order was kept. A kept order is allowed 400 expansions per part (at least the default 20000), because it still searches contour order and entries. diff --git a/docs/post-verification.md b/docs/post-verification.md index 704a621..9414397 100644 --- a/docs/post-verification.md +++ b/docs/post-verification.md @@ -17,7 +17,10 @@ shows all three check categories, even when there are no findings: Findings identify the plate and part's position in the cutting sequence. Read the scrollable report, cancel to fix the nest, then post again to rerun verification. -Uncheckable geometry is reported as incomplete, not passed. Multiple interrupted +Uncheckable geometry is reported as incomplete, not passed. The overlap check reads +clean drawing programs made of the built-in instructions only; a missing instruction +list, any other instruction type, or a hole stored as an absolute-mode subprogram +(OpenNest writes them incremental) makes that part's check incomplete. Multiple interrupted cut fragments, or a lead-out after an open contour that might cut through a tab, also require manual review rather than being assumed retained.