CommonSubdivision::interpolateEdgeLengthsA/B: resolve source faces unambiguously on Delta-complexes#252
Open
designbynumbers wants to merge 2 commits into
Open
Conversation
…lexes interpolateEdgeLengthsA/B resolved the source face containing each common subdivision edge via sharedFace() on the endpoint SurfacePoints. On a Delta-complex this is ambiguous: two vertices may be joined by several edges (e.g. after edge flips near inserted vertices), and sharedFace() can return a face adjacent to the wrong connecting edge. The endpoints are then interpreted as corners of that wrong face, and the computed length is that of an entirely different edge (we observed a common subdivision edge of length 3.93 reported as 5.53). Resolve the source face from an adjacent common subdivision face via sourceFaceA/sourceFaceB instead, which is unambiguous: every common subdivision edge lies inside (the closure of) its neighboring faces' source faces, and corner-based barycentric coordinates within a single face are well defined even on a Delta-complex. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single edge flip on a tetrahedron joins two vertices by two distinct edges; interpolateEdgeLengthsA/B used to resolve common subdivision edges from their endpoint SurfacePoints, which is ambiguous in this configuration and returned the length of the wrong edge (error ~1.2 on the unit tetrahedron). This is the minimal no-insertions reproducer for the fix in the preceding commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
interpolateEdgeLengthsA/Bresolve the source face containing each commonsubdivision edge with
sharedFace()on the two endpointSurfacePoints.On a Delta-complex this is ambiguous: two vertices can be joined by more
than one edge, and
sharedFace()may return a face adjacent to the wrongconnecting edge. The endpoints are then interpreted as corners of that
face, and the returned "length" is the length of an entirely different
edge. Since intrinsic triangulations routinely become Delta-complexes
after edge flips, this affects ordinary usage.
This PR resolves the source face through the common subdivision's own
provenance data instead:
Face f = sourceFaceB[e.halfedge().face()]; // was: sharedFace(tail, tip)(and
sourceFaceA[...]ininterpolateEdgeLengthsA). Every commonsubdivision edge lies inside the closure of its adjacent faces' source
faces, and corner-based barycentric coordinates within a single face are
well defined even on a Delta-complex, so this resolution is unambiguous.
How to verify
The minimal failing case is a single edge flip on a tetrahedron: flipping
edge ab creates a second edge between c and d, parallel to the
existing one. The common subdivision itself is fine, but
interpolateEdgeLengthsBreports the length of the wrong cd edge:On current master (b34974d) this fails with a worst per-edge error of
1.195 on the unit tetrahedron (flipping edge 1 or 5; the other edges pass
by luck of face-iteration order). With the fix the worst error is ~1e-15
for every flip choice. The PR includes this check as a test
(
IntrinsicTriangulationSuite.CommonSubdivisionEdgeLengthsDeltaComplex),which exercises all six flip choices and both the A and B variants.
interpolateEdgeLengthsAhas the identical latent problem when mesh A isa Delta-complex input, though the common simplicial-input case masks it;
the PR fixes both for symmetry.
As with #248, we checked this against the
int-tri-updatesbranch: theaffected code is identical there, so the fix applies cleanly to both
lines and is independent of that branch's reworked tracing.
🤖 Generated with Claude Code (Claude Fable 5)