Stop create_mesh flinging a centroid down a hollow - #83
Draft
akaszynski wants to merge 1 commit into
Draft
Conversation
On a hollow part, a cluster covering the wall either side of a slot can have its centroid land in the slot. The ray cast along the cluster normal then misses that wall, carries on down the hollow and hits the far side, so the point ends up flung through the part and the triangles around it become a visible wedge. How far the ray travels does not on its own separate that from a genuine projection. Clustering a surface of revolution gives rings, and a ring's centroid sits on the axis with its normal along it, so the ray properly runs out to the tip: 11.99 cluster radii on a Cone(height=128), further than any of the runaways. What does separate them is where the centroid started. A runaway starts on the wall it covers, a fraction of the cluster size from the nearest face; a ring's centroid is a full cluster radius clear of anything. So refuse the projection only when both hold. Over a corpus of 43 meshes from 10 to 20000 clusters and 0 to 3 subdivisions the only meshes landing in that corner are the ones that are hollow, plus two clusters on the ant that move 0.03% of its bounding box either way. Of 566 remeshes compared against the old behaviour, the 9 that change are all part.stl, the issue's minimal repro and the test's slotted wall. A hollow room scan changes too, at a cluster count outside that run. Fixes #70 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pv46gGGU77uTBDVHSdJRtY
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #70.
create_mesh(moveclus=True)could fling a cluster centroid clean through a part, leaving a wedge across the opening of a channel section.What happens
moveclusprojects each cluster centroid onto the surface along the cluster normal. On a hollow section, a cluster lying flat on a wall can have an area-weighted centroid that lands just off the material, over a slot cut in that wall. The ray misses the near wall, runs down the hollow — the side walls are parallel to it, so they fail the|det| < EPSILONtest and are skipped — and hits the far wall instead. On the reporter's mesh 3 of 17562 clusters came back at 87 units, the depth of the section, where legitimate projections top out around 4.There is no nearer intersection to find; brute-forcing all 151888 faces returns the same 87.
moveclusandray_tracearrived in v0.3.0, which is why 0.2 was fine.The fix
Leave the centroid unprojected when the ray has clearly skimmed off down a hollow, which is what 0.2.x did. Two conditions must both hold: the centroid already sat on the surface before projecting, and the ray then travelled more than three cluster-sizes.
The first condition is what makes this safe. A centroid that starts off the surface is closing a real gap and must keep its projection — a cone apex is the clean example, where ACVD produces ring-shaped clusters whose centroid lands on the axis of revolution and whose ray legitimately exits through the tip. An earlier version of this patch capped on distance alone and broke exactly that case, taking a cone from a surface deviation of 0 to 0.18.
Reporter's mesh: max edge 91.40 to 33.76, against 30.94 for
moveclus=False. The extracted component in #70's attachment goes 89.26 to 25.56, matchingmoveclus=Falseexactly.Verified unchanged across 566 remeshes over 43 meshes — cones from height 0.5 to 128, conical countersinks, cylinders, spheres, tori, and the usual example meshes. The only outputs that move are the hollow ones: the two meshes from the issue, the synthetic wall, and a room scan whose max edge improves.
Keeping
moveclus=Trueas the default. Away from this defect it is a real accuracy gain — p99 distance from output points to the input surface is 0.0 on 0.3.x against 2.63 on 0.2.11.Caveats
Both thresholds are empirical and neither margin is wide. Across the corpus, runaway projections run 3.09 to 9.49 cluster-sizes while the closest legitimate approach from a non-hollow mesh is 2.57; the on-surface ratio separates 0.162–0.462 from 0.517. The constants sit near the geometric midpoint of each pair.
Two of 41 cluster counts on the synthetic wall still fling a point. Loosening either threshold enough to catch them starts suppressing legitimate projections on the cow, human and airplane meshes, so I left them.
A principled version would use the hit face index, which
ray_tracealready returns andcreate_meshcurrently discards: a legitimate hit lands on a face near the cluster in mesh connectivity, a runaway lands on an unrelated wall. That is a larger change and wants its own PR.AI Usage
Drafted with Claude Opus 5, reviewed by me before pushing. A second Claude reviewed the first version cold and found it made cones worse, which is what produced the on-surface condition; it also found the original test passed with the cap disabled entirely. Threshold corpus, the 566-config comparison and the mutation check are all its work, spot-checked by me.