Repository navigation
Fix FlipEdgeNetwork::getPathPolyline on degenerate meshes: trace intrinsic edges between vertices robustly - #261
Open
mfagerlund wants to merge 3 commits into
Open
mfagerlund wants to merge 3 commits into
mfagerlund wants to merge 3 commits into
Conversation
traceIntrinsicHalfedgeAlongInput traced an edge between two input vertices with traceGeodesic and kept the untrimmed trace when it missed the end vertex. On meshes with zero-area faces, or where the ray passes exactly through a vertex, the trace ends inside a face or turns off course, and the exported polyline is not a path between the two vertices (FlipEdgeNetwork::getPathPolyline on Thingi10K meshes). New traceGeodesicBetweenVertices: the trimmed trace is kept when it reaches the end vertex with the edge's length (1e-9 relative), so clean meshes give the same points bit for bit; otherwise the edge is traced again by unfolding its faces along the line (traceGeodesicByUnfolding: layout from positions where available, both sides tried at a vertex hit, re-aimed at the end vertex's unfolded position). surfacePathLength measures a surface point path. Tests: through a vertex of a zero-area face, from a corner of angle pi, FlipEdgeNetwork's polyline through a zero-area face, and bit-identity with the old trace on a clean mesh.
With vertex positions the length is summed from the positions; the faces are only needed to check that consecutive points share one, which is topological. Same result bit for bit, without requiring halfedgeVectorsInFace per call: the per-edge check in traceGeodesicBetweenVertices is now cheap on clean meshes.
…x without measuring it A trace whose untrimmed end lies within lengthTol of endVert (bounded from the edge lengths, O(1)) and that crossed only edge interiors on the way is kept as it is; only the others are measured with surfacePathLength and, if off, traced again by unfolding. At a vertex the trace picks a new direction and can still end at endVert by another route (near-coincident vertices), so such traces are still measured. The result is moved out instead of copied. Same polylines as before on every pair tested; on clean meshes the export costs the same as the old one.
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 goes wrong
On meshes with zero-area faces or near-coincident vertices,
FlipEdgeNetwork::getPathPolyline()can return apolyline that is not a path on the surface between the two endpoints. It stops short of the target vertex, two
consecutive points share no face, or its length differs from
FlipEdgeNetwork::length(). The path FlipOutcomputed is fine; only the export is wrong.
Why
SignpostIntrinsicTriangulation::traceIntrinsicHalfedgeAlongInputtraces each intrinsic edge of the path fromits start vertex along its signpost direction for its length with
traceGeodesic, then trims the trace to theend vertex's 1-ring with
trimTraceResult. When the trim fails, it keeps the untrimmed trace, which does not endat the end vertex. When the trim succeeds, the result is not checked.
The trace goes wrong in four ways on degenerate input:
step and ends inside the face.
clamped and the trace turns off course. On the meshes below this happens at vertices of zero-area faces.
trimmed path takes a detour through the duplicate.
1e-5 rad on the pairs below) while the edge lengths stay right. On a long edge the trace then misses the end vertex.
The fix
New
traceGeodesicBetweenVertices(geom, startVert, traceVec, endVert, options, lengthTol = 1e-9)intrace_geodesic.h, used bytraceIntrinsicHalfedgeAlongInputwhen both ends of the intrinsic edge are inputvertices:
Trace with
traceGeodesicand trim withtrimTraceResult, as before. If the trim succeeds and the trace isright, return it: this is the old result, bit for bit. The trace is right if
lengths of the end point's edge or face) and it crossed only edge interiors on the way, or else
surfacePathLength, from the vertex positions) is within 1e-9 relative of theedge length.
The first test is what makes the check free on clean meshes. The second catches a trace that hit a vertex,
turned, and still ended at the end vertex by another route (Thingi10K 106830 below, near-coincident
vertices).
Otherwise, trace the edge again with
traceGeodesicByUnfolding. It lays the faces out one by one along theline in the segment's frame, from the vertex positions when the geometry has them (edge lengths lose the
width of a sliver), and picks each exit edge by the side of the line the new vertex lies on. Where the line
passes within 1e-9 of a vertex, both sides are tried. It arrives where the end vertex is laid out at a point
whose chord from the start has the edge's length (1e-6 relative) and crosses every edge crossed so far; the
crossings are then put on that chord, so the path ends exactly at the end vertex. The direction only aims
the line: where it misses, it is turned toward the end vertex's unfolded position and unfolded again, up to
8 times.
If that fails too, return the old result (trimmed if the trim succeeded, else the untrimmed trace).
surfacePathLength(geom, path)measures a path of surface points and returns NaN if two consecutive pointsshare no face. With vertex positions it needs no face layout (the shared face is a topological test).
Clean meshes are unchanged. Step 1 returns the old trace whenever it was already a path to the end vertex
with the edge's length, which is every edge on a clean mesh. The test
CleanMeshUnchangedcompares the oldtrace and the new function point by point on FlipOut paths on
bob_small.ply. On 14 standard meshes (300random pairs each, 4200 paths) the polylines are bit-identical before and after, and so are FlipOut's lengths.
Cost. On the same 14 meshes (300 pairs each, single thread, old and new export alternated in one process,
min of 3 per pair):
getPathPolylinetakes 0.999x the old export's time (geometric mean over meshes; 0.979x to1.025x per mesh), and
iterativeShortenplusgetPathPolyline1.0000x (0.9998x to 1.0003x). The result is movedout instead of copied, which pays for the check. Measuring every trace with
surfacePathLengthinstead (nofirst test) costs 1.20x on
getPathPolyline: about 45 ns per path point against about 700 ns per point forthe trace itself.
Tests
test/src/trace_geodesic_test.cpp, 4 tests inTraceGeodesicBetweenVertices:ThroughVertexOfZeroAreaFace: a planar patch where vertex M = (1, 0, 0) lies on edge A-B, so face (A, B, M)has zero area; the segment C -> D runs straight down through M.
FromCornerOfAnglePi: the segment M -> D, leaving M through the zero-area face's corner of angle pi.FlipOutPolylineThroughZeroAreaFace: the public path.FlipEdgeNetwork::constructFromDijkstraPathC -> D onthe same patch,
iterativeShorten,getPathPolyline; the polyline must run from C to D, step inside facesand have length 2.
CleanMeshUnchanged: bit-identity with the old trace on a clean mesh.Upstream's suite with the change: 205 of 205 pass (201 existing, 4 new).
Before the fix (the test file built against master; master lacks the two new functions, so the test file was
compiled with a shim that defines
traceGeodesicBetweenVerticesas master's export, i.e.traceGeodesic+trimTraceResult+ end vertex appended, andsurfacePathLengthas in the fix), 3 of the 4 fail:On master, FlipOut's polyline from C to D stops at length 1 (at M, the vertex of the zero-area face) instead
of reaching D at length 2. With the fix all 4 pass.
Thingi10K
419 vertex pairs on 188 Thingi10K models where
getPathPolylinewas invalid on master. The meshes are theThingi10K models with vertices of identical coordinates merged, vertices sorted lexicographically and unused
vertices removed (that fixes the vertex indices below, 0-based); no other repair. Per pair: the shipped
FlipOut as a user runs it (
constructFromDijkstraPath,iterativeShorten,getPathPolyline, defaultsettings), on master and on the branch. A polyline is valid if it starts at v0, ends at v1, every two
consecutive points share a face, and its length from the vertex positions equals
length()to 1e-9relative.
FlipOut's reported length is bit-identical before and after on all 419 pairs; only the export changed.
Every number here is the same with and without the O(1) test in step 1: the 419 polylines are identical point
for point. The 419 split in two sets:
Invalid 214 -> 7.
The 7 left (Thingi10K 109089, 270395, 285376, 331335, 521601, 331795, 331796) are intrinsic edges with no
straight trace on the input surface: the end vertex is never laid out near the line, or only at a chord
1.9e-4 longer than the intrinsic length. 6 of the 7 become valid when vertices closer than 1e-10 of the
bounding box diagonal are merged first.
205 -> 112. Of the 112, 81 are length mismatches below 1e-6: there the polyline is a straight chord through
the input faces and FlipOut's summed intrinsic edge lengths have drifted from the surface distance by
1e-9 .. 1e-6 relative on near-degenerate intrinsic triangles. The other 31 are untraceable edges as above,
27 of them valid once near-coincident vertices are merged.
Representative pairs:
In brackets for "valid": the relative difference between the polyline's length and
length(). Causes aretaken from the first intrinsic edge that the old export got wrong: the faces its trace touched and the two end
fans (a face with 2 area / longest edge^2 below 1e-12 is zero-area; an edge below 1e-10 of the bounding box
diagonal is near-coincident; a trace through a corner or an edge end is a vertex hit; otherwise the angle
between the old trace's first step and the new one).
How to reproduce
Or, without Thingi10K, the patch from the tests: vertices (0,0,0), (1,0,0), (2,0,0), (1,1,0), (1,-1,0), faces
{0,1,3}, {1,2,3}, {0,2,1}, {2,0,4}, FlipOut from vertex 3 to vertex 4.
Note
The
FlipOutContextexport in #259 traces intrinsic edges the same way and will calltraceGeodesicBetweenVerticesonce this lands.