From 896ed2026ad4f9122abc84056877c7379eab054a Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Fri, 25 Sep 2026 08:56:49 -0400 Subject: [PATCH] fix(engine): leave scribe marks out of layout bounds and salvage The job runner already checked sheet bounds on material contours only, but the benchmark validator and salvage scoring used Part.BoundingBox, which includes scribe/etch moves. A PEP bend tick that ends a hair past the part's edge passed the runner yet failed the benchmark when placed flush to the sheet edge, and it could shrink the credited offcut. Marks only mark the surface, so bounds and salvage now use material only. Benchmark before/after (all five built-in engines, local fixtures, salvage 0.5): no job changed validity or cost. Regression tests pin the new rule: a protruding tick flush to the sheet edge is valid in all four quadrants, and a tick past the parts envelope no longer shrinks salvage (targeted fixture cost 130 -> 120). Co-Authored-By: Codex Co-Authored-By: Claude Opus 5.5 --- .../Jobs/NestJobCostTests.cs | 26 ++++++---- OpenNest.Engine/Jobs/NestJobCost.cs | 6 +-- OpenNest.Engine/Jobs/NestLayoutCheck.cs | 9 +++- OpenNest.Engine/Jobs/NestTolerances.cs | 4 +- .../Benchmark/NestLayoutCheckMarksTests.cs | 48 +++++++++++++++++++ 5 files changed, 79 insertions(+), 14 deletions(-) create mode 100644 OpenNest.Tests/Benchmark/NestLayoutCheckMarksTests.cs diff --git a/OpenNest.Engine.Tests/Jobs/NestJobCostTests.cs b/OpenNest.Engine.Tests/Jobs/NestJobCostTests.cs index 4aedddf..1a5c82c 100644 --- a/OpenNest.Engine.Tests/Jobs/NestJobCostTests.cs +++ b/OpenNest.Engine.Tests/Jobs/NestJobCostTests.cs @@ -59,13 +59,18 @@ public class NestJobCostTests } [Theory] - [InlineData(0)] - [InlineData(0.5)] - public void RotatedMarksAndMultipleSheetsKeepExactLegacyCost(double rate) + [InlineData(0, false)] + [InlineData(0.5, false)] + [InlineData(0, true)] + [InlineData(0.5, true)] + public void RotatedPartsAndMultipleSheetsUseMaterialCost(double rate, bool marked) { var program = TestDrawingFactory.Rectangle(4, 3); - program.MoveTo(2, 2); - program.Codes.Add(new LinearMove(9, 2) { Layer = LayerType.Scribe }); + if (marked) + { + program.MoveTo(2, 2); + program.Codes.Add(new LinearMove(9, 2) { Layer = LayerType.Scribe }); + } var part = new NestJobPart("part", PartGeometrySnapshot.FromProgram(program), 5); var stock = new NestPlateStock("sheet", new Size(30, 40)); var other = new NestPlateStock("large", new Size(50, 50)); @@ -75,15 +80,18 @@ public class NestJobCostTests builder.AddSheet(stock, new[] { ("part", 10.123, 8.456, 0.37), ("part", 25.789, 17.321, 1.12) }); builder.AddSheet(other, new[] { ("part", 12.345, 19.876, 2.13) }); var result = builder.Build(NestJobStopReason.NoPlacementFound); + var cleanPart = new NestJobPart("part", PartGeometrySnapshot.FromProgram( + TestDrawingFactory.Rectangle(4, 3)), 5); + var cleanJob = new NestJob(new[] { cleanPart }, job.Plates, job.Options); foreach (var sheet in result.Plates) - Assert.Equal(LegacyNestJobCost.EstimateNetArea(job, sheet), NestJobCost.NetSheetArea(job, sheet)); + Assert.Equal(LegacyNestJobCost.EstimateNetArea(cleanJob, sheet), NestJobCost.NetSheetArea(job, sheet)); Assert.Equal(2500, NestJobCost.UnplacedPartPenalty(job)); - Assert.Equal(result.Plates.Sum(sheet => LegacyNestJobCost.EstimateNetArea(job, sheet)) + 5000, + Assert.Equal(result.Plates.Sum(sheet => LegacyNestJobCost.EstimateNetArea(cleanJob, sheet)) + 5000, NestJobCost.Evaluate(job, result)); } [Fact] - public void ScoringKeepsEtchBoundsEvenThoughMaterialGeometryExcludesThem() + public void EtchBeyondMaterialDoesNotShrinkSalvageOffcut() { var program = TestDrawingFactory.Rectangle(4, 3); program.MoveTo(2, 2); @@ -95,7 +103,7 @@ public class NestJobCostTests var sheet = new NestJobPlateResult(0, stock, new[] { new NestJobPlacement("part", 0, 0, 0, 0) }); Assert.Equal(130, LegacyNestJobCost.EstimateNetArea(job, sheet)); - Assert.Equal(130, NestJobCost.NetSheetArea(job, sheet)); + Assert.Equal(120, NestJobCost.NetSheetArea(job, sheet)); Assert.Equal(120, NestJobCost.NetSheetArea(job.Options, stock, JobPartGeometry.Read(part.Geometry).Bounds)); } diff --git a/OpenNest.Engine/Jobs/NestJobCost.cs b/OpenNest.Engine/Jobs/NestJobCost.cs index 4d883d2..c18fa33 100644 --- a/OpenNest.Engine/Jobs/NestJobCost.cs +++ b/OpenNest.Engine/Jobs/NestJobCost.cs @@ -10,7 +10,7 @@ public static class NestJobCost /// /// Sheet area less SalvageRate times the largest usable full-width/full-length edge offcut. /// Both offcut dimensions must meet MinimumSalvageDimension, which must be positive. - /// Bounds intentionally use Part.BoundingBox (including marks) to preserve benchmark scores. + /// Bounds use rotated material contours only; scribe/etch marks never shrink salvage offcuts. /// public static double NetSheetArea(NestJob job, NestJobPlateResult sheet) { @@ -26,7 +26,7 @@ public static class NestJobCost part.Rotate(p.Rotation); part.Location = new OpenNest.Geometry.Vector(p.X, p.Y); part.UpdateBounds(); - return part.BoundingBox; + return NestLayoutCheck.MaterialBounds(part); }) .ToList(); return NetSheetArea(job.Options, sheet.Stock, boxes.Min(b => b.Left), @@ -35,7 +35,7 @@ public static class NestJobCost /// /// Computes net area from an existing placed-parts envelope without rebuilding geometry. - /// For benchmark parity the envelope must include the same marks as Part.BoundingBox. + /// The envelope must contain material only; exclude scribe/etch marks. /// Empty sheets should use the sheet overload, which returns their full area. /// public static double NetSheetArea(NestJobOptions options, NestPlateStock stock, Box partsEnvelope) => diff --git a/OpenNest.Engine/Jobs/NestLayoutCheck.cs b/OpenNest.Engine/Jobs/NestLayoutCheck.cs index 2d1e271..c3e2ddd 100644 --- a/OpenNest.Engine/Jobs/NestLayoutCheck.cs +++ b/OpenNest.Engine/Jobs/NestLayoutCheck.cs @@ -225,7 +225,7 @@ public static class NestLayoutCheck foreach (var part in parts) { - var bb = part.BoundingBox; + var bb = MaterialBounds(part); var outLeft = bb.Left < workArea.X - Tolerance.Epsilon; var outBottom = bb.Bottom < workArea.Y - Tolerance.Epsilon; @@ -339,6 +339,13 @@ public static class NestLayoutCheck } } + /// Analytic world-space material bounds; surface marks never bound material. + internal static Box MaterialBounds(Part part) => + ConvertProgram.ToGeometry(part.Program) + .Where(e => SpecialLayers.IsMaterial(e.Layer)) + .GetBoundingBox() + .Translate(part.Location); + private static bool BoxesTouch(Box a, Box b) => a.Left <= b.Right + Tolerance.Epsilon && b.Left <= a.Right + Tolerance.Epsilon diff --git a/OpenNest.Engine/Jobs/NestTolerances.cs b/OpenNest.Engine/Jobs/NestTolerances.cs index 932c0e3..a40de18 100644 --- a/OpenNest.Engine/Jobs/NestTolerances.cs +++ b/OpenNest.Engine/Jobs/NestTolerances.cs @@ -3,7 +3,9 @@ using OpenNest.Geometry; namespace OpenNest.Engine.Jobs; -/// Shared numerical contract for layout validation. +/// Shared numerical contract for layout validation. +/// Only material contours (SpecialLayers.IsMaterial) count for bounds, clearance and scoring. +/// Scribe/etch marks never affect these checks or salvage envelopes, even outside the sheet. public static class NestTolerances { /// Arc chord tolerance used by both validators. The layout check circumscribes diff --git a/OpenNest.Tests/Benchmark/NestLayoutCheckMarksTests.cs b/OpenNest.Tests/Benchmark/NestLayoutCheckMarksTests.cs new file mode 100644 index 0000000..44fe9d6 --- /dev/null +++ b/OpenNest.Tests/Benchmark/NestLayoutCheckMarksTests.cs @@ -0,0 +1,48 @@ +using OpenNest.Benchmark; +using OpenNest.CNC; +using OpenNest.Engine.Jobs; +using OpenNest.Engine.Jobs.Adapters; +using OpenNest.Geometry; + +namespace OpenNest.Tests.Benchmark; + +public class NestLayoutCheckMarksTests +{ + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public void EtchTickPastFlushMaterialEdgePassesBoundsAndLeavesSalvageUnchanged(int quadrant) + { + var program = new Program(); + program.MoveTo(0, 0); + program.LineTo(4, 0); + program.LineTo(4, 3); + program.LineTo(0, 3); + program.LineTo(0, 0); + var clean = new NestJobPart("part", PartGeometrySnapshot.FromProgram(program), 1); + program.MoveTo(3.75, 2); + program.Codes.Add(new LinearMove(4.25, 2) { Layer = LayerType.Scribe }); + var marked = new NestJobPart("part", PartGeometrySnapshot.FromProgram(program), 1); + var stock = new NestPlateStock("sheet", new Size(10, 20), quadrant: quadrant); + var job = new NestJob(new[] { marked }, new[] { stock }, + new NestJobOptions(salvageRate: 0.5, minimumSalvageDimension: 1)); + var builder = new NestJobResultBuilder(job); + builder.AddSheet(stock, new[] { (marked.Id, stock.WorkArea.Right - 4, stock.WorkArea.Bottom, 0.0) }); + var result = builder.Build(NestJobStopReason.Completed); + Assert.Empty(NestLayoutCheck.Violations(job, result)); + var cleanJob = new NestJob(new[] { clean }, job.Plates, job.Options); + Assert.Equal(NestJobCost.Evaluate(cleanJob, result), NestJobCost.Evaluate(job, result)); + + var materialized = NestResultMaterializer.Materialize(job, result); + var requirements = new Dictionary + { + [materialized.DrawingsByPartId[marked.Id]] = (marked.Id, 1), + }; + var runs = materialized.Nest.Plates.Select(p => (p, p.Parts.ToList())).ToList(); + Assert.Contains(LegacyNestValidator.Validate(runs, requirements).Violations, + message => message.Contains("outside the work area")); + Assert.True(NestValidator.Validate(runs, requirements).Valid); + } +}