From 073ead9b79cca33b13f0de7e00738025afec444a Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Mon, 21 Sep 2026 16:36:24 -0400 Subject: [PATCH] refactor(engine): share jobs placement identity and progress mechanics --- .../Jobs/CandidatePlacementContextTests.cs | 116 ++++++++++++++++++ .../Jobs/Adapters/LegacyPlateNesterAdapter.cs | 15 +-- .../Placement/CandidatePlacementContext.cs | 68 ++++++++++ .../Jobs/Placement/DefaultPlateNester.cs | 50 ++------ .../Jobs/Placement/StripPlateNester.cs | 46 +------ 5 files changed, 197 insertions(+), 98 deletions(-) create mode 100644 OpenNest.Engine.Tests/Jobs/CandidatePlacementContextTests.cs create mode 100644 OpenNest.Engine/Jobs/Placement/CandidatePlacementContext.cs diff --git a/OpenNest.Engine.Tests/Jobs/CandidatePlacementContextTests.cs b/OpenNest.Engine.Tests/Jobs/CandidatePlacementContextTests.cs new file mode 100644 index 0000000..4884cab --- /dev/null +++ b/OpenNest.Engine.Tests/Jobs/CandidatePlacementContextTests.cs @@ -0,0 +1,116 @@ +using OpenNest.CNC; +using OpenNest.Geometry; +using OpenNest.Engine.Jobs; +using OpenNest.Engine.Jobs.Placement; + +namespace OpenNest.Engine.Tests.Jobs; + +public class CandidatePlacementContextTests +{ + [Fact] + public void FreshItemsReusePrivateDrawingsAndReflectCurrentRequirement() + { + var context = new CandidatePlacementContext(); + var geometry = PartGeometrySnapshot.FromProgram(TestDrawingFactory.Rectangle(6, 4)); + + var first = Assert.Single( + context.CreateItems( + new[] + { + new NestJobPart( + "part", + geometry, + 3, + priority: 7, + rotation: RotationPolicy.BoundedSweep(0.1, 0.7, 0.2) + ), + } + ) + ); + var current = Assert.Single( + context.CreateItems( + new[] + { + new NestJobPart( + "part", + geometry, + 1, + priority: 4, + rotation: RotationPolicy.Fixed(0.5) + ), + } + ) + ); + + Assert.NotSame(first, current); + Assert.Same(first.Drawing, current.Drawing); + Assert.Equal(1, current.Quantity); + Assert.Equal(4, current.Priority); + Assert.Equal(OpenNest.Math.Angle.TwoPI, current.StepAngle); + Assert.Equal(0.5, current.RotationStart); + Assert.Equal(0.5, current.RotationEnd); + } + + [Fact] + public void PlacementMappingUsesPrivateDrawingIdentityAndRejectsInvalidParts() + { + var context = new CandidatePlacementContext(); + var items = context.CreateItems( + new[] + { + new NestJobPart( + "part", + PartGeometrySnapshot.FromProgram(TestDrawingFactory.Rectangle(6, 4)), + 1 + ), + } + ); + var placed = new Part(items[0].Drawing, new Vector(7, 11)); + placed.Rotate(0.3); + + var placement = Assert.Single(context.MapPlacements(new[] { placed })); + + Assert.Equal("part", placement.PartId); + Assert.Equal(0, placement.InstanceIndex); + // The mapping carries the part's committed pose; Rotate moves both program and location. + Assert.Equal(placed.Location.X, placement.X, 9); + Assert.Equal(placed.Location.Y, placement.Y, 9); + Assert.Equal(placed.Rotation, placement.Rotation, 9); + } + + [Fact] + public void NullPartIsRejected() + { + var context = new CandidatePlacementContext(); + Assert.Throws( + () => + { + _ = context.MapPlacements(new List { null! }); + } + ); + } + + [Fact] + public void ForeignDrawingWithKnownNameIsRejected() + { + var context = new CandidatePlacementContext(); + _ = context.CreateItems( + new[] + { + new NestJobPart( + "part", + PartGeometrySnapshot.FromProgram(TestDrawingFactory.Rectangle(6, 4)), + 1 + ), + } + ); + // Same requirement name, but a foreign Drawing instance: identity is by reference. + var foreign = new Part(new Drawing("part", TestDrawingFactory.Rectangle(6, 4))); + Assert.Throws( + () => + { + _ = context.MapPlacements(new[] { foreign }); + } + ); + } +} diff --git a/OpenNest.Engine/Jobs/Adapters/LegacyPlateNesterAdapter.cs b/OpenNest.Engine/Jobs/Adapters/LegacyPlateNesterAdapter.cs index 320f3b3..90c1403 100644 --- a/OpenNest.Engine/Jobs/Adapters/LegacyPlateNesterAdapter.cs +++ b/OpenNest.Engine/Jobs/Adapters/LegacyPlateNesterAdapter.cs @@ -53,8 +53,7 @@ public sealed class LegacyPlateNesterAdapter : IPlateNester var engine = engineFactory(plate) ?? throw new InvalidOperationException("Legacy engine factory returned null."); - var legacyProgress = - progress == null ? null : new LegacyProgress(progress, request.Stock.Id); + var legacyProgress = CandidateProgressBridge.Create(progress, request.Stock.Id); var parts = engine.Nest(items, legacyProgress, token); token.ThrowIfCancellationRequested(); if (parts == null) @@ -72,16 +71,4 @@ public sealed class LegacyPlateNesterAdapter : IPlateNester } return new PlateCandidate(placements); } - - private sealed class LegacyProgress(IProgress progress, string stockId) - : IProgress - { - public void Report(NestProgress value) - { - ArgumentNullException.ThrowIfNull(value); - progress.Report( - new NestJobProgress(NestJobStage.EvaluatingCandidate, stockId, -1, 0, 0, value) - ); - } - } } diff --git a/OpenNest.Engine/Jobs/Placement/CandidatePlacementContext.cs b/OpenNest.Engine/Jobs/Placement/CandidatePlacementContext.cs new file mode 100644 index 0000000..c6c336a --- /dev/null +++ b/OpenNest.Engine/Jobs/Placement/CandidatePlacementContext.cs @@ -0,0 +1,68 @@ +using System; +using System.Collections.Generic; + +using OpenNest.Engine.Jobs.Adapters; +namespace OpenNest.Engine.Jobs.Placement; + +/// +/// Owns the private Drawing-to-requirement identity map for one plate-nester run. +/// It creates fresh mutable legacy items for each candidate while only returned poses cross back +/// into the immutable jobs boundary. +/// +internal sealed class CandidatePlacementContext +{ + private readonly Dictionary drawingsById = new(StringComparer.Ordinal); + private readonly Dictionary idByDrawing = new( + ReferenceEqualityComparer.Instance + ); + + internal List CreateItems(IEnumerable requirements) + { + ArgumentNullException.ThrowIfNull(requirements); + + var items = new List(); + foreach (var requirement in requirements) + { + ArgumentNullException.ThrowIfNull(requirement); + if (!drawingsById.TryGetValue(requirement.Id, out var drawing)) + { + drawing = DrawingJobMapper.CreateDrawing(requirement); + drawingsById.Add(requirement.Id, drawing); + idByDrawing.Add(drawing, requirement.Id); + } + + items.Add( + new NestItem + { + Drawing = drawing, + Quantity = requirement.Quantity, + Priority = requirement.Priority, + StepAngle = DrawingJobMapper.LegacyStep(requirement.Rotation), + RotationStart = requirement.Rotation.Start, + RotationEnd = requirement.Rotation.End, + } + ); + } + + return items; + } + + internal List MapPlacements(IEnumerable parts) + { + ArgumentNullException.ThrowIfNull(parts); + + var placements = new List(); + foreach (var part in parts) + { + if (part?.BaseDrawing == null || !idByDrawing.TryGetValue(part.BaseDrawing, out var id)) + throw new InvalidOperationException( + "Placement does not reference a known requirement drawing." + ); + placements.Add( + new NestJobPlacement(id, 0, part.Location.X, part.Location.Y, part.Rotation) + ); + } + + return placements; + } +} diff --git a/OpenNest.Engine/Jobs/Placement/DefaultPlateNester.cs b/OpenNest.Engine/Jobs/Placement/DefaultPlateNester.cs index b5d0650..eedb3e7 100644 --- a/OpenNest.Engine/Jobs/Placement/DefaultPlateNester.cs +++ b/OpenNest.Engine/Jobs/Placement/DefaultPlateNester.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Threading; @@ -13,7 +12,8 @@ namespace OpenNest.Engine.Jobs.Placement; /// mutations never feed back into job accounting. /// /// -/// A private per requirement is created once per solve and reused across every +/// The identity/progress boundary mechanics live in : one +/// private per requirement is created once per solve and reused across every /// candidate trial (the runner reuses one instance per job). This is safe /// because the engines mutate (per-trial) and canonical-frame copies, /// never the shared or its Quantity. Identity is by Drawing reference, @@ -23,10 +23,7 @@ public sealed class DefaultPlateNester : IPlateNester { private readonly Func engineFactory; private readonly OrderedPlateNester restrictedRotationNester = new(); - private readonly Dictionary drawingsById = new(StringComparer.Ordinal); - private readonly Dictionary idByDrawing = new( - ReferenceEqualityComparer.Instance - ); + private readonly CandidatePlacementContext context = new(); public DefaultPlateNester() : this(static plate => new DefaultNestEngine(plate)) { } @@ -55,30 +52,9 @@ public sealed class DefaultPlateNester : IPlateNester return restrictedRotationNester.Place(request, progress, token); var plate = DrawingJobMapper.CreatePlate(request.Stock); - var items = new List(request.Parts.Count); - foreach (var requirement in request.Parts) - { - if (!drawingsById.TryGetValue(requirement.Id, out var drawing)) - { - drawing = DrawingJobMapper.CreateDrawing(requirement); - drawingsById.Add(requirement.Id, drawing); - idByDrawing.Add(drawing, requirement.Id); - } - - // Quantity is the request's remaining demand; the engine may mutate this per-trial item, - // and that mutation is deliberately discarded — placement counts come from the result. - items.Add( - new NestItem - { - Drawing = drawing, - Quantity = requirement.Quantity, - Priority = requirement.Priority, - StepAngle = DrawingJobMapper.LegacyStep(requirement.Rotation), - RotationStart = requirement.Rotation.Start, - RotationEnd = requirement.Rotation.End, - } - ); - } + // Quantity is the request's remaining demand; the engine may mutate these per-trial items, + // and that mutation is deliberately discarded — placement counts come from the result. + var items = context.CreateItems(request.Parts); var engine = engineFactory(plate) @@ -89,18 +65,6 @@ public sealed class DefaultPlateNester : IPlateNester if (parts == null) throw new InvalidOperationException("Engine returned null placements."); - var placements = new List(parts.Count); - foreach (var part in parts) - { - if (part?.BaseDrawing == null || !idByDrawing.TryGetValue(part.BaseDrawing, out var id)) - throw new InvalidOperationException( - "Placement does not reference a known requirement drawing." - ); - placements.Add( - new NestJobPlacement(id, 0, part.Location.X, part.Location.Y, part.Rotation) - ); - } - - return new PlateCandidate(placements); + return new PlateCandidate(context.MapPlacements(parts)); } } diff --git a/OpenNest.Engine/Jobs/Placement/StripPlateNester.cs b/OpenNest.Engine/Jobs/Placement/StripPlateNester.cs index 5794da5..73b4ba3 100644 --- a/OpenNest.Engine/Jobs/Placement/StripPlateNester.cs +++ b/OpenNest.Engine/Jobs/Placement/StripPlateNester.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Threading; using OpenNest.Engine.Jobs.Adapters; @@ -11,17 +10,15 @@ namespace OpenNest.Engine.Jobs.Placement; /// remaining demand is read from the request and placement counts are derived from returned placements. /// /// -/// A private per requirement is created once per solve and reused across trials +/// The identity/progress boundary mechanics live in : a private +/// per requirement is created once per solve and reused across trials /// (safe: the engine mutates per-trial and canonical copies, never the /// shared Drawing). Identity is by Drawing reference. Each trial gets a fresh private . /// public sealed class StripPlateNester : IPlateNester { private readonly Func engineFactory; - private readonly Dictionary drawingsById = new(StringComparer.Ordinal); - private readonly Dictionary idByDrawing = new( - ReferenceEqualityComparer.Instance - ); + private readonly CandidatePlacementContext context = new(); public StripPlateNester() : this(static plate => new StripNestEngine(plate)) { } @@ -43,28 +40,7 @@ public sealed class StripPlateNester : IPlateNester token.ThrowIfCancellationRequested(); var plate = DrawingJobMapper.CreatePlate(request.Stock); - var items = new List(request.Parts.Count); - foreach (var requirement in request.Parts) - { - if (!drawingsById.TryGetValue(requirement.Id, out var drawing)) - { - drawing = DrawingJobMapper.CreateDrawing(requirement); - drawingsById.Add(requirement.Id, drawing); - idByDrawing.Add(drawing, requirement.Id); - } - - items.Add( - new NestItem - { - Drawing = drawing, - Quantity = requirement.Quantity, - Priority = requirement.Priority, - StepAngle = DrawingJobMapper.LegacyStep(requirement.Rotation), - RotationStart = requirement.Rotation.Start, - RotationEnd = requirement.Rotation.End, - } - ); - } + var items = context.CreateItems(request.Parts); var engine = engineFactory(plate) @@ -75,18 +51,6 @@ public sealed class StripPlateNester : IPlateNester if (parts == null) throw new InvalidOperationException("Engine returned null placements."); - var placements = new List(parts.Count); - foreach (var part in parts) - { - if (part?.BaseDrawing == null || !idByDrawing.TryGetValue(part.BaseDrawing, out var id)) - throw new InvalidOperationException( - "Placement does not reference a known requirement drawing." - ); - placements.Add( - new NestJobPlacement(id, 0, part.Location.X, part.Location.Y, part.Rotation) - ); - } - - return new PlateCandidate(placements); + return new PlateCandidate(context.MapPlacements(parts)); } }