Skip to content

Commit a79222b

Browse files
committed
fix: spatially compute geofencing boundaries for split vertices
Split vertices on boundary-crossing edges were getting wrong geofencing boundaries because copyRentalRestrictionsFrom blind-copied the entering flag from parent vertices. The entering flag encodes position (outside= true, inside=false), so a split vertex on the opposite side of the zone boundary from its parent got the wrong flag, breaking the pairing check in updateGeofencingZones. Now VertexLinker.createSplitVertex queries the spatial zone index to determine which zones contain the split point and sets the correct entering flag accordingly. For permanent split vertices at concave zone corners where no permanent neighbor is inside the zone, the missing boundary is lazily added to the parent edge's fromVertex. Also fixes GeofencingZoneApplier to add boundary extensions to both endpoints of boundary-crossing edges (not just fromv), and aligns GeofencingZoneIndex to use covers() matching the applier's semantics.
1 parent d8a9f8a commit a79222b

6 files changed

Lines changed: 271 additions & 21 deletions

File tree

street/src/main/java/org/opentripplanner/service/vehiclerental/street/GeofencingZoneApplier.java

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -144,27 +144,51 @@ private Map<StreetEdge, GeofencingBoundaryExtension> addBoundaryExtensions(
144144

145145
for (var e : candidates) {
146146
if (e instanceof StreetEdge streetEdge) {
147-
var fromVertex = streetEdge.getFromVertex();
148-
var toVertex = streetEdge.getToVertex();
149-
150-
boolean fromInZone = vertexInZone.computeIfAbsent(fromVertex, v ->
151-
isVertexInZone(v.getCoordinate(), zoneBBox, preparedZone, reusablePoint)
152-
);
153-
boolean toInZone = vertexInZone.computeIfAbsent(toVertex, v ->
154-
isVertexInZone(v.getCoordinate(), zoneBBox, preparedZone, reusablePoint)
147+
addBoundaryIfCrossing(
148+
streetEdge,
149+
zone,
150+
zoneBBox,
151+
preparedZone,
152+
reusablePoint,
153+
vertexInZone,
154+
edgesUpdated
155155
);
156-
157-
if (fromInZone != toInZone) {
158-
var ext = new GeofencingBoundaryExtension(zone, toInZone);
159-
streetEdge.addGeofencingBoundary(ext);
160-
edgesUpdated.put(streetEdge, ext);
161-
}
162156
}
163157
}
164158
}
165159
return edgesUpdated;
166160
}
167161

162+
private static void addBoundaryIfCrossing(
163+
StreetEdge streetEdge,
164+
GeofencingZone zone,
165+
Envelope zoneBBox,
166+
PreparedGeometry preparedZone,
167+
Point reusablePoint,
168+
Map<Vertex, Boolean> vertexInZone,
169+
Map<StreetEdge, GeofencingBoundaryExtension> edgesUpdated
170+
) {
171+
var fromVertex = streetEdge.getFromVertex();
172+
var toVertex = streetEdge.getToVertex();
173+
174+
boolean fromInZone = vertexInZone.computeIfAbsent(fromVertex, v ->
175+
isVertexInZone(v.getCoordinate(), zoneBBox, preparedZone, reusablePoint)
176+
);
177+
boolean toInZone = vertexInZone.computeIfAbsent(toVertex, v ->
178+
isVertexInZone(v.getCoordinate(), zoneBBox, preparedZone, reusablePoint)
179+
);
180+
181+
if (fromInZone != toInZone) {
182+
var ext = new GeofencingBoundaryExtension(zone, toInZone);
183+
streetEdge.addGeofencingBoundary(ext);
184+
// Also add to tov with opposite entering flag so that the pairing check
185+
// in updateGeofencingZones works for all vertices, including permanent
186+
// split vertices that may only be tov of boundary-crossing edges.
187+
toVertex.addGeofencingBoundary(new GeofencingBoundaryExtension(zone, !toInZone));
188+
edgesUpdated.put(streetEdge, ext);
189+
}
190+
}
191+
168192
private static boolean isVertexInZone(
169193
Coordinate coord,
170194
Envelope zoneBBox,

street/src/main/java/org/opentripplanner/service/vehiclerental/street/GeofencingZoneIndex.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,18 @@ public GeofencingZoneIndex(Collection<GeofencingZone> zones) {
3535
}
3636

3737
/**
38-
* Returns all zones whose geometry contains the given coordinate.
38+
* Returns all zones whose geometry covers the given coordinate. Uses {@code covers()}
39+
* (not {@code contains()}) to include points on the zone boundary, matching the semantics
40+
* of {@link GeofencingZoneApplier#isVertexInZone} which uses {@code covers()} for
41+
* boundary detection during graph building.
3942
*/
4043
@SuppressWarnings("unchecked")
4144
public Set<GeofencingZone> getZonesContaining(Coordinate coord) {
4245
var point = GeometryUtils.getGeometryFactory().createPoint(coord);
4346
List<GeofencingZone> candidates = index.query(new Envelope(coord));
4447
return candidates
4548
.stream()
46-
.filter(z -> preparedGeometries.get(z).contains(point))
49+
.filter(z -> preparedGeometries.get(z).covers(point))
4750
.collect(Collectors.toSet());
4851
}
4952
}

street/src/main/java/org/opentripplanner/street/linking/VertexLinker.java

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import org.locationtech.jts.geom.LineString;
1818
import org.locationtech.jts.linearref.LinearLocation;
1919
import org.locationtech.jts.linearref.LocationIndexedLine;
20+
import org.opentripplanner.service.vehiclerental.model.GeofencingZone;
21+
import org.opentripplanner.service.vehiclerental.street.GeofencingBoundaryExtension;
2022
import org.opentripplanner.street.Scope;
2123
import org.opentripplanner.street.geometry.GeometryUtils;
2224
import org.opentripplanner.street.geometry.SphericalDistanceLibrary;
@@ -562,9 +564,38 @@ private SplitterVertex createSplitVertex(
562564
v.addBusinessAreaBorderNetwork(network);
563565
}
564566
}
565-
for (var boundary : toVertex.getGeofencingBoundaries()) {
566-
if (!v.getGeofencingBoundaries().contains(boundary)) {
567-
v.addGeofencingBoundary(boundary);
567+
// Compute geofencing boundaries spatially for the split vertex.
568+
// The entering flag encodes position: outside=true, inside=false.
569+
// Blind-copying from parent vertices would give the wrong flag when
570+
// the split point is on the opposite side of the zone boundary.
571+
var fromBoundaries = originalEdge.getFromVertex().getGeofencingBoundaries();
572+
var toBoundaries = toVertex.getGeofencingBoundaries();
573+
if (!fromBoundaries.isEmpty() || !toBoundaries.isEmpty()) {
574+
var splitCoord = new Coordinate(x, y);
575+
var containingZones = graph.getGeofencingZonesContaining(splitCoord);
576+
var boundaryZones = new HashSet<GeofencingZone>();
577+
for (var b : fromBoundaries) {
578+
boundaryZones.add(b.zone());
579+
}
580+
for (var b : toBoundaries) {
581+
boundaryZones.add(b.zone());
582+
}
583+
var fromVertex = originalEdge.getFromVertex();
584+
for (var zone : boundaryZones) {
585+
boolean splitInZone = containingZones.contains(zone);
586+
v.addGeofencingBoundary(new GeofencingBoundaryExtension(zone, !splitInZone));
587+
588+
// If the split vertex is inside the zone but the parent edge's fromVertex
589+
// has no boundary for this zone (e.g., a permanent SplitterVertex in a
590+
// concave corner where all permanent neighbors are outside the zone),
591+
// add the missing boundary to fromVertex. Without this, the pairing check
592+
// in updateGeofencingZones would fail and the zone would never enter state.
593+
if (splitInZone) {
594+
boolean fromvHasBoundary = fromBoundaries.stream().anyMatch(b -> b.zone().equals(zone));
595+
if (!fromvHasBoundary) {
596+
fromVertex.addGeofencingBoundary(new GeofencingBoundaryExtension(zone, true));
597+
}
598+
}
568599
}
569600
}
570601

street/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,15 +358,16 @@ public void removeAllBusinessAreaBorders() {
358358
}
359359

360360
/**
361-
* Copy all rental restriction data from another vertex to this one.
361+
* Copy business area border data from another vertex to this one.
362+
* Geofencing boundaries are NOT copied — they require spatial computation
363+
* based on the vertex's actual position (see VertexLinker.createSplitVertex).
362364
*/
363365
public void copyRentalRestrictionsFrom(Vertex other) {
364366
if (other.businessAreaBorder != null) {
365367
for (var network : other.businessAreaBorder.networks()) {
366368
addBusinessAreaBorderNetwork(network);
367369
}
368370
}
369-
this.geofencingBoundaries = other.geofencingBoundaries;
370371
}
371372

372373
/**

street/src/test/java/org/opentripplanner/service/vehiclerental/street/GeofencingZoneIndexTest.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,19 @@ void pointOutsideAllZones() {
5151
assertTrue(zones.isEmpty());
5252
}
5353

54+
@Test
55+
void pointOnZoneBoundaryIsIncluded() {
56+
// A point exactly on the polygon boundary should be classified as inside,
57+
// matching GeofencingZoneApplier.isVertexInZone which uses covers().
58+
// Use the first vertex of the Oslo polygon (which is on its boundary).
59+
var boundaryCoord = new Coordinate(10.62535658370308, 59.961055202323195);
60+
var zones = index.getZonesContaining(boundaryCoord);
61+
assertTrue(
62+
zones.contains(oslo),
63+
"Point on zone boundary should be included (covers semantics)"
64+
);
65+
}
66+
5467
@Test
5568
void emptyIndex() {
5669
var emptyIndex = new GeofencingZoneIndex(List.of());
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package org.opentripplanner.street.linking;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertTrue;
5+
import static org.opentripplanner.street.model.StreetModelFactory.intersectionVertex;
6+
import static org.opentripplanner.street.model.StreetModelFactory.streetEdge;
7+
8+
import java.util.List;
9+
import java.util.Set;
10+
import org.junit.jupiter.api.Test;
11+
import org.locationtech.jts.geom.Coordinate;
12+
import org.locationtech.jts.geom.Polygon;
13+
import org.opentripplanner.core.model.i18n.I18NString;
14+
import org.opentripplanner.core.model.id.FeedScopedId;
15+
import org.opentripplanner.service.vehiclerental.model.GeofencingZone;
16+
import org.opentripplanner.service.vehiclerental.street.GeofencingBoundaryExtension;
17+
import org.opentripplanner.service.vehiclerental.street.GeofencingZoneIndex;
18+
import org.opentripplanner.street.geometry.GeometryUtils;
19+
import org.opentripplanner.street.graph.Graph;
20+
import org.opentripplanner.street.model.StreetConstants;
21+
import org.opentripplanner.street.model.edge.TemporaryFreeEdge;
22+
import org.opentripplanner.street.model.vertex.SplitterVertex;
23+
import org.opentripplanner.street.model.vertex.StreetVertex;
24+
import org.opentripplanner.street.model.vertex.TemporaryStreetLocation;
25+
import org.opentripplanner.street.search.TraverseModeSet;
26+
27+
/**
28+
* Tests that split vertices on boundary-crossing edges get spatially correct
29+
* geofencing boundary extensions instead of blind-copied ones from parent vertices.
30+
*/
31+
class VertexLinkerGeofencingTest {
32+
33+
// Zone polygon: a rectangle covering lon [10.70, 10.71], lat [59.92, 59.93]
34+
private static final Polygon ZONE_POLYGON = GeometryUtils.getGeometryFactory().createPolygon(
35+
new Coordinate[] {
36+
new Coordinate(10.70, 59.92),
37+
new Coordinate(10.71, 59.92),
38+
new Coordinate(10.71, 59.93),
39+
new Coordinate(10.70, 59.93),
40+
new Coordinate(10.70, 59.92),
41+
}
42+
);
43+
44+
private static final GeofencingZone NO_DROP_OFF_ZONE = new GeofencingZone(
45+
new FeedScopedId("tier", "park"),
46+
null,
47+
ZONE_POLYGON,
48+
true,
49+
false
50+
);
51+
52+
// A is outside the zone (lon=10.695), B is inside (lon=10.705)
53+
// Both at lat=59.925 (midpoint of zone's lat range)
54+
private final StreetVertex vertexOutside = intersectionVertex("A", 59.925, 10.695);
55+
private final StreetVertex vertexInside = intersectionVertex("B", 59.925, 10.705);
56+
57+
@Test
58+
void splitVertexInsideZoneGetsEnteringFalse() {
59+
// Split at a point inside the zone (lon=10.703, which is inside [10.70, 10.71])
60+
var splitVertex = linkAndFindSplitVertex(59.925, 10.703);
61+
62+
var boundaries = splitVertex.getGeofencingBoundaries();
63+
assertEquals(1, boundaries.size());
64+
var boundary = boundaries.getFirst();
65+
assertEquals(NO_DROP_OFF_ZONE, boundary.zone());
66+
// Inside the zone → entering=false
67+
assertEquals(false, boundary.entering());
68+
}
69+
70+
@Test
71+
void splitVertexOutsideZoneGetsEnteringTrue() {
72+
// Split at a point outside the zone (lon=10.698, which is outside [10.70, 10.71])
73+
var splitVertex = linkAndFindSplitVertex(59.925, 10.698);
74+
75+
var boundaries = splitVertex.getGeofencingBoundaries();
76+
assertEquals(1, boundaries.size());
77+
var boundary = boundaries.getFirst();
78+
assertEquals(NO_DROP_OFF_ZONE, boundary.zone());
79+
// Outside the zone → entering=true
80+
assertEquals(true, boundary.entering());
81+
}
82+
83+
@Test
84+
void splitVertexOnNonBoundaryEdgeGetsNoBoundaries() {
85+
// Two vertices both outside the zone
86+
var v1 = intersectionVertex("C", 59.925, 10.690);
87+
var v2 = intersectionVertex("D", 59.925, 10.695);
88+
streetEdge(v1, v2);
89+
90+
var graph = new Graph();
91+
graph.addVertex(v1);
92+
graph.addVertex(v2);
93+
registerZoneIndex(graph);
94+
graph.index();
95+
96+
var linker = new VertexLinker(
97+
graph,
98+
VisibilityMode.COMPUTE_AREA_VISIBILITY_LINES,
99+
StreetConstants.DEFAULT_MAX_AREA_NODES,
100+
true
101+
);
102+
103+
var split = new TemporaryStreetLocation(
104+
new Coordinate(10.6925, 59.925),
105+
I18NString.of("split")
106+
);
107+
var disposable = linker.linkVertexForRequest(
108+
split,
109+
TraverseModeSet.allModes(),
110+
LinkingDirection.BIDIRECTIONAL,
111+
(sv1, sv2) ->
112+
List.of(TemporaryFreeEdge.createTemporaryFreeEdge((TemporaryStreetLocation) sv1, sv2))
113+
);
114+
115+
// Find the split vertex
116+
var splitterVertex = findSplitterVertex(split);
117+
assertTrue(
118+
splitterVertex.getGeofencingBoundaries().isEmpty(),
119+
"Split vertex on non-boundary edge should have no geofencing boundaries"
120+
);
121+
122+
disposable.disposeEdges();
123+
}
124+
125+
private SplitterVertex linkAndFindSplitVertex(double lat, double lon) {
126+
streetEdge(vertexOutside, vertexInside);
127+
128+
// Add boundary extensions to simulate what GeofencingZoneApplier does:
129+
// A (outside) gets entering=true, B (inside) gets entering=false
130+
vertexOutside.addGeofencingBoundary(new GeofencingBoundaryExtension(NO_DROP_OFF_ZONE, true));
131+
vertexInside.addGeofencingBoundary(new GeofencingBoundaryExtension(NO_DROP_OFF_ZONE, false));
132+
133+
var graph = new Graph();
134+
graph.addVertex(vertexOutside);
135+
graph.addVertex(vertexInside);
136+
registerZoneIndex(graph);
137+
graph.index();
138+
139+
var linker = new VertexLinker(
140+
graph,
141+
VisibilityMode.COMPUTE_AREA_VISIBILITY_LINES,
142+
StreetConstants.DEFAULT_MAX_AREA_NODES,
143+
true
144+
);
145+
146+
var split = new TemporaryStreetLocation(new Coordinate(lon, lat), I18NString.of("split"));
147+
var disposable = linker.linkVertexForRequest(
148+
split,
149+
TraverseModeSet.allModes(),
150+
LinkingDirection.BIDIRECTIONAL,
151+
(v1, v2) ->
152+
List.of(TemporaryFreeEdge.createTemporaryFreeEdge((TemporaryStreetLocation) v1, v2))
153+
);
154+
155+
var splitterVertex = findSplitterVertex(split);
156+
disposable.disposeEdges();
157+
return splitterVertex;
158+
}
159+
160+
private static SplitterVertex findSplitterVertex(TemporaryStreetLocation location) {
161+
for (var edge : location.getOutgoing()) {
162+
if (edge.getToVertex() instanceof SplitterVertex sv) {
163+
return sv;
164+
}
165+
}
166+
for (var edge : location.getIncoming()) {
167+
if (edge.getFromVertex() instanceof SplitterVertex sv) {
168+
return sv;
169+
}
170+
}
171+
throw new AssertionError("No SplitterVertex found connected to the temporary location");
172+
}
173+
174+
private static void registerZoneIndex(Graph graph) {
175+
var index = new GeofencingZoneIndex(Set.of(NO_DROP_OFF_ZONE));
176+
graph.setGeofencingZoneIndex("tier", index);
177+
}
178+
}

0 commit comments

Comments
 (0)