refactor(engine): one PlateNester and one strategy name table

DefaultPlateNester, StripPlateNester and RemnantPlateNester differed only
in the filler they built. They are replaced by one internal PlateNester
over a Func<Plate, PlateFiller>, which keeps the old Place exactly:
restricted rotation goes to OrderedPlateNester, and each trial gets a fresh
private plate and filler.

PlacementStrategies maps Fill (and its legacy "Default" alias), Strip and
both Remnants to a filler policy and layout. PlateNesterFactory reads it
case-sensitively and PlateFillService ignores case, as before; each keeps
its own error type and message. Nothing outside the Engine and its tests
used the three public nester types, so they are removed.

No layout changes: all 29 fill-stack pins pass with the pin file untouched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
ajandClaude Opus 5.5 committed 2026-10-25 23:44:51 -04:00
1 parent 5761eefd37
commit 5611d9a663
9 files changed
+164 -171

No files matched your search

+28 -2
View File
@@ -67,11 +67,37 @@ public class FillStrategyNameTests
Assert.Equal(6, parts.Count);
}
[Theory]
[InlineData("strip")]
[InlineData("HORIZONTAL REMNANT")]
[InlineData("default")]
public void FillServiceIgnoresCaseWhereTheJobFactoryDoesNot(string strategy)
{
var plate = new Plate(new Size(60, 80));
var drawing = new Drawing("part", TestDrawingFactory.Rectangle(6, 4));
var parts = PlateFillService.FillItem(strategy, plate,
new NestItem { Drawing = drawing, Quantity = 6 }, plate.WorkArea(), null, CancellationToken.None);
Assert.Equal(6, parts.Count);
Assert.Throws<NotSupportedException>(() => PlateNesterFactory.Create(strategy));
}
[Fact]
public void FillServiceRejectsUnknownNamesWithTheKnownList()
{
var error = Assert.Throws<NotSupportedException>(() => PlateFillService.FillItem("Mystery",
new Plate(new Size(60, 80)), new NestItem(), new Box(0, 0, 1, 1), null, CancellationToken.None));
Assert.Equal("Unknown placement strategy: Mystery. Known strategies: "
+ "Fill, Strip, Vertical Remnant, Horizontal Remnant.", error.Message);
}
[Fact]
public void WholeJobRunnerAcceptsFillAndItsLegacyName()
{
Assert.IsType<DefaultPlateNester>(PlateNesterFactory.Create("Fill"));
Assert.IsType<DefaultPlateNester>(PlateNesterFactory.Create("Default"));
Assert.IsType<PlateNester>(PlateNesterFactory.Create("Fill"));
Assert.IsType<PlateNester>(PlateNesterFactory.Create("Default"));
Assert.Equal("Fill", new NestJobOptions().PlacementStrategy);
var job = FiniteStockJobTests.Job(1);
@@ -85,7 +85,7 @@ public class NestJobCancellationTests
new[] { part },
new[] { new NestPlateStock("stock", new Size(20, 30), 1) }
);
var runner = new NestJobRunner(_ => new DefaultPlateNester(plate => new ReportingFiller(
var runner = new NestJobRunner(_ => new PlateNester(plate => new ReportingFiller(
plate
)));
@@ -1,6 +1,7 @@
using OpenNest.CNC;
using OpenNest.Engine.Jobs;
using OpenNest.Engine.Jobs.Placement;
using OpenNest.Engine.Jobs.Placement.Fillers;
using OpenNest.Geometry;
using Xunit;
@@ -59,7 +60,7 @@ public class PlateNesterParityTests
),
};
var result = Solve(new DefaultPlateNester(), Job(parts));
var result = Solve(PlateNesterFactory.Create("Fill"), Job(parts));
Assert.Equal(NestJobStatus.Complete, result.Status);
Assert.Equal(4, ByPart(result)["a"].Placed);
@@ -86,7 +87,7 @@ public class PlateNesterParityTests
),
};
var result = Solve(new StripPlateNester(), Job(parts, strategy: "Strip"));
var result = Solve(PlateNesterFactory.Create("Strip"), Job(parts, strategy: "Strip"));
Assert.Equal(NestJobStatus.Complete, result.Status);
Assert.Equal(4, ByPart(result)["a"].Placed);
@@ -97,13 +98,55 @@ public class PlateNesterParityTests
[Fact]
public void Builtins_AreResolvedByProductionFactory()
{
Assert.IsType<DefaultPlateNester>(PlateNesterFactory.Create("Fill"));
Assert.IsType<StripPlateNester>(PlateNesterFactory.Create("Strip"));
Assert.IsType<RemnantPlateNester>(PlateNesterFactory.Create("Vertical Remnant"));
Assert.IsType<RemnantPlateNester>(PlateNesterFactory.Create("Horizontal Remnant"));
Assert.IsType<PlateNester>(PlateNesterFactory.Create("Fill"));
Assert.IsType<PlateNester>(PlateNesterFactory.Create("Strip"));
Assert.IsType<PlateNester>(PlateNesterFactory.Create("Vertical Remnant"));
Assert.IsType<PlateNester>(PlateNesterFactory.Create("Horizontal Remnant"));
Assert.Throws<NotSupportedException>(() => PlateNesterFactory.Create("not registered"));
}
[Theory]
[InlineData("Fill", "Fill")]
[InlineData("Default", "Fill")]
[InlineData("Strip", "Strip")]
[InlineData("Vertical Remnant", "Vertical Remnant")]
[InlineData("Horizontal Remnant", "Horizontal Remnant")]
public void StrategyTable_MapsEachNameToItsFiller(string name, string canonical)
{
var entry = PlacementStrategies.Find(name, StringComparison.Ordinal);
Assert.NotNull(entry);
Assert.Equal(canonical, entry.Name);
var (policy, layout) = canonical switch
{
"Fill" => (PlateFillPolicy.Fill, PlateLayout.FillThenPack),
"Strip" => (PlateFillPolicy.Fill, PlateLayout.Strips),
"Vertical Remnant" => (PlateFillPolicy.VerticalRemnant, PlateLayout.FillThenPack),
_ => (PlateFillPolicy.HorizontalRemnant, PlateLayout.FillThenPack),
};
Assert.Same(policy, entry.Policy);
Assert.Equal(layout, entry.Layout);
}
[Fact]
public void StrategyTable_ListsTheBuiltInStrategiesInOrder() =>
Assert.Equal(
PlateFillService.BuiltInStrategies,
PlacementStrategies.Entries.Select(entry => entry.Name)
);
[Theory]
[InlineData("fill")]
[InlineData("default")]
[InlineData("STRIP")]
[InlineData("vertical remnant")]
[InlineData("")]
public void Factory_MatchesNamesCaseSensitively(string strategy)
{
var error = Assert.Throws<NotSupportedException>(() => PlateNesterFactory.Create(strategy));
Assert.Equal($"Unknown placement strategy: {strategy}.", error.Message);
}
[Fact]
public void AsymmetricPart_ValidAndFulfilled()
{
@@ -126,7 +169,7 @@ public class PlateNesterParityTests
2
),
};
var result = Solve(new DefaultPlateNester(), Job(parts));
var result = Solve(PlateNesterFactory.Create("Fill"), Job(parts));
Assert.Equal(NestJobStatus.Complete, result.Status);
Assert.Equal(3, ByPart(result)["l"].Placed);
@@ -162,7 +205,7 @@ public class PlateNesterParityTests
new NestJobPart("holed", PartGeometrySnapshot.FromProgram(holed), 2),
new NestJobPart("arc", PartGeometrySnapshot.FromProgram(arc), 2),
};
var result = Solve(new DefaultPlateNester(), Job(parts));
var result = Solve(PlateNesterFactory.Create("Fill"), Job(parts));
Assert.Equal(NestJobStatus.Complete, result.Status);
Assert.Equal(2, ByPart(result)["holed"].Placed);
@@ -194,7 +237,7 @@ public class PlateNesterParityTests
2,
rotation: RotationPolicy.Fixed(0)
);
var result = Solve(new DefaultPlateNester(), Job(new[] { part }));
var result = Solve(PlateNesterFactory.Create("Fill"), Job(new[] { part }));
Assert.Equal(NestJobStatus.Complete, result.Status);
var placements = result.Plates.SelectMany(p => p.Placements).ToList();
@@ -219,7 +262,7 @@ public class PlateNesterParityTests
2,
rotation: RotationPolicy.Fixed(0)
);
var nester = new DefaultPlateNester(_ =>
var nester = new PlateNester(_ =>
throw new InvalidOperationException("filler must not run for restricted rotation")
);
@@ -243,7 +286,7 @@ public class PlateNesterParityTests
2,
rotation: RotationPolicy.Fixed(0)
);
var nester = new RemnantPlateNester(_ =>
var nester = new PlateNester(_ =>
throw new InvalidOperationException("filler must not run for restricted rotation")
);
@@ -267,7 +310,7 @@ public class PlateNesterParityTests
2,
rotation: RotationPolicy.Fixed(0)
);
var nester = new StripPlateNester(_ =>
var nester = new PlateNester(_ =>
throw new InvalidOperationException("filler must not run for restricted rotation")
);
@@ -289,7 +332,7 @@ public class PlateNesterParityTests
new NestJobPart("first", PartGeometrySnapshot.FromProgram(program), 2),
new NestJobPart("second", PartGeometrySnapshot.FromProgram(program), 1),
};
var result = Solve(new DefaultPlateNester(), Job(parts));
var result = Solve(PlateNesterFactory.Create("Fill"), Job(parts));
Assert.Equal(NestJobStatus.Complete, result.Status);
Assert.Equal(2, ByPart(result)["first"].Placed);
@@ -311,7 +354,7 @@ public class PlateNesterParityTests
program.LineTo(12, 7);
var part = new NestJobPart("offset", PartGeometrySnapshot.FromProgram(program), 2);
var result = Solve(new DefaultPlateNester(), Job(new[] { part }));
var result = Solve(PlateNesterFactory.Create("Fill"), Job(new[] { part }));
Assert.Equal(NestJobStatus.Complete, result.Status);
Assert.Equal(2, ByPart(result)["offset"].Placed);
@@ -330,7 +373,7 @@ public class PlateNesterParityTests
5
);
var stock = new NestPlateStock("stock", new Size(30, 20), 3);
var nester = new DefaultPlateNester();
var nester = PlateNesterFactory.Create("Fill");
var result = new NestJobRunner(_ => nester).Solve(
new NestJob(new[] { part }, new[] { stock })
);
@@ -0,0 +1,49 @@
using System;
using System.Collections.Generic;
using OpenNest.Engine.Jobs.Placement.Fillers;
namespace OpenNest.Engine.Jobs.Placement;
/// <summary>
/// The built-in placement strategies by name: the filler policy and layout behind each one.
/// <see cref="PlateNesterFactory"/> (whole jobs) and <see cref="PlateFillService"/> (interactive
/// fills) both resolve names here, each with its own case rule.
/// </summary>
internal static class PlacementStrategies
{
/// <summary>Name of the Fill strategy in earlier releases; accepted as an alias.</summary>
private const string LegacyFillName = "Default";
/// <summary>One built-in strategy: its canonical name and the filler it runs.</summary>
internal sealed record Entry(string Name, PlateFillPolicy Policy, PlateLayout Layout)
{
public PlateFiller CreateFiller(Plate plate) => new(plate, Policy, Layout);
}
/// <summary>In <see cref="PlateFillService.BuiltInStrategies"/> order.</summary>
internal static IReadOnlyList<Entry> Entries { get; } =
[
new("Fill", PlateFillPolicy.Fill, PlateLayout.FillThenPack),
new("Strip", PlateFillPolicy.Fill, PlateLayout.Strips),
new("Vertical Remnant", PlateFillPolicy.VerticalRemnant, PlateLayout.FillThenPack),
new("Horizontal Remnant", PlateFillPolicy.HorizontalRemnant, PlateLayout.FillThenPack),
];
/// <summary>
/// Finds the strategy <paramref name="name"/> names under <paramref name="comparison"/>;
/// "Default", the Fill strategy's earlier name, finds Fill. Null when none matches.
/// </summary>
internal static Entry Find(string name, StringComparison comparison)
{
if (name == null)
return null;
if (name.Equals(LegacyFillName, comparison))
return Entries[0];
foreach (var entry in Entries)
{
if (entry.Name.Equals(name, comparison))
return entry;
}
return null;
}
}
@@ -20,9 +20,6 @@ public static class PlateFillService
/// <summary>Strategy used when a caller selection names none.</summary>
public const string DefaultStrategy = "Fill";
/// <summary>Name of the Fill strategy in earlier releases; accepted as an alias.</summary>
private const string LegacyFillName = "Default";
/// <summary>The four built-in strategy names, in registry display order.</summary>
public static IReadOnlyList<string> BuiltInStrategies { get; } =
[
@@ -414,30 +411,20 @@ public static class PlateFillService
);
}
if (strategy.Equals(LegacyFillName, StringComparison.OrdinalIgnoreCase))
return DefaultStrategy;
foreach (var candidate in BuiltInStrategies)
{
if (candidate.Equals(strategy, StringComparison.OrdinalIgnoreCase))
return candidate;
}
return Find(strategy).Name;
}
throw new NotSupportedException(
private static PlacementStrategies.Entry Find(string strategy) =>
PlacementStrategies.Find(strategy, StringComparison.OrdinalIgnoreCase)
?? throw new NotSupportedException(
$"Unknown placement strategy: {strategy}. Known strategies: {string.Join(", ", BuiltInStrategies)}."
);
}
/// <summary>Builds the filler for an optional strategy (null/empty = Fill).</summary>
private static PlateFiller CreateFiller(string strategy, Plate plate)
{
ArgumentNullException.ThrowIfNull(plate);
return ResolveStrategy(strategy) switch
{
"Fill" => new PlateFiller(plate),
"Strip" => new PlateFiller(plate, PlateFillPolicy.Fill, PlateLayout.Strips),
"Vertical Remnant" => new PlateFiller(plate, PlateFillPolicy.VerticalRemnant),
_ => new PlateFiller(plate, PlateFillPolicy.HorizontalRemnant),
};
return Find(ResolveStrategy(strategy)).CreateFiller(plate);
}
/// <summary>Public operations require an explicit strategy name (null is an argument error).</summary>
@@ -7,11 +7,11 @@ using OpenNest.Engine.Jobs.Placement.Fillers;
namespace OpenNest.Engine.Jobs.Placement;
/// <summary>
/// Built-in placement strategy for the whole-job runner. Fills each candidate trial with a
/// <see cref="PlateFiller"/> on a fresh private plate and owns its own run-scoped
/// bookkeeping: remaining demand is read from the request and placement counts are derived from
/// returned placements, so the filler's private <see cref="NestItem.Quantity"/> mutations never
/// feed back into job accounting.
/// Built-in placement strategy for the whole-job runner, for every entry in
/// <see cref="PlacementStrategies"/>. Fills each candidate trial with a <see cref="PlateFiller"/>
/// on a fresh private plate and owns its own run-scoped bookkeeping: remaining demand is read
/// from the request and placement counts are derived from returned placements, so the filler's
/// private <see cref="NestItem.Quantity"/> mutations never feed back into job accounting.
/// </summary>
/// <remarks>
/// The identity/progress boundary mechanics live in <see cref="CandidatePlacementContext"/>: one
@@ -19,19 +19,17 @@ namespace OpenNest.Engine.Jobs.Placement;
/// candidate trial (the runner reuses one <see cref="IPlateNester"/> instance per job). This is safe
/// because the fillers mutate <see cref="NestItem.Quantity"/> (per-trial) and canonical-frame copies,
/// never the shared <see cref="Drawing"/> or its <c>Quantity</c>. Identity is by Drawing reference,
/// never by name. Each trial still gets a fresh private <see cref="Plate"/>.
/// never by name. Each trial gets a fresh private <see cref="Plate"/> and filler, so learned angles
/// never cross trials.
/// </remarks>
public sealed class DefaultPlateNester : IPlateNester
internal sealed class PlateNester : IPlateNester
{
private readonly Func<Plate, PlateFiller> fillerFactory;
private readonly OrderedPlateNester restrictedRotationNester = new();
private readonly CandidatePlacementContext context = new();
public DefaultPlateNester()
: this(static plate => new PlateFiller(plate)) { }
/// <param name="fillerFactory">Injectable for tests; defaults to <see cref="PlateFiller"/>.</param>
internal DefaultPlateNester(Func<Plate, PlateFiller> fillerFactory)
/// <param name="fillerFactory">Builds the trial's filler; injectable for tests.</param>
internal PlateNester(Func<Plate, PlateFiller> fillerFactory)
{
this.fillerFactory =
fillerFactory ?? throw new ArgumentNullException(nameof(fillerFactory));
@@ -46,10 +44,11 @@ public sealed class DefaultPlateNester : IPlateNester
ArgumentNullException.ThrowIfNull(request);
token.ThrowIfCancellationRequested();
// The Default fill pipeline cannot express a locked or bounded rotation (start == end == 0
// reads as "unconstrained") and its Pairs/RectBestFit strategies rotate freely, so it can
// return poses the requirement's RotationPolicy forbids. Restricted requirements go to the
// policy-aware ordered nester, which only proposes allowed angles and validates each pose.
// No filler can express a locked or bounded rotation (start == end == 0 reads as
// "unconstrained"): the Pairs/RectBestFit strategies rotate freely and the strip fills
// try two classification angles, so they can return poses the requirement's
// RotationPolicy forbids. Restricted requirements go to the policy-aware ordered nester,
// which only proposes allowed angles and validates each pose.
if (request.Parts.Any(part => part.Rotation.Kind != RotationPolicyKind.Automatic))
return restrictedRotationNester.Place(request, progress, token);
@@ -1,40 +0,0 @@
using System;
using System.Threading;
using OpenNest.Engine.Jobs.Placement.Fillers;
namespace OpenNest.Engine.Jobs.Placement;
/// <summary>
/// Built-in remnant placement strategy for the whole-job runner. Fills one candidate per trial with
/// a <see cref="PlateFiller"/> with the vertical or horizontal remnant <see cref="PlateFillPolicy"/>. Placement is
/// <see cref="DefaultPlateNester"/>'s with that filler: the same fresh private plate per trial and
/// the same run-scoped identity, progress, and accounting boundaries.
/// </summary>
/// <remarks>
/// The remnant fillers share the Fill pipeline's automatic-rotation limitation, so non-automatic
/// requirements route through <see cref="OrderedPlateNester"/> exactly as they do for Fill.
/// </remarks>
public sealed class RemnantPlateNester : IPlateNester
{
private readonly DefaultPlateNester nester;
internal RemnantPlateNester(Func<Plate, PlateFiller> fillerFactory)
{
nester = new DefaultPlateNester(
fillerFactory ?? throw new ArgumentNullException(nameof(fillerFactory)));
}
/// <summary>Vertical-remnant policy: minimize X-extent, prefer horizontal placement.</summary>
internal static RemnantPlateNester Vertical() =>
new(plate => new PlateFiller(plate, PlateFillPolicy.VerticalRemnant));
/// <summary>Horizontal-remnant policy: minimize Y-extent, prefer vertical placement.</summary>
internal static RemnantPlateNester Horizontal() =>
new(plate => new PlateFiller(plate, PlateFillPolicy.HorizontalRemnant));
public PlateCandidate Place(
PlatePlacementRequest request,
IProgress<NestJobProgress> progress = null,
CancellationToken token = default
) => nester.Place(request, progress, token);
}
@@ -1,67 +0,0 @@
using System;
using System.Linq;
using System.Threading;
using OpenNest.Engine.Jobs.Adapters;
using OpenNest.Engine.Jobs.Placement.Fillers;
namespace OpenNest.Engine.Jobs.Placement;
/// <summary>
/// Built-in placement strategy for the whole-job runner. Runs a <see cref="PlateFiller"/> with the
/// <see cref="PlateLayout.Strips"/> layout (iterative shrink-fill strips, then a pack)
/// with the same run-scoped bookkeeping as
/// <see cref="DefaultPlateNester"/>: remaining demand is read from the request and placement counts
/// are derived from returned placements.
/// </summary>
/// <remarks>
/// The identity/progress boundary mechanics live in <see cref="CandidatePlacementContext"/>: a private
/// <see cref="Drawing"/> per requirement is created once per solve and reused across trials
/// (safe: the filler mutates per-trial <see cref="NestItem.Quantity"/> and canonical copies, never the
/// shared Drawing). Identity is by Drawing reference. Each trial gets a fresh private <see cref="Plate"/>.
/// </remarks>
public sealed class StripPlateNester : IPlateNester
{
private readonly Func<Plate, PlateFiller> fillerFactory;
private readonly OrderedPlateNester restrictedRotationNester = new();
private readonly CandidatePlacementContext context = new();
public StripPlateNester()
: this(static plate => new PlateFiller(plate, PlateFillPolicy.Fill, PlateLayout.Strips)) { }
/// <param name="fillerFactory">Injectable for tests; defaults to the strip-layout <see cref="PlateFiller"/>.</param>
internal StripPlateNester(Func<Plate, PlateFiller> fillerFactory)
{
this.fillerFactory =
fillerFactory ?? throw new ArgumentNullException(nameof(fillerFactory));
}
public PlateCandidate Place(
PlatePlacementRequest request,
IProgress<NestJobProgress> progress = null,
CancellationToken token = default
)
{
ArgumentNullException.ThrowIfNull(request);
token.ThrowIfCancellationRequested();
// The strip fills try two classification angles and ignore the requirement's rotation
// bounds, so they can return poses its RotationPolicy forbids. As in
// DefaultPlateNester, restricted requirements go to the policy-aware ordered nester.
if (request.Parts.Any(part => part.Rotation.Kind != RotationPolicyKind.Automatic))
return restrictedRotationNester.Place(request, progress, token);
var plate = DrawingJobMapper.CreatePlate(request.Stock);
var items = context.CreateItems(request.Parts);
var filler =
fillerFactory(plate)
?? throw new InvalidOperationException("Filler factory returned null.");
var legacyProgress = CandidateProgressBridge.Create(progress, request.Stock.Id);
var parts = filler.Nest(items, legacyProgress, token);
token.ThrowIfCancellationRequested();
if (parts == null)
throw new InvalidOperationException("Filler returned null placements.");
return new PlateCandidate(context.MapPlacements(parts));
}
}
+7 -11
View File
@@ -5,22 +5,18 @@ namespace OpenNest.Engine.Jobs;
/// <summary>
/// Instance-scoped strategy resolution for the whole-job runner. All four built-in strategies
/// resolve directly to filler-backed plate nesters. Selection is explicit per job; no shared
/// mutable strategy setting is read or modified. Unknown keys reject.
/// resolve to a filler-backed plate nester from <see cref="PlacementStrategies"/>. Selection is
/// explicit per job; no shared mutable strategy setting is read or modified. Names match
/// case-sensitively ("Default" is Fill's name in earlier releases); unknown keys reject.
/// </summary>
public static class PlateNesterFactory
{
public static IPlateNester Create(string strategy)
{
ArgumentNullException.ThrowIfNull(strategy);
return strategy switch
{
// "Default" is the strategy's name in earlier releases.
"Fill" or "Default" => new DefaultPlateNester(),
"Strip" => new StripPlateNester(),
"Vertical Remnant" => RemnantPlateNester.Vertical(),
"Horizontal Remnant" => RemnantPlateNester.Horizontal(),
_ => throw new NotSupportedException($"Unknown placement strategy: {strategy}."),
};
var entry =
PlacementStrategies.Find(strategy, StringComparison.Ordinal)
?? throw new NotSupportedException($"Unknown placement strategy: {strategy}.");
return new PlateNester(entry.CreateFiller);
}
}