Issue #40: phone-side map slice geometry (clip, project, simplify) #117

Merged

robert merged 1 commit from area/map-viewport-geometry into main 2026-09-05 23:06:36 +02:00

Conversation 0 Commits 1 Files changed 17 +12549 -9

robert commented 2026-09-05 23:03:37 +02:00

Owner

Copy link

Implements #40 per D39's 2026-09-02 update, which supersedes the issue's own original acceptance criteria (screen-coordinate projection, hard-coded 200x228 grid). Scope is strictly the phone-side geometry pipeline: anchor selection, clip, metric-frame project, Douglas-Peucker simplify, cue placement, and the refresh decision. #41 (chunked MAP_POLYLINE transport) and #42 (watch-side rendering) are untouched.

New types (companion/map)

• MapAnchor + metersEastNorthOf: plain signed lat/lon anchor; flat-earth projection of a GpxPoint to meters east/north of it, shared across one whole slice (not re-derived per segment, unlike core's per-segment projections).
• MetricOffsetMeters (pre-quantisation Double meters) vs. MetricPoint (the wire's own int16 decimetre pair) -- kept as two types on purpose so Douglas-Peucker runs before quantisation, not after.
• MetricPolylineSimplifier: Douglas-Peucker over the already-projected Cartesian metric frame.
• MapCueMarker / MapSlice: output types, with MapSlice.needsRefresh() implementing D39's "as the rider nears the edge of the sent slice" decision, including a route-end special case.
• MapSliceBuilder: the pipeline itself.

Reused vs. new, and why

Reused directly:

• RouteSnapper/RouteSnap (#31) for "where is the rider right now" -- MapSliceBuilder never re-derives a position from a raw GPS fix; it only reads RouteSnap.distanceAlongRouteMeters (not segmentStartIndex, which it doesn't need since it re-scans its own clip window independently).
• Cue/Direction/EnrichmentTier as-is. Direction is carried through to MapCueMarker unchanged, not pre-encoded to a wire byte -- that numeric mapping is #41's call (and Direction's own KDoc already says nothing should depend on its ordinal for a wire value).
• cumulativeDistancesMeters, widened from internal to public in RouteGeodesy.kt -- its first cross-module consumer. internal in Kotlin is module-private, not package-private, so this single, behaviour-preserving visibility change was necessary for a straight reuse rather than a reimplementation. (haversineMeters, locateAlongPolyline, PolylineLocation were left internal -- no call site here needed them directly.)

Deliberately NOT reused, with the reason documented in code:

• PolylineSimplifier (core, GPX import's Douglas-Peucker): operates on GpxPoint (lat/lon) and re-derives a local equirectangular projection (cos(lat)) per RDP run, because its callers hand it raw lat/lon with no shared frame. This issue's input is already Cartesian (every point already shares one fixed anchor), so there is no lat/lon and no trigonometry left to do -- reusing it would mean re-deriving lat/lon just to have it immediately re-projected back to meters. It is also internal to :companion:core in any case, so a straight cross-module call isn't even available. MetricPolylineSimplifier is a second, small, well-justified implementation, mirroring PolylineSimplifier's own iterative-stack design for the identical reason (avoiding recursion-depth blowup on a pathological zigzag).
• locateAlongPolyline: not needed -- RouteSnapper already solves "where is the rider" (including the self-crossing disambiguation from #39's audit), and cues are placed via route.points[cue.polylineIndex] directly (a Cue already carries the exact vertex it was enriched against).

Key design decisions

• Anchor is the rider's own on-route position, interpolated (not snapped to the nearest vertex) at the exact distanceAlongRouteMeters RouteSnap reports -- MapSliceBuilder.pointAtDistance is the (new, small) inverse of locateAlongPolyline: distance -> point, which nothing existing provides. One direct, tested consequence: MapSlice.riderPosition is always exactly MetricPoint(0, 0).
• Anchor representation stays plain signed degrees, not the wire's +90/+180-offset-to-unsigned trick (MAP_ANCHOR_LAT/MAP_ANCHOR_LON are uint32). That offset is an AppMessage encoding necessity applied once, at serialization time -- #41's job -- not a domain concept; baking it in here would just make #41 undo it.
• Douglas-Peucker tolerance is derived from the specific connected watch's screenWidthPx/screenHeightPx (received once at handshake, SCREEN_W/SCREEN_H per docs/PROTOCOL.md section 3), passed into MapSliceBuilder.build as plain parameters -- never a hard-coded 200x228 constant, which is exactly what D39's update killed. screenDerivedToleranceMeters uses max(width, height) as the reference dimension (not min): heading-up rotation means either axis can end up aligned with the direction of travel, so the longer axis (more pixels, finer resolution) is the conservative choice -- using the shorter axis would discard detail the watch's longer axis could actually render. One pixel-equivalent of the clip window's real-world span is the tolerance, floored at MIN_TOLERANCE_METERS (0.1 m) -- the wire's own decimetre quantisation grain, below which a finer tolerance is meaningless.
• int16/decimetre range safety: MetricPoint's init hard-requires both axes into Short.MIN_VALUE..Short.MAX_VALUE (+-3.2 km, D39's own number) rather than silently clamping -- an out-of-range point is a bug in the caller (too large a clip window), not something to paper over. MapSliceBuilder's default window (100 m behind + 400 m ahead = 500 m) sits far inside that range by construction (arc length along the route is always >= the anchor-to-point straight-line distance). Verified directly in MetricPointTest at the exact boundary: 32767 dm (3276.7 m) accepted, 32768 dm (3276.8 m) rejected.
• needsRefresh fires when the rider is within a configurable margin (default 50 m) of the slice's forward edge -- "nears the edge", not "at" or "every frame/second" -- with a route-end special case so a slice that already reaches the route's own end never asks for another that can't exist.

Wire shapes verified, not guessed

Checked shared/message_keys.json and docs/PROTOCOL.md section 2.5 for MAP_ANCHOR_LAT/MAP_ANCHOR_LON/MAP_POLYLINE/MAP_CUES/MAP_POS/MAP_HEADING before writing any type -- MetricPoint matches MAP_POLYLINE/MAP_CUES/MAP_POS's int16-decimetre-pair shape exactly, MapCueMarker matches MAP_CUES's east, north, direction triple.

Tests -- real host runs, not hand-review

./gradlew :companion:map:test --rerun-tasks: 39 cases, all green. Also ran :companion:core:test, :companion:route:test (unaffected, still green) and :companion:assembleDebug (whole app still assembles after removing the MapViewport.kt placeholder).

Covers: int16 range/overflow at the exact +-3.2 km boundary; the anchor-relative projection round-tripping to known real-world distances (0.001 deg latitude ~= 111.32 m, longitude scaled by cos(lat)); Douglas-Peucker genuinely dropping a sub-tolerance wiggle while preserving a real corner (both synthetic, exact-value tests, and against the real fixture); cue markers independently re-derived and checked against the identical anchor/projection the builder used; the full needsRefresh decision matrix including the route-end special case; and an end-to-end run wiring a real RouteSnapper into MapSliceBuilder against the same real komoot export RouteSnapperTest uses (real-komoot-havelchaussee-glienicker-bruecke.gpx, copied into this module's own test resources -- no shared java-test-fixtures setup exists yet to avoid the duplication cleanly).

Note on test design: most scenarios construct RouteSnap directly at a caller-chosen distanceAlongRouteMeters rather than driving a fresh RouteSnapper to an arbitrary position -- RouteSnapper.snap always starts its search window at cursor 0 (by design, per its own KDoc), so reaching a position kilometers into a real route from a fresh instance either needs a long walked fix sequence or isn't representative of what MapSliceBuilder actually consumes (only the distance number). One genuine end-to-end test exercises the real RouteSnapper -> MapSliceBuilder path near the route's start, where a fresh snap naturally lands correctly.

Claude-Session: https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

Implements #40 per D39's 2026-09-02 update, which supersedes the issue's own original acceptance criteria (screen-coordinate projection, hard-coded 200x228 grid). Scope is strictly the phone-side geometry pipeline: anchor selection, clip, metric-frame project, Douglas-Peucker simplify, cue placement, and the refresh decision. #41 (chunked MAP_POLYLINE transport) and #42 (watch-side rendering) are untouched. ### New types (`companion/map`) - `MapAnchor` + `metersEastNorthOf`: plain signed lat/lon anchor; flat-earth projection of a `GpxPoint` to meters east/north of it, shared across one whole slice (not re-derived per segment, unlike core's per-segment projections). - `MetricOffsetMeters` (pre-quantisation `Double` meters) vs. `MetricPoint` (the wire's own `int16` decimetre pair) -- kept as two types on purpose so Douglas-Peucker runs before quantisation, not after. - `MetricPolylineSimplifier`: Douglas-Peucker over the already-projected Cartesian metric frame. - `MapCueMarker` / `MapSlice`: output types, with `MapSlice.needsRefresh()` implementing D39's "as the rider nears the edge of the sent slice" decision, including a route-end special case. - `MapSliceBuilder`: the pipeline itself. ### Reused vs. new, and why **Reused directly:** - `RouteSnapper`/`RouteSnap` (#31) for "where is the rider right now" -- `MapSliceBuilder` never re-derives a position from a raw GPS fix; it only reads `RouteSnap.distanceAlongRouteMeters` (not `segmentStartIndex`, which it doesn't need since it re-scans its own clip window independently). - `Cue`/`Direction`/`EnrichmentTier` as-is. `Direction` is carried through to `MapCueMarker` unchanged, not pre-encoded to a wire byte -- that numeric mapping is #41's call (and `Direction`'s own KDoc already says nothing should depend on its ordinal for a wire value). - `cumulativeDistancesMeters`, widened from `internal` to `public` in `RouteGeodesy.kt` -- its first cross-module consumer. `internal` in Kotlin is module-private, not package-private, so this single, behaviour-preserving visibility change was necessary for a straight reuse rather than a reimplementation. (`haversineMeters`, `locateAlongPolyline`, `PolylineLocation` were left `internal` -- no call site here needed them directly.) **Deliberately NOT reused, with the reason documented in code:** - `PolylineSimplifier` (core, GPX import's Douglas-Peucker): operates on `GpxPoint` (lat/lon) and re-derives a local equirectangular projection (`cos(lat)`) *per RDP run*, because its callers hand it raw lat/lon with no shared frame. This issue's input is already Cartesian (every point already shares one fixed anchor), so there is no lat/lon and no trigonometry left to do -- reusing it would mean re-deriving lat/lon just to have it immediately re-projected back to meters. It is also `internal` to `:companion:core` in any case, so a straight cross-module call isn't even available. `MetricPolylineSimplifier` is a second, small, well-justified implementation, mirroring `PolylineSimplifier`'s own iterative-stack design for the identical reason (avoiding recursion-depth blowup on a pathological zigzag). - `locateAlongPolyline`: not needed -- `RouteSnapper` already solves "where is the rider" (including the self-crossing disambiguation from #39's audit), and cues are placed via `route.points[cue.polylineIndex]` directly (a `Cue` already carries the exact vertex it was enriched against). ### Key design decisions - **Anchor** is the rider's own on-route position, *interpolated* (not snapped to the nearest vertex) at the exact `distanceAlongRouteMeters` `RouteSnap` reports -- `MapSliceBuilder.pointAtDistance` is the (new, small) inverse of `locateAlongPolyline`: distance -> point, which nothing existing provides. One direct, tested consequence: `MapSlice.riderPosition` is always exactly `MetricPoint(0, 0)`. - **Anchor representation stays plain signed degrees**, not the wire's `+90`/`+180`-offset-to-unsigned trick (`MAP_ANCHOR_LAT`/`MAP_ANCHOR_LON` are `uint32`). That offset is an AppMessage encoding necessity applied once, at serialization time -- #41's job -- not a domain concept; baking it in here would just make #41 undo it. - **Douglas-Peucker tolerance** is derived from the *specific connected watch's* `screenWidthPx`/`screenHeightPx` (received once at handshake, `SCREEN_W`/`SCREEN_H` per docs/PROTOCOL.md section 3), passed into `MapSliceBuilder.build` as plain parameters -- never a hard-coded 200x228 constant, which is exactly what D39's update killed. `screenDerivedToleranceMeters` uses `max(width, height)` as the reference dimension (not `min`): heading-up rotation means either axis can end up aligned with the direction of travel, so the *longer* axis (more pixels, finer resolution) is the conservative choice -- using the shorter axis would discard detail the watch's longer axis could actually render. One pixel-equivalent of the clip window's real-world span is the tolerance, floored at `MIN_TOLERANCE_METERS` (0.1 m) -- the wire's own decimetre quantisation grain, below which a finer tolerance is meaningless. - **int16/decimetre range safety**: `MetricPoint`'s `init` hard-`require`s both axes into `Short.MIN_VALUE..Short.MAX_VALUE` (+-3.2 km, D39's own number) rather than silently clamping -- an out-of-range point is a bug in the caller (too large a clip window), not something to paper over. `MapSliceBuilder`'s default window (100 m behind + 400 m ahead = 500 m) sits far inside that range by construction (arc length along the route is always >= the anchor-to-point straight-line distance). Verified directly in `MetricPointTest` at the exact boundary: 32767 dm (3276.7 m) accepted, 32768 dm (3276.8 m) rejected. - **needsRefresh** fires when the rider is within a configurable margin (default 50 m) of the slice's forward edge -- "nears the edge", not "at" or "every frame/second" -- with a route-end special case so a slice that already reaches the route's own end never asks for another that can't exist. ### Wire shapes verified, not guessed Checked `shared/message_keys.json` and `docs/PROTOCOL.md` section 2.5 for `MAP_ANCHOR_LAT`/`MAP_ANCHOR_LON`/`MAP_POLYLINE`/`MAP_CUES`/`MAP_POS`/`MAP_HEADING` before writing any type -- `MetricPoint` matches `MAP_POLYLINE`/`MAP_CUES`/`MAP_POS`'s `int16`-decimetre-pair shape exactly, `MapCueMarker` matches `MAP_CUES`'s `east, north, direction` triple. ### Tests -- real host runs, not hand-review `./gradlew :companion:map:test --rerun-tasks`: 39 cases, all green. Also ran `:companion:core:test`, `:companion:route:test` (unaffected, still green) and `:companion:assembleDebug` (whole app still assembles after removing the `MapViewport.kt` placeholder). Covers: int16 range/overflow at the exact +-3.2 km boundary; the anchor-relative projection round-tripping to known real-world distances (0.001 deg latitude ~= 111.32 m, longitude scaled by `cos(lat)`); Douglas-Peucker genuinely dropping a sub-tolerance wiggle while preserving a real corner (both synthetic, exact-value tests, and against the real fixture); cue markers independently re-derived and checked against the identical anchor/projection the builder used; the full `needsRefresh` decision matrix including the route-end special case; and an end-to-end run wiring a real `RouteSnapper` into `MapSliceBuilder` against the same real komoot export `RouteSnapperTest` uses (`real-komoot-havelchaussee-glienicker-bruecke.gpx`, copied into this module's own test resources -- no shared `java-test-fixtures` setup exists yet to avoid the duplication cleanly). Note on test design: most scenarios construct `RouteSnap` directly at a caller-chosen `distanceAlongRouteMeters` rather than driving a fresh `RouteSnapper` to an arbitrary position -- `RouteSnapper.snap` always starts its search window at cursor 0 (by design, per its own KDoc), so reaching a position kilometers into a real route from a fresh instance either needs a long walked fix sequence or isn't representative of what `MapSliceBuilder` actually consumes (only the distance number). One genuine end-to-end test exercises the real `RouteSnapper` -> `MapSliceBuilder` path near the route's start, where a fresh snap naturally lands correctly. Claude-Session: https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

robert added 1 commit 2026-09-05 23:03:37 +02:00

Issue #40: phone-side map slice geometry (clip, project, simplify) ... 4eb83c04a4

Per D39's 2026-09-02 update to #40, which supersedes the issue's original
acceptance criteria: the phone clips and Douglas-Peucker-simplifies a slice
of route geometry in a stable anchor-relative metric frame. It does not
project to screen coordinates -- the watch does that (#42), out of scope
here -- and it does not put bytes on the wire (#41, also out of scope).

New in :companion:map:
- MapAnchor + metersEastNorthOf: a plain signed lat/lon anchor and the
  flat-earth projection of a GpxPoint into meters east/north of it, shared
  across one whole slice rather than re-derived per segment.
- MetricOffsetMeters / MetricPoint: pre-quantisation Double meters vs. the
  wire's own int16-decimetre type, with a hard range check at the +-3.2 km
  D39 states.
- MetricPolylineSimplifier: Douglas-Peucker over the already-projected
  Cartesian metric frame -- a second, deliberately separate implementation
  from core's lat/lon PolylineSimplifier (see its own KDoc for why: no
  lat/lon, no per-run trigonometry left to do once points are already in
  one shared metric frame).
- MapCueMarker / MapSlice: the upcoming-cue and full-slice output types,
  with MapSlice.needsRefresh() implementing D39's "as the rider nears the
  edge of the sent slice" refresh decision (with a route-end special case
  so a slice already reaching the route's own end never asks again).
- MapSliceBuilder: the pipeline -- interpolates the anchor at the rider's
  exact distance-along-route, clips to a behind/ahead window measured in
  real distance, projects, derives a Douglas-Peucker tolerance from the
  *actual connected watch's* screenWidthPx/screenHeightPx (never a
  hard-coded 200x228 grid), simplifies, quantises, and places upcoming cues
  in the same frame.

Reused rather than reimplemented:
- RouteSnapper/RouteSnap (#31) for "where is the rider right now" --
  MapSliceBuilder never re-derives a position from a raw GPS fix.
- core's cumulativeDistancesMeters, widened from internal to public (its
  first cross-module consumer) for measuring the real-world clip window.
- Cue/Direction/EnrichmentTier as-is for cue markers -- Direction is
  carried through, not pre-encoded to a wire byte (that's #41's call).

Deliberately not reused: core's PolylineSimplifier (operates on lat/lon
with per-run trigonometry the metric frame no longer needs) and
locateAlongPolyline (RouteSnapper already solves "where is the rider").

companion/map/build.gradle.kts gains the same JUnit-Platform testOptions
and Kotest testImplementation trio :companion:location/:companion:pebble
already use for their own first host-testable, non-android.* classes.

Placeholder MapViewport.kt removed (nothing referenced it but a stale
"still a placeholder" doc comment in RoutePreview.kt, updated).

Tests (39 cases, real host runs via
`./gradlew :companion:map:test --rerun-tasks`, all green): int16
range/overflow at the exact +-3.2 km boundary, anchor-relative projection
round-tripping to known real-world distances, Douglas-Peucker genuinely
dropping sub-tolerance wiggle while preserving a real corner, cue markers
verified in the identical frame as the polyline, the needs-refresh decision
including the route-end special case, and an end-to-end run against the
same real komoot fixture RouteSnapperTest uses (copied into this module's
own test resources -- no shared test-fixtures module exists yet).

Claude-Session: https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

robert merged commit 2e8e896b48 into main 2026-09-05 23:06:36 +02:00

robert referenced this pull request from a commit 2026-09-05 23:06:37 +02:00

Merge pull request 'Issue #40: phone-side map slice geometry (clip, project, simplify)' (#117) from area/map-viewport-geometry into main

robert referenced this pull request 2026-09-05 23:06:56 +02:00

Viewport selection, polyline clip, project and simplify #40

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:companion

Android companion app

  

area:docs

Documentation

  

area:shared

Message contract shared by both sides

  

area:tooling

Build tooling, SDK, dev scripts

  

area:watchapp

Pebble C watchapp

  

blocker

Blocks a large part of the project until resolved

  

kind:chore

Setup, scaffolding, maintenance

  

kind:feature

New functionality

  

kind:spike

Time-boxed investigation to de-risk a decision

  

kind:test

Tests and fixtures

No labels

area:companion

area:docs

area:shared

area:tooling

area:watchapp

blocker

kind:chore

kind:feature

kind:spike

kind:test

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

robert/PedalPebble!117

No description provided.