Skip to content

Document distance_to_segment limits; fix epsilon doc and test helper#26

Open
gistrec wants to merge 2 commits into
masterfrom
fix/docs-and-test-helpers
Open

Document distance_to_segment limits; fix epsilon doc and test helper#26
gistrec wants to merge 2 commits into
masterfrom
fix/docs-and-test-helpers

Conversation

@gistrec

@gistrec gistrec commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Summary

Four follow-ups from the June code review — no library behavior changes (item 4 only changes behavior on inputs that previously produced NaN).

1. kDefaultEpsilon doc was off by 1000×. 1e-12 degrees is ~111 nm ≈ 0.1 µm on Earth, not "0.1 nanometers". Fixed in latlng.hpp and docs/api.md.

2. distance_to_segment approximation documented. The closest point is found by planar projection in raw (lat, lng) coordinate space (same algorithm as android-maps-utils PolyUtil.distanceToLine), then the great-circle distance to it is returned:

  • longitude is not scaled by cos(lat), so the projection skews at high latitudes — the result can overshoot by a few percent around lat 80°;
  • longitudes are used as-is, so a segment crossing the antimeridian (±180°) is treated as spanning nearly the whole globe — a point lying exactly on such a segment reported ~11 km in the review probe.

Both poly.hpp and docs/api.md now state this and point tolerance checks at on_path (which handles geodesic segments correctly).

3. EXPECT_NEAR_LatLng antimeridian fragility. The test helper wrapped each longitude independently, so a pair like 179.9999999 vs -179.9999999 — the same meridian within tolerance — compared as ~360° apart and failed. It now compares the wrapped difference wrap(expected.lng - actual.lng, -180, 180) and prints both raw longitudes on failure. Also fixes the stale comment: wrap() yields [-180, 180), not (-180, 180].

4. hav_from_sin NaN guard. Its only caller (is_on_segment_gc) passes sin_dist13 * sin_bearing, where sin_bearing is the quotient (a·d − b·c)/√((a²+b²)(c²+d²)) — mathematically ≤ 1 by Cauchy–Schwarz, but rounding can push it 1 ulp above 1. Then sqrt(1 − x²) of a negative argument returns NaN, every subsequent NaN comparison is false, and both rejection checks in is_on_segment_gc are silently skipped — on_path/on_edge (geodesic, their default mode) return a false positive for segments under ~2.1 rad of arc. Clamp to 1 (correct limit hav(90°) = 0.5), matching the existing arc_hav clamp; regular values are unchanged bit-for-bit.

Measured cost (Apple M1, -O2, medians of per-pair deltas across 10 interleaved before/after runs):

benchmark Δ absolute
bare hav_from_sin in a tight loop +20% +0.20 ns/call (0.99 → 1.20 ns; the relative number is large only because the function costs ~1 ns)
on_path geodesic, worst case: 50-segment full scan, point off path +3.4% +1.5 ns/segment
everything else 0% hav_from_sin has no other callers

Test plan

  • New test Math.hav_from_sin_clamp: ±(1 + 1 ulp) must yield 0.5, not NaN (fails before the fix)
  • Full unit-test suite: 35/35 pass (the EXPECT_NEAR_LatLng change is strictly more permissive only at the ±180 boundary and identical elsewhere, so existing assertions keep their strength)

gistrec added 2 commits June 10, 2026 21:50
- kDefaultEpsilon was documented as "0.1 nanometers" on Earth in
  latlng.hpp and api.md; 1e-12 degrees is actually ~111 nm (~0.1 um),
  a 1000x understatement.
- distance_to_segment projects in raw (lat, lng) space: no cos(lat)
  scaling (overshoots a few percent at high latitudes) and no
  antimeridian handling (a point on a segment crossing +-180 can
  report kilometers). Document the approximation in poly.hpp and
  api.md and point tolerance checks at on_path.
- EXPECT_NEAR_LatLng wrapped each longitude independently, so pairs
  straddling +-180 (179.9999999 vs -179.9999999) compared ~360 deg
  apart and failed despite being the same meridian within tolerance.
  Compare the wrapped difference instead, print raw longitudes on
  failure, and fix the comment: wrap() yields [-180, 180), not
  (-180, 180].
The only caller passes sin_dist13 * sin_bearing, where sin_bearing is
the quotient (a*d - b*c)/sqrt((a^2+b^2)*(c^2+d^2)). Mathematically it
is <= 1 by Cauchy-Schwarz, but rounding can push it 1 ulp above 1; the
resulting sqrt(negative) NaN propagates through is_on_segment_gc where
every NaN comparison is false, silently turning on_path/on_edge into a
false positive for segments under ~2.1 rad of arc. Clamp x^2 to 1,
matching the existing arc_hav clamp. Measured cost: +0.2 ns/call on
the bare function, ~+3% on a worst-case geodesic on_path scan.
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