Skip to content

feat(area): solve plazas up to 256 vertices, not 40 - #7698

Merged
DennisOSRM merged 1 commit into
masterfrom
area-geodesic-vertex-limit
Aug 16, 2026
Merged

feat(area): solve plazas up to 256 vertices, not 40#7698
DennisOSRM merged 1 commit into
masterfrom
area-geodesic-vertex-limit

Conversation

@DennisOSRM

Copy link
Copy Markdown
Collaborator

Issue

No issue. Found by driving a live osrm-routed on an Ile-de-France extract and noticing
that 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_VERTICES vertices, and
what 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:

limit 40 limit 256
worst detour 2.159x 1.003x
median 1.145x 1.000x
pairs over 1.1x 226 of 386 0 of 386

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:

percentile vertices
50th 22
90th 75
95th 119
99th 283
max 2,821

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:

limit 40     first 287.3   median 288.3   max 297.6 ms
limit 128    first 287.3   median 288.9   max 315.7 ms
limit 256    first 287.3   median 289.0   max 302.0 ms

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() runs visible_vertices() from
each vertex and that is itself quadratic, so the build is cubic. From
src/benchmarks/area_geodesic.cpp:

vertices     40     104     200     260     404     580
build      0.8ms   9.0ms    50ms   101ms   356ms   996ms

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 quadratic
per-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

  • self-review code for correctness and following the coding guidelines
  • add tests
  • update relevant wiki pages
  • review
  • adjust for comments

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:

  • Place de la République has only 38 vertices, so it was always solved, yet 59 of 604 clear-line
    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.
  • A plaza request costs about 288 ms server-side against 0.8 ms for a comparable route that
    touches no area. Unrelated to the solver, since it is unchanged when the solver declines.

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.
Copilot AI lite review requested due to automatic review settings August 16, 2026 16:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.74%. Comparing base (9229f17) to head (f341788).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DennisOSRM
DennisOSRM merged commit b61eb11 into master Aug 16, 2026
22 of 23 checks passed
@DennisOSRM
DennisOSRM deleted the area-geodesic-vertex-limit branch August 16, 2026 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants