Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 8, 2026

PR #75 prevented infill/skin overlap by subtracting skin areas from infill boundaries. PR #98 fixed nested structures infill by disabling this subtraction entirely. This reconciles both features.

Root Cause

Exposure-detected skin areas include regions inside holes where nested structures exist. Subtracting these from infill boundaries prevented nested structures from generating their own infill.

Solution

Filter skin areas to exclude regions inside holes before passing to infill generation. This matches the existing behavior in skin generation (line 740), which already skips areas inside holes via isAreaInsideAnyHoleWall().

# Filter skin areas before passing to infill
skinAreasForInfill = []
for skinArea in skinAreas
    if not coverage.isAreaInsideAnyHoleWall(skinArea, holeSkinWalls, holeInnerWalls, holeOuterWalls)
        skinAreasForInfill.push(skinArea)

infillModule.generateInfillGCode(..., skinAreasForInfill)

Result

Testing

Added 4 comprehensive automated tests in src/slicer/slice.test.coffee to verify the reconciliation:

  1. Basic overlap prevention - Validates the mechanism on simple structures
  2. Nested structures with infill - Confirms both nested structures receive infill despite overlap prevention being active
  3. Skin area filtering verification - Tests the core filtering mechanism across multiple layers
  4. Three nested structures - Validates the solution works with deeper nesting levels

All 569 tests pass (565 existing + 4 new), ensuring the reconciliation continues to work correctly.

Original prompt

In PR 98 (#98) we made some excellent progress fixing an issue with infill generation for nested structures. However, I noticed that we also lost a feature introduced in PR 75 (#75) that made sure infill and skin don't overlap on adaptive skin layers. Do you think you could study these two features and try to reconcile the implantation so that we still generate infill for nested structures AND also restore not getting overlapping infill and skin on adaptive skin layers?


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix infill generation for nested structures and restore skin overlap Reconcile infill/skin overlap prevention with nested structures infill Jan 8, 2026
Copilot AI requested a review from jgphilpott January 8, 2026 12:19
@jgphilpott
Copy link
Owner

This is great @copilot! Its working well but could you also please add some automated tests to verify this behavior?

Copy link
Contributor Author

Copilot AI commented Jan 9, 2026

This is great @copilot! Its working well but could you also please add some automated tests to verify this behavior?

Added 4 comprehensive automated tests in commit b337d91:

  1. Basic overlap prevention test - Verifies the mechanism works on simple structures
  2. Nested structures with infill test - Confirms both nested structures receive infill despite overlap prevention
  3. Skin area filtering verification - Tests the core filtering mechanism across all layers
  4. Three nested structures test - Validates the solution works with deeper nesting

All 569 tests (565 existing + 4 new) pass, ensuring the reconciliation of PR #75 and PR #98 is working correctly.

@jgphilpott jgphilpott marked this pull request as ready for review January 9, 2026 00:07
@jgphilpott jgphilpott merged commit edeec5f into main Jan 9, 2026
1 check passed
@jgphilpott jgphilpott deleted the copilot/reconcile-infill-skin-overlap branch January 9, 2026 00:10
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.

2 participants