refactor(engine): list the built-in fill strategies instead of scanning

FillStrategyRegistry now holds a fixed, ordered list of the seven
built-in strategies (Pairs, Row, Column, RectBestFit, Extents, Linear,
TiltedRemnant, in their Order sequence) instead of reflecting over the
Engine assembly. LoadFrom and LoadPlugins are gone; nothing outside
tests called them, so outside IFillStrategy plug-ins were never loaded
by a shipped host and none are loaded now. IFillStrategy stays public.

The disabled set stays process-wide and keeps its Disable, Enable,
SetEnabled, Strategies, AllStrategies and DisabledNames behavior for the
Options form. DefaultPlateFiller still reads Strategies, so interactive
Fill, the fixed-strategy engines, Irregular's block catalog and private
plate fill, the cutout lattice fill and the brute-force runner all keep
honoring it.

PlateViewFillLifetimeTests drives fills through PlateView, which builds
its own filler, so a filler-level seam can't reach it. The registry gets
an internal UseForTesting hook that swaps the list until disposed, and
the WinForms test project can see Engine internals for it.

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:50 -04:00
1 parent 73a7c8d6a2
commit 0c7ad5aefb
4 files changed
+56 -102

No files matched your search

+1
View File
@@ -8,6 +8,7 @@
<InternalsVisibleTo Include="OpenNest.Tests" />
<InternalsVisibleTo Include="OpenNest.Benchmark" />
<InternalsVisibleTo Include="OpenNest.Engine.Tests" />
<InternalsVisibleTo Include="OpenNest.WinForms.Tests" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\OpenNest.Core\OpenNest.Core.csproj" />
@@ -1,31 +1,40 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Reflection;
namespace OpenNest.Engine.Strategies
{
/// <summary>
/// The built-in fill strategies in pipeline order, with one process-wide disabled set that the
/// desktop Options form sets. Every fill that runs the strategy pipeline reads it.
/// </summary>
public static class FillStrategyRegistry
{
private static readonly List<IFillStrategy> strategies = new();
// Pipeline order, ascending by each strategy's Order value.
private static readonly IReadOnlyList<IFillStrategy> builtIn = Array.AsReadOnly(
new IFillStrategy[]
{
new PairsFillStrategy(),
new RowFillStrategy(),
new ColumnFillStrategy(),
new RectBestFitStrategy(),
new ExtentsFillStrategy(),
new LinearFillStrategy(),
new TiltedRemnantStrategy(),
}
);
private static IReadOnlyList<IFillStrategy> strategies = builtIn;
private static List<IFillStrategy> sorted;
private static HashSet<string> enabledFilter;
private static readonly HashSet<string> disabled = new(StringComparer.OrdinalIgnoreCase);
static FillStrategyRegistry()
{
LoadFrom(typeof(FillStrategyRegistry).Assembly);
}
public static IReadOnlyList<IFillStrategy> Strategies => sorted ??= FilterStrategies();
/// <summary>
/// Returns all registered strategies regardless of enabled/disabled state.
/// Returns all strategies regardless of enabled/disabled state.
/// </summary>
public static IReadOnlyList<IFillStrategy> AllStrategies =>
strategies.OrderBy(s => s.Order).ToList();
public static IReadOnlyList<IFillStrategy> AllStrategies => strategies;
/// <summary>
/// Returns the names of all permanently disabled strategies.
@@ -38,11 +47,11 @@ namespace OpenNest.Engine.Strategies
enabledFilter != null
? strategies.Where(s => enabledFilter.Contains(s.Name))
: strategies.Where(s => !disabled.Contains(s.Name));
return source.OrderBy(s => s.Order).ToList();
return source.ToList();
}
/// <summary>
/// Permanently disables strategies by name. They remain registered
/// Permanently disables strategies by name. They remain listed
/// but are excluded from the default pipeline.
/// </summary>
public static void Disable(params string[] names)
@@ -75,79 +84,31 @@ namespace OpenNest.Engine.Strategies
sorted = null;
}
public static void LoadFrom(Assembly assembly)
/// <summary>
/// Replaces the built-in list, in the given order, until the result is disposed. For tests
/// that drive fills through a host that builds its own filler, such as the plate view.
/// </summary>
internal static IDisposable UseForTesting(IReadOnlyList<IFillStrategy> replacement)
{
foreach (var type in assembly.GetTypes())
{
if (
type.IsAbstract
|| type.IsInterface
|| !typeof(IFillStrategy).IsAssignableFrom(type)
)
continue;
var ctor = type.GetConstructor(Type.EmptyTypes);
if (ctor == null)
{
Debug.WriteLine(
$"[FillStrategyRegistry] Skipping {type.Name}: no parameterless constructor"
);
continue;
}
try
{
var instance = (IFillStrategy)ctor.Invoke(null);
if (
strategies.Any(s =>
s.Name.Equals(instance.Name, StringComparison.OrdinalIgnoreCase)
)
)
{
Debug.WriteLine(
$"[FillStrategyRegistry] Duplicate strategy '{instance.Name}' skipped"
);
continue;
}
strategies.Add(instance);
Debug.WriteLine(
$"[FillStrategyRegistry] Registered: {instance.Name} (Order={instance.Order})"
);
}
catch (Exception ex)
{
Debug.WriteLine(
$"[FillStrategyRegistry] Failed to instantiate {type.Name}: {ex.Message}"
);
}
}
var prior = strategies;
strategies = replacement;
sorted = null;
return new Restore(prior);
}
public static void LoadPlugins(string directory)
private sealed class Restore : IDisposable
{
if (!Directory.Exists(directory))
return;
private IReadOnlyList<IFillStrategy> prior;
foreach (var dll in Directory.GetFiles(directory, "*.dll"))
public Restore(IReadOnlyList<IFillStrategy> prior) => this.prior = prior;
public void Dispose()
{
try
{
var assembly = Assembly.LoadFrom(dll);
LoadFrom(assembly);
Debug.WriteLine(
$"[FillStrategyRegistry] Loaded plugin assembly: {Path.GetFileName(dll)}"
);
}
catch (Exception ex)
{
Debug.WriteLine(
$"[FillStrategyRegistry] Failed to load {Path.GetFileName(dll)}: {ex.Message}"
);
}
if (prior == null)
return;
strategies = prior;
sorted = null;
prior = null;
}
}
}
@@ -6,20 +6,16 @@ namespace OpenNest.Tests.Strategies;
public class FillStrategyRegistryTests
{
[Fact]
public void Registry_DiscoversBuiltInStrategies()
public void Registry_ListsBuiltInStrategiesInPipelineOrder()
{
var strategies = FillStrategyRegistry.Strategies;
Assert.True(
strategies.Count >= 6,
$"Expected at least 6 built-in strategies, got {strategies.Count}"
Assert.Equal(
["Pairs", "Row", "Column", "RectBestFit", "Extents", "Linear", "TiltedRemnant"],
FillStrategyRegistry.AllStrategies.Select(s => s.Name)
);
Assert.Equal(
[100, 150, 160, 200, 300, 400, 500],
FillStrategyRegistry.AllStrategies.Select(s => s.Order)
);
Assert.Contains(strategies, s => s.Name == "Pairs");
Assert.Contains(strategies, s => s.Name == "RectBestFit");
Assert.Contains(strategies, s => s.Name == "Extents");
Assert.Contains(strategies, s => s.Name == "Linear");
Assert.Contains(strategies, s => s.Name == "Row");
Assert.Contains(strategies, s => s.Name == "Column");
}
[Fact]
@@ -168,8 +168,8 @@ public class PlateViewFillLifetimeTests
});
}
// Use the real strategy registry rather than a production-only injection hook.
public sealed class BlockingFillStrategy : IFillStrategy
// PlateView builds its own filler, so the registry's list is swapped for the test's strategy.
private sealed class BlockingFillStrategy : IFillStrategy
{
public string Name => "WinForms lifetime regression";
public NestPhase Phase => NestPhase.Linear;
@@ -183,7 +183,7 @@ public class PlateViewFillLifetimeTests
private readonly ManualResetEventSlim entered = new();
private readonly ManualResetEventSlim release = new();
private readonly BlockingFillStrategy strategy;
private readonly string[] priorStrategies;
private readonly IDisposable strategies;
private readonly string priorEngine;
private readonly SynchronizationContext? priorContext;
@@ -197,14 +197,10 @@ public class PlateViewFillLifetimeTests
public FillRun()
{
priorStrategies = FillStrategyRegistry.Strategies.Select(s => s.Name).ToArray();
priorEngine = EngineSelection.EngineName;
FillStrategyRegistry.LoadFrom(typeof(BlockingFillStrategy).Assembly);
strategy = Assert.Single(FillStrategyRegistry.AllStrategies.OfType<BlockingFillStrategy>());
FillStrategyRegistry.Disable(strategy.Name);
FillStrategyRegistry.SetEnabled(strategy.Name);
strategy = new BlockingFillStrategy { Run = Fill };
strategies = FillStrategyRegistry.UseForTesting([strategy]);
EngineSelection.EngineName = EngineSelection.DefaultEngineName;
strategy.Run = Fill;
var program = new CNC.Program();
program.Codes.Add(new RapidMove(0, 0));
@@ -260,7 +256,7 @@ public class PlateViewFillLifetimeTests
finally
{
strategy.Run = null;
FillStrategyRegistry.SetEnabled(priorStrategies);
strategies.Dispose();
EngineSelection.EngineName = priorEngine;
SynchronizationContext.SetSynchronizationContext(priorContext);
View.Dispose();