Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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<Stop>): List<Pair<Stop, Coordinates>> {
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<Stop>,
): 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<Long, Long> =
(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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -388,9 +398,13 @@ internal class MapLibreOpenPtvMap
) {
val style = map.style ?: return
val source = style.getSourceAs<GeoJsonSource>(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)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}