merge: resolve polylabel conflicts, keep remote version with hole support
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,197 @@
|
||||
# Engine Refactor: Extract Shared Algorithms from DefaultNestEngine and StripNestEngine
|
||||
|
||||
## Problem
|
||||
|
||||
`DefaultNestEngine` (~550 lines) mixes phase orchestration with strategy-specific logic (pair candidate selection, angle building, pattern helpers). `StripNestEngine` (~450 lines) duplicates patterns that DefaultNestEngine also uses: shrink-to-fit loops, iterative remnant filling, and progress accumulation. Both engines would benefit from extracting shared algorithms into focused, reusable classes.
|
||||
|
||||
## Approach
|
||||
|
||||
Extract five classes from the two engines. No new interfaces or strategy patterns — just focused helper classes that each engine composes.
|
||||
|
||||
## Extracted Classes
|
||||
|
||||
### 1. PairFiller
|
||||
|
||||
**Source:** DefaultNestEngine lines 362-489 (`FillWithPairs`, `SelectPairCandidates`, `BuildRemainderPatterns`, `MinPairCandidates`, `PairTimeLimit`).
|
||||
|
||||
**API:**
|
||||
```csharp
|
||||
public class PairFiller
|
||||
{
|
||||
public PairFiller(Size plateSize, double partSpacing) { }
|
||||
|
||||
public List<Part> Fill(NestItem item, Box workArea,
|
||||
int plateNumber = 0,
|
||||
CancellationToken token = default,
|
||||
IProgress<NestProgress> progress = null);
|
||||
}
|
||||
```
|
||||
|
||||
**Details:**
|
||||
- Constructor takes plate size and spacing — decoupled from `Plate` object.
|
||||
- `SelectPairCandidates` and `BuildRemainderPatterns` become private methods.
|
||||
- Uses `BestFitCache.GetOrCompute()` internally (same as today).
|
||||
- Calls `BuildRotatedPattern` and `FillPattern` — these become `internal static` methods on DefaultNestEngine so PairFiller can call them without ceremony.
|
||||
- Returns `List<Part>` (empty list if no result), same contract as today.
|
||||
- Progress reporting: PairFiller accepts `IProgress<NestProgress>` and `int plateNumber` in its `Fill` method to maintain per-candidate progress updates. The caller passes these through from the engine.
|
||||
|
||||
**Caller:** `DefaultNestEngine.FindBestFill` replaces `this.FillWithPairs(...)` with `new PairFiller(Plate.Size, Plate.PartSpacing).Fill(...)`.
|
||||
|
||||
### 2. AngleCandidateBuilder
|
||||
|
||||
**Source:** DefaultNestEngine lines 279-347 (`BuildCandidateAngles`, `knownGoodAngles` HashSet, `ForceFullAngleSweep` property).
|
||||
|
||||
**API:**
|
||||
```csharp
|
||||
public class AngleCandidateBuilder
|
||||
{
|
||||
public bool ForceFullSweep { get; set; }
|
||||
|
||||
public List<double> Build(NestItem item, double bestRotation, Box workArea);
|
||||
|
||||
public void RecordProductive(List<AngleResult> angleResults);
|
||||
}
|
||||
```
|
||||
|
||||
**Details:**
|
||||
- Owns `knownGoodAngles` state — lives as long as the engine instance so pruning accumulates across fills.
|
||||
- `Build()` encapsulates the full pipeline: base angles, sweep check, ML prediction, known-good pruning.
|
||||
- `RecordProductive()` replaces the inline loop that feeds `knownGoodAngles` after the linear phase.
|
||||
- `ForceFullAngleSweep` moves from DefaultNestEngine to `AngleCandidateBuilder.ForceFullSweep`. DefaultNestEngine keeps a forwarding property `ForceFullAngleSweep` that delegates to its `AngleCandidateBuilder` instance, so `BruteForceRunner` (which sets `engine.ForceFullAngleSweep = true`) continues to work without changes.
|
||||
|
||||
**Caller:** DefaultNestEngine creates one `AngleCandidateBuilder` instance as a field and calls `Build()`/`RecordProductive()` from `FindBestFill`.
|
||||
|
||||
### 3. ShrinkFiller
|
||||
|
||||
**Source:** StripNestEngine `TryOrientation` shrink loop (lines 188-215) and `ShrinkFill` (lines 358-418).
|
||||
|
||||
**API:**
|
||||
```csharp
|
||||
public static class ShrinkFiller
|
||||
{
|
||||
public static ShrinkResult Shrink(
|
||||
Func<NestItem, Box, List<Part>> fillFunc,
|
||||
NestItem item, Box box,
|
||||
double spacing,
|
||||
ShrinkAxis axis,
|
||||
CancellationToken token = default,
|
||||
int maxIterations = 20);
|
||||
}
|
||||
|
||||
public enum ShrinkAxis { Width, Height }
|
||||
|
||||
public class ShrinkResult
|
||||
{
|
||||
public List<Part> Parts { get; set; }
|
||||
public double Dimension { get; set; }
|
||||
}
|
||||
```
|
||||
|
||||
**Details:**
|
||||
- `fillFunc` delegate decouples ShrinkFiller from any specific engine — the caller provides how to fill.
|
||||
- `ShrinkAxis` determines which dimension to reduce. `TryOrientation` maps strip direction to axis: `StripDirection.Bottom` → `ShrinkAxis.Height`, `StripDirection.Left` → `ShrinkAxis.Width`. `ShrinkFill` calls `Shrink` twice (width then height).
|
||||
- Loop logic: fill initial box, measure placed bounding box, reduce dimension by `spacing`, retry until count drops below initial count. Dimension is measured as `placedBox.Right - box.X` for Width or `placedBox.Top - box.Y` for Height.
|
||||
- Returns both the best parts and the final tight dimension (needed by `TryOrientation` to compute the remnant box).
|
||||
- **Two-axis independence:** When `ShrinkFill` calls `Shrink` twice, each axis shrinks against the **original** box dimensions, not the result of the prior axis. This preserves the current behavior where width and height are shrunk independently.
|
||||
|
||||
**Callers:**
|
||||
- `StripNestEngine.TryOrientation` replaces its inline shrink loop.
|
||||
- `StripNestEngine.ShrinkFill` replaces its two-axis inline shrink loops.
|
||||
|
||||
### 4. RemnantFiller
|
||||
|
||||
**Source:** StripNestEngine remnant loop (lines 253-343) and the simpler version in NestEngineBase.Nest (lines 74-97).
|
||||
|
||||
**API:**
|
||||
```csharp
|
||||
public class RemnantFiller
|
||||
{
|
||||
public RemnantFiller(Box workArea, double spacing) { }
|
||||
|
||||
public void AddObstacles(IEnumerable<Part> parts);
|
||||
|
||||
public List<Part> FillItems(
|
||||
List<NestItem> items,
|
||||
Func<NestItem, Box, List<Part>> fillFunc,
|
||||
CancellationToken token = default,
|
||||
IProgress<NestProgress> progress = null);
|
||||
}
|
||||
```
|
||||
|
||||
**Details:**
|
||||
- Owns a `RemnantFinder` instance internally.
|
||||
- `AddObstacles` registers already-placed parts (bounding boxes offset by spacing).
|
||||
- `FillItems` runs the iterative loop: find remnants, try each item in each remnant, fill, update obstacles, repeat until no progress.
|
||||
- Local quantity tracking (dictionary keyed by drawing name) stays internal — does not mutate the input `NestItem` quantities. Returns the placed parts; the caller deducts quantities.
|
||||
- Uses minimum-remnant-size filtering (smallest remaining part dimension), same as StripNestEngine today.
|
||||
- `fillFunc` delegate allows callers to provide any fill strategy (DefaultNestEngine.Fill, ShrinkFill, etc.).
|
||||
|
||||
**Callers:**
|
||||
- `StripNestEngine.TryOrientation` replaces its inline remnant loop with `RemnantFiller.FillItems(...)`.
|
||||
- `NestEngineBase.Nest` replaces its hand-rolled largest-remnant loop. **Note:** This is a deliberate behavioral improvement — the base class currently uses only the single largest remnant, while `RemnantFiller` tries all remnants iteratively with minimum-size filtering. This may produce better fill results for engines that rely on the base `Nest` method.
|
||||
|
||||
**Unchanged:** `NestEngineBase.Nest` phase 2 (bin-packing single-quantity items via `PackArea`, lines 100-119) is not affected by this change.
|
||||
|
||||
### 5. AccumulatingProgress
|
||||
|
||||
**Source:** StripNestEngine nested class (lines 425-449).
|
||||
|
||||
**API:**
|
||||
```csharp
|
||||
internal class AccumulatingProgress : IProgress<NestProgress>
|
||||
{
|
||||
public AccumulatingProgress(IProgress<NestProgress> inner, List<Part> previousParts) { }
|
||||
public void Report(NestProgress value);
|
||||
}
|
||||
```
|
||||
|
||||
**Details:**
|
||||
- Moved from private nested class to standalone `internal` class in OpenNest.Engine.
|
||||
- No behavioral change — wraps an `IProgress<NestProgress>` and prepends previously placed parts to each report.
|
||||
|
||||
## What Stays on Each Engine
|
||||
|
||||
### DefaultNestEngine (~200 lines after extraction)
|
||||
|
||||
- `Fill(NestItem, Box, ...)` — public entry point, unchanged.
|
||||
- `Fill(List<Part>, Box, ...)` — group-parts overload, unchanged.
|
||||
- `PackArea` — bin packing delegation, unchanged.
|
||||
- `FindBestFill` — orchestration, now ~30 lines: calls `AngleCandidateBuilder.Build()`, `PairFiller.Fill()`, linear angle loop, `FillRectangleBestFit`, picks best.
|
||||
- `FillRectangleBestFit` — 6-line private method, too small to extract.
|
||||
- `BuildRotatedPattern` / `FillPattern` — become `internal static`, used by both the linear loop and PairFiller.
|
||||
- `QuickFillCount` — stays (used by binary search, not shared).
|
||||
|
||||
### StripNestEngine (~200 lines after extraction)
|
||||
|
||||
- `Nest` — orchestration, unchanged.
|
||||
- `TryOrientation` — becomes thinner: calls `DefaultNestEngine.Fill` for initial fill, `ShrinkFiller.Shrink()` for tightening, `RemnantFiller.FillItems()` for remnants.
|
||||
- `ShrinkFill` — replaced by two `ShrinkFiller.Shrink()` calls.
|
||||
- `SelectStripItemIndex` / `EstimateStripDimension` — stay private, strip-specific.
|
||||
- `AccumulatingProgress` — removed, uses shared class.
|
||||
|
||||
### NestEngineBase
|
||||
|
||||
- `Nest` — switches from hand-rolled remnant loop to `RemnantFiller.FillItems()`.
|
||||
- All other methods unchanged.
|
||||
|
||||
## File Layout
|
||||
|
||||
All new classes go in `OpenNest.Engine/`:
|
||||
|
||||
```
|
||||
OpenNest.Engine/
|
||||
PairFiller.cs
|
||||
AngleCandidateBuilder.cs
|
||||
ShrinkFiller.cs
|
||||
RemnantFiller.cs
|
||||
AccumulatingProgress.cs
|
||||
```
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- No new interfaces or strategy patterns.
|
||||
- No changes to FillLinear, FillBestFit, PackBottomLeft, or any other existing algorithm.
|
||||
- No changes to NestEngineRegistry or the plugin system.
|
||||
- No changes to public API surface — all existing callers continue to work unchanged. One deliberate behavioral improvement: `NestEngineBase.Nest` gains multi-remnant filling (see RemnantFiller section).
|
||||
- PatternHelper extraction deferred — `BuildRotatedPattern`/`FillPattern` become `internal static` on DefaultNestEngine for now. Extract if a third consumer appears.
|
||||
- StripNestEngine continues to create fresh `DefaultNestEngine` instances per fill call. Sharing an `AngleCandidateBuilder` across sub-fills to enable angle pruning is a potential future optimization, not part of this refactor.
|
||||
@@ -0,0 +1,82 @@
|
||||
# Polylabel Part Label Positioning
|
||||
|
||||
**Date:** 2026-03-16
|
||||
**Status:** Approved
|
||||
|
||||
## Problem
|
||||
|
||||
Part ID labels in `PlateView` are drawn at `PathPoints[0]` — the first point of the graphics path, which sits on the part contour edge. This causes labels to overlap adjacent parts and be unreadable, especially in dense nests.
|
||||
|
||||
## Solution
|
||||
|
||||
Implement the polylabel algorithm (pole of inaccessibility) to find the point inside each part's polygon with maximum distance from all edges, including hole edges. Draw the part ID label centered on that point.
|
||||
|
||||
## Design
|
||||
|
||||
### Part 1: Polylabel Algorithm
|
||||
|
||||
Add `PolyLabel` static class in `OpenNest.Geometry` namespace (file: `OpenNest.Core/Geometry/PolyLabel.cs`).
|
||||
|
||||
**Public API:**
|
||||
|
||||
```csharp
|
||||
public static class PolyLabel
|
||||
{
|
||||
public static Vector Find(Polygon outer, IList<Polygon> holes = null, double precision = 0.5);
|
||||
}
|
||||
```
|
||||
|
||||
**Algorithm:**
|
||||
|
||||
1. Compute bounding box of the outer polygon.
|
||||
2. Divide into a grid of cells (cell size = shorter bbox dimension).
|
||||
3. For each cell, compute signed distance from cell center to nearest edge on any ring (outer boundary + all holes). Use `Polygon.ContainsPoint` for sign (negative if outside outer polygon or inside a hole).
|
||||
4. Track the best interior point found so far.
|
||||
5. Use a priority queue (sorted list) ordered by maximum possible distance for each cell.
|
||||
6. Subdivide promising cells that could beat the current best; discard the rest.
|
||||
7. Stop when the best cell's potential improvement over the current best is less than the precision tolerance.
|
||||
|
||||
**Dependencies within codebase:**
|
||||
|
||||
- `Polygon.ContainsPoint(Vector)` — ray-casting point-in-polygon test (already exists).
|
||||
- Point-to-segment distance — compute from `Line` or inline (distance from point to each polygon edge).
|
||||
|
||||
**Fallback:** If the polygon is degenerate (< 3 vertices) or the program has no geometry, fall back to the bounding box center.
|
||||
|
||||
**No external dependencies.**
|
||||
|
||||
### Part 2: Label Rendering in LayoutPart
|
||||
|
||||
Modify `LayoutPart` in `OpenNest/LayoutPart.cs`.
|
||||
|
||||
**Changes:**
|
||||
|
||||
1. Add a cached `Vector? _labelPoint` field in **program-local coordinates** (pre-transform). Invalidated when `IsDirty` is set.
|
||||
2. When computing the label point (on first draw after invalidation):
|
||||
- Convert the part's `Program` to geometry via `ConvertProgram.ToGeometry`.
|
||||
- Build shapes via `ShapeBuilder.GetShapes`.
|
||||
- Identify the outer contour using `ShapeProfile` (the `Perimeter` shape) and convert cutouts to hole polygons.
|
||||
- Run `PolyLabel.Find(outer, holes)` on the result.
|
||||
- Cache the `Vector` in program-local coordinates.
|
||||
3. In `Draw(Graphics g, string id)`:
|
||||
- Offset the cached label point by `BasePart.Location`.
|
||||
- Transform through the current view matrix (handles zoom/pan without cache invalidation).
|
||||
- Draw the ID string centered using `StringFormat` with `Alignment = Center` and `LineAlignment = Center`.
|
||||
|
||||
**Coordinate pipeline:** polylabel runs once in program-local coordinates (expensive, cached). Location offset + matrix transform happen every frame (cheap, no caching needed). This matches how the existing `GraphicsPath` pipeline works and avoids stale cache on zoom/pan.
|
||||
|
||||
## Scope
|
||||
|
||||
- **In scope:** polylabel algorithm, label positioning change in `LayoutPart.Draw`.
|
||||
- **Out of scope:** changing part origins, modifying the nesting engine, any changes to `Part`, `Drawing`, or `Program` classes.
|
||||
|
||||
## Testing
|
||||
|
||||
- Unit tests for `PolyLabel.Find()` with known polygons:
|
||||
- Square — label at center.
|
||||
- L-shape — label in the larger lobe.
|
||||
- C-shape — label inside the concavity, not at bounding box center.
|
||||
- Triangle — label at incenter.
|
||||
- Thin rectangle (10:1 aspect ratio) — label centered along the short axis.
|
||||
- Square with large centered hole — label avoids the hole.
|
||||
- Verify the returned point is inside the polygon and has the expected distance from edges.
|
||||
Reference in New Issue
Block a user