diff --git a/mobile/feature/nearby/src/main/kotlin/ac/jfx/openptv/feature/nearby/ColocationSpread.kt b/mobile/feature/nearby/src/main/kotlin/ac/jfx/openptv/feature/nearby/ColocationSpread.kt new file mode 100644 index 00000000..f9382d40 --- /dev/null +++ b/mobile/feature/nearby/src/main/kotlin/ac/jfx/openptv/feature/nearby/ColocationSpread.kt @@ -0,0 +1,76 @@ +package ac.jfx.openptv.feature.nearby + +import ac.jfx.openptv.core.model.Coordinates +import ac.jfx.openptv.core.model.Stop +import kotlin.math.PI +import kotlin.math.cos +import kotlin.math.roundToLong +import kotlin.math.sin + +/** + * Issue #172: PTV gives different modes serving one physical station identical coordinates — the + * Richmond train stop and the V/Line "Richmond Railway Station" are both `stop_id 1162` at the same + * lat/lng. Drawn as-is their map dots stack on a single pixel, so only the top one is visible and + * tappable. + * + * [spreadColocatedStops] resolves each stop to the coordinate it should actually be drawn at: a stop + * with no neighbour on its point keeps its exact coordinate; a group sharing a point is fanned evenly + * around a small circle. Slot order within a group is sorted by route type then stop id, so a stop's + * nudged position is deterministic and never jitters between renders (the train dot always sits in + * the same place relative to its V/Line twin). [MapLibreOpenPtvMap] uses this for both rendering and + * the tap hit-test, so a fanned-out dot is selectable exactly where it's shown. + * + * Grouping is expected to run on the already-filtered stop list, so only stops that genuinely overlap + * under the current route-type filters are nudged — a lone V/Line stop (train filtered off) stays on + * its true coordinate. + */ +internal fun spreadColocatedStops(stops: List): List> { + val byLocation = stops.groupBy { colocationKey(it.latitude, it.longitude) } + return stops.map { stop -> + val group = byLocation.getValue(colocationKey(stop.latitude, stop.longitude)) + stop to displayCoordinate(stop, group) + } +} + +/** + * Where [stop] should be drawn given the [group] of stops sharing its point. The longitude offset is + * divided by cos(latitude) so the nudge stays roughly circular on the ground rather than stretched + * east-west at Melbourne's latitude. + */ +private fun displayCoordinate( + stop: Stop, + group: List, +): Coordinates { + if (group.size <= 1) return Coordinates(lat = stop.latitude, lng = stop.longitude) + val ordered = group.sortedWith(compareBy({ it.routeType.toCode() }, { it.id.value })) + val index = ordered.indexOf(stop) + val angle = 2.0 * PI * index / ordered.size + val latitudeOffset = COLOCATION_OFFSET_DEG * sin(angle) + val longitudeOffset = + COLOCATION_OFFSET_DEG * cos(angle) / cos(stop.latitude * PI / DEGREES_PER_RADIAN) + return Coordinates( + lat = stop.latitude + latitudeOffset, + lng = stop.longitude + longitudeOffset, + ) +} + +/** + * Bucket key for co-location detection: round lat/lng onto [COLOCATION_GRID] so stops on the same + * (or all-but-identical) point hash together. + */ +private fun colocationKey( + latitude: Double, + longitude: Double, +): Pair = + (latitude * COLOCATION_GRID).roundToLong() to (longitude * COLOCATION_GRID).roundToLong() + +// Grid for grouping stops that share a coordinate. 1e5 → ~1.1 m cells, so exact and near-exact +// duplicates land in the same bucket without false-merging genuinely distinct neighbouring stops. +private const val COLOCATION_GRID: Double = 1e5 + +// Radius each co-located dot is nudged from the shared point. ~20 m ≈ one pin diameter of +// separation at street zoom; small enough that the stop isn't meaningfully misplaced. +private const val COLOCATION_OFFSET_METERS: Double = 20.0 +private const val METERS_PER_DEGREE_LAT: Double = 111_320.0 +private const val COLOCATION_OFFSET_DEG: Double = COLOCATION_OFFSET_METERS / METERS_PER_DEGREE_LAT +private const val DEGREES_PER_RADIAN: Double = 180.0 diff --git a/mobile/feature/nearby/src/main/kotlin/ac/jfx/openptv/feature/nearby/MapLibreOpenPtvMap.kt b/mobile/feature/nearby/src/main/kotlin/ac/jfx/openptv/feature/nearby/MapLibreOpenPtvMap.kt index 6a38c936..a066dd65 100644 --- a/mobile/feature/nearby/src/main/kotlin/ac/jfx/openptv/feature/nearby/MapLibreOpenPtvMap.kt +++ b/mobile/feature/nearby/src/main/kotlin/ac/jfx/openptv/feature/nearby/MapLibreOpenPtvMap.kt @@ -65,6 +65,14 @@ import javax.inject.Singleton * pins can be on screen at once, so the unclustered point count stays well within MapLibre's * comfort zone. * + * **Co-located stops (issue #172).** Different modes routinely share one physical station, and PTV + * gives them identical coordinates — e.g. the Richmond train stop and the V/Line "Richmond Railway + * Station" are both `stop_id 1162` at the same lat/lng. Rendered as-is their circles stack on the + * same pixel and only the top one is visible/tappable. [applyPins] fans any group of stops sharing + * a coordinate out around a small fixed-radius circle (see [spreadColocatedStops]); a lone stop + * keeps its exact coordinate. We deliberately do *not* re-enable clustering to fix this (that brings back + * the #124 zoom-extreme bug) — the offset is a few metres, well under one stop's spacing. + * * Going with [CircleLayer] (rather than icon bitmaps) keeps the impl asset-free — every colour * is derivable from a constant, and adding a route type is one row in [routeTypeColor]. If a * future Roborazzi screenshot ever needs distinct icons we'll swap in a bitmap factory; until @@ -207,16 +215,18 @@ internal class MapLibreOpenPtvMap ) val tap = Coordinates(lat = latLng.latitude, lng = latLng.longitude) + // Issue #172: hit-test against the displayed positions so + // co-located stops fanned apart by [spreadColocatedStops] + // are each individually tappable; tapping the shared + // underlying coordinate would otherwise always resolve to + // the same one. val hit = - pinsLatest.minByOrNull { stop -> - tap.distanceTo(Coordinates(stop.latitude, stop.longitude)) - } - val dist = - hit?.let { - tap.distanceTo(Coordinates(it.latitude, it.longitude)) + spreadColocatedStops(pinsLatest).minByOrNull { (_, coord) -> + tap.distanceTo(coord) } + val dist = hit?.let { tap.distanceTo(it.second) } if (hit != null && dist != null && dist <= PIN_HIT_RADIUS_METERS) { - onPinClickedLatest(hit) + onPinClickedLatest(hit.first) } } } @@ -388,9 +398,13 @@ internal class MapLibreOpenPtvMap ) { val style = map.style ?: return val source = style.getSourceAs(SOURCE_PINS) ?: return + // Issue #172: draw each stop at its (possibly fanned-out) display position so + // co-located stops don't stack on one pixel. [spreadColocatedStops] is the single source + // of these positions — the tap hit-test uses it too, so a fanned-out dot is tappable + // where it's actually drawn rather than at the shared underlying coordinate. val features = - pins.map { stop -> - Feature.fromGeometry(Point.fromLngLat(stop.longitude, stop.latitude)).apply { + spreadColocatedStops(pins).map { (stop, coord) -> + Feature.fromGeometry(Point.fromLngLat(coord.lng, coord.lat)).apply { addNumberProperty(GEOJSON_PROP_ROUTE_TYPE, stop.routeType.toCode()) addNumberProperty(GEOJSON_PROP_STOP_ID, stop.id.value) } diff --git a/mobile/feature/nearby/src/test/kotlin/ac/jfx/openptv/feature/nearby/ColocationSpreadTest.kt b/mobile/feature/nearby/src/test/kotlin/ac/jfx/openptv/feature/nearby/ColocationSpreadTest.kt new file mode 100644 index 00000000..c4bd648b --- /dev/null +++ b/mobile/feature/nearby/src/test/kotlin/ac/jfx/openptv/feature/nearby/ColocationSpreadTest.kt @@ -0,0 +1,96 @@ +package ac.jfx.openptv.feature.nearby + +import ac.jfx.openptv.core.model.Coordinates +import ac.jfx.openptv.core.model.RouteType +import ac.jfx.openptv.core.testing.StopMother +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +/** + * Coverage for [spreadColocatedStops] — the pure fan-out that stops co-located map pins (e.g. the + * Richmond train stop and the V/Line "Richmond Railway Station", both `stop_id 1162` at identical + * lat/lng) from stacking on a single pixel (issue #172). + */ +class ColocationSpreadTest { + @Test + fun `a lone stop keeps its exact coordinate`() { + val stop = StopMother.aStop().withLatitude(RICHMOND_LAT).withLongitude(RICHMOND_LNG).build() + + val result = spreadColocatedStops(listOf(stop)) + + assertThat(result).hasSize(1) + val (resolved, coord) = result.single() + assertThat(resolved).isEqualTo(stop) + assertThat(coord).isEqualTo(Coordinates(lat = RICHMOND_LAT, lng = RICHMOND_LNG)) + } + + @Test + fun `two stops on the same point are fanned apart so each dot is distinct`() { + val train = + StopMother.aStop() + .withId(1162) + .withRouteType(RouteType.Train) + .withLatitude(RICHMOND_LAT) + .withLongitude(RICHMOND_LNG) + .build() + val vline = + StopMother.aStop() + .withId(1162) + .withRouteType(RouteType.VLine) + .withLatitude(RICHMOND_LAT) + .withLongitude(RICHMOND_LNG) + .build() + + val byStop = spreadColocatedStops(listOf(train, vline)).toMap() + + val shared = Coordinates(lat = RICHMOND_LAT, lng = RICHMOND_LNG) + val trainCoord = byStop.getValue(train) + val vlineCoord = byStop.getValue(vline) + + // Each dot is nudged off the shared point by roughly the configured ~20 m radius... + assertThat(shared.distanceTo(trainCoord)).isWithin(TOLERANCE_M).of(OFFSET_M) + assertThat(shared.distanceTo(vlineCoord)).isWithin(TOLERANCE_M).of(OFFSET_M) + // ...and they end up well separated from each other (≈ two radii apart). + assertThat(trainCoord.distanceTo(vlineCoord)).isGreaterThan(2 * OFFSET_M - TOLERANCE_M) + } + + @Test + fun `fan-out is deterministic across calls and independent of input order`() { + val train = + StopMother.aStop().withId(1162).withRouteType(RouteType.Train) + .withLatitude(RICHMOND_LAT).withLongitude(RICHMOND_LNG).build() + val vline = + StopMother.aStop().withId(1162).withRouteType(RouteType.VLine) + .withLatitude(RICHMOND_LAT).withLongitude(RICHMOND_LNG).build() + + val first = spreadColocatedStops(listOf(train, vline)).toMap() + val reversed = spreadColocatedStops(listOf(vline, train)).toMap() + + // Slot is keyed by the stop (sorted by route type then id), not its position in the input, + // so a re-render with a reordered list places each dot identically — no jitter. + assertThat(reversed.getValue(train)).isEqualTo(first.getValue(train)) + assertThat(reversed.getValue(vline)).isEqualTo(first.getValue(vline)) + } + + @Test + fun `distinct nearby stops are not merged and stay put`() { + val a = StopMother.aStop().withId(1).withLatitude(RICHMOND_LAT).withLongitude(RICHMOND_LNG).build() + // ~150 m north — a genuinely different stop, outside the co-location grid cell. + val b = StopMother.aStop().withId(2).withLatitude(RICHMOND_LAT + 0.0013).withLongitude(RICHMOND_LNG).build() + + val byStop = spreadColocatedStops(listOf(a, b)).toMap() + + assertThat(byStop.getValue(a)).isEqualTo(Coordinates(lat = RICHMOND_LAT, lng = RICHMOND_LNG)) + assertThat(byStop.getValue(b)).isEqualTo(Coordinates(lat = RICHMOND_LAT + 0.0013, lng = RICHMOND_LNG)) + } + + private companion object { + // Richmond Railway Station — the real shared point from PTV (train + V/Line, stop_id 1162). + private const val RICHMOND_LAT = -37.82407 + private const val RICHMOND_LNG = 144.99016 + + // The fan-out radius mirrors COLOCATION_OFFSET_METERS in the production code. + private const val OFFSET_M = 20.0 + private const val TOLERANCE_M = 3.0 + } +}