From 0ca4e64189c34d96060dcd80faa109fac92ebc3a Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Tue, 20 Oct 2026 22:04:23 -0400 Subject: [PATCH] fix(bestfit): drop saved pair sets whose canonical frame changed Saved best-fit pairs are in the drawing's canonical frame, but the canonical angle is recomputed on load and was not saved with the set. When that computation changes (RotatingCalipers checking every hull edge, marks no longer counted as material), old pairs were replayed in a turned frame and could overlap. Each saved set now records canonicalAngle, and BestFitCacheStore.Import skips sets whose angle no longer matches the recomputed one (1e-9 rad, modulo a full turn). Sets from older files have no angle and are dropped; their pairs are recomputed when next needed. Co-Authored-By: Claude Opus 5.5 --- OpenNest.Core/BestFit/IBestFitStore.cs | 14 +++- OpenNest.Engine/BestFit/BestFitCacheStore.cs | 29 ++++++- .../BestFitStorePersistenceTests.cs | 19 ++++- OpenNest.IO/NestFormat.cs | 1 + OpenNest.IO/NestReader.cs | 3 +- OpenNest.IO/NestWriter.cs | 1 + .../BestFit/BestFitCacheStoreTests.cs | 78 ++++++++++++++++++- OpenNest.Tests/IO/BestFitPersistenceTests.cs | 64 ++++++++++++++- docs/nest-file-format.md | 21 +++-- 9 files changed, 213 insertions(+), 17 deletions(-) diff --git a/OpenNest.Core/BestFit/IBestFitStore.cs b/OpenNest.Core/BestFit/IBestFitStore.cs index 47a2731a..8a0c8fce 100644 --- a/OpenNest.Core/BestFit/IBestFitStore.cs +++ b/OpenNest.Core/BestFit/IBestFitStore.cs @@ -21,12 +21,17 @@ namespace OpenNest.BestFit IReadOnlyList HullAngles ); - /// The saved best-fit results for one drawing at one plate size and spacing. + /// + /// The saved best-fit results for one drawing at one plate size and spacing. + /// is the drawing's canonical angle (radians) when the + /// set was saved: the frame its pair geometry is in. Null means the frame is unknown. + /// public sealed record SavedPairSet( double PlateWidth, double PlateHeight, double Spacing, - IReadOnlyList Results + IReadOnlyList Results, + double? CanonicalAngle = null ); /// @@ -45,7 +50,10 @@ namespace OpenNest.BestFit IReadOnlySet<(double Width, double Length, double Spacing)> plateKeys ); - /// Restores saved sets for . + /// + /// Restores saved sets for . Sets saved in a frame other + /// than the drawing's current canonical frame, or in an unknown one, are stale. + /// void Import(Drawing drawing, IReadOnlyList sets); } } diff --git a/OpenNest.Engine/BestFit/BestFitCacheStore.cs b/OpenNest.Engine/BestFit/BestFitCacheStore.cs index 2a555809..ba6e2739 100644 --- a/OpenNest.Engine/BestFit/BestFitCacheStore.cs +++ b/OpenNest.Engine/BestFit/BestFitCacheStore.cs @@ -8,9 +8,17 @@ namespace OpenNest.Engine.BestFit /// /// Saves and restores entries for .nest archives. Only kept /// results are exported; imported sets are added without replacing computed ones. + /// Each set records the canonical angle its pairs were computed in, and import drops sets + /// whose angle no longer matches the drawing's recomputed one. /// public sealed class BestFitCacheStore : IBestFitStore { + /// + /// Largest difference (radians) at which a saved canonical angle still matches. The + /// angle round-trips exactly, so this only absorbs floating-point noise in recomputing it. + /// + private const double FrameTolerance = 1e-9; + /// The store over the process-wide . public static BestFitCacheStore Shared { get; } = new(); @@ -50,7 +58,8 @@ namespace OpenNest.Engine.BestFit )) .ToList(); - sets.Add(new SavedPairSet(kvp.Key.PlateWidth, kvp.Key.PlateHeight, kvp.Key.Spacing, results)); + sets.Add(new SavedPairSet( + kvp.Key.PlateWidth, kvp.Key.PlateHeight, kvp.Key.Spacing, results, FrameAngle(drawing))); } return sets; @@ -64,7 +73,7 @@ namespace OpenNest.Engine.BestFit foreach (var set in sets) { - if (set.Results.Count == 0) + if (set.Results.Count == 0 || !MatchesFrame(set, drawing)) continue; canonical ??= CanonicalFrame.AsCanonicalCopy(drawing); @@ -95,5 +104,21 @@ namespace OpenNest.Engine.BestFit BestFitCache.Populate(drawing, set.PlateWidth, set.PlateHeight, set.Spacing, results); } } + + private static double FrameAngle(Drawing drawing) => drawing.Source?.Angle ?? 0.0; + + /// + /// True when the set was saved in the drawing's current canonical frame. A changed + /// canonical-angle computation would replay old pairs in a turned frame, where they can + /// overlap; sets with no saved angle (older files) are treated the same way. + /// + private static bool MatchesFrame(SavedPairSet set, Drawing drawing) + { + if (set.CanonicalAngle is not double saved || !double.IsFinite(saved)) + return false; + + var difference = System.Math.IEEERemainder(saved - FrameAngle(drawing), 2.0 * System.Math.PI); + return System.Math.Abs(difference) <= FrameTolerance; + } } } diff --git a/OpenNest.IO.Tests/BestFitStorePersistenceTests.cs b/OpenNest.IO.Tests/BestFitStorePersistenceTests.cs index 0cf0d5f8..f533c788 100644 --- a/OpenNest.IO.Tests/BestFitStorePersistenceTests.cs +++ b/OpenNest.IO.Tests/BestFitStorePersistenceTests.cs @@ -45,7 +45,7 @@ public class BestFitStorePersistenceTests [Fact] public void Read_WithStore_ImportsSavedSetsWithWireDefaults() { - var bytes = Save(CreateNest(), new RecordingStore { ToExport = [new SavedPairSet(60, 40, 0.25, [Result])] }); + var bytes = Save(CreateNest(), new RecordingStore { ToExport = [new SavedPairSet(60, 40, 0.25, [Result], 0.75)] }); var store = new RecordingStore(); var nest = new NestReader(new MemoryStream(bytes)) { BestFitStore = store }.Read(); @@ -54,12 +54,29 @@ public class BestFitStorePersistenceTests Assert.Same(nest.Drawings.Single(), imported.Drawing); var set = Assert.Single(imported.Sets); Assert.Equal((60.0, 40.0, 0.25), (set.PlateWidth, set.PlateHeight, set.Spacing)); + Assert.Equal(0.75, set.CanonicalAngle); var result = Assert.Single(set.Results); // A null reason and hull list are written as "" and []. Assert.Equal(Result with { Reason = "", HullAngles = result.HullAngles }, result); Assert.Empty(result.HullAngles); } + [Fact] + public void Read_SetWithoutCanonicalAngle_ImportsAnUnknownFrame() + { + // Files saved before sets recorded their frame; the store decides whether they are stale. + const string olderSets = """ + [{ "plateWidth": 60, "plateHeight": 40, "spacing": 0.25, "results": [] }] + """; + var bytes = AddEntry(Save(CreateNest(), store: null), "bestfits/bestfit-1", olderSets); + var store = new RecordingStore(); + + new NestReader(new MemoryStream(bytes)) { BestFitStore = store }.Read(); + + var set = Assert.Single(Assert.Single(store.Imported).Sets); + Assert.Null(set.CanonicalAngle); + } + [Fact] public void Read_WithoutStore_SkipsBestFitEntriesUnparsed() { diff --git a/OpenNest.IO/NestFormat.cs b/OpenNest.IO/NestFormat.cs index 97101877..ce8cbffb 100644 --- a/OpenNest.IO/NestFormat.cs +++ b/OpenNest.IO/NestFormat.cs @@ -726,6 +726,7 @@ namespace OpenNest.IO public double PlateWidth { get; init; } public double PlateHeight { get; init; } public double Spacing { get; init; } + public double? CanonicalAngle { get; init; } public List Results { get; init; } = new(); } diff --git a/OpenNest.IO/NestReader.cs b/OpenNest.IO/NestReader.cs index b109220f..67c82dbe 100644 --- a/OpenNest.IO/NestReader.cs +++ b/OpenNest.IO/NestReader.cs @@ -465,7 +465,8 @@ namespace OpenNest.IO r.Reason, r.TrueArea, r.HullAngles - )).ToList() + )).ToList(), + set.CanonicalAngle ); /// diff --git a/OpenNest.IO/NestWriter.cs b/OpenNest.IO/NestWriter.cs index 52358bb2..363dc1e1 100644 --- a/OpenNest.IO/NestWriter.cs +++ b/OpenNest.IO/NestWriter.cs @@ -460,6 +460,7 @@ namespace OpenNest.IO PlateWidth = set.PlateWidth, PlateHeight = set.PlateHeight, Spacing = set.Spacing, + CanonicalAngle = set.CanonicalAngle, Results = set .Results.Select(r => new BestFitResultDto { diff --git a/OpenNest.Tests/BestFit/BestFitCacheStoreTests.cs b/OpenNest.Tests/BestFit/BestFitCacheStoreTests.cs index 44bcd55e..f3ce506a 100644 --- a/OpenNest.Tests/BestFit/BestFitCacheStoreTests.cs +++ b/OpenNest.Tests/BestFit/BestFitCacheStoreTests.cs @@ -53,6 +53,7 @@ public class BestFitCacheStoreTests var set = Assert.Single(sets); Assert.Equal((60.0, 40.0, Spacing), (set.PlateWidth, set.PlateHeight, set.Spacing)); + Assert.Equal(drawing.Source.Angle, set.CanonicalAngle); var result = Assert.Single(set.Results); Assert.Equal( new SavedPairResult(0.5, System.Math.PI, 3.25, -1.5, 2, 7, Spacing, 90, 12, 7.5, 0.1, true, "Valid", 70, result.HullAngles), @@ -78,7 +79,8 @@ public class BestFitCacheStoreTests var drawing = MakeDrawing(); var saved = new SavedPairResult(0.5, System.Math.PI, 3.25, -1.5, 2, 7, Spacing, 90, 12, 7.5, 0.1, true, "Valid", 70, new[] { 0.25, -0.5 }); - BestFitCacheStore.Shared.Import(drawing, new[] { new SavedPairSet(60, 40, Spacing, new[] { saved }) }); + BestFitCacheStore.Shared.Import( + drawing, new[] { new SavedPairSet(60, 40, Spacing, new[] { saved }, drawing.Source.Angle) }); var restored = Assert.Single(BestFitCache.GetAllForDrawing(drawing)); Assert.Equal((60.0, 40.0, Spacing), restored.Key); @@ -111,7 +113,8 @@ public class BestFitCacheStoreTests BestFitCache.Populate(drawing, 60, 40, Spacing, existing); var saved = new SavedPairResult(0, 0, 1, 1, 0, 1, Spacing, 5, 1, 5, 0, true, "Valid", 4, new double[0]); - BestFitCacheStore.Shared.Import(drawing, new[] { new SavedPairSet(60, 40, Spacing, new[] { saved }) }); + BestFitCacheStore.Shared.Import( + drawing, new[] { new SavedPairSet(60, 40, Spacing, new[] { saved }, drawing.Source.Angle) }); Assert.Same(existing, BestFitCache.GetAllForDrawing(drawing)[(60, 40, Spacing)]); } @@ -122,8 +125,77 @@ public class BestFitCacheStoreTests BestFitCache.Clear(); var drawing = MakeDrawing(); - BestFitCacheStore.Shared.Import(drawing, new[] { new SavedPairSet(60, 40, Spacing, new SavedPairResult[0]) }); + BestFitCacheStore.Shared.Import( + drawing, new[] { new SavedPairSet(60, 40, Spacing, new SavedPairResult[0], drawing.Source.Angle) }); Assert.Empty(BestFitCache.GetAllForDrawing(drawing)); } + + [Fact] + public void Import_SetSavedUnderTheCurrentCanonicalAngle_IsKept() + { + BestFitCache.Clear(); + var drawing = MakeRotatedDrawing(); + BestFitCache.Populate(drawing, 60, 40, Spacing, new() { MakeResult(drawing, keep: true) }); + var sets = BestFitCacheStore.Shared.Export(drawing, new HashSet<(double, double, double)> { (60, 40, Spacing) }); + BestFitCache.Clear(); + + BestFitCacheStore.Shared.Import(drawing, sets); + + Assert.Single(Assert.Single(BestFitCache.GetAllForDrawing(drawing)).Value); + } + + [Fact] + public void Import_SetSavedUnderAnotherCanonicalAngle_IsDropped() + { + // A changed canonical-angle computation turns the drawing's frame; pairs saved in the + // old frame would be replayed turned and could overlap. + BestFitCache.Clear(); + var drawing = MakeRotatedDrawing(); + BestFitCache.Populate(drawing, 60, 40, Spacing, new() { MakeResult(drawing, keep: true) }); + var sets = BestFitCacheStore.Shared.Export(drawing, new HashSet<(double, double, double)> { (60, 40, Spacing) }); + BestFitCache.Clear(); + drawing.Source.Angle += 0.35; + + BestFitCacheStore.Shared.Import(drawing, sets); + + Assert.Empty(BestFitCache.GetAllForDrawing(drawing)); + } + + [Theory] + [InlineData(null)] + [InlineData(double.NaN)] + public void Import_SetWithoutAUsableCanonicalAngle_IsDropped(double? angle) + { + BestFitCache.Clear(); + var drawing = MakeDrawing(); + var saved = new SavedPairResult(0, 0, 1, 1, 0, 1, Spacing, 5, 1, 5, 0, true, "Valid", 4, new double[0]); + + BestFitCacheStore.Shared.Import(drawing, new[] { new SavedPairSet(60, 40, Spacing, new[] { saved }, angle) }); + + Assert.Empty(BestFitCache.GetAllForDrawing(drawing)); + } + + [Fact] + public void Import_AngleAWholeTurnAway_MatchesTheSameFrame() + { + BestFitCache.Clear(); + var drawing = MakeRotatedDrawing(); + var saved = new SavedPairResult(0, 0, 1, 1, 0, 1, Spacing, 5, 1, 5, 0, true, "Valid", 4, new double[0]); + + BestFitCacheStore.Shared.Import( + drawing, + new[] { new SavedPairSet(60, 40, Spacing, new[] { saved }, drawing.Source.Angle - 2 * System.Math.PI) }); + + Assert.Single(BestFitCache.GetAllForDrawing(drawing)); + } + + private static Drawing MakeRotatedDrawing() + { + var drawing = MakeDrawing(); + drawing.Program.Rotate(0.5, drawing.Program.BoundingBox().Center); + drawing.RecomputeCanonicalAngle(); + Assert.NotEqual(0.0, drawing.Source.Angle); + return drawing; + } } diff --git a/OpenNest.Tests/IO/BestFitPersistenceTests.cs b/OpenNest.Tests/IO/BestFitPersistenceTests.cs index f060029c..3c75a80c 100644 --- a/OpenNest.Tests/IO/BestFitPersistenceTests.cs +++ b/OpenNest.Tests/IO/BestFitPersistenceTests.cs @@ -2,6 +2,7 @@ using System.IO; using System.IO.Compression; using System.Linq; using System.Text; +using System.Text.Json.Nodes; using OpenNest.Engine; using OpenNest.Engine.BestFit; using OpenNest.Geometry; @@ -28,13 +29,15 @@ public class BestFitPersistenceTests // Narrow enough that the filter drops some computed pairs (the T-shape pairs are ~8 x 17). private const double NarrowWidth = 8.1; - // Captured from the writer before the best-fit mapping moved out of OpenNest.IO. + // Captured from the writer before the best-fit mapping moved out of OpenNest.IO, plus the + // canonical angle each set now records. private const string GoldenBestFitJson = """ [ { "plateWidth": 60, "plateHeight": 40, "spacing": 0.25, + "canonicalAngle": 1.5707963267948966, "results": [ { "part1Rotation": 0, @@ -170,6 +173,45 @@ public class BestFitPersistenceTests } } + [Fact] + public void Read_SetSavedUnderAnotherCanonicalAngle_IsDropped() + { + // As if saved before a change to the canonical-angle computation turned this drawing's + // frame: replaying those pairs in the new frame could overlap them. + BestFitCache.Clear(); + var nest = CreateNest(MakeRotatedTShape()); + var source = nest.Drawings.First(); + BestFitCache.GetOrCompute(source, UsedWidth, UsedLength, Spacing); + var bytes = RewriteBestFits(Save(nest), sets => + { + foreach (var set in sets) + set["canonicalAngle"] = source.Source.Angle + 0.35; + }); + + BestFitCache.Clear(); + var drawing = Load(bytes).Drawings.Single(); + + Assert.Empty(BestFitCache.GetAllForDrawing(drawing)); + } + + [Fact] + public void Read_SetFromAnOlderFileWithoutACanonicalAngle_IsDropped() + { + BestFitCache.Clear(); + var nest = CreateNest(MakeTShape()); + PopulateHandBuiltSets(nest.Drawings.First()); + var bytes = RewriteBestFits(Save(nest), sets => + { + foreach (var set in sets) + set.Remove("canonicalAngle"); + }); + + BestFitCache.Clear(); + var drawing = Load(bytes).Drawings.Single(); + + Assert.Empty(BestFitCache.GetAllForDrawing(drawing)); + } + [Fact] public void RoundTrip_NoBestFits_LoadsAndSavesUnchanged() { @@ -296,6 +338,26 @@ public class BestFitPersistenceTests private static Nest Load(byte[] bytes) => new NestReader(new MemoryStream(bytes)) { BestFitStore = BestFitCacheStore.Shared }.Read(); + /// Edits every saved set of drawing 1 in place, as an older or foreign writer might have saved it. + private static byte[] RewriteBestFits(byte[] bytes, System.Action> edit) + { + var entries = ReadEntries(bytes); + var sets = JsonNode.Parse(new MemoryStream(entries["bestfits/bestfit-1"]))!.AsArray(); + edit(sets.Select(n => n!.AsObject()).ToList()); + entries["bestfits/bestfit-1"] = Encoding.UTF8.GetBytes(sets.ToJsonString()); + + using var stream = new MemoryStream(); + using (var zip = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true)) + { + foreach (var (name, content) in entries) + { + using var entryStream = zip.CreateEntry(name).Open(); + entryStream.Write(content); + } + } + return stream.ToArray(); + } + private static System.Collections.Generic.Dictionary ReadEntries(byte[] bytes) { using var zip = new ZipArchive(new MemoryStream(bytes), ZipArchiveMode.Read); diff --git a/docs/nest-file-format.md b/docs/nest-file-format.md index 589fd978..a031d380 100644 --- a/docs/nest-file-format.md +++ b/docs/nest-file-format.md @@ -149,12 +149,21 @@ it now explicitly refreshes only the edited drawing's layout colors. Optional `bestfits/bestfit-N` entries hold drawing N's cached best-fit pair results: a JSON array of sets, each with `plateWidth`, `plateHeight` and -`spacing` (the plate size and part spacing it was filtered for) and `results`. -Each result has `part1Rotation`, `part2Rotation`, `part2OffsetX`, -`part2OffsetY`, `strategyType`, `testNumber`, `candidateSpacing`, -`rotatedArea`, `boundingWidth`, `boundingHeight`, `optimalRotation`, `keep`, -`reason`, `trueArea` and `hullAngles`. Angles are radians; pair geometry is in -the drawing's canonical frame. +`spacing` (the plate size and part spacing it was filtered for), +`canonicalAngle` and `results`. Each result has `part1Rotation`, +`part2Rotation`, `part2OffsetX`, `part2OffsetY`, `strategyType`, `testNumber`, +`candidateSpacing`, `rotatedArea`, `boundingWidth`, `boundingHeight`, +`optimalRotation`, `keep`, `reason`, `trueArea` and `hullAngles`. Angles are +radians; pair geometry is in the drawing's canonical frame. + +The canonical angle is not saved with the drawing: readers recompute it from +the program, so a change to that computation turns the frame of drawings saved +before it. `canonicalAngle` records the drawing's canonical angle when the set +was saved, and `BestFitCacheStore` drops a set on load unless it matches the +recomputed angle (within 1e-9 radians, modulo a full turn), since pairs replayed +in a turned frame can overlap. Sets from files saved before the field existed +have no angle and are dropped the same way; their pairs are recomputed when +next needed. The entries are only a cache: readers may ignore them and writers may omit them, at the cost of recomputing pairs. Writers save only sets whose