Fill planar holes in mesh space - #6739
Merged
Merged
Conversation
…o a plane The hole loops are triangulated in the mesh's own coordinates and the patch is stitched through the patch->mesh edge map, so the projection round-trip, the coordinate-based vertex merging and the border point fixup all go away. Loops sharing a vertex are now merged by identity rather than by position. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The copied ring order comes from the input surface, whose facing is independent of the patch normal the sweep frame follows, so at a vertex the boundary visits twice the inherited order could pair the wrong sectors and the fill came out folded (correct net winding, faces of both senses). Now both splices of a new edge are deferred until its endpoints are known and the insertion position is found with findClosestToFront in projected coordinates - the same rule mergeSamePoints_ uses, so pinched fills match the contour-rebuild path exactly. Tests for both sweep-frame orientations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fedr
approved these changes
Sep 1, 2026
| return 0; | ||
| } | ||
|
|
||
| int findClosestToFront( const MeshTopology& tp, const SweepLinePredicates& predicates, |
Contributor
There was a problem hiding this comment.
add descriptions to new functions
Review follow-up, comments only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fedr
added a commit
that referenced
this pull request
Sep 1, 2026
Merges master (bool-returning addPartByMask of #6744, mesh-space fill and pinched tests of #6739) and builds on both: MeshTopology::addPartByMask now simulates the future edge ring at every target vertex the contours visit more than once, before anything is modified: walking from each stitched edge through the patch and back, all stitched edges of the vertex must fall in one cycle, otherwise the vertex would be left with several disjoint edge rings and the call returns false keeping the target intact. Inputs whose ring composition it cannot simulate (a gap in the from-ring fixed by the near-stitch seam code) keep the old behavior. fillContours2D on a pinched hole loop now fails cleanly instead of producing an invalid mesh: the mirror patch is pinched at the same vertex and pairs each lobe with itself, which no vertex duplication on the patch side can fix; the plan-based fillContours2DPlan remains the way to fill such loops (disk-like). Both formerly DISABLED_ tests are enabled and rewritten to the new behavior: fillContours2DPinchedHoleValidity expects a clean failure with the mesh intact, AddPartByMaskAndStitchPinched expects the pinched stitch rejected and also checks that a fan patch reaching the pinch through two distinct vertices is still accepted (5 verts, 6 faces, closed, valid). Master's fillContours2DPinchedBoundary is updated to the same clean-failure expectation; its triangulation-level sibling keeps checking the patch itself.
Grantim
added a commit
that referenced
this pull request
Sep 1, 2026
Since fillContours2D moved to mesh space (#6739) nothing constructs a HolesVertIds or asks for outBoundaries, so the identity-merge and rebuild-from-contours paths they fed are unreachable. Dropping them leaves mergeSamePoints_ a pure positional weld and reduces the mesh overload of triangulateDisjointContours to its single copy-the- sub-topology path. Public API break: TriangulationParameters::holeVertsIds and the holeVertsIds/outBoundaries parameters of triangulateDisjointContours are gone. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 1, 2026
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.
What
Two commits:
fillContours2Dfills holes in mesh space. The hole loops go throughtriangulateDisjointContours( mesh, loops, normal, nullptr, &patchToMesh )and the patch is stitched through the patch→mesh edge map, aligned to the input loops by identity. Deleted: the best-fit-plane projection round-trip (projectHoles/fillProjected), theHolesVertIds-driven vertex merge, and the border point snap-back — patch boundary vertices now carry the exact mesh coordinates by construction.validateAndFixPatchand theaddMeshPartstitch are unchanged.findClosestToFrontin projected coordinates (new overload taking the reference-ray base explicitly) — the same rulemergeSamePoints_uses, so pinched fills match the contour-rebuild path exactly. This fixes the mesh-space triangulation itself, not onlyfillContours2D.Verification
fillContours2Dwith heap corruption (quantized self-touching boundaries; reproduced on clean master) return a clean error instead.fillContours2DandtriangulateDisjointContours).Follow-ups (separate PRs)
outBoundariesmesh-overload parameter and its rebuild branch, 2DholeVertsIds/outBoundariesparameters,HolesVertIdstypes andfindHoleVertIdsByHoleEdges, the identity-guard inmergeSamePoints_.🤖 Generated with Claude Code