fix(fill): pick FillPattern winners in angle order, not thread order

FillPattern gathered per-angle results in a ConcurrentBag and kept the first best it
enumerated, so an exact tie between angles went to whichever worker finished first:
identical inputs could return different, equally scored layouts. Results now sit in
slots indexed by angle, V before H within an angle (the order the single-angle bag
already produced and the existing tie tests pin), and ties keep the earliest slot.
Single-angle behaviour is unchanged.

Regression: a never-prefer comparer over one 0-degree and fifteen 180-degree angles
must keep the 0-degree layout in 50 of 50 calls; it failed on the old code.
This commit is contained in:
aj committed 2026-10-04 19:50:30 -04:00
1 parent 418f75916b
commit 4842b80ceb
2 files changed
+34 -11

No files matched your search

+13 -8
View File
@@ -1,5 +1,4 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Threading.Tasks;
using OpenNest.Engine.Fill;
@@ -40,24 +39,27 @@ namespace OpenNest.Engine.Strategies
IFillComparer comparer = null
)
{
var results = new ConcurrentBag<(List<Part> Parts, FillScore Score)>();
// Slots in angle order, V before H within an angle: ties keep the earliest slot
// however the workers finish.
var results = new (List<Part> Parts, FillScore Score)[angles.Count * 2];
Parallel.ForEach(
angles,
angle =>
Parallel.For(
0,
angles.Count,
i =>
{
var pattern = BuildRotatedPattern(groupParts, angle);
var pattern = BuildRotatedPattern(groupParts, angles[i]);
if (pattern.Parts.Count == 0)
return;
var h = engine.Fill(pattern, NestDirection.Horizontal);
if (h != null && h.Count > 0)
results.Add((h, comparer == null ? FillScore.Compute(h, workArea) : default));
results[2 * i + 1] = (h, comparer == null ? FillScore.Compute(h, workArea) : default);
var v = engine.Fill(pattern, NestDirection.Vertical);
if (v != null && v.Count > 0)
results.Add((v, comparer == null ? FillScore.Compute(v, workArea) : default));
results[2 * i] = (v, comparer == null ? FillScore.Compute(v, workArea) : default);
}
);
@@ -66,6 +68,9 @@ namespace OpenNest.Engine.Strategies
foreach (var res in results)
{
if (res.Parts == null)
continue;
if (comparer != null)
{
if (best == null || comparer.IsBetter(res.Parts, best, workArea))
+21 -3
View File
@@ -154,7 +154,7 @@ public class FillHelpersTests
var actual = FillHelpers.FillPattern(engine, group, angles, workArea, comparer);
// One angle writes H then V on one worker's bag queue; enumeration is V then H.
// Within one angle V is considered before H.
// Pin both arguments and the call count, not just the eventual winning score.
var call = Assert.Single(comparer.Calls);
AssertSameLayout(h, call.Candidate);
@@ -173,7 +173,7 @@ public class FillHelpersTests
[Theory]
[InlineData(false)]
[InlineData(true)]
public void FillPattern_TiedScores_PreservesBagOrderAndStillCallsCustomComparer(bool acceptCandidate)
public void FillPattern_TiedScores_KeepsVerticalBeforeHorizontalAndStillCallsCustomComparer(bool acceptCandidate)
{
var drawing = new RectangleShape { Length = 2, Width = 1 }.GetDrawing();
var group = new List<Part> { new(drawing, new Vector(11, 13)) };
@@ -191,7 +191,7 @@ public class FillHelpersTests
var byScore = FillHelpers.FillPattern(engine, group, angles, workArea);
var byComparer = FillHelpers.FillPattern(engine, group, angles, workArea, comparer);
AssertSameLayout(v, byScore); // Strict > retains the first bag result on a tie.
AssertSameLayout(v, byScore); // Strict > retains the earlier (V) result on a tie.
var call = Assert.Single(comparer.Calls);
AssertSameLayout(h, call.Candidate);
AssertSameLayout(v, call.Current);
@@ -202,6 +202,24 @@ public class FillHelpersTests
AssertValidLayout(byComparer, workArea);
}
[Fact]
public void FillPattern_ConsidersAnglesInInputOrderWhateverTheThreadOrder()
{
var drawing = new RectangleShape { Length = 2, Width = 1 }.GetDrawing();
var group = new List<Part> { new(drawing, new Vector(11, 13)) };
var workArea = new Box(3, 5, 5, 4);
var engine = new FillLinear(workArea, 0.25);
// Only the first angle's layouts keep rotation zero. A comparer that never prefers the
// candidate keeps whichever result is considered first: the first angle's V.
var angles = new List<double> { 0 };
angles.AddRange(Enumerable.Repeat(System.Math.PI, 15));
var expected = engine.Fill(FillHelpers.BuildRotatedPattern(group, 0), NestDirection.Vertical);
var comparer = new RecordingComparer((_, _, _) => false);
for (var run = 0; run < 50; run++)
AssertSameLayout(expected, FillHelpers.FillPattern(engine, group, angles, workArea, comparer));
}
[Fact]
public void FillPattern_RotatedGroup_PreservesDrawingIdentityPosesAndInputPrograms()
{