Skip to content

Fix dome zenith skin infill suppressed by hole paths misclassified as covered regions#187

Merged
jgphilpott merged 4 commits intomainfrom
copilot/fix-dome-skin-infill-regression
Mar 13, 2026
Merged

Fix dome zenith skin infill suppressed by hole paths misclassified as covered regions#187
jgphilpott merged 4 commits intomainfrom
copilot/fix-dome-skin-infill-regression

Conversation

Copy link
Contributor

Copilot AI commented Mar 13, 2026

PR 182 removed the 10% minimum size ratio from findCoveredRegions to detect small lego-stud features. This regressed the dome example: small circular hole boundaries near the cavity zenith passed all remaining checks and were incorrectly classified as fully covered regions, suppressing their skin infill entirely.

Root cause

coveringRegionsBelow for dome zenith layers contains both the outer square boundary (filtered by the boundary-touch check) and the small circular hole boundary (not filtered). The hole circle is interior, has a tiny size ratio vs the outer square (≪55%), and is fully overlapped by the above layer's solid — so it was added to fullyCoveredSkinWalls and excluded from infill generation.

Fix

cavity.coffee — After the boundary-touch check, classify each candidate using the even-odd nesting parity rule: count how many other paths in regionCandidates contain the candidate's bounding-box centre (using a bounding-box prefilter before the full pointInPolygon test). An odd count means the candidate sits inside an odd number of enclosing paths — it is empty space (a hole or cavity) in the adjacent layer and is skipped. An even count (including 0) means it is a solid structure and proceeds normally.

nestingCount = 0

for otherPath in regionCandidates
    continue if otherPath is candidate
    continue if otherPath.length < 3
    otherBounds = bounds.calculatePathBounds(otherPath)
    continue unless otherBounds?
    # Cheap bounding-box pre-filter.
    continue if candidateCenterX < otherBounds.minX or candidateCenterX > otherBounds.maxX
    continue if candidateCenterY < otherBounds.minY or candidateCenterY > otherBounds.maxY
    if primitives.pointInPolygon({ x: candidateCenterX, y: candidateCenterY }, otherPath)
        nestingCount += 1

# Odd nesting count → hole/cavity path; skip it.
continue if nestingCount % 2 is 1

This correctly handles arbitrarily deep nesting (structure → hole → structure → …):

  • outer structure: 0 containing paths (even) → solid ✓
  • hole inside that structure: 1 containing path (odd) → hole → skipped ✓
  • structure inside the hole: 2 containing paths (even) → solid ✓

And correctly distinguishes the original two cases:

  • Dome zenith hole — circle centre is inside the outer square (nesting count = 1, odd) → skipped → skin infill restored ✓
  • Lego stud — stud centre is not enclosed by anything in coveringRegionsAbove (nesting count = 0, even) → still detected as covered ✓

cavity.test.coffee — Added Dome Zenith Skin Infill (Regression) test verifying skin infill is generated on layers 49–54 near the dome zenith. The test sets setNozzleDiameter, setExposureDetectionResolution, and setInfillPattern explicitly to avoid reliance on slicer defaults. Assertions use a per-layer presence check (at least 3 of 6 zenith layers must have skin infill) rather than a brittle absolute count.

resources/gcode/skin/dome/ — Regenerated upright, flipped, and sideways GCode examples with the fix applied.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…dRegions

Co-authored-by: jgphilpott <4128208+jgphilpott@users.noreply.github.com>
Copilot AI changed the title [WIP] [PR-182] Investigate dome skin infill regression due to coverage fix Fix dome zenith skin infill suppressed by hole paths misclassified as covered regions Mar 13, 2026
Copilot AI requested a review from jgphilpott March 13, 2026 08:39
@jgphilpott jgphilpott marked this pull request as ready for review March 13, 2026 09:00
Copilot AI review requested due to automatic review settings March 13, 2026 09:00
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 a regression in exposure-based skin generation where small hole boundaries near a dome cavity zenith were being misclassified as “fully covered regions,” which incorrectly suppressed skin infill. This fits into the slicer’s exposure/cavity detection pipeline used to decide where adaptive skin should (and should not) be generated.

Changes:

  • Update covered-region detection to skip candidates deemed to be hole paths (enclosed by another path in the same candidate set).
  • Add a dome-zenith regression test to ensure skin infill is generated near the cavity zenith.
  • Regenerate several committed G-code example/benchmark outputs (Git LFS pointers updated).

Reviewed changes

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

File Description
src/slicer/skin/exposure/cavity.coffee Adds a hole-path exclusion check when identifying fully covered regions.
src/slicer/skin/exposure/cavity.test.coffee Adds a regression test asserting skin infill exists near the dome zenith.
resources/gcode//.gcode Updates Git LFS pointers for regenerated reference G-code outputs.

You can also share your feedback on Copilot code review. Take the survey.

@jgphilpott
Copy link
Owner

@copilot apply changes based on the comments in this thread

…t config, stable assertions

Co-authored-by: jgphilpott <4128208+jgphilpott@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 13, 2026

@copilot apply changes based on the comments in this thread

Done in commit 27555e5. All four review comments are addressed:

  1. Nesting parity (cavity.coffee:42-60): The single-containment check is replaced with an even-odd nesting count — the loop now tallies how many paths in regionCandidates contain the candidate's centre; an odd count means the candidate is a hole/cavity and is skipped. This correctly handles nested structure→hole→structure chains at any depth.

  2. Bounding-box prefilter (cavity.coffee:46-55): calculatePathBounds is called for each otherPath and the centre point is tested against those bounds before calling pointInPolygon, avoiding the expensive polygon test for the majority of non-enclosing paths.

  3. Explicit slicer config (cavity.test.coffee:1299-1305): setNozzleDiameter(0.4), setExposureDetectionResolution(961), and setInfillPattern('grid') are now set explicitly in the dome zenith regression test.

  4. Stable assertions (cavity.test.coffee:1335-1349): The brittle totalSkinInfill > 50 is replaced with a per-layer count: at least 3 of the 6 zenith layers (49–54) must have at least one skin infill line. All 17 tests pass.

@jgphilpott jgphilpott merged commit 153203e into main Mar 13, 2026
1 check passed
@jgphilpott jgphilpott deleted the copilot/fix-dome-skin-infill-regression branch March 13, 2026 09:34
Copilot AI mentioned this pull request Mar 14, 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