docs: address spec review feedback for trim-to-count

Clarify sort direction (ascending, keep nearest to origin), document
parameter changes, MeasureDimension behavior, and behavioral trade-off.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-03-19 23:56:14 -04:00
parent 11f605801f
commit fe2a293128

View File

@@ -17,10 +17,9 @@ internal static List<Part> TrimToCount(List<Part> parts, int targetCount, Shrink
```
- Returns input unchanged if `parts.Count <= targetCount`
- Sorts by trailing edge descending:
- `ShrinkAxis.Width` → sort by `BoundingBox.Right`
- `ShrinkAxis.Height` → sort by `BoundingBox.Top`
- Removes parts from the far end until `targetCount` remains
- Sorts ascending by trailing edge, takes the first `targetCount` parts (keeps parts nearest to origin, discards farthest):
- `ShrinkAxis.Width` → sort ascending by `BoundingBox.Right`
- `ShrinkAxis.Height` → sort ascending by `BoundingBox.Top`
- Returns a new list (does not mutate input)
### Changes to `ShrinkFiller.Shrink`
@@ -33,7 +32,7 @@ Replace the iterative shrink loop:
4. Report progress once after trim
5. Return `ShrinkResult`
The `maxIterations` parameter becomes unused and can be removed.
Parameters removed from `Shrink`: `maxIterations` (no loop). The `spacing` parameter is kept (used by `EstimateStartBox`). `CancellationToken` is kept in the signature for API consistency even though the loop no longer uses it.
### Changes to `DefaultNestEngine.Fill`
@@ -56,9 +55,12 @@ Defaults to `ShrinkAxis.Width` (trim by right edge) since this is the natural "e
- **Axis-aware trimming**: Height shrink trims by top edge, width shrink trims by right edge. This respects the strip direction.
- **No pair integrity**: Trimming may split interlocking pairs. This is acceptable because if the layout is suboptimal, a better candidate will replace it during evaluation.
- **No edge spacing concerns**: The new dimension is simply the max edge of remaining parts. No snapping to spacing increments.
- **`MeasureDimension` unchanged**: It measures the occupied extent of remaining parts relative to `box.X`/`box.Y` (the work area origin). This works correctly after trimming.
- **`EstimateStartBox` preserved**: It was designed to accelerate the iterative loop, which is now gone. It still helps by producing a smaller starting fill, but could be simplified in a future pass.
- **Behavioral trade-off**: The shrink loop found the smallest box fitting N parts; trim-to-count reports the actual extent of the N nearest parts, which may be slightly less tight if there are gaps. In practice this is negligible since fill algorithms pack densely.
## Files Changed
- `OpenNest.Engine/Fill/ShrinkFiller.cs` — add `TrimToCount`, replace shrink loop, remove `maxIterations`
- `OpenNest.Engine/DefaultNestEngine.cs` — replace `Take(N)` with `TrimToCount`
- `OpenNest.Tests/ShrinkFillerTests.cs`update tests for new behavior, add `TrimToCount` tests
- `OpenNest.Tests/ShrinkFillerTests.cs`delete `Shrink_RespectsMaxIterations` test (concept no longer exists), update remaining tests, add `TrimToCount` tests