diff --git a/OpenNest.Engine.Tests/Jobs/NestJobEngineSelectionTests.cs b/OpenNest.Engine.Tests/Jobs/NestJobEngineSelectionTests.cs new file mode 100644 index 0000000..e15fd1d --- /dev/null +++ b/OpenNest.Engine.Tests/Jobs/NestJobEngineSelectionTests.cs @@ -0,0 +1,86 @@ +using OpenNest.CNC; +using OpenNest.Geometry; + +namespace OpenNest.Engine.Tests.Jobs; + +/// +/// Strategy selection must be instance-scoped: explicit engine choices work without touching the +/// process-global NestEngineRegistry.ActiveEngineName, and unknown strategies are rejected. +/// +public class NestJobEngineSelectionTests +{ + [Fact] + public void ExplicitDefaultAndStripSelectionsDoNotTouchGlobalRegistry() + { + var original = NestEngineRegistry.ActiveEngineName; + var job = FiniteStockJobTests.Job(1); + + var defaultResult = new NestJobRunner(PlateNesterFactory.Create).Solve(job); + Assert.Equal(NestJobStatus.Complete, defaultResult.Status); + + var stripResult = new NestJobRunner(PlateNesterFactory.Create) + .Solve(new NestJob(job.Parts, job.Plates, new NestJobOptions("Strip"))); + Assert.Equal(NestJobStatus.Complete, stripResult.Status); + + Assert.Equal(original, NestEngineRegistry.ActiveEngineName); + } + + [Fact] + public void FactoryResolvesNamedEnginesWithoutGlobalState() + { + var original = NestEngineRegistry.ActiveEngineName; + var defaultNester = PlateNesterFactory.Create("Default"); + var stripNester = PlateNesterFactory.Create("Strip"); + var verticalNester = PlateNesterFactory.Create("Vertical Remnant"); + var horizontalNester = PlateNesterFactory.Create("Horizontal Remnant"); + + Assert.NotNull(defaultNester); + Assert.NotNull(stripNester); + Assert.NotNull(verticalNester); + Assert.NotNull(horizontalNester); + Assert.NotSame(defaultNester, stripNester); + Assert.Equal(original, NestEngineRegistry.ActiveEngineName); + } + + [Fact] + public void UnknownStrategyIsRejected() + { + Assert.Throws(() => PlateNesterFactory.Create("Not A Real Engine")); + var job = FiniteStockJobTests.Job(1); + Assert.Throws(() => + new NestJobRunner(key => throw new NotSupportedException($"Unknown placement strategy: {key}")) + .Solve(new NestJob(job.Parts, job.Plates, new NestJobOptions("Bogus")))); + } + + [Fact] + public void LegacyRegistryPluginsDoNotLeakIntoJobSelection() + { + // A plugin engine registered through the legacy registry must not become selectable + // through the job factory; the new boundary is independent of registry state. + NestEngineRegistry.Register("ProbePlugin", "test plugin", plate => new PluginShapeEngine(plate)); + Assert.Contains(NestEngineRegistry.AvailableEngines, e => e.Name == "ProbePlugin"); + + Assert.Throws(() => PlateNesterFactory.Create("ProbePlugin")); + Assert.NotNull(PlateNesterFactory.Create("Default")); + } + + [Fact] + public void StripEngineEndToEndPlacesAndAccounts() + { + var drawing = new Drawing("strip part", TestDrawingFactory.Rectangle(30, 30)); + var job = new NestJob(new[] { DrawingJobMapper.FromDrawing("part", drawing, 2) }, + new[] { new NestPlateStock("s", new Size(90, 90), 1) }); + var result = new NestJobRunner(PlateNesterFactory.Create) + .Solve(new NestJob(job.Parts, job.Plates, new NestJobOptions("Strip"))); + + Assert.True(result.Plates.SelectMany(p => p.Placements).Count() >= 1); + foreach (var f in result.Fulfillment) + Assert.Equal(f.Requested, f.Placed + f.Unplaced); + } + + private sealed class PluginShapeEngine(Plate plate) : NestEngineBase(plate) + { + public override string Name => "ProbePlugin"; + public override string Description => "registered via legacy registry only"; + } +} diff --git a/OpenNest.Engine.Tests/Jobs/NestJobIdentityTests.cs b/OpenNest.Engine.Tests/Jobs/NestJobIdentityTests.cs new file mode 100644 index 0000000..568508c --- /dev/null +++ b/OpenNest.Engine.Tests/Jobs/NestJobIdentityTests.cs @@ -0,0 +1,149 @@ +using OpenNest.CNC; +using OpenNest.Engine.Fill; +using OpenNest.Geometry; + +namespace OpenNest.Engine.Tests.Jobs; + +/// +/// Names are never identity: two distinct drawings that share a display name must keep +/// independent quantities, and two requirements that share one source drawing must not +/// cross-count each other's placements through the legacy engine paths. +/// +public class NestJobIdentityTests +{ + [Fact] + public void DistinctDrawingsWithSameNameKeepIndependentQuantitiesInRealEngine() + { + var a = new Drawing("identical", TestDrawingFactory.Rectangle(40, 40)); + var b = new Drawing("identical", TestDrawingFactory.Rectangle(40, 40)); + var job = new NestJob(new[] + { + DrawingJobMapper.FromDrawing("a", a, 2), + DrawingJobMapper.FromDrawing("b", b, 2) + }, new[] { new NestPlateStock("s", new Size(90, 90), 1) }); + + var result = new NestJobRunner(LegacyPlateNesterAdapter.Create).Solve(job); + + // Every placed part maps to a known requirement ID; no part is invented or cross-counted. + Assert.True(result.Plates.SelectMany(p => p.Placements).All(p => p.PartId is "a" or "b")); + var counts = result.Plates.SelectMany(p => p.Placements).GroupBy(p => p.PartId) + .ToDictionary(g => g.Key, g => g.Count()); + foreach (var (id, placed) in counts) + Assert.True(placed <= 2, $"Requirement {id} placed {placed} > requested 2"); + + // Fulfillment conservation for both IDs. + foreach (var f in result.Fulfillment) + Assert.Equal(f.Requested, f.Placed + f.Unplaced); + } + + [Fact] + public void TwoRequirementsOnSameSourceDrawingKeepIndependentQuantities() + { + var source = new Drawing("shared", TestDrawingFactory.Rectangle(30, 30)); + var job = new NestJob(new[] + { + DrawingJobMapper.FromDrawing("first", source, 2), + DrawingJobMapper.FromDrawing("second", source, 2) + }, new[] { new NestPlateStock("s", new Size(90, 90), 1) }); + + var result = new NestJobRunner(LegacyPlateNesterAdapter.Create).Solve(job); + + Assert.Equal(new[] { "first", "second" }, result.Fulfillment.Select(f => f.PartId)); + foreach (var f in result.Fulfillment) + Assert.Equal(f.Requested, f.Placed + f.Unplaced); + + // Output drawings are distinct even though the input is the same Drawing instance. + var output = NestResultMaterializer.Materialize(job, result); + Assert.NotSame(output.DrawingsByPartId["first"], output.DrawingsByPartId["second"]); + // Caller source is untouched. + Assert.Equal(0, source.Quantity.Nested); + } + + [Fact] + public void EngineDeductionCountsByDrawingReferenceNotName() + { + // Plate 90x40 fits exactly two 40x40 parts. The engine fills item A with both and + // starves item B. Name-based deduction would then zero BOTH items (the two placed + // parts carry the shared name, so each item counts 2 as "its own"). Reference-based + // deduction leaves B at 2. + var a = new Drawing("dup", TestDrawingFactory.Rectangle(40, 40)); + var b = new Drawing("dup", TestDrawingFactory.Rectangle(40, 40)); + var plate = new Plate(new Size(90, 40)); + var items = new List + { + new() { Drawing = a, Quantity = 2 }, + new() { Drawing = b, Quantity = 2 } + }; + // Place exactly 2 parts from item A and none from item B, then run the base-class + // deduction. Deterministic regardless of any fill heuristic. + var placed = new StarvingProbe(plate).Nest(items, null, default); + + var aPlaced = placed.Count(p => ReferenceEquals(p.BaseDrawing, a)); + var bPlaced = placed.Count(p => ReferenceEquals(p.BaseDrawing, b)); + Assert.Equal(2, placed.Count); + Assert.Equal(2, aPlaced); + Assert.Equal(0, bPlaced); + + // Invariant: remaining = requested - own placements. Under name-based counting, + // both items would read 0 here because the two placed parts match the shared name. + Assert.Equal(0, items[0].Quantity); + Assert.Equal(2, items[1].Quantity); + } + + [Fact] + public void SameNameSinglesAreBothReturnedByPackPhase() + { + var a = new Drawing("samesingle", TestDrawingFactory.Rectangle(30, 30)); + var b = new Drawing("samesingle", TestDrawingFactory.Rectangle(30, 30)); + var plate = new Plate(new Size(100, 100)); + var items = new List + { + new() { Drawing = a, Quantity = 1 }, + new() { Drawing = b, Quantity = 1 } + }; + var placed = new BaseNestEngineProbe(plate).Nest(items, null, default); + Assert.Equal(2, placed.Count); + Assert.Equal(new[] { 0, 0 }, new[] { items[0].Quantity, items[1].Quantity }); + } + + private sealed class BaseNestEngineProbe(Plate plate) : NestEngineBase(plate) + { + public override string Name => "probe"; + public override string Description => "probe"; + public override List Fill(NestItem item, Box workArea, + IProgress progress, CancellationToken token) + => new DefaultNestEngine(Plate).Fill(item, workArea, progress, token); + public override List Fill(List groupParts, Box workArea, + IProgress progress, CancellationToken token) + => new DefaultNestEngine(Plate).Fill(groupParts, workArea, progress, token); + public override List PackArea(Box box, List items, + IProgress progress, CancellationToken token) + => new DefaultNestEngine(Plate).PackArea(box, items, progress, token); + } + + /// Places exactly 2 parts from the first multi-quantity item and none from the + /// rest, forcing the base-class deduction to run on an asymmetric placement result. + private sealed class StarvingProbe(Plate plate) : NestEngineBase(plate) + { + private int _first = -1; + public override string Name => "starving"; + public override string Description => "starves all but the first fill item"; + public override List Fill(NestItem item, Box workArea, + IProgress progress, CancellationToken token) + { + if (_first < 0) _first = 1; + if (_first++ != 1) + return new List(); + var parts = new List(); + var x = 0.0; + for (var i = 0; i < 2; i++) + { + var p = new Part(item.Drawing); + p.Offset(new Vector(x, 0)); + x += item.Drawing.Program.BoundingBox().Width + Plate.PartSpacing; + parts.Add(p); + } + return parts; + } + } +} diff --git a/OpenNest.Engine/Fill/IterativeShrinkFiller.cs b/OpenNest.Engine/Fill/IterativeShrinkFiller.cs index a7ed1cf..790148c 100644 --- a/OpenNest.Engine/Fill/IterativeShrinkFiller.cs +++ b/OpenNest.Engine/Fill/IterativeShrinkFiller.cs @@ -1,6 +1,7 @@ using OpenNest.Geometry; using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -130,17 +131,13 @@ namespace OpenNest.Engine.Fill var placed = filler.FillItems(workItems, shrinkWrapper, token); - // Build leftovers: compare placed count to original quantities. - // RemnantFiller.FillItems does NOT mutate NestItem.Quantity. + // Build leftovers: compare placed count to original quantities by drawing + // reference. RemnantFiller.FillItems does NOT mutate NestItem.Quantity. var leftovers = new List(); foreach (var item in items) { - var placedCount = 0; - foreach (var p in placed) - { - if (p.BaseDrawing.Name == item.Drawing.Name) - placedCount++; - } + var placedCount = placed.Count(p => + ReferenceEquals(p.BaseDrawing, item.Drawing)); if (item.Quantity <= 0) continue; // unlimited items are always "satisfied" — no leftover diff --git a/OpenNest.Engine/Fill/RemnantFiller.cs b/OpenNest.Engine/Fill/RemnantFiller.cs index c6eb110..35015d1 100644 --- a/OpenNest.Engine/Fill/RemnantFiller.cs +++ b/OpenNest.Engine/Fill/RemnantFiller.cs @@ -58,20 +58,20 @@ namespace OpenNest.Engine.Fill return allParts; } - private static Dictionary BuildLocalQuantities(List items) + private static Dictionary BuildLocalQuantities(List items) { - var localQty = new Dictionary(items.Count); + var localQty = new Dictionary(items.Count, ReferenceEqualityComparer.Instance); foreach (var item in items) - localQty[item.Drawing.Name] = item.Quantity; + localQty[item.Drawing] = item.Quantity; return localQty; } - private static double FindMinItemDimension(List items, Dictionary localQty) + private static double FindMinItemDimension(List items, Dictionary localQty) { var minDim = double.MaxValue; foreach (var item in items) { - if (localQty[item.Drawing.Name] <= 0) + if (localQty[item.Drawing] <= 0) continue; var bb = item.Drawing.Program.BoundingBox(); var dim = System.Math.Min(bb.Width, bb.Length); @@ -84,7 +84,7 @@ namespace OpenNest.Engine.Fill private bool TryFillOneItem( List items, List freeBoxes, - Dictionary localQty, + Dictionary localQty, Func> fillFunc, List allParts, CancellationToken token) @@ -94,7 +94,7 @@ namespace OpenNest.Engine.Fill if (token.IsCancellationRequested) return false; - var qty = localQty[item.Drawing.Name]; + var qty = localQty[item.Drawing]; if (qty <= 0) continue; @@ -110,7 +110,7 @@ namespace OpenNest.Engine.Fill RemoveTopmostPart(placed); allParts.AddRange(placed); - localQty[item.Drawing.Name] = System.Math.Max(0, qty - placed.Count); + localQty[item.Drawing] = System.Math.Max(0, qty - placed.Count); // Add the envelope of all placed parts as a single obstacle // rather than individual bounding boxes, preventing the diff --git a/OpenNest.Engine/Jobs/PlateNesterFactory.cs b/OpenNest.Engine/Jobs/PlateNesterFactory.cs new file mode 100644 index 0000000..c60b1be --- /dev/null +++ b/OpenNest.Engine/Jobs/PlateNesterFactory.cs @@ -0,0 +1,23 @@ +using System; +namespace OpenNest; + +/// +/// Instance-scoped strategy resolution for the whole-job runner. The built-in strategies map +/// to private engine factories; the process-global NestEngineRegistry (including plugin +/// registrations and ActiveEngineName) is neither read nor modified. Unknown keys reject. +/// +public static class PlateNesterFactory +{ + public static IPlateNester Create(string strategy) + { + ArgumentNullException.ThrowIfNull(strategy); + return strategy switch + { + "Default" => new LegacyPlateNesterAdapter(plate => new DefaultNestEngine(plate)), + "Strip" => new LegacyPlateNesterAdapter(plate => new StripNestEngine(plate)), + "Vertical Remnant" => new LegacyPlateNesterAdapter(plate => new VerticalRemnantEngine(plate)), + "Horizontal Remnant" => new LegacyPlateNesterAdapter(plate => new HorizontalRemnantEngine(plate)), + _ => throw new NotSupportedException($"Unknown placement strategy: {strategy}.") + }; + } +} diff --git a/OpenNest.Engine/NestEngineBase.cs b/OpenNest.Engine/NestEngineBase.cs index 54bf64c..a79503e 100644 --- a/OpenNest.Engine/NestEngineBase.cs +++ b/OpenNest.Engine/NestEngineBase.cs @@ -113,11 +113,11 @@ namespace OpenNest { allParts.AddRange(fillParts); - // Deduct placed quantities + // Deduct placed quantities by drawing reference, not name. foreach (var item in fillItems) { var placed = fillParts.Count(p => - p.BaseDrawing.Name == item.Drawing.Name); + ReferenceEquals(p.BaseDrawing, item.Drawing)); item.Quantity = System.Math.Max(0, item.Quantity - placed); } @@ -147,10 +147,11 @@ namespace OpenNest { allParts.AddRange(packParts); + // Deduct placed quantities by drawing reference, not name. foreach (var item in regularPackItems) { var placed = packParts.Count(p => - p.BaseDrawing.Name == item.Drawing.Name); + ReferenceEquals(p.BaseDrawing, item.Drawing)); item.Quantity = System.Math.Max(0, item.Quantity - placed); } } diff --git a/OpenNest.Engine/StripNestEngine.cs b/OpenNest.Engine/StripNestEngine.cs index cec9ed0..d25cb57 100644 --- a/OpenNest.Engine/StripNestEngine.cs +++ b/OpenNest.Engine/StripNestEngine.cs @@ -128,13 +128,14 @@ namespace OpenNest } } - // Deduct placed quantities from original items. + // Deduct placed quantities from original items by drawing reference. foreach (var item in items) { if (item.Quantity <= 0) continue; - var placed = allParts.Count(p => p.BaseDrawing.Name == item.Drawing.Name); + var placed = allParts.Count(p => + ReferenceEquals(p.BaseDrawing, item.Drawing)); item.Quantity = System.Math.Max(0, item.Quantity - placed); } diff --git a/README.md b/README.md index 83c63fa..7b23baf 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ The new whole-job contracts in `OpenNest.Engine/Jobs` (`namespace OpenNest`) use **Current scope (minimum runnable slice, not release-ready):** `NestJobRunner.Solve` allocates one stock entry across multiple physical sheets, respecting finite inventory (`null` is unlimited), positive remaining demand, and `MaxPlates`. Empty parts complete without consuming stock; empty/unavailable stock returns `Incomplete/StockExhausted`. A zero-placement candidate stops with `NoPlacementFound` and consumes no sheet. Nonempty jobs with multiple stock entries explicitly throw `NotSupportedException`; mixed-stock selection and optional optimizations are not implemented. -`DrawingJobMapper` snapshots caller drawings/items under explicit requirement IDs. `LegacyPlateNesterAdapter` creates fresh private legacy drawings, items, and plates for each trial and maps returned drawings **by reference**, never by name. Mutable legacy quantities never drive the fulfillment ledger. The minimal built-in factory accepts only `Default`; injected factories must explicitly reject unknown keys or return null. `NestResultMaterializer` returns a detached domain nest and `DrawingsByPartId` identity map. Each output plate represents one physical sheet (`Quantity = 1`), and each placement is attached exactly once so domain quantity events do not double count. +`DrawingJobMapper` snapshots caller drawings/items under explicit requirement IDs. `LegacyPlateNesterAdapter` creates fresh private legacy drawings, items, and plates for each trial and maps returned drawings **by reference**, never by name. Mutable legacy quantities never drive the fulfillment ledger. `PlateNesterFactory` resolves the built-in strategies (`Default`, `Strip`, `Vertical Remnant`, `Horizontal Remnant`) to private engine factories; it neither reads nor changes the process-global `NestEngineRegistry`, and unknown keys reject. Quantity deduction in the engine paths the runner reaches (base-class fill/pack, strip deduction, remnant-fill ledger, shrink-leftover counting) is keyed by drawing reference, not display name, so same-name drawings and repeated requirements stay independent. `NestResultMaterializer` returns a detached domain nest and `DrawingsByPartId` identity map. Each output plate represents one physical sheet (`Quantity = 1`), and each placement is attached exactly once so domain quantity events do not double count. ```csharp var job = new NestJob(