Repository navigation
Exact geodesics: honour max_propagation_distance without stop points; require curvatures once - #262
Open
mfagerlund wants to merge 2 commits into
Open
mfagerlund wants to merge 2 commits into
mfagerlund wants to merge 2 commits into
Conversation
The saddle test in GeodesicAlgorithmExact::propagate required and unrequired the curvatures for every popped interval. Require them once before the loop and release them after it instead. Results are unchanged; add a test that the distance field does not depend on whether the caller holds the curvatures.
check_stop_conditions returned false whenever there were no stop vertices, so propagate(source, max_propagation_distance) ignored the bound and covered the whole mesh. Stop once the queue passes the bound when no stop points are given; the behaviour with stop points is unchanged. Add tests with and without stop points.
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.
Two small changes to
GeodesicAlgorithmExact(src/surface/exact_geodesics.cpp), one commit each.1.
max_propagation_distanceis ignored when there are no stop pointsThe documentation says: "Once
propagateidentifies all vertices withinmax_propagation_distanceof the source points, it stops." Without stop points it never stops early:check_stop_conditionsreturns false wheneverm_stop_verticesis empty, so the bound has no effect and the propagation covers the whole mesh.Reproduction:
Fix: with no stop vertices,
check_stop_conditionsreturns true once the queue has passed a finite bound. With no bound and no stop points it still runs to the end. Vertices beyond the bound may now be left atGEODESIC_INF. Vertices closer than the bound keep their exact distance.Behaviour with stop points is unchanged. When both a finite bound and stop points are given, propagation continues until both conditions hold. The documentation can be read either way for that case, so I left it alone.
2. Vertex Gaussian curvatures are required per popped interval
The saddle test in
propagatecalledrequireVertexGaussianCurvatures()andunrequireVertexGaussianCurvatures()for each endpoint of each popped interval. This change requires them once before the loop and releases them after it.This is a cleanup rather than a large speed-up:
unrequire()does not free the buffer, so the repeated calls cost a counter update each, not a recomputation.Results
Measured with master's and this branch's
exact_geodesics.cpp, on 14 common test meshes (1.8k to 112k vertices) and 10 Thingi10K models, 20 vertex pairs each.Stop-vertex queries (
propagate(v0, GEODESIC_INF, {v1})): 480 of 480 distances are bit-identical to master. There is no speed change to claim: the removed calls cost about 13 cycles per require/unrequire pair, against 2900 to 9600 cycles per popped interval, so about 0.3 to 0.9% of a query by that count. A master-vs-branch comparison could not resolve this on a loaded machine (geometric mean of CPU cycles master/branch 0.999; the same comparison between two identical builds gave 1.015, with per-mesh ratios of 0.87 to 1.25 and 0.93 to 1.15 respectively).Bounded propagation without stop points (
propagate(v0, d), withdthe distance fromv0tov1, 3 pairs per mesh):v1itself, on the bound, and the difference is rounding.On a larger model, Thingi10K 719791 (871k vertices), three nearby pairs: bounded propagation reached 1466, 4933 and 6985 vertices in 11 to 24 Gcycles, about what the matching stop-vertex query costs. Those three stop-vertex distances are bit-identical to master. Master's bounded run was not measured there, since it covers the whole mesh: a single stop-vertex query between distant vertices had not finished after 84 CPU minutes.
Tests
New
test/src/exact_geodesics_test.cpp:MaxPropagationDistanceWithoutStopPoints: fails on master, passes here.MaxPropagationDistanceWithStopPoints: stop points alone, and stop points with a bound.ResultIndependentOfRequiredQuantities: the distance field is the same whether or not the caller holds the curvatures.The full test suite passes (204 tests).