From 953429dae9295f53735bff2b770eaef431159870 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Sat, 28 Mar 2026 22:08:38 -0400 Subject: [PATCH] fix: add overlap safety check and diagnostics to FillGrid Step 2 FillGrid had no overlap check after perpendicular tiling of the row pattern (Step 2), unlike Step 1 which had one. When geometry-aware FindPatternCopyDistance underestimated row spacing, overlapping parts were returned unchecked. Changes: - Make FillLinear.HasOverlappingParts shape-aware (bbox pre-filter + Part.Intersects) instead of bbox-only, preventing false positives on interlocking pairs while catching real overlaps - Add missing overlap safety check after Step 2 perpendicular tiling with bbox fallback - Add diagnostic Debug.WriteLine logging when overlap fallback triggers, including engine label, step, direction, work area, spacing, pattern details, and overlapping part locations/rotations for reproduction - Add FillLinear.Label property set at all callsites for log traceability - Refactor LinearFillStrategy and ExtentsFillStrategy to use shared FillHelpers.BestOverAngles helper for angle-sweep logic Co-Authored-By: Claude Opus 4.6 (1M context) --- OpenNest.Engine/DefaultNestEngine.cs | 2 +- OpenNest.Engine/Fill/FillLinear.cs | 63 +++++++++++++++++-- OpenNest.Engine/Fill/PairFiller.cs | 4 +- OpenNest.Engine/Fill/StripeFiller.cs | 8 +-- .../Strategies/ExtentsFillStrategy.cs | 20 ++---- OpenNest.Engine/Strategies/FillHelpers.cs | 43 +++++++++++++ .../Strategies/LinearFillStrategy.cs | 53 ++++++---------- OpenNest.Tests/StrategyOverlapTests.cs | 1 + OpenNest/Forms/PatternTileForm.cs | 6 +- 9 files changed, 133 insertions(+), 67 deletions(-) diff --git a/OpenNest.Engine/DefaultNestEngine.cs b/OpenNest.Engine/DefaultNestEngine.cs index dfa7558..c9463f0 100644 --- a/OpenNest.Engine/DefaultNestEngine.cs +++ b/OpenNest.Engine/DefaultNestEngine.cs @@ -94,7 +94,7 @@ namespace OpenNest // Multi-part group: linear pattern fill only. PhaseResults.Clear(); - var engine = new FillLinear(workArea, Plate.PartSpacing); + var engine = new FillLinear(workArea, Plate.PartSpacing) { Label = "GroupPattern" }; var angles = RotationAnalysis.FindHullEdgeAngles(groupParts); var best = FillHelpers.FillPattern(engine, groupParts, angles, workArea, Comparer); PhaseResults.Add(new PhaseResult(NestPhase.Linear, best?.Count ?? 0, 0)); diff --git a/OpenNest.Engine/Fill/FillLinear.cs b/OpenNest.Engine/Fill/FillLinear.cs index 31682db..600e37a 100644 --- a/OpenNest.Engine/Fill/FillLinear.cs +++ b/OpenNest.Engine/Fill/FillLinear.cs @@ -1,6 +1,7 @@ using OpenNest.Geometry; using OpenNest.Math; using System.Collections.Generic; +using System.Diagnostics; using System.Threading.Tasks; namespace OpenNest.Engine.Fill @@ -19,6 +20,11 @@ namespace OpenNest.Engine.Fill public double HalfSpacing => PartSpacing / 2; + /// + /// Diagnostic label set by callers to identify the engine/context in overlap logs. + /// + public string Label { get; set; } + private static Vector MakeOffset(NestDirection direction, double distance) { return direction == NestDirection.Horizontal @@ -323,7 +329,7 @@ namespace OpenNest.Engine.Fill return result; } - private static bool HasOverlappingParts(List parts) + private static bool HasOverlappingParts(List parts, out int overlapA, out int overlapB) { for (var i = 0; i < parts.Count; i++) { @@ -338,11 +344,20 @@ namespace OpenNest.Engine.Fill var overlapY = System.Math.Min(b1.Top, b2.Top) - System.Math.Max(b1.Bottom, b2.Bottom); - if (overlapX > Tolerance.Epsilon && overlapY > Tolerance.Epsilon) + if (overlapX <= Tolerance.Epsilon || overlapY <= Tolerance.Epsilon) + continue; + + if (parts[i].Intersects(parts[j], out _)) + { + overlapA = i; + overlapB = j; return true; + } } } + overlapA = -1; + overlapB = -1; return false; } @@ -384,10 +399,9 @@ namespace OpenNest.Engine.Fill var row = new List(pattern.Parts); row.AddRange(TilePattern(pattern, direction, boundaries)); - // Safety: if geometry-aware spacing produced overlapping parts, - // fall back to bbox-based spacing for this axis. - if (pattern.Parts.Count > 1 && HasOverlappingParts(row)) + if (pattern.Parts.Count > 1 && HasOverlappingParts(row, out var a1, out var b1)) { + LogOverlap("Step1-Primary", direction, pattern, row, a1, b1); row = new List(pattern.Parts); row.AddRange(TilePatternBbox(pattern, direction)); } @@ -397,8 +411,9 @@ namespace OpenNest.Engine.Fill { row.AddRange(TilePattern(pattern, perpAxis, boundaries)); - if (pattern.Parts.Count > 1 && HasOverlappingParts(row)) + if (pattern.Parts.Count > 1 && HasOverlappingParts(row, out var a2, out var b2)) { + LogOverlap("Step1-PerpOnly", perpAxis, pattern, row, a2, b2); row = new List(pattern.Parts); row.AddRange(TilePatternBbox(pattern, perpAxis)); } @@ -415,9 +430,45 @@ namespace OpenNest.Engine.Fill var gridResult = new List(rowPattern.Parts); gridResult.AddRange(TilePattern(rowPattern, perpAxis, rowBoundaries)); + if (HasOverlappingParts(gridResult, out var a3, out var b3)) + { + LogOverlap("Step2-Perp", perpAxis, rowPattern, gridResult, a3, b3); + gridResult = new List(rowPattern.Parts); + gridResult.AddRange(TilePatternBbox(rowPattern, perpAxis)); + } + return gridResult; } + private void LogOverlap(string step, NestDirection tilingDir, + Pattern pattern, List parts, int idxA, int idxB) + { + var pa = parts[idxA]; + var pb = parts[idxB]; + var ba = pa.BoundingBox; + var bb = pb.BoundingBox; + + Debug.WriteLine($"[FillLinear] OVERLAP FALLBACK ({Label ?? "unknown"})"); + Debug.WriteLine($" Step: {step}, TilingDir: {tilingDir}"); + Debug.WriteLine($" WorkArea: ({WorkArea.X:F4},{WorkArea.Y:F4}) {WorkArea.Width:F4}x{WorkArea.Length:F4}, Spacing: {PartSpacing}"); + Debug.WriteLine($" Pattern: {pattern.Parts.Count} parts, bbox {pattern.BoundingBox.Width:F4}x{pattern.BoundingBox.Length:F4}"); + Debug.WriteLine($" Total parts after tiling: {parts.Count}"); + Debug.WriteLine($" Overlapping pair [{idxA}] vs [{idxB}]:"); + Debug.WriteLine($" [{idxA}]: drawing={pa.BaseDrawing?.Name ?? "?"} rot={Angle.ToDegrees(pa.Rotation):F2}° " + + $"loc=({pa.Location.X:F4},{pa.Location.Y:F4}) bbox=({ba.Left:F4},{ba.Bottom:F4})-({ba.Right:F4},{ba.Top:F4})"); + Debug.WriteLine($" [{idxB}]: drawing={pb.BaseDrawing?.Name ?? "?"} rot={Angle.ToDegrees(pb.Rotation):F2}° " + + $"loc=({pb.Location.X:F4},{pb.Location.Y:F4}) bbox=({bb.Left:F4},{bb.Bottom:F4})-({bb.Right:F4},{bb.Top:F4})"); + + // Log all pattern seed parts for reproduction + Debug.WriteLine($" Pattern seed parts:"); + for (var i = 0; i < pattern.Parts.Count; i++) + { + var p = pattern.Parts[i]; + Debug.WriteLine($" [{i}]: drawing={p.BaseDrawing?.Name ?? "?"} rot={Angle.ToDegrees(p.Rotation):F2}° " + + $"loc=({p.Location.X:F4},{p.Location.Y:F4}) bbox={p.BoundingBox.Width:F4}x{p.BoundingBox.Length:F4}"); + } + } + /// /// Fills a single row of identical parts along one axis using geometry-aware spacing. /// diff --git a/OpenNest.Engine/Fill/PairFiller.cs b/OpenNest.Engine/Fill/PairFiller.cs index 791ad6c..194da8a 100644 --- a/OpenNest.Engine/Fill/PairFiller.cs +++ b/OpenNest.Engine/Fill/PairFiller.cs @@ -195,7 +195,7 @@ namespace OpenNest.Engine.Fill if (pattern.Parts.Count == 0) continue; - var engine = new FillLinear(workArea, partSpacing); + var engine = new FillLinear(workArea, partSpacing) { Label = "Pairs" }; foreach (var dir in new[] { NestDirection.Horizontal, NestDirection.Vertical }) { if (!dedup.TryAdd(pattern.BoundingBox, workArea, dir)) @@ -321,7 +321,7 @@ namespace OpenNest.Engine.Fill return cachedResult; } - var filler = new FillLinear(remnantBox, partSpacing); + var filler = new FillLinear(remnantBox, partSpacing) { Label = "Pairs-Remnant" }; List parts = null; foreach (var angle in new[] { 0.0, Angle.HalfPI }) diff --git a/OpenNest.Engine/Fill/StripeFiller.cs b/OpenNest.Engine/Fill/StripeFiller.cs index f882b02..79f1a17 100644 --- a/OpenNest.Engine/Fill/StripeFiller.cs +++ b/OpenNest.Engine/Fill/StripeFiller.cs @@ -121,7 +121,7 @@ public class StripeFiller if (!_dedup.TryAdd(rotatedPattern.BoundingBox, workArea, primaryAxis)) return null; - var stripeEngine = new FillLinear(stripeBox, spacing); + var stripeEngine = new FillLinear(stripeBox, spacing) { Label = "Stripe" }; var stripeParts = stripeEngine.Fill(rotatedPattern, primaryAxis); if (stripeParts == null || stripeParts.Count == 0) @@ -136,7 +136,7 @@ public class StripeFiller stripePattern.Parts.AddRange(stripeParts); stripePattern.UpdateBounds(); - var gridEngine = new FillLinear(workArea, spacing); + var gridEngine = new FillLinear(workArea, spacing) { Label = "Stripe-Grid" }; var gridParts = gridEngine.Fill(stripePattern, perpAxis); if (gridParts == null || gridParts.Count == 0) @@ -244,7 +244,7 @@ public class StripeFiller return cachedResult; } - var filler = new FillLinear(remnantBox, spacing); + var filler = new FillLinear(remnantBox, spacing) { Label = "Stripe-Remnant" }; List best = null; foreach (var angle in new[] { 0.0, Angle.HalfPI }) @@ -396,7 +396,7 @@ public class StripeFiller var stripeBox = axis == NestDirection.Horizontal ? new Box(0, 0, sheetSpan, perpDim) : new Box(0, 0, perpDim, sheetSpan); - var engine = new FillLinear(stripeBox, spacing); + var engine = new FillLinear(stripeBox, spacing) { Label = "Stripe-EstimateRow" }; var filled = engine.Fill(rotated, axis); var n = filled?.Count ?? 0; diff --git a/OpenNest.Engine/Strategies/ExtentsFillStrategy.cs b/OpenNest.Engine/Strategies/ExtentsFillStrategy.cs index 0efbc6b..3c4f487 100644 --- a/OpenNest.Engine/Strategies/ExtentsFillStrategy.cs +++ b/OpenNest.Engine/Strategies/ExtentsFillStrategy.cs @@ -20,22 +20,10 @@ namespace OpenNest.Engine.Strategies var angles = new[] { bestRotation, bestRotation + Angle.HalfPI }; - List best = null; - var comparer = context.Policy?.Comparer ?? new DefaultFillComparer(); - - foreach (var angle in angles) - { - context.Token.ThrowIfCancellationRequested(); - var result = filler.Fill(context.Item.Drawing, angle, - context.PlateNumber, context.Token, context.Progress); - if (result != null && result.Count > 0) - { - if (best == null || comparer.IsBetter(result, best, context.WorkArea)) - best = result; - } - } - - return best ?? new List(); + return FillHelpers.BestOverAngles(context, angles, + angle => filler.Fill(context.Item.Drawing, angle, + context.PlateNumber, context.Token, context.Progress), + NestPhase.Extents, "Extents"); } } } diff --git a/OpenNest.Engine/Strategies/FillHelpers.cs b/OpenNest.Engine/Strategies/FillHelpers.cs index db541b9..a951aa1 100644 --- a/OpenNest.Engine/Strategies/FillHelpers.cs +++ b/OpenNest.Engine/Strategies/FillHelpers.cs @@ -110,6 +110,49 @@ namespace OpenNest.Engine.Strategies return fallback ?? new List(); } + /// + /// Sweeps a list of angles, calling fillAtAngle for each, and returns + /// the best result according to the context's comparer. Handles + /// cancellation and progress reporting. + /// + public static List BestOverAngles( + FillContext context, + IReadOnlyList angles, + Func> fillAtAngle, + NestPhase phase, + string phaseLabel) + { + var workArea = context.WorkArea; + var comparer = context.Policy?.Comparer ?? new DefaultFillComparer(); + List best = null; + + for (var i = 0; i < angles.Count; i++) + { + context.Token.ThrowIfCancellationRequested(); + + var angle = angles[i]; + var result = fillAtAngle(angle); + var angleDeg = Angle.ToDegrees(angle); + + if (result != null && result.Count > 0) + { + if (best == null || comparer.IsBetter(result, best, workArea)) + best = result; + } + + NestEngineBase.ReportProgress(context.Progress, new ProgressReport + { + Phase = phase, + PlateNumber = context.PlateNumber, + Parts = best, + WorkArea = workArea, + Description = $"{phaseLabel}: {i + 1}/{angles.Count} angles, {angleDeg:F0}° best = {best?.Count ?? 0} parts", + }); + } + + return best ?? new List(); + } + /// /// Checks if any pair of parts geometrically overlap. Uses bounding box /// pre-filtering for performance, then falls back to shape intersection. diff --git a/OpenNest.Engine/Strategies/LinearFillStrategy.cs b/OpenNest.Engine/Strategies/LinearFillStrategy.cs index a0af093..a9a7265 100644 --- a/OpenNest.Engine/Strategies/LinearFillStrategy.cs +++ b/OpenNest.Engine/Strategies/LinearFillStrategy.cs @@ -19,45 +19,28 @@ namespace OpenNest.Engine.Strategies var workArea = context.WorkArea; var comparer = context.Policy?.Comparer ?? new DefaultFillComparer(); var preferred = context.Policy?.PreferredDirection; - List best = null; - for (var ai = 0; ai < angles.Count; ai++) - { - context.Token.ThrowIfCancellationRequested(); - - var angle = angles[ai]; - var engine = new FillLinear(workArea, context.Plate.PartSpacing); - - var result = FillHelpers.FillWithDirectionPreference( - dir => engine.Fill(context.Item.Drawing, angle, dir), - preferred, comparer, workArea); - - var angleDeg = Angle.ToDegrees(angle); - - if (result != null && result.Count > 0) + return FillHelpers.BestOverAngles(context, angles, + angle => { - context.AngleResults.Add(new AngleResult + var engine = new FillLinear(workArea, context.Plate.PartSpacing) { Label = "Linear" }; + var result = FillHelpers.FillWithDirectionPreference( + dir => engine.Fill(context.Item.Drawing, angle, dir), + preferred, comparer, workArea); + + if (result != null && result.Count > 0) { - AngleDeg = angleDeg, - Direction = preferred ?? NestDirection.Horizontal, - PartCount = result.Count - }); + context.AngleResults.Add(new AngleResult + { + AngleDeg = Angle.ToDegrees(angle), + Direction = preferred ?? NestDirection.Horizontal, + PartCount = result.Count + }); + } - if (best == null || comparer.IsBetter(result, best, workArea)) - best = result; - } - - NestEngineBase.ReportProgress(context.Progress, new ProgressReport - { - Phase = NestPhase.Linear, - PlateNumber = context.PlateNumber, - Parts = best, - WorkArea = workArea, - Description = $"Linear: {ai + 1}/{angles.Count} angles, {angleDeg:F0}° best = {best?.Count ?? 0} parts", - }); - } - - return best ?? new List(); + return result; + }, + NestPhase.Linear, "Linear"); } } } diff --git a/OpenNest.Tests/StrategyOverlapTests.cs b/OpenNest.Tests/StrategyOverlapTests.cs index e67081d..98536d9 100644 --- a/OpenNest.Tests/StrategyOverlapTests.cs +++ b/OpenNest.Tests/StrategyOverlapTests.cs @@ -107,4 +107,5 @@ public class StrategyOverlapTests Assert.Empty(failures); } + } diff --git a/OpenNest/Forms/PatternTileForm.cs b/OpenNest/Forms/PatternTileForm.cs index 5e1089c..3945204 100644 --- a/OpenNest/Forms/PatternTileForm.cs +++ b/OpenNest/Forms/PatternTileForm.cs @@ -220,7 +220,7 @@ namespace OpenNest.Forms return; var workArea = new Box(0, 0, plateSize.Length, plateSize.Width); - var filler = new FillLinear(workArea, PartSpacing); + var filler = new FillLinear(workArea, PartSpacing) { Label = "PatternTile-H" }; var hParts = filler.Fill(pattern, NestDirection.Horizontal); foreach (var part in hParts) @@ -228,7 +228,7 @@ namespace OpenNest.Forms hLabel.Text = $"Horizontal — {hParts.Count} parts"; hPreview.ZoomToFit(); - var vFiller = new FillLinear(workArea, PartSpacing); + var vFiller = new FillLinear(workArea, PartSpacing) { Label = "PatternTile-V" }; var vParts = vFiller.Fill(pattern, NestDirection.Vertical); foreach (var part in vParts) vPreview.Plate.Parts.Add(part); @@ -328,7 +328,7 @@ namespace OpenNest.Forms if (pattern == null) return; - var filler = new FillLinear(new Box(0, 0, plateSize.Length, plateSize.Width), PartSpacing); + var filler = new FillLinear(new Box(0, 0, plateSize.Length, plateSize.Width), PartSpacing) { Label = "PatternTile-Apply" }; var tiledParts = filler.Fill(pattern, applyDirection); Result = new PatternTileResult