From c6bb35e088be1e0b4c3c8c508b1a3705c0b444b4 Mon Sep 17 00:00:00 2001 From: Jason Cantarella Date: Wed, 10 Jun 2026 19:41:05 -0400 Subject: [PATCH 1/2] Fix source-face resolution in interpolateEdgeLengthsA/B on Delta-complexes 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 --- src/surface/common_subdivision.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/surface/common_subdivision.cpp b/src/surface/common_subdivision.cpp index 0cf727c6..d4961f55 100644 --- a/src/surface/common_subdivision.cpp +++ b/src/surface/common_subdivision.cpp @@ -193,7 +193,12 @@ EdgeData CommonSubdivision::interpolateEdgeLengthsA(const EdgeDataedges()) { SurfacePoint tail = sourcePoints[e.halfedge().tailVertex()]->posA; SurfacePoint tip = sourcePoints[e.halfedge().tipVertex()]->posA; - Face f = sharedFace(tail, tip); + // Resolve the containing source face from an adjacent common subdivision + // face rather than from the endpoint SurfacePoints: on a Delta-complex, + // the endpoints may share several elements (e.g. two vertices joined by + // multiple edges), and sharedFace() could pick a face adjacent to the + // wrong one, producing a wildly wrong length. + Face f = sourceFaceA[e.halfedge().face()]; GC_SAFETY_ASSERT(f != Face(), "common subdivision edges must be contained in a face"); tail = tail.inFace(f); tip = tip.inFace(f); @@ -216,7 +221,10 @@ EdgeData CommonSubdivision::interpolateEdgeLengthsB(const EdgeDataedges()) { SurfacePoint tail = sourcePoints[e.halfedge().tailVertex()]->posB; SurfacePoint tip = sourcePoints[e.halfedge().tipVertex()]->posB; - Face f = sharedFace(tail, tip); + // See the comment in interpolateEdgeLengthsA: resolve the containing + // source face from an adjacent common subdivision face, since the + // endpoint SurfacePoints alone are ambiguous on a Delta-complex. + Face f = sourceFaceB[e.halfedge().face()]; GC_SAFETY_ASSERT(f != Face(), "common subdivision edges must be contained in a face"); tail = tail.inFace(f); tip = tip.inFace(f); From 96634ea9037c0de6111d6f402a3b0b79fda8a6a9 Mon Sep 17 00:00:00 2001 From: Jason Cantarella Date: Thu, 11 Jun 2026 11:06:44 -0400 Subject: [PATCH 2/2] Add Delta-complex test for common subdivision edge length interpolation 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 --- test/src/intrinsic_triangulation_test.cpp | 35 +++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/src/intrinsic_triangulation_test.cpp b/test/src/intrinsic_triangulation_test.cpp index 01128fdc..4672bf05 100644 --- a/test/src/intrinsic_triangulation_test.cpp +++ b/test/src/intrinsic_triangulation_test.cpp @@ -370,6 +370,41 @@ TEST_F(IntrinsicTriangulationSuite, CommonSubdivisionCompareIntegerSignpost) { // TODO: also test with signposts? +// A single edge flip on a tetrahedron creates a Delta-complex in which two +// vertices are joined by two distinct edges. interpolateEdgeLengthsA/B used +// to resolve each common subdivision edge's containing face from its +// endpoint SurfacePoints alone, which is ambiguous in this configuration +// and produced lengths of the wrong edge. +TEST_F(IntrinsicTriangulationSuite, CommonSubdivisionEdgeLengthsDeltaComplex) { + for (size_t iE = 0; iE < 6; iE++) { + auto a = getAsset("tet.obj", true); + ManifoldSurfaceMesh& mesh = *a.manifoldMesh; + VertexPositionGeometry& origGeometry = *a.geometry; + + IntegerCoordinatesIntrinsicTriangulation tri(mesh, origGeometry); + + if (!tri.flipEdgeIfPossible(tri.intrinsicMesh->edge(iE))) continue; + + CommonSubdivision& cs = tri.getCommonSubdivision(); + cs.constructMesh(); + + // Ground truth: every common subdivision edge lies in a single face of + // both meshes, so extrinsic positions interpolated across mesh A give + // exact lengths + VertexPositionGeometry csGeo(*cs.mesh, cs.interpolateAcrossA(origGeometry.vertexPositions)); + csGeo.requireEdgeLengths(); + + EdgeData lengthsFromLenB = cs.interpolateEdgeLengthsB(tri.edgeLengths); + origGeometry.requireEdgeLengths(); + EdgeData lengthsFromLenA = cs.interpolateEdgeLengthsA(origGeometry.edgeLengths); + + for (Edge e : cs.mesh->edges()) { + EXPECT_NEAR(csGeo.edgeLengths[e], lengthsFromLenB[e], 1e-5); + EXPECT_NEAR(csGeo.edgeLengths[e], lengthsFromLenA[e], 1e-5); + } + } +} + TEST_F(IntrinsicTriangulationSuite, FunctionTransfer) { for (const MeshAsset& a : {getAsset("fox.ply", true)}) { a.printThyName();