feat(area): solve plazas up to 256 vertices, not 40 - #7698
Merged
Conversation
A journey with both ends inside one open area is answered by the geodesic solver, and the solver declines any area with more vertices than this. What it declines falls back to the mesh, and the mesh holds only the shortest-path trees rooted at the entry points, so the answer walks out towards an entry point and comes back instead of going straight. That is visible on real data. On the Notre-Dame parvis, 120 vertices, 226 of 386 sampled crossings whose straight line was entirely inside the free space came back more than a tenth longer than that line, the worst at 2.16 times. All 386 are within 1.003 of it once the area is solved. The old limit was chosen against Monaco, whose largest area has 24 vertices, and the comment said it was a guard against the pathological rather than a real constraint. That does not hold for a city. In Ile-de-France the median pedestrian area has 22 vertices but the 95th percentile has 119, and 40 declines a quarter of all 6321 of them. 256 covers 98.7%. The cost is a one-off build, cached per area per thread, and the benchmark puts it at about 100 ms at the new ceiling. Nothing is added to the per-request path: the same plaza request measured at limits 40, 128 and 256 takes 288 ms in all three cases. What is still declined are the genuinely large areas, up to 2821 vertices here, where a cubic build cannot be paid anywhere in a request. Going further wants the build to get cheaper first; the extractor already solves the same problem with a rotational sweep at O(n log n) per vertex.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7698 +/- ##
==========================================
- Coverage 94.75% 94.74% -0.02%
==========================================
Files 519 519
Lines 41582 41582
==========================================
- Hits 39402 39397 -5
- Misses 2180 2185 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Issue
No issue. Found by driving a live
osrm-routedon an Ile-de-France extract and noticingthat a walk across the Notre-Dame parvis came back as a right angle where a straight line
was available.
What is wrong
A journey with both ends inside one open area is answered by the geodesic solver added in
#7685. The solver declines any area with more than
GEODESIC_MAX_VERTICESvertices, andwhat it declines falls back to the mesh. The mesh deliberately holds only the shortest-path
trees rooted at the entry points, so a journey between two interior points walks out
towards an entry point and comes back rather than going straight.
The Notre-Dame parvis has 120 vertices, three times the old limit of 40.
Sampling 386 pairs of interior points on that plaza whose straight line stays entirely
inside the free space:
Five other Paris plazas behave the same way. At 256, Madeleine, Bastille, Châtelet and
Sorbonne all come out at worst 1.003x with zero pairs over 1.1x.
Why 40 was wrong
The limit was measured against Monaco, and the comment said so: "Areas that occur are far
smaller than either -- Monaco's largest has 24 -- so the limit is a guard against the
pathological rather than a real constraint."
That does not survive contact with a city. Over all 6,321 pedestrian areas in
Ile-de-France:
40 declines a quarter of them. 256 covers 98.7%. So it was a real constraint on real data,
not a guard.
What it costs
Nothing on the per-request path. The same plaza request, measured server-side at three
different limits, thirty times each:
Identical. (That 288 ms is itself worth looking at, but it is not this: it is the same with
the solver declining the area entirely. Separate matter, noted below.)
A one-off build, cached per area per thread.
solve()runsvisible_vertices()fromeach vertex and that is itself quadratic, so the build is cubic. From
src/benchmarks/area_geodesic.cpp:About 100 ms at the new ceiling, once. I checked the cache actually amortises it rather
than assuming: instrumenting the build counter shows one solve across repeated requests to
the same area on a single-threaded server.
What is still declined, deliberately
The genuinely large areas. Ile-de-France has one with 2,821 vertices; at a cubic build that
cannot be paid anywhere in a request, cached or not.
Going higher wants the build to get cheaper first rather than trading correctness against
latency. The extractor already solves the same problem with a rotational sweep at O(n log n)
per vertex in
src/extractor/area/visibility_graph.cpp, against the engine's quadraticper-vertex scan. Reusing it would move this number rather than being traded against it.
Testing
All fourteen unit suites pass. Full cucumber: 1478 scenarios, 1463 passed, 15 skipped, 0
failed, the same as before the change. Verified live on Ile-de-France
across seven named Paris plazas, with the interior-crossing numbers above.
Was this change primarily generated using an AI tool? Yes.
🤖 Claude Code, Claude Opus 5
Tasklist
Requirements / Relations
Needs #7695 to build a real extract at all, and #7697 to serve one with steps.
Two things this does not fix, both found while measuring it and both separate:
interior pairs still exceed 1.1x and the worst of them visit the same coordinate twice,
going out and doubling back. Different cause, not diagnosed.
touches no area. Unrelated to the solver, since it is unchanged when the solver declines.