Skip to content

Fix: small covered regions (e.g. lego studs) not detected as fully covered on transition layers#182

Merged
jgphilpott merged 4 commits intomainfrom
copilot/debug-skin-generation-issue
Mar 8, 2026
Merged

Fix: small covered regions (e.g. lego studs) not detected as fully covered on transition layers#182
jgphilpott merged 4 commits intomainfrom
copilot/debug-skin-generation-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 8, 2026

  • Understand the issue: stud circles (~3-5% of brick area) fail the 10% minimum size ratio threshold in findCoveredRegions
  • Fix cavity.coffee: remove the 10% minimum size ratio lower bound; keep 55% upper bound and all other guards
  • Add test for "multiple small covered regions above large region" (lego stud scenario) in cavity.test.coffee
  • Regenerate lego-brick.gcode benchmark — layers 151-154 now show SKIN (outer ring) + FILL (under each stud) instead of solid SKIN
  • All 16 cavity tests pass (15 existing + 1 new)
  • Fix cavity.coffee comment: "its area" → "the candidate region's area" (reviewer feedback)
  • Fix test comment: area ratio ~2% not ~3-5% (reviewer feedback)
  • Add explicit setNozzleDiameter(0.4) and setExposureDetectionResolution(961) to test (reviewer feedback)
  • Strengthen fill assertion: count distinct fillRegions and assert ≥6 (one per stud) (reviewer feedback)

🔒 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.

…regions (lego studs)

Co-authored-by: jgphilpott <4128208+jgphilpott@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix skin generation for Lego brick G-code Fix: small covered regions (e.g. lego studs) not detected as fully covered on transition layers Mar 8, 2026
@jgphilpott jgphilpott marked this pull request as ready for review March 8, 2026 06:20
Copilot AI review requested due to automatic review settings March 8, 2026 06:20
Copy link
Copy Markdown
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 exposure-detection behavior on transition layers where small covered regions (e.g., lego studs) were not being recognized as “fully covered,” causing those areas to get skin infill instead of regular infill.

Changes:

  • Remove the 10% minimum size-ratio guard in findCoveredRegions, allowing legitimately small covered features to be detected.
  • Add a regression test covering the “small cylinders on a slab” scenario and asserting TYPE: SKIN + TYPE: FILL behavior on a transition layer.
  • Regenerate the lego-brick.gcode benchmark artifact (LFS pointer update).

Reviewed changes

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

File Description
src/slicer/skin/exposure/cavity.coffee Updates covered-region detection heuristic by removing the minimum size-ratio bound and documenting the rationale.
src/slicer/skin/exposure/cavity.test.coffee Adds a lego-stud-style regression test intended to ensure small covered features produce regular infill under them on transition layers.
resources/gcode/benchmarks/lego-brick.gcode Updates the stored benchmark output (Git LFS pointer) to reflect new slicing behavior.

@jgphilpott
Copy link
Copy Markdown
Owner

@copilot apply changes based on the comments in this thread

…s, stronger fill-region assertion

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

Copilot AI commented Mar 8, 2026

@copilot apply changes based on the comments in this thread

Applied all four review suggestions in commit 3660889:

  • cavity.coffee line 9: "its area" → "the candidate region's area"
  • Test comment line 1161: area ratio corrected to "~2%"
  • Test setup: added explicit setNozzleDiameter(0.4) and setExposureDetectionResolution(961) calls
  • Assertion strengthened: now tracks fillRegions (distinct consecutive FILL sections) and asserts fillRegions >= 6 — one region per stud

@jgphilpott jgphilpott merged commit ff898c9 into main Mar 8, 2026
1 check passed
@jgphilpott jgphilpott deleted the copilot/debug-skin-generation-issue branch March 8, 2026 07:51
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