From fdc6f074eec340becb421846576b67ccbbd5e244 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Tue, 13 Oct 2026 21:11:25 -0400 Subject: [PATCH] fix(expand): distinguish part blockers from plate edges --- OpenNest.Engine.Tests/Fill/ExpanderTests.cs | 43 +++++++++++++++ OpenNest.Engine/Fill/Expander.cs | 60 +++++++++++---------- docs/geometry/expand-spacing.md | 2 +- 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/OpenNest.Engine.Tests/Fill/ExpanderTests.cs b/OpenNest.Engine.Tests/Fill/ExpanderTests.cs index 6e488b4..4fb55d6 100644 --- a/OpenNest.Engine.Tests/Fill/ExpanderTests.cs +++ b/OpenNest.Engine.Tests/Fill/ExpanderTests.cs @@ -536,6 +536,49 @@ public class ExpanderTests Assert.Equal(0, host.Location.X); } + [Fact] + public void Separate_ReportsPartBlockerRatherThanPlateEdge() + { + var plate = MakePlate(20, 20, edge: 0); + var pinned = AddSquare(plate, 8, 8); + AddSquare(plate, 3.8, 8); + AddSquare(plate, 12.2, 8); + AddSquare(plate, 8, 3.8); + AddSquare(plate, 8, 12.2); + + var (_, poses, violations) = Expander.Separate(new List { pinned }, plate, 1); + Assert.NotEmpty(violations); + Assert.All(violations, v => Assert.False(v.BlockedByEdge, + $"{v.B.BaseDrawing.Name}: blocker before the work-area edge, pose={poses[0]}")); + } + + [Fact] + public void Separate_ReportsActualPlateEdgeWhenItLimitsMovement() + { + var plate = MakePlate(12, 12, edge: 0); + var anchor = AddSquare(plate, 1, 4, 2); + var moving = AddSquare(plate, 7, 4, 2); + + var (_, poses, violations) = Expander.Separate(new List { anchor, moving }, plate, 9); + var pair = Assert.Single(violations.Where(v => ReferenceEquals(v.A, anchor) && ReferenceEquals(v.B, moving))); + Assert.True(pair.BlockedByEdge); + Assert.True(poses[1].X + 2 <= 12 + 1e-6); + } + + [Fact] + public void Separate_ReportsPartBeforeFartherEdge_OnNarrowPlate() + { + var plate = MakePlate(9, 2, edge: 0); + var anchor = AddSquare(plate, 0, 0, 2); + var moving = AddSquare(plate, 3, 0, 2); + var wall = AddRectangle(plate, 5.5, 0, 0.2, 2); + + var (_, poses, violations) = Expander.Separate(new List { anchor, moving }, plate, 4); + var pair = Assert.Single(violations.Where(v => ReferenceEquals(v.A, anchor) && ReferenceEquals(v.B, moving))); + Assert.False(pair.BlockedByEdge); + Assert.True(poses[1].X + 2 <= wall.Location.X + 1e-6); + } + [Fact] public void Expand_WallsAndUnselectedPairs_StayExactlyAtClearance() { diff --git a/OpenNest.Engine/Fill/Expander.cs b/OpenNest.Engine/Fill/Expander.cs index 4d730a7..e0a479f 100644 --- a/OpenNest.Engine/Fill/Expander.cs +++ b/OpenNest.Engine/Fill/Expander.cs @@ -14,16 +14,15 @@ namespace OpenNest.Engine.Fill /// boundaries. The inverse of : it grows gaps instead /// of closing them. /// - /// Input with overlaps is accepted: an overlapping pair is a pair with - /// negative clearance and is separated along the minimum-translation - /// direction. Moves are straight-line only — a pair that could separate only - /// by routing around a blocker is reported as a violation instead. Final - /// positions are always overlap-free; nothing moves off the work area. + /// Input with overlaps is accepted for safe direct separation; unresolved + /// overlaps remain in the violation report. Moves are straight-line only — + /// a pair that requires routing around a blocker remains unresolved. + /// Plate edges retain their EdgeSpacing floor. /// /// - /// Anchor policy: within a violated pair the later-indexed selected part - /// moves; the counterpart moves only as a fallback when the anchor mover is - /// fully blocked by a work-area edge. Walls (non-selected parts) never move. + /// Anchor policy: within a violated selected pair only its later-indexed + /// selected part moves. Nonselected parts never move; there is no counterpart + /// fallback when the prescribed mover is blocked. /// /// public static class Expander @@ -67,7 +66,8 @@ namespace OpenNest.Engine.Fill /// True when the run was cancelled; no positions were changed. public bool Cancelled; - /// Pairs that could not reach . + /// Pairs still below the attempted spacing, including + /// unresolved negative clearances when no positive target was feasible. public List Violations = new(); } @@ -76,8 +76,10 @@ namespace OpenNest.Engine.Fill /// and its other parts allow, applying the best spacing found. Plate /// PartSpacing/EdgeSpacing are not modified; edges keep their own /// EdgeSpacing floor while only part-to-part clearance chases the target. - /// Mutates locations; probing never touches - /// them, so a failed or cancelled run leaves the layout unchanged. + /// Mutates locations only after the final + /// proposal passes validation; failed individual probes and cancellation + /// leave all live locations unchanged. A completed run can apply safe + /// partial separation and report any remaining violations. /// public static Result Expand( List selected, @@ -263,7 +265,6 @@ namespace OpenNest.Engine.Fill ) { var positions = new List(start); - var stuck = new HashSet(); var violations = new List(); var epsMove = 1e-4; @@ -278,16 +279,12 @@ namespace OpenNest.Engine.Fill if (token.IsCancellationRequested) return (false, start, violations); - // Parking is pass-local. A blocker later in this pass may move, - // so retry the pair on the next pass; a pass with no progress - // terminates pinned layouts without exhausting the iteration cap. - stuck.Clear(); + // Retry every pair on the next pass if another blocker moved; + // a complete pass without progress stops genuinely pinned layouts. var moved = 0.0; foreach (var pair in context.Pairs) { - if (stuck.Contains(pair.Id)) - continue; var (distance, directionA) = context.PairClearance( pair.IndexA, @@ -351,9 +348,8 @@ namespace OpenNest.Engine.Fill continue; } - // Zero progress parks the pair for this pass; movement of a - // different part can unblock it on the next pass. - stuck.Add(pair.Id); + // No progress now; a later part may unblock this pair on + // the next pass. The final sweep remeasures all pairs. } if (moved < epsMove) @@ -368,7 +364,7 @@ namespace OpenNest.Engine.Fill violations.Clear(); foreach (var pair in context.Pairs) { - var (distance, _) = context.PairClearance( + var (distance, directionA) = context.PairClearance( pair.IndexA, pair.IndexB, positions @@ -377,13 +373,27 @@ namespace OpenNest.Engine.Fill if (distance >= spacing) continue; + var direction = pair.Mover == pair.IndexA ? directionA : -directionA; + var need = target - distance; + var attempt = ProposeMove(context, positions, pair, distance, direction, need, epsMove); + if (attempt.Applied <= 0 && distance <= 1e-7) + { + var opposite = ProposeMove(context, positions, pair, distance, -direction, need, epsMove); + if (opposite.Applied > attempt.Applied) + attempt = opposite; + } + // Only an actual first work-area limit is an edge blocker. + // An available move at the iteration budget, or a part/floor + // limiting first, is not evidence of an edge obstruction. + var blockedByEdge = attempt.EdgeLimited && !attempt.PartLimited + && attempt.Applied < need - epsMove; violations.Add( new Violation { A = context.PartOf(pair.IndexA), B = context.PartOf(pair.IndexB), Achieved = distance, - BlockedByEdge = stuck.Contains(pair.Id), + BlockedByEdge = blockedByEdge, } ); } @@ -426,7 +436,6 @@ namespace OpenNest.Engine.Fill /// One constrained part↔part pair with its anchor mover. private sealed class Pair { - public int Id; public int IndexA; public int IndexB; public int Mover; @@ -602,8 +611,6 @@ namespace OpenNest.Engine.Fill private void BuildPairs() { - var id = 0; - for (var a = 0; a < parts.Count; a++) { for (var b = a + 1; b < parts.Count; b++) @@ -617,7 +624,6 @@ namespace OpenNest.Engine.Fill Pairs.Add( new Pair { - Id = id++, IndexA = a, IndexB = b, // Anchor policy: the later-index mover moves. diff --git a/docs/geometry/expand-spacing.md b/docs/geometry/expand-spacing.md index df46a36..27c5ac6 100644 --- a/docs/geometry/expand-spacing.md +++ b/docs/geometry/expand-spacing.md @@ -2,6 +2,6 @@ `Plate > Expand Spacing` moves selected parts by translation only. The first selected part is the anchor for selected pairs; the later selected member moves, and nonselected parts remain fixed. The plate retains its own `EdgeSpacing` floor; the command does not change `PartSpacing` or `EdgeSpacing`. -Every selected/selected and selected/nonselected pair has an operation-entry clearance floor, measured against material with cutouts subtracted. A later move may reuse slack opened by an earlier move, but it cannot reduce a pair below its own starting gap. Existing overlaps are accepted for attempted separation; a remaining overlap is reported as a violation, not a successful positive gap. Temporarily blocked pairs are retried after another part moves, while a pass without positive movement stops the relaxation. +Every selected/selected and selected/nonselected pair has an operation-entry clearance floor, measured against material with cutouts subtracted. A later move may reuse slack opened by an earlier move, but it cannot reduce a pair below its own starting gap. Existing overlaps are accepted for attempted separation; a remaining overlap is reported as a violation, not a successful positive gap. Temporarily blocked pairs are retried after another part moves, while a pass without positive movement stops the relaxation. An individual violation's `BlockedByEdge` flag is true only when the work-area edge is the first limiting cause; the desktop status currently displays a blocked-pair count, not those individual causes. Probes run on scratch poses. Each trial translation is clipped against blocking material along its entire straight path, including cutout walls. Initially overlapping convex line profiles can take a certified direct exit; unsupported or ambiguous overlap paths stay put and remain reported. When a part touches a cutout wall and the usual nearest-boundary direction makes no safe progress, the tool tries the opposite direction through the same checks. Tiny clamp keep-outs use a conservative native-bound motion limit when tolerant geometry cannot certify them. Cancellation before application leaves the live layout unchanged; `Separate` never changes it. The desktop command is synchronous and has no Cancel dialog or automatic save. A bounded search reports the spacing it found, not an optimum. See `docs/geometry/directional-slides.md` for directional contact behavior.