Skip to content

Commit 27555e5

Browse files
Copilotjgphilpott
andcommitted
Address review feedback: nesting parity, bbox prefilter, explicit test config, stable assertions
Co-authored-by: jgphilpott <4128208+jgphilpott@users.noreply.github.com>
1 parent 6849004 commit 27555e5

File tree

2 files changed

+44
-18
lines changed

2 files changed

+44
-18
lines changed

src/slicer/skin/exposure/cavity.coffee

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ primitives = require('../../utils/primitives')
66
# Scan regionCandidates against regionRefs and return candidates that qualify as
77
# fully covered interior regions. A candidate qualifies when:
88
# - It is interior to currentPath (does not touch its boundary).
9-
# - It is NOT a hole path (i.e. not enclosed by another path in the same layer set).
10-
# Hole paths represent empty space in an adjacent layer, not solid features.
9+
# - It is a solid feature at its nesting level, NOT a hole path. Nesting parity is
10+
# determined by counting how many other paths in regionCandidates contain the
11+
# candidate's centre (even = solid structure, odd = hole/cavity).
1112
# - It is the smaller of the two paired regions (candidateArea < refArea).
1213
# - A reference region covers ≥50% of the candidate region's area.
1314
# - The size ratio between the two regions is below the step-transition ceiling (<55%).
@@ -39,25 +40,42 @@ findCoveredRegions = (regionCandidates, regionRefs, currentPathBounds, currentAr
3940
)
4041
continue if touchesBoundary
4142

42-
# Skip candidates that are hole paths (enclosed by another path in the same set).
43-
# Hole paths represent empty space (cavities/openings) in the adjacent layer, not
44-
# solid features. Treating them as covered regions would suppress skin infill
45-
# on the corresponding exposure patches in the current layer (e.g. dome zenith).
43+
# Determine whether this candidate is a hole path using the even-odd nesting rule.
44+
# Count how many other paths in regionCandidates contain the candidate's centre point.
45+
# Odd containment count → the candidate is inside an odd number of paths → it is
46+
# empty space (a hole or cavity) in the adjacent layer, not a solid feature.
47+
# Even containment count (0 included) → it is a solid structure at that nesting level.
48+
#
49+
# This handles arbitrarily deep nesting (structure→hole→structure→…) correctly:
50+
# - outer structure: 0 containing paths (even) → structure ✓
51+
# - hole inside that structure: 1 containing path (odd) → hole ✓
52+
# - structure inside the hole: 2 containing paths (even) → structure ✓
53+
#
54+
# Bounding-box containment is checked first as a cheap pre-filter so that the more
55+
# expensive pointInPolygon call is only made for paths whose bounds actually enclose
56+
# the candidate centre.
4657
candidateCenterX = (candidateBounds.minX + candidateBounds.maxX) / 2
4758
candidateCenterY = (candidateBounds.minY + candidateBounds.maxY) / 2
48-
isHolePath = false
59+
nestingCount = 0
4960

5061
for otherPath in regionCandidates
5162

5263
continue if otherPath is candidate
5364
continue if otherPath.length < 3
5465

66+
otherBounds = bounds.calculatePathBounds(otherPath)
67+
continue unless otherBounds?
68+
69+
# Cheap bounding-box pre-filter.
70+
continue if candidateCenterX < otherBounds.minX or candidateCenterX > otherBounds.maxX
71+
continue if candidateCenterY < otherBounds.minY or candidateCenterY > otherBounds.maxY
72+
5573
if primitives.pointInPolygon({ x: candidateCenterX, y: candidateCenterY }, otherPath)
5674

57-
isHolePath = true
58-
break
75+
nestingCount += 1
5976

60-
continue if isHolePath
77+
# Odd nesting count → hole/cavity path; skip it.
78+
continue if nestingCount % 2 is 1
6179

6280
candidateWidth = candidateBounds.maxX - candidateBounds.minX
6381
candidateHeight = candidateBounds.maxY - candidateBounds.minY

src/slicer/skin/exposure/cavity.test.coffee

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,13 +1296,16 @@ describe 'Exposure Detection - Cavity and Hole Detection', ->
12961296
finalMesh.updateMatrixWorld()
12971297

12981298
# Configure slicer with exposure detection enabled.
1299+
slicer.setNozzleDiameter(0.4)
1300+
slicer.setExposureDetectionResolution(961)
12991301
slicer.setLayerHeight(0.2)
13001302
slicer.setShellSkinThickness(0.8) # 4 skin layers.
13011303
slicer.setShellWallThickness(0.8)
13021304
slicer.setVerbose(true)
13031305
slicer.setAutohome(false)
13041306
slicer.setExposureDetection(true)
13051307
slicer.setInfillDensity(20)
1308+
slicer.setInfillPattern('grid')
13061309

13071310
# Slice the mesh.
13081311
result = slicer.slice(finalMesh)
@@ -1329,8 +1332,8 @@ describe 'Exposure Detection - Cavity and Hole Detection', ->
13291332
# Exposure patches appear at layers near the zenith where the small hole disappears.
13301333
# Before the fix: hole paths from adjacent layers were classified as covered regions,
13311334
# suppressing all skin infill in that area.
1332-
# After the fix: those hole paths are recognised as holes (enclosed by the outer
1333-
# square boundary in the same set) and are no longer classified as covered regions.
1335+
# After the fix: those hole paths are recognised as holes (nesting parity = odd)
1336+
# and are no longer classified as covered regions.
13341337
# Layers 49-54 should therefore have skin infill.
13351338
zenithLayerTotal = 0
13361339

@@ -1341,9 +1344,14 @@ describe 'Exposure Detection - Cavity and Hole Detection', ->
13411344
# Verify that skin infill is generated at the dome zenith exposure patches.
13421345
expect(zenithLayerTotal).toBeGreaterThan(0)
13431346

1344-
# Also verify lego-stud-style covered region detection still works: the pyramid
1345-
# test in the 'Fully Covered Areas Exclusion' suite is the canonical check, but
1346-
# as a sanity guard verify that the total skin infill count is reasonable
1347-
# (dome should have significantly more skin than a solid box of the same size).
1348-
totalSkinInfill = (result.match(/Moving to skin infill line/g) || []).length
1349-
expect(totalSkinInfill).toBeGreaterThan(50)
1347+
# Verify per-layer presence: at least 3 of the 6 zenith layers must have
1348+
# skin infill. This is more stable than checking an absolute count because
1349+
# it tolerates minor geometry/tessellation changes while still catching a
1350+
# full regression (where all zenith layers produce zero infill lines).
1351+
zenithLayersWithInfill = 0
1352+
1353+
for layerIndex in [49..54]
1354+
1355+
zenithLayersWithInfill += 1 if (skinInfillByLayer[layerIndex] || 0) > 0
1356+
1357+
expect(zenithLayersWithInfill).toBeGreaterThanOrEqual(3)

0 commit comments

Comments
 (0)