fix: account for road width in map matching#7643
Conversation
ede0921 to
3854d35
Compare
5423fac to
9b795db
Compare
9b795db to
537f42f
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates OSRM’s extraction → data facade → map matching pipeline to account for tagged/derived road surface width, improving matching robustness when GPS points are near the road edge rather than near the centerline (Issue #4480).
Changes:
- Persist profile-derived road width (and lane-count fallback) into edge annotations and expose them via the engine DataFacade.
- Adjust map matching emission probabilities to treat points within the road surface as having zero “outside-road” distance.
- Expand candidate lookup bounds in geospatial queries (capped) and add a testbot regression scenario covering wide-road cases.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| unit_tests/mocks/mock_datafacade.hpp | Updates the mock facade to satisfy the extended BaseDataFacade interface. |
| unit_tests/extractor/intersection_analysis_tests.cpp | Updates annotation aggregate initialization for new fields. |
| unit_tests/engine/offline_facade.cpp | Updates the offline facade test implementation with the new DataFacade methods. |
| src/extractor/scripting_environment_lua.cpp | Exposes road_width to Lua profiles via ExtractionWay. |
| src/extractor/extractor_callbacks.cpp | Encodes and stores lane count + road width into per-edge annotations during extraction. |
| src/engine/routing_algorithms/map_matching.cpp | Uses “distance outside road surface” for emission probability calculations. |
| profiles/testbot.lua | Adds lane/width handling to the testbot profile (via Guidance + lanes tag). |
| profiles/lib/measure.lua | Adds missing lib/set dependency used by the module. |
| profiles/lib/guidance.lua | Implements get_road_width parsing for multiple width tagging schemes. |
| include/extractor/node_data_container.hpp | Adds accessors to read lanes/width from stored annotations. |
| include/extractor/node_based_edge.hpp | Extends NodeBasedEdgeAnnotation to store lanes + width and includes them in comparisons. |
| include/extractor/intersection/constants.hpp | Introduces a cap for road-surface width approximation. |
| include/extractor/extraction_way.hpp | Adds road_width to the extraction result structure and resets it in clear(). |
| include/engine/geospatial_query.hpp | Broadens candidate search by capped half-width and applies per-segment half-width in filtering. |
| include/engine/datafacade/datafacade_base.hpp | Adds GetNumberOfLanes / GetRoadWidth to the facade interface. |
| include/engine/datafacade/contiguous_internalmem_datafacade.hpp | Implements the new facade methods via edge-based node data. |
| features/testbot/matching.feature | Adds a regression scenario ensuring wide roads are matched correctly over parallel distractors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
defeb76 to
b2fd876
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #7643 +/- ##
==========================================
+ Coverage 94.20% 94.22% +0.01%
==========================================
Files 483 483
Lines 37670 37843 +173
==========================================
+ Hits 35487 35656 +169
- Misses 2183 2187 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
b2fd876 to
a46d09e
Compare
DennisOSRM
left a comment
There was a problem hiding this comment.
Thanks so much for the contribution. I left a couple of comments/questions on the PR. Could you provide some experimental results showing this leads to improved matching results?
| return nil | ||
| end | ||
|
|
||
| local function first_width(way, keys) |
There was a problem hiding this comment.
Should this maybe be called find_first_width to indicate that it greedily searches for the first width tag it can find?
| end | ||
| end | ||
|
|
||
| local function sum_width_lanes(value) |
There was a problem hiding this comment.
Should this be called sum_lane_width?
|
|
||
| if found then | ||
| return total | ||
| end |
There was a problem hiding this comment.
Should this return a nil value explicitly in case the sum is zero?
| end | ||
|
|
||
| local total = 0 | ||
| local found = false |
There was a problem hiding this comment.
Is this variable actually necessary? Would it be enough to check if sum is zero or not?
| @@ -1,4 +1,5 @@ | |||
| local Sequence = require('lib/sequence') | |||
| local Set = require('lib/set') | |||
There was a problem hiding this comment.
Is this an unrelated change?
Fixes #4480.
Summary
width,est_width,width:carriageway, directional width tags, andwidth:lanes*variants.laneswhen no explicit width is tagged, capped by the maximum road-surface width.Testing
git diff --checkjq empty taginfo.jsonPR_TITLE="fix: account for road width in map matching" node scripts/check_pr_title.jsnode_modules/.bin/cucumber-js --dry-run features/testbot/matching.featurecmake --build build-local --target util-tests engine-tests extractor-tests -j 30./build-local/unit_tests/util-tests --run_test=static_rtree --log_level=test_suite./build-local/unit_tests/engine-tests --log_level=test_suite./build-local/unit_tests/extractor-tests --run_test=graph_compressor --log_level=test_suitecodex review --base origin/master --title "fix: account for road width in map matching"