Repository navigation
Fix locate_navcel on multi-surface maps and speed up queries - #29
Merged
Merged
Conversation
locate_navcel now returns the surface closest in z, whatever the order of the surfaces, also when the point lies exactly on a surface. A NavCel -> surface index replaces the linear owner lookups, the hint walk steps across the right edge and re-checks height_eps where it ends, and both the hint and the search paths project the point vertically. The closest-hit raycast visits surfaces by AABB entry and prunes by the best t, BVH traversals use a fixed stack, and closest_navcel visits the nearer child first. Adds randomized tests against brute-force references, robustness tests, multi-floor tests, and an optional micro-benchmark (NAVMAP_BUILD_BENCH). Fixes #28 Signed-off-by: Francisco Martín Rico <fmrico@gmail.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.
Fixes #28
locate_navcelcould return the wrong floor, and with a hint it was much slower than without one. Covering the fix with randomized tests turned up four more problems in the same code path; the list is in the issue comment.Changes in
navmap_corerebuild_geometry_accels()and kept on copy, assign and move, replaces three linearstd::findlookups. New publicsurface_of(cid)andkNoSurface. If the index is stale, it falls back to the linear search.neighbor[(i+1)%3]).height_epson the cell where it ends.std::stackper query.closest_navcelvisits the nearer child first.Tests
test_navmap_properties: random multi-surface maps, with sloped and bumpy patches overlapping in XY and interleaved NavCel ids. It checkslocate_navcel(with and without hints),raycast,raycast_manyandclosest_navcelagainst brute-force references, and checks adjacency symmetry. It also has robustness cases: NaN/inf queries, empty map and empty surfaces, degenerate triangles, points on vertices and edges, every cell as a hint, a cell in two surfaces, and repeated rebuilds.test_locate_multisurface: floors in normal and reversed order, points on, between, above and below floors, valid and invalid hints, hints on another floor, the hint walk,surface_ofafter copy, assign and move or with a stale index, andset_area/sample_layer_aton the right floor. The regression tests fail without the fix.navmap_coretests also pass under ASan/UBSan.navmap_ros,navmap_examplesand the EasyNav NavMap maps manager, planner and localizer pass unchanged.Benchmark
Optional
navmap_bench(-DNAVMAP_BUILD_BENCH=ON, off by default). 5 cm cells, before vs after:A deep copy costs about 10% more because it now copies the surface index.