Skip to content

Fix concentric infill generating inside holes (dome/cavity cross-sections)#167

Merged
jgphilpott merged 4 commits intomainfrom
copilot/fix-dome-slice-infill-issue
Mar 1, 2026
Merged

Fix concentric infill generating inside holes (dome/cavity cross-sections)#167
jgphilpott merged 4 commits intomainfrom
copilot/fix-dome-slice-infill-issue

Conversation

Copy link
Contributor

Copilot AI commented Mar 1, 2026

  • Understand the bug: concentric infill skipped entire loops based on 50% point-in-hole sampling, so loops that partially overlapped holes were printed entirely — including portions inside the hole
  • Fix concentric.coffee: when holes exist, clip each loop edge by edge using clipLineWithHoles, group resulting valid segments into polylines, render each polyline with combing travel moves
  • Add regression test using ExtrudeGeometry (rectangular outer boundary + circular inner hole) that specifically tests the partial-overlap scenario
  • Address review comment #2868826589: extract parseFillExtrusionCoords helper to eliminate duplicated G-code coordinate parsing across both hole tests
  • Address review comment #2868826571: hoist HOLE_TOLERANCE = 0.5 as a shared describe-scope constant; apply to both hole-radius assertions to absorb polygon-approximation boundary noise
  • Address review comment #2868826580: replace for polyline in polylines with nearest-neighbour while loop (picks closest start/end, optionally flips polyline) to minimise travel in holes-present branch
  • Simplify remainingPolylines.length > 1 guard to just if lastEndPoint? per secondary review feedback
  • All 7 concentric infill tests pass

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…ing points

Co-authored-by: jgphilpott <4128208+jgphilpott@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix infill clipping issue in dome slicing example Fix concentric infill generating inside holes (dome/cavity cross-sections) Mar 1, 2026
@jgphilpott jgphilpott marked this pull request as ready for review March 1, 2026 10:54
Copilot AI review requested due to automatic review settings March 1, 2026 10:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes concentric infill so it no longer generates extrusion paths inside holes (notably in dome/cavity-like cross-sections) by switching from point-sampling “skip loop” logic to geometric line clipping against hole boundaries.

Changes:

  • Replace majority point-sampling hole detection with per-edge clipLineWithHoles clipping and polyline rendering when holes exist.
  • Remove the unused primitives import and add clipping dependency in the concentric pattern.
  • Add a regression test covering a rectangular loop intersecting a circular hole (the previously missed 50% sampled-points case).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/slicer/infill/patterns/concentric.coffee Clips concentric loop edges against holes and renders the remaining segments as polylines to prevent infill inside holes.
src/slicer/infill/patterns/concentric.test.coffee Adds a regression test for partial-overlap rectangular-vs-circular hole cases and keeps/extends hole-avoidance assertions.

@jgphilpott
Copy link
Owner

@copilot apply changes based on the comments in this thread

…nt, nearest-neighbour polyline ordering

Co-authored-by: jgphilpott <4128208+jgphilpott@users.noreply.github.com>
@jgphilpott jgphilpott merged commit d624707 into main Mar 1, 2026
1 check passed
@jgphilpott jgphilpott deleted the copilot/fix-dome-slice-infill-issue branch March 1, 2026 11:26
Copilot AI mentioned this pull request Mar 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants