From f8f085f2ef4233cfe4ba940f0319d4bbd9d98ec1 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Fri, 24 Feb 2023 13:37:33 +0200 Subject: [PATCH 01/17] Move area linking code to vertex linker. Link points which map to an area edge with all visibility points. --- .../module/OsmBoardingLocationsModule.java | 3 +- .../linking/DisposableEdgeCollection.java | 4 + .../routing/linking/FlexLocationAdder.java | 4 +- .../routing/linking/VertexLinker.java | 160 ++++++++++++++++-- .../street/model/edge/AreaEdgeList.java | 94 +--------- 5 files changed, 150 insertions(+), 115 deletions(-) diff --git a/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java b/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java index 42da90bea39..0766e932b96 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java @@ -170,8 +170,7 @@ private boolean connectVertexToStop(TransitStopVertex ts, StreetIndex index, Gra name, edgeList.references ); - edgeList.addVertex(boardingLocation); - + linker.addPermanentAreaVertex(boardingLocation, edgeList); linkBoardingLocationToStop(ts, stopCode, boardingLocation); return true; } diff --git a/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java b/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java index a8c09e92081..cf2af6e2041 100644 --- a/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java +++ b/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java @@ -33,6 +33,10 @@ public void addEdge(Edge edge) { this.edges.add(edge); } + public int edgeCount() { + return this.edges.size(); + } + /** * Removes all the edges in this collection from the graph. */ diff --git a/src/main/java/org/opentripplanner/routing/linking/FlexLocationAdder.java b/src/main/java/org/opentripplanner/routing/linking/FlexLocationAdder.java index 646b23a82f6..f8963781d1b 100644 --- a/src/main/java/org/opentripplanner/routing/linking/FlexLocationAdder.java +++ b/src/main/java/org/opentripplanner/routing/linking/FlexLocationAdder.java @@ -6,13 +6,13 @@ import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.street.model.StreetTraversalPermission; import org.opentripplanner.street.model.edge.StreetEdge; -import org.opentripplanner.street.model.vertex.SplitterVertex; +import org.opentripplanner.street.model.vertex.IntersectionVertex; import org.opentripplanner.transit.model.site.AreaStop; import org.opentripplanner.transit.service.StopModel; class FlexLocationAdder { - static void addFlexLocations(StreetEdge edge, SplitterVertex v0, StopModel stopModel) { + static void addFlexLocations(StreetEdge edge, IntersectionVertex v0, StopModel stopModel) { if (edge.getPermission().allows(StreetTraversalPermission.PEDESTRIAN_AND_CAR)) { Point p = GeometryUtils.getGeometryFactory().createPoint(v0.getCoordinate()); Envelope env = p.getEnvelopeInternal(); diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index 071025cfc4f..1687f09c520 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -1,5 +1,6 @@ package org.opentripplanner.routing.linking; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -8,18 +9,24 @@ import java.util.stream.Collectors; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Envelope; +import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.geom.Polygon; import org.locationtech.jts.linearref.LinearLocation; import org.locationtech.jts.linearref.LocationIndexedLine; +import org.locationtech.jts.operation.distance.DistanceOp; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.geometry.SphericalDistanceLibrary; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graph.index.EdgeSpatialIndex; import org.opentripplanner.street.model.edge.AreaEdge; +import org.opentripplanner.street.model.edge.AreaEdgeList; import org.opentripplanner.street.model.edge.Edge; +import org.opentripplanner.street.model.edge.NamedArea; import org.opentripplanner.street.model.edge.StreetEdge; +import org.opentripplanner.street.model.vertex.IntersectionVertex; import org.opentripplanner.street.model.vertex.SplitterVertex; import org.opentripplanner.street.model.vertex.StreetVertex; import org.opentripplanner.street.model.vertex.TemporarySplitterVertex; @@ -67,7 +74,7 @@ public class VertexLinker { private final StopModel stopModel; // TODO Temporary code until we refactor WalkableAreaBuilder (#3152) - private Boolean addExtraEdgesToAreas = false; + private Boolean addExtraEdgesToAreas = true; /** * Construct a new VertexLinker. NOTE: Only one VertexLinker should be active on a graph at any @@ -351,6 +358,8 @@ private StreetVertex link( LinearLocation ll = il.project(new Coordinate(vertex.getLon() * xScale, vertex.getLat())); double length = SphericalDistanceLibrary.length(orig); + IntersectionVertex start = null; + // if we're very close to one end of the line or the other, or endwise, don't bother to split, // cut to the chase and link directly // We use a really tiny epsilon here because we only want points that actually snap to exactly the same location on the @@ -359,37 +368,33 @@ private StreetVertex link( ll.getSegmentIndex() == 0 && (ll.getSegmentFraction() < 1e-8 || ll.getSegmentFraction() * length < 0.1) ) { - return (StreetVertex) edge.getFromVertex(); + start = (IntersectionVertex) edge.getFromVertex(); } // -1 converts from count to index. Because of the fencepost problem, npoints - 1 is the "segment" // past the last point else if (ll.getSegmentIndex() == orig.getNumPoints() - 1) { - return (StreetVertex) edge.getToVertex(); + start = (IntersectionVertex) edge.getToVertex(); } // nPoints - 2: -1 to correct for index vs count, -1 to account for fencepost problem else if ( ll.getSegmentIndex() == orig.getNumPoints() - 2 && (ll.getSegmentFraction() > 1 - 1e-8 || (1 - ll.getSegmentFraction()) * length < 0.1) ) { - return (StreetVertex) edge.getToVertex(); + start = (IntersectionVertex) edge.getToVertex(); } else { // split the edge, get the split vertex - SplitterVertex v0 = split(edge, ll, scope, direction, tempEdges); - - // If splitter vertex is part of area; link splittervertex to all other vertexes in area, this creates - // edges that were missed by WalkableAreaBuilder - // TODO Temporary code until we refactor the WalkableAreaBuilder (#3152) - if (scope == Scope.PERMANENT && this.addExtraEdgesToAreas && edge instanceof AreaEdge) { - ((AreaEdge) edge).getArea().addVertex(v0); - } - - // TODO Consider moving this code - if (OTPFeature.FlexRouting.isOn()) { - FlexLocationAdder.addFlexLocations(edge, v0, stopModel); - } + start = (IntersectionVertex) split(edge, ll, scope, direction, tempEdges); + } - return v0; + if (this.addExtraEdgesToAreas && edge instanceof AreaEdge aEdge) { + addAreaVertex(start, aEdge.getArea(), scope, tempEdges); } + // TODO Consider moving this code + if (OTPFeature.FlexRouting.isOn()) { + FlexLocationAdder.addFlexLocations(edge, start, stopModel); + } + + return start; } /** @@ -500,4 +505,123 @@ public boolean equals(Object o) { } private record StreetEdgePair(StreetEdge e0, StreetEdge e1) {} + + /** + * Link a new vertex permanently with area geometry + */ + public void addPermanentAreaVertex(IntersectionVertex newVertex, AreaEdgeList edgeList) { + addAreaVertex(newVertex, edgeList, Scope.PERMANENT, null); + } + + /** + * Safely add a vertex to an area. This creates edges to all other vertices unless those edges + * would cross one of the original edges. + */ + + public void addAreaVertex( + IntersectionVertex newVertex, + AreaEdgeList edgeList, + Scope scope, + DisposableEdgeCollection tempEdges + ) { + List areas = edgeList.getAreas(); + Geometry origPolygon = edgeList.getGeometry(); + Geometry polygon = origPolygon.union(origPolygon.getBoundary()).buffer(0.000001); + HashSet visibilityVertices = edgeList.visibilityVertices; + + // Due to truncating of precision in storage of the edge geometry, the new split vertex + // might be located just outside the area, so we calculate the point closest to the polygon + // for the comparison. + Coordinate[] nearestPoints = DistanceOp.nearestPoints( + polygon, + GEOMETRY_FACTORY.createPoint(newVertex.getCoordinate()) + ); + + int added = 0; + + for (IntersectionVertex v : visibilityVertices) { + LineString newGeometry = GEOMETRY_FACTORY.createLineString( + new Coordinate[] { nearestPoints[0], v.getCoordinate() } + ); + + // ensure that new edge does not leave the bounds of the original area, or + // fall into any holes + if (!polygon.contains(newGeometry)) { + continue; + } + + // check to see if this splits multiple NamedAreas. This code is rather similar to + // code in OSMGBI, but the data structures are different + createSegments(newVertex, v, edgeList, areas, scope, tempEdges); + added++; + } + + // TODO: Temporary fix for unconnected area edges. This should go away when moving walkable + // area calculation to be done after stop linking + if (added == 0) { + for (IntersectionVertex v : visibilityVertices) { + createSegments(newVertex, v, edgeList, areas, scope, tempEdges); + } + } + if (scope == Scope.PERMANENT) { + visibilityVertices.add(newVertex); + } + } + + private void createSegments( + IntersectionVertex from, + IntersectionVertex to, + AreaEdgeList ael, + List areas, + Scope scope, + DisposableEdgeCollection tempEdges + ) { + LineString line = GEOMETRY_FACTORY.createLineString( + new Coordinate[] { from.getCoordinate(), to.getCoordinate() } + ); + + List intersects = new ArrayList<>(); + for (NamedArea area : areas) { + Geometry polygon = area.getPolygon(); + Geometry intersection = polygon.intersection(line); + if (intersection.getLength() > 0.000001) { + intersects.add(area); + } + } + if (!intersects.isEmpty()) { + // If more than one area intersects, we pick one by random for the name & properties + NamedArea area = intersects.get(0); + + double length = SphericalDistanceLibrary.distance(to.getCoordinate(), from.getCoordinate()); + + AreaEdge ae = new AreaEdge( + from, + to, + line, + area.getName(), + length, + area.getPermission(), + false, + ael + ); + if (scope != Scope.PERMANENT) { + tempEdges.addEdge(ae); + } + + ae = + new AreaEdge( + to, + from, + line.reverse(), + area.getName(), + length, + area.getPermission(), + true, + ael + ); + if (scope != Scope.PERMANENT) { + tempEdges.addEdge(ae); + } + } + } } diff --git a/src/main/java/org/opentripplanner/street/model/edge/AreaEdgeList.java b/src/main/java/org/opentripplanner/street/model/edge/AreaEdgeList.java index 832b196b39a..504233c0b95 100644 --- a/src/main/java/org/opentripplanner/street/model/edge/AreaEdgeList.java +++ b/src/main/java/org/opentripplanner/street/model/edge/AreaEdgeList.java @@ -5,19 +5,13 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; -import org.locationtech.jts.geom.GeometryFactory; -import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.Polygon; -import org.locationtech.jts.operation.distance.DistanceOp; -import org.opentripplanner.framework.geometry.GeometryUtils; -import org.opentripplanner.framework.geometry.SphericalDistanceLibrary; import org.opentripplanner.street.model.vertex.IntersectionVertex; /** * This is a representation of a set of contiguous OSM areas, used for various tasks related to edge - * splitting, such as start/endpoint snapping and adding new edges during transit linking. + * splitting, such as adding new edges during transit linking. * * @author novalis */ @@ -47,53 +41,6 @@ public String toString() { ); } - /** - * Safely add a vertex to this area. This creates edges to all other vertices unless those edges - * would cross one of the original edges. - */ - public void addVertex(IntersectionVertex newVertex) { - GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); - - Geometry polygon = originalEdges.union(originalEdges.getBoundary()).buffer(0.000001); - - // Due to truncating of precision in storage of the edge geometry, the new split vertex - // might be located just outside the area, so we calculate the point closest to the polygon - // for the comparison. - Coordinate[] nearestPoints = DistanceOp.nearestPoints( - polygon, - geometryFactory.createPoint(newVertex.getCoordinate()) - ); - - int added = 0; - - for (IntersectionVertex v : visibilityVertices) { - LineString newGeometry = geometryFactory.createLineString( - new Coordinate[] { nearestPoints[0], v.getCoordinate() } - ); - - // ensure that new edge does not leave the bounds of the original area, or - // fall into any holes - if (!polygon.contains(newGeometry)) { - continue; - } - - // check to see if this splits multiple NamedAreas. This code is rather similar to - // code in OSMGBI, but the data structures are different - createSegments(newVertex, v, areas); - added++; - } - - // TODO: Temporary fix for unconnected area edges. This should go away when moving walkable - // area calculation to be done after stop linking - if (added == 0) { - for (IntersectionVertex v : visibilityVertices) { - createSegments(newVertex, v, areas); - } - } - - visibilityVertices.add(newVertex); - } - public void addArea(NamedArea namedArea) { areas.add(namedArea); } @@ -105,43 +52,4 @@ public List getAreas() { public Geometry getGeometry() { return originalEdges; } - - private void createSegments( - IntersectionVertex from, - IntersectionVertex to, - List areas - ) { - GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); - - LineString line = geometryFactory.createLineString( - new Coordinate[] { from.getCoordinate(), to.getCoordinate() } - ); - - List intersects = new ArrayList<>(); - for (NamedArea area : areas) { - Geometry polygon = area.getPolygon(); - Geometry intersection = polygon.intersection(line); - if (intersection.getLength() > 0.000001) { - intersects.add(area); - } - } - if (!intersects.isEmpty()) { - // If more than one area intersects, we pick one by random for the name & properties - NamedArea area = intersects.get(0); - - double length = SphericalDistanceLibrary.distance(to.getCoordinate(), from.getCoordinate()); - - new AreaEdge(from, to, line, area.getName(), length, area.getPermission(), false, this); - new AreaEdge( - to, - from, - line.reverse(), - area.getName(), - length, - area.getPermission(), - true, - this - ); - } - } } From cf5d59f5ac26e8a821c62abeaf2dc8d45145c6b2 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Sat, 25 Feb 2023 17:05:53 +0200 Subject: [PATCH 02/17] Bugfix: temporary edges from access/eggress routing must not be disposed before graph paths have been fully processed It is a bit unclear why transit routing did not crash before this change --- .../raptoradapter/router/TransitRouter.java | 76 +++++++++---------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java index 55452d1c72e..a1629a21c1b 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java @@ -70,10 +70,19 @@ public static TransitRouterResult route( additionalSearchDays, debugTimingAggregator ); - return transitRouter.route(); + try ( + var temporaryVertices = new TemporaryVerticesContainer( + serverContext.graph(), + request, + request.journey().access().mode(), + request.journey().egress().mode() + ) + ) { + return transitRouter.route(temporaryVertices); + } } - private TransitRouterResult route() { + private TransitRouterResult route(TemporaryVerticesContainer temporaryVertices) { if (!request.journey().transit().enabled()) { return new TransitRouterResult(List.of(), null); } @@ -92,7 +101,7 @@ private TransitRouterResult route() { debugTimingAggregator.finishedPatternFiltering(); - var accessEgresses = getAccessEgresses(); + var accessEgresses = getAccessEgresses(temporaryVertices); debugTimingAggregator.finishedAccessEgress( accessEgresses.getAccesses().size(), @@ -150,46 +159,37 @@ private TransitRouterResult route() { return new TransitRouterResult(itineraries, transitResponse.requestUsed().searchParams()); } - private AccessEgresses getAccessEgresses() { + private AccessEgresses getAccessEgresses(TemporaryVerticesContainer temporaryVertices) { var accessEgressMapper = new AccessEgressMapper(); var accessList = new ArrayList(); var egressList = new ArrayList(); - try ( - var temporaryVertices = new TemporaryVerticesContainer( - serverContext.graph(), - request, - request.journey().access().mode(), - request.journey().egress().mode() - ) - ) { - var accessCalculator = (Runnable) () -> { - debugTimingAggregator.startedAccessCalculating(); - accessList.addAll(getAccessEgresses(accessEgressMapper, temporaryVertices, false)); - debugTimingAggregator.finishedAccessCalculating(); - }; - - var egressCalculator = (Runnable) () -> { - debugTimingAggregator.startedEgressCalculating(); - egressList.addAll(getAccessEgresses(accessEgressMapper, temporaryVertices, true)); - debugTimingAggregator.finishedEgressCalculating(); - }; - - if (OTPFeature.ParallelRouting.isOn()) { - try { - CompletableFuture - .allOf( - CompletableFuture.runAsync(accessCalculator), - CompletableFuture.runAsync(egressCalculator) - ) - .join(); - } catch (CompletionException e) { - RoutingValidationException.unwrapAndRethrowCompletionException(e); - } - } else { - accessCalculator.run(); - egressCalculator.run(); + var accessCalculator = (Runnable) () -> { + debugTimingAggregator.startedAccessCalculating(); + accessList.addAll(getAccessEgresses(accessEgressMapper, temporaryVertices, false)); + debugTimingAggregator.finishedAccessCalculating(); + }; + + var egressCalculator = (Runnable) () -> { + debugTimingAggregator.startedEgressCalculating(); + egressList.addAll(getAccessEgresses(accessEgressMapper, temporaryVertices, true)); + debugTimingAggregator.finishedEgressCalculating(); + }; + + if (OTPFeature.ParallelRouting.isOn()) { + try { + CompletableFuture + .allOf( + CompletableFuture.runAsync(accessCalculator), + CompletableFuture.runAsync(egressCalculator) + ) + .join(); + } catch (CompletionException e) { + RoutingValidationException.unwrapAndRethrowCompletionException(e); } + } else { + accessCalculator.run(); + egressCalculator.run(); } verifyAccessEgress(accessList, egressList); From 52c2c170442395a29f2567a2904879aa62a81946 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 27 Feb 2023 08:37:38 +0200 Subject: [PATCH 03/17] Do not snap vertex inside an area into closest edge --- .../routing/linking/VertexLinker.java | 77 ++++++++++++------- 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index 1687f09c520..11e26308360 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -382,8 +382,27 @@ else if ( ) { start = (IntersectionVertex) edge.getToVertex(); } else { - // split the edge, get the split vertex - start = (IntersectionVertex) split(edge, ll, scope, direction, tempEdges); + boolean split = true; + // if vertex is inside an area, no need to snap to nearest edge and split it + if (this.addExtraEdgesToAreas && edge instanceof AreaEdge aEdge) { + if ( + aEdge + .getArea() + .getGeometry() + .contains(GEOMETRY_FACTORY.createPoint(vertex.getCoordinate())) + ) { + if (vertex instanceof IntersectionVertex iv) { + start = iv; + } else { + start = splitVertex(aEdge, scope, direction, vertex.getLon(), vertex.getLat()); + } + split = false; + } + } + if (split) { + // split the edge, get the split vertex + start = (IntersectionVertex) split(edge, ll, scope, direction, tempEdges); + } } if (this.addExtraEdgesToAreas && edge instanceof AreaEdge aEdge) { @@ -418,32 +437,7 @@ private SplitterVertex split( // create the geometries Coordinate splitPoint = ll.getCoordinate(geometry); - - SplitterVertex v; - String uniqueSplitLabel = "split_" + graph.nextSplitNumber++; - - if (scope != Scope.PERMANENT) { - TemporarySplitterVertex tsv = new TemporarySplitterVertex( - uniqueSplitLabel, - splitPoint.x, - splitPoint.y, - originalEdge, - direction == LinkingDirection.OUTGOING - ); - tsv.setWheelchairAccessible(originalEdge.isWheelchairAccessible()); - v = tsv; - } else { - v = - new SplitterVertex( - graph, - uniqueSplitLabel, - splitPoint.x, - splitPoint.y, - originalEdge.getName() - ); - } - - v.addRentalRestriction(originalEdge.getFromVertex().rentalRestrictions()); + SplitterVertex v = splitVertex(originalEdge, scope, direction, splitPoint.x, splitPoint.y); // Split the 'edge' at 'v' in 2 new edges and connect these 2 edges to the // existing vertices @@ -474,6 +468,33 @@ private SplitterVertex split( return v; } + private SplitterVertex splitVertex( + StreetEdge originalEdge, + Scope scope, + LinkingDirection direction, + double x, + double y + ) { + SplitterVertex v; + String uniqueSplitLabel = "split_" + graph.nextSplitNumber++; + + if (scope != Scope.PERMANENT) { + TemporarySplitterVertex tsv = new TemporarySplitterVertex( + uniqueSplitLabel, + x, + y, + originalEdge, + direction == LinkingDirection.OUTGOING + ); + tsv.setWheelchairAccessible(originalEdge.isWheelchairAccessible()); + v = tsv; + } else { + v = new SplitterVertex(graph, uniqueSplitLabel, x, y, originalEdge.getName()); + } + v.addRentalRestriction(originalEdge.getFromVertex().rentalRestrictions()); + return v; + } + private static class DistanceTo { T item; From 169fdc8ce61eea702e0e32b2e7b24fd7beebd37a Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 27 Feb 2023 15:53:13 +0200 Subject: [PATCH 04/17] Do not link if vertex got linked directly --- .../graph_builder/module/OsmBoardingLocationsModule.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java b/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java index 0766e932b96..f82a15905bf 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java @@ -123,6 +123,9 @@ private boolean connectVertexToStop(TransitStopVertex ts, StreetIndex index, Gra new TraverseModeSet(TraverseMode.WALK), LinkingDirection.BOTH_WAYS, (osmBoardingLocationVertex, splitVertex) -> { + if (osmBoardingLocationVertex == splitVertex) { + return List.of(); + } // the OSM boarding location vertex is not connected to the street network, so we // need to link it first return List.of( From 15c99896f91b271ed3621934f56444b3ec563f6e Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 27 Feb 2023 15:53:59 +0200 Subject: [PATCH 05/17] Add checks to avoid adding duplicate linking Vertex linking no longer always splits an edge with a new vertex, but sometimes picks an existing vertex which may be alrweady linked. Also area linking may get executed several times. --- .../routing/linking/VertexLinker.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index 11e26308360..435e2b6eaae 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -280,10 +280,11 @@ private Set linkToStreetEdges( } Set> closesEdges = getClosestEdgesPerMode(traverseModes, candidateEdges); - + Set linkedAreas = new HashSet<>(); return closesEdges .stream() - .map(ce -> link(vertex, ce.item, xscale, scope, direction, tempEdges)) + .map(ce -> link(vertex, ce.item, xscale, scope, direction, tempEdges, linkedAreas)) + .filter(v -> v != null) .collect(Collectors.toSet()); } @@ -349,7 +350,8 @@ private StreetVertex link( double xScale, Scope scope, LinkingDirection direction, - DisposableEdgeCollection tempEdges + DisposableEdgeCollection tempEdges, + Set linkedAreas ) { // TODO: we've already built this line string, we should save it LineString orig = edge.getGeometry(); @@ -385,6 +387,11 @@ else if ( boolean split = true; // if vertex is inside an area, no need to snap to nearest edge and split it if (this.addExtraEdgesToAreas && edge instanceof AreaEdge aEdge) { + // do not relink again to same area when many edges are equally close + if (linkedAreas.contains(aEdge.getArea())) { + return null; + } + if ( aEdge .getArea() @@ -597,6 +604,13 @@ private void createSegments( Scope scope, DisposableEdgeCollection tempEdges ) { + // Check that vertices are not yet linked + for (Edge e : from.getOutgoing()) { + if (e.getToVertex() == to) { + return; + } + } + LineString line = GEOMETRY_FACTORY.createLineString( new Coordinate[] { from.getCoordinate(), to.getCoordinate() } ); From 5fbc927eb1d84e076ec11fbc0cdc406bab510130 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 27 Feb 2023 15:59:48 +0200 Subject: [PATCH 06/17] Platform linking test must define a proper area geometry Area linking runs some geometry.contains() tests, which fail if geometry is void. --- .../linking/LinkStopToPlatformTest.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java index 3cbb6ff0409..253f333dff9 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java @@ -40,7 +40,15 @@ public class LinkStopToPlatformTest { @BeforeEach public void before() { - // Set up transit platform + /* Set up transit platform with 5 corners and a stop outside the platform. It looks like this: + ________________________________________ + | | + | | + | / + | / + ------------------------------------- + * + */ var deduplicator = new Deduplicator(); var stopModel = new StopModel(); @@ -56,7 +64,18 @@ public void before() { vertices.add(new IntersectionVertex(graph, "5", 10.22056, 59.13575, "Platform vertex 5")); AreaEdgeList areaEdgeList = new AreaEdgeList( - GeometryUtils.getGeometryFactory().createPolygon(), + GeometryUtils + .getGeometryFactory() + .createPolygon( + new Coordinate[] { + new Coordinate(59.13568, 10.22054), + new Coordinate(59.13519, 10.22432), + new Coordinate(59.13514, 10.22492), + new Coordinate(59.13518, 10.22493), + new Coordinate(59.13575, 10.22056), + new Coordinate(59.13568, 10.22054), + } + ), Set.of() ); From 412f921d504db87b1b4a8e6267a8ad07ba550158 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 27 Feb 2023 18:00:29 +0200 Subject: [PATCH 07/17] Fix typo --- .../org/opentripplanner/routing/linking/VertexLinker.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index 435e2b6eaae..ea734a11927 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -279,9 +279,9 @@ private Set linkToStreetEdges( return Set.of(); } - Set> closesEdges = getClosestEdgesPerMode(traverseModes, candidateEdges); + Set> closestEdges = getClosestEdgesPerMode(traverseModes, candidateEdges); Set linkedAreas = new HashSet<>(); - return closesEdges + return closestEdges .stream() .map(ce -> link(vertex, ce.item, xscale, scope, direction, tempEdges, linkedAreas)) .filter(v -> v != null) From 1d0980bf189c7d1b36234e0a1b961f51f3846ff1 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Tue, 28 Feb 2023 10:29:33 +0200 Subject: [PATCH 08/17] No need to itertate all named areas, whren only first one is used --- .../routing/linking/VertexLinker.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index ea734a11927..dd498853e46 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -279,7 +279,10 @@ private Set linkToStreetEdges( return Set.of(); } - Set> closestEdges = getClosestEdgesPerMode(traverseModes, candidateEdges); + Set> closestEdges = getClosestEdgesPerMode( + traverseModes, + candidateEdges + ); Set linkedAreas = new HashSet<>(); return closestEdges .stream() @@ -616,26 +619,26 @@ private void createSegments( ); List intersects = new ArrayList<>(); + NamedArea hit = null; for (NamedArea area : areas) { Geometry polygon = area.getPolygon(); Geometry intersection = polygon.intersection(line); if (intersection.getLength() > 0.000001) { - intersects.add(area); + hit = area; + break; } } - if (!intersects.isEmpty()) { + if (hit != null) { // If more than one area intersects, we pick one by random for the name & properties - NamedArea area = intersects.get(0); - double length = SphericalDistanceLibrary.distance(to.getCoordinate(), from.getCoordinate()); AreaEdge ae = new AreaEdge( from, to, line, - area.getName(), + hit.getName(), length, - area.getPermission(), + hit.getPermission(), false, ael ); @@ -648,9 +651,9 @@ private void createSegments( to, from, line.reverse(), - area.getName(), + hit.getName(), length, - area.getPermission(), + hit.getPermission(), true, ael ); From c605ed8e9dbc66e6b5fad9b4b46820582c5d5d18 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Thu, 2 Mar 2023 13:40:51 +0200 Subject: [PATCH 09/17] Update the linking test and add more --- .../linking/LinkStopToPlatformTest.java | 125 +++++++++--------- 1 file changed, 63 insertions(+), 62 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java index 253f333dff9..f37043885eb 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java @@ -31,85 +31,98 @@ import org.opentripplanner.transit.model.site.RegularStop; import org.opentripplanner.transit.service.StopModel; import org.opentripplanner.transit.service.TransitModel; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class LinkStopToPlatformTest { - private static final GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); - - private Graph graph; + private static final Logger LOG = LoggerFactory.getLogger(LinkStopToPlatformTest.class); - @BeforeEach - public void before() { - /* Set up transit platform with 5 corners and a stop outside the platform. It looks like this: - ________________________________________ - | | - | | - | / - | / - ------------------------------------- - * - */ + private static final GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); + private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate stop) { var deduplicator = new Deduplicator(); var stopModel = new StopModel(); - graph = new Graph(deduplicator); + Graph graph = new Graph(deduplicator); var transitModel = new TransitModel(stopModel, deduplicator); - ArrayList vertices = new ArrayList<>(); + Coordinate[] closedGeom = new Coordinate[platform.length + 1]; - vertices.add(new IntersectionVertex(graph, "1", 10.22054, 59.13568, "Platform vertex 1")); - vertices.add(new IntersectionVertex(graph, "2", 10.22432, 59.13519, "Platform vertex 2")); - vertices.add(new IntersectionVertex(graph, "3", 10.22492, 59.13514, "Platform vertex 3")); - vertices.add(new IntersectionVertex(graph, "4", 10.22493, 59.13518, "Platform vertex 4")); - vertices.add(new IntersectionVertex(graph, "5", 10.22056, 59.13575, "Platform vertex 5")); + for (int i = 0; i < platform.length; i++) { + Coordinate c = platform[i]; + vertices.add( + new IntersectionVertex(graph, String.valueOf(i), c.x, c.y, "Platform vertex " + i) + ); + closedGeom[i] = c; + } + closedGeom[platform.length] = closedGeom[0]; AreaEdgeList areaEdgeList = new AreaEdgeList( - GeometryUtils - .getGeometryFactory() - .createPolygon( - new Coordinate[] { - new Coordinate(59.13568, 10.22054), - new Coordinate(59.13519, 10.22432), - new Coordinate(59.13514, 10.22492), - new Coordinate(59.13518, 10.22493), - new Coordinate(59.13575, 10.22056), - new Coordinate(59.13568, 10.22054), - } - ), + GeometryUtils.getGeometryFactory().createPolygon(closedGeom), Set.of() ); - ArrayList edges = new ArrayList<>(); + // visibility vertices are platform entrance points and convex corners + // which should be directly linked with stops + for (int i : visible) { + areaEdgeList.visibilityVertices.add(vertices.get(i)); + } - edges.add(createAreaEdge(vertices.get(0), vertices.get(1), areaEdgeList, "edge 1")); - edges.add(createAreaEdge(vertices.get(1), vertices.get(2), areaEdgeList, "edge 2")); - edges.add(createAreaEdge(vertices.get(2), vertices.get(3), areaEdgeList, "edge 3")); - edges.add(createAreaEdge(vertices.get(3), vertices.get(4), areaEdgeList, "edge 4")); - edges.add(createAreaEdge(vertices.get(4), vertices.get(0), areaEdgeList, "edge 5")); + ArrayList edges = new ArrayList<>(); - edges.add(createAreaEdge(vertices.get(1), vertices.get(0), areaEdgeList, "edge 6")); - edges.add(createAreaEdge(vertices.get(2), vertices.get(1), areaEdgeList, "edge 7")); - edges.add(createAreaEdge(vertices.get(3), vertices.get(2), areaEdgeList, "edge 8")); - edges.add(createAreaEdge(vertices.get(4), vertices.get(3), areaEdgeList, "edge 9")); - edges.add(createAreaEdge(vertices.get(0), vertices.get(4), areaEdgeList, "edge 10")); + for (int i = 0; i < platform.length; i++) { + int next_i = (i + 1) % platform.length; + edges.add(createAreaEdge(vertices.get(i), vertices.get(next_i), areaEdgeList, "edge " + i)); + edges.add( + createAreaEdge( + vertices.get(next_i), + vertices.get(i), + areaEdgeList, + "edge " + String.valueOf(i + platform.length) + ) + ); + } - RegularStop stop = TransitModelForTest + RegularStop transitStop = TransitModelForTest .stop("TestStop") - .withCoordinate(59.13545, 10.22213) + .withCoordinate(stop.y, stop.x) .build(); transitModel.index(); graph.index(transitModel.getStopModel()); - new TransitStopVertexBuilder().withGraph(graph).withStop(stop).build(); + new TransitStopVertexBuilder().withGraph(graph).withStop(transitStop).build(); + + LOG.info("Test graph size {}", graph.getEdges().size()); + return graph; } /** * Tests that extra edges are added when linking stops to platform areas to prevent detours around - * the platform. */ @Test - public void testLinkStopWithoutExtraEdges() { + public void testLinkStopOutsideArea() { + // test platform is a simple rectangle. It creates a graph of 8 edges. + Coordinate platform[] = { + new Coordinate(60.0001, 10), + new Coordinate(60.0001, 10.0002), + new Coordinate(60, 10.0002), + new Coordinate(60, 10), + }; + // add entrance to every corner of the platform (this array defines indices) + int visibilityPoints[] = { 0, 1, 2, 3 }; + + // place one stop outside the platform, halway under the bottom edge + Coordinate stop = new Coordinate(59.9999, 10.0001); + + Graph graph = prepareTest(platform, visibilityPoints, stop); + linkStops(graph); + + // bottom edge gets split into half and split point connects to the stop. 4 new eges get added. + assertEquals(12, graph.getEdges().size()); + } + + private void linkStops(Graph graph) { VertexLinker linker = graph.getLinker(); for (TransitStopVertex tStop : graph.getVerticesOfType(TransitStopVertex.class)) { @@ -124,19 +137,6 @@ public void testLinkStopWithoutExtraEdges() { ) ); } - - assertEquals(16, graph.getEdges().size()); - } - - @Test - @Disabled - public void testLinkStopWithExtraEdges() { - // TODO Reimplement this functionality #3152 - // SimpleStreetSplitter splitter = SimpleStreetSplitter.createForTest(graph); - // splitter.setAddExtraEdgesToAreas(true); - // splitter.link(); - // - // assertEquals(38, graph.getEdges().size()); } private AreaEdge createAreaEdge( @@ -150,6 +150,7 @@ private AreaEdge createAreaEdge( ); I18NString name = new LocalizedString(nameString); + LOG.info("edgelen {}", line.getLength()); return new AreaEdge( v1, v2, From 54e0069dc1ca9bc243dde90df4b577346beb4987 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 6 Mar 2023 09:56:17 +0200 Subject: [PATCH 10/17] Add convenient method for testing vertex linking --- .../street/model/vertex/Vertex.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java index 5261da76228..95a5e598306 100644 --- a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java +++ b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java @@ -200,6 +200,23 @@ public List getOutgoingStreetEdges() { return result; } + /** + * Returns true if vertex is connected to another one by an edge + */ + public boolean isConnected(Vertex v) { + for (Edge e : this.getOutgoing()) { + if (e.getToVertex() == v) { + return true; + } + } + for (Edge e : v.getOutgoing()) { + if (e.getToVertex() == this) { + return true; + } + } + return false; + } + public boolean rentalTraversalBanned(State currentState) { return rentalRestrictions.traversalBanned(currentState); } From b15614930f5590a3dc77a9606b726731e745de1a Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 6 Mar 2023 10:46:36 +0200 Subject: [PATCH 11/17] Use correct logic for skipping duplicate linking --- .../routing/linking/VertexLinker.java | 25 ++++++------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index dd498853e46..217ea81776a 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -390,17 +390,12 @@ else if ( boolean split = true; // if vertex is inside an area, no need to snap to nearest edge and split it if (this.addExtraEdgesToAreas && edge instanceof AreaEdge aEdge) { - // do not relink again to same area when many edges are equally close - if (linkedAreas.contains(aEdge.getArea())) { - return null; - } - - if ( - aEdge - .getArea() - .getGeometry() - .contains(GEOMETRY_FACTORY.createPoint(vertex.getCoordinate())) - ) { + AreaEdgeList ael = aEdge.getArea(); + if (ael.getGeometry().contains(GEOMETRY_FACTORY.createPoint(vertex.getCoordinate()))) { + // do not relink again to the area when many edges are equally close + if (!linkedAreas.add(ael)) { + return null; + } if (vertex instanceof IntersectionVertex iv) { start = iv; } else { @@ -586,7 +581,6 @@ public void addAreaVertex( createSegments(newVertex, v, edgeList, areas, scope, tempEdges); added++; } - // TODO: Temporary fix for unconnected area edges. This should go away when moving walkable // area calculation to be done after stop linking if (added == 0) { @@ -608,12 +602,9 @@ private void createSegments( DisposableEdgeCollection tempEdges ) { // Check that vertices are not yet linked - for (Edge e : from.getOutgoing()) { - if (e.getToVertex() == to) { - return; - } + if (from.isConnected(to)) { + return; } - LineString line = GEOMETRY_FACTORY.createLineString( new Coordinate[] { from.getCoordinate(), to.getCoordinate() } ); From 74400ac5b99c32f188f732cb77c113a624b7ea88 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 6 Mar 2023 10:57:43 +0200 Subject: [PATCH 12/17] Add unit tests for new linking functionality --- .../linking/LinkStopToPlatformTest.java | 131 ++++++++++++++---- 1 file changed, 107 insertions(+), 24 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java index 6fc3aec1da1..cbf36c4312a 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java @@ -1,6 +1,8 @@ package org.opentripplanner.graph_builder.linking; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; import java.util.List; @@ -11,6 +13,7 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.geom.Polygon; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.framework.i18n.LocalizedString; @@ -21,10 +24,12 @@ import org.opentripplanner.street.model.edge.AreaEdge; import org.opentripplanner.street.model.edge.AreaEdgeList; import org.opentripplanner.street.model.edge.Edge; +import org.opentripplanner.street.model.edge.NamedArea; import org.opentripplanner.street.model.edge.StreetTransitStopLink; import org.opentripplanner.street.model.vertex.IntersectionVertex; import org.opentripplanner.street.model.vertex.TransitStopVertex; import org.opentripplanner.street.model.vertex.TransitStopVertexBuilder; +import org.opentripplanner.street.model.vertex.Vertex; import org.opentripplanner.street.search.TraverseMode; import org.opentripplanner.street.search.TraverseModeSet; import org.opentripplanner.transit.model._data.TransitModelForTest; @@ -38,10 +43,9 @@ public class LinkStopToPlatformTest { private static final Logger LOG = LoggerFactory.getLogger(LinkStopToPlatformTest.class); - private static final GeometryFactory geometryFactory = GeometryUtils.getGeometryFactory(); - private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate stop) { + private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate[] stops) { var deduplicator = new Deduplicator(); var stopModel = new StopModel(); Graph graph = new Graph(deduplicator); @@ -58,10 +62,8 @@ private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate stop) } closedGeom[platform.length] = closedGeom[0]; - AreaEdgeList areaEdgeList = new AreaEdgeList( - GeometryUtils.getGeometryFactory().createPolygon(closedGeom), - Set.of() - ); + Polygon polygon = GeometryUtils.getGeometryFactory().createPolygon(closedGeom); + AreaEdgeList areaEdgeList = new AreaEdgeList(polygon, Set.of()); // visibility vertices are platform entrance points and convex corners // which should be directly linked with stops @@ -69,6 +71,13 @@ private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate stop) areaEdgeList.visibilityVertices.add(vertices.get(i)); } + // AreaEdgeList must include a valid NamedArea which defines area atttributes + NamedArea namedArea = new NamedArea(); + namedArea.setName(new LocalizedString("test platform")); + namedArea.setPermission(StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE); + namedArea.setOriginalEdges(polygon); + areaEdgeList.addArea(namedArea); + ArrayList edges = new ArrayList<>(); for (int i = 0; i < platform.length; i++) { @@ -84,22 +93,25 @@ private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate stop) ); } - RegularStop transitStop = TransitModelForTest - .stop("TestStop") - .withCoordinate(stop.y, stop.x) - .build(); + RegularStop transitStops[] = new RegularStop[stops.length]; + for (int i = 0; i < stops.length; i++) { + Coordinate stop = stops[i]; + transitStops[i] = + TransitModelForTest.stop("TestStop " + i).withCoordinate(stop.y, stop.x).build(); + } transitModel.index(); graph.index(transitModel.getStopModel()); - new TransitStopVertexBuilder().withGraph(graph).withStop(transitStop).build(); + for (RegularStop s : transitStops) { + new TransitStopVertexBuilder().withGraph(graph).withStop(s).build(); + } - LOG.info("Test graph size {}", graph.getEdges().size()); return graph; } /** - * Link stop outside platform area to platform. Adds direct connection to closest edges. + * Link stop outside platform area to platform. */ @Test public void testLinkStopOutsideArea() { @@ -114,14 +126,20 @@ public void testLinkStopOutsideArea() { int visibilityPoints[] = { 0, 1, 2, 3 }; // place one stop outside the platform, halway under the bottom edge - Coordinate stop = new Coordinate(10.001, 59.9999); + Coordinate stops[] = { new Coordinate(10.001, 59.9999) }; - Graph graph = prepareTest(platform, visibilityPoints, stop); + Graph graph = prepareTest(platform, visibilityPoints, stops); linkStops(graph); - // Two bottom edges gets split into half and both split points - // connected to the stop bidirectonally. 6 new eges get added. - assertEquals(14, graph.getEdges().size()); + for (Edge e : graph.getEdges()) { + LOG.debug("Edge {}", e); + } + + // Two bottom edges gets split into half (+2 edges) + // both split points are linked to the stop bidirectonally (+4 edges). + // both split points also link to 2 visibility points at opposite side (+8 edges) + // 14 new edges in total + assertEquals(22, graph.getEdges().size()); } /** @@ -140,16 +158,81 @@ public void testLinkStopInsideArea() { int visibilityPoints[] = { 0, 1, 2, 3 }; // place one stop inside the platform, near bottom left corner - Coordinate stop = new Coordinate(10.001, 60.001); + Coordinate stops[] = { new Coordinate(10.001, 60.001) }; - Graph graph = prepareTest(platform, visibilityPoints, stop); + Graph graph = prepareTest(platform, visibilityPoints, stops); linkStops(graph); - for (Edge e : graph.getEdges()) { - LOG.info("Edge {}", e); + // stop links to a new street vertex with 2 edges + // new vertex connects to all 4 visibility points with 4*2 new edges + assertEquals(18, graph.getEdges().size()); + } + + /** + * Link stop which is very close to a platform vertex. + * Linking snaps directly to the vertex. + */ + @Test + public void testLinkStopNearPlatformVertex() { + Coordinate platform[] = { + new Coordinate(10, 60.002), + new Coordinate(10.004, 60.002), + new Coordinate(10.004, 60), + new Coordinate(10, 60), + }; + // add entrance to every corner of the platform + int visibilityPoints[] = { 0, 1, 2, 3 }; + + // place one stop inside the platform, very near of the bottom left corner + Coordinate stops[] = { new Coordinate(10.00000001, 60.00000001) }; + + Graph graph = prepareTest(platform, visibilityPoints, stops); + linkStops(graph); + + // stop links to a existing vertex with 2 edges + // selected vertex connects to oppiste corner with 2 new edges + assertEquals(12, graph.getEdges().size()); + } + + /** + * Link two stops inside platform area to platform. + * Stops will get linked directly. + */ + @Test + public void testLinkTwoStopsInsideArea() { + Coordinate platform[] = { + new Coordinate(10, 60.002), + new Coordinate(10.004, 60.002), + new Coordinate(10.004, 60), + new Coordinate(10, 60), + }; + // entrance to every corner of the platform + int visibilityPoints[] = { 0, 1, 2, 3 }; + + // place two stops inside the platform + Coordinate stops[] = { new Coordinate(10.001, 60.001), new Coordinate(10.003, 60.001) }; + + Graph graph = prepareTest(platform, visibilityPoints, stops); + linkStops(graph); + + // stops are linked with 2 new street vertices with 2 edges each (+4) + // new vertices connects to original 4 visibility points with 2*4*2 new edges (+16) + // stops are also linked directly (+2) + assertEquals(30, graph.getEdges().size()); + + // verify direct linking + List transitStops = graph.getVerticesOfType(TransitStopVertex.class); + Vertex v1 = null; + Vertex v2 = null; + for (Edge e : transitStops.get(0).getOutgoing()) { + v1 = e.getToVertex(); + } + for (Edge e : transitStops.get(1).getOutgoing()) { + v2 = e.getToVertex(); } - // stop connects to all 4 visibility points and 4*2 new edges will be added - assertEquals(16, graph.getEdges().size()); + assertNotNull(v1); + assertNotNull(v2); + assertTrue(v1.isConnected(v2)); } private void linkStops(Graph graph) { From 66648f7a421302b7f25ae0a29d7867cf74c59ac8 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 6 Mar 2023 14:27:25 +0200 Subject: [PATCH 13/17] Fix array notation --- .../linking/LinkStopToPlatformTest.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java index cbf36c4312a..ca573d4a172 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java @@ -93,7 +93,7 @@ private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate[] sto ); } - RegularStop transitStops[] = new RegularStop[stops.length]; + RegularStop[] transitStops = new RegularStop[stops.length]; for (int i = 0; i < stops.length; i++) { Coordinate stop = stops[i]; transitStops[i] = @@ -116,17 +116,17 @@ private Graph prepareTest(Coordinate[] platform, int[] visible, Coordinate[] sto @Test public void testLinkStopOutsideArea() { // test platform is a simple rectangle. It creates a graph of 8 edges. - Coordinate platform[] = { + Coordinate[] platform = { new Coordinate(10, 60.001), new Coordinate(10.002, 60.001), new Coordinate(10.002, 60), new Coordinate(10, 60), }; // add entrance to every corner of the platform (this array defines indices) - int visibilityPoints[] = { 0, 1, 2, 3 }; + int[] visibilityPoints = { 0, 1, 2, 3 }; // place one stop outside the platform, halway under the bottom edge - Coordinate stops[] = { new Coordinate(10.001, 59.9999) }; + Coordinate[] stops = { new Coordinate(10.001, 59.9999) }; Graph graph = prepareTest(platform, visibilityPoints, stops); linkStops(graph); @@ -148,17 +148,17 @@ public void testLinkStopOutsideArea() { @Test public void testLinkStopInsideArea() { // test platform is a simple rectangle. It creates a graph of 8 edges. - Coordinate platform[] = { + Coordinate[] platform = { new Coordinate(10, 60.002), new Coordinate(10.004, 60.002), new Coordinate(10.004, 60), new Coordinate(10, 60), }; // add entrance to every corner of the platform (this array defines indices) - int visibilityPoints[] = { 0, 1, 2, 3 }; + int[] visibilityPoints = { 0, 1, 2, 3 }; // place one stop inside the platform, near bottom left corner - Coordinate stops[] = { new Coordinate(10.001, 60.001) }; + Coordinate[] stops = { new Coordinate(10.001, 60.001) }; Graph graph = prepareTest(platform, visibilityPoints, stops); linkStops(graph); @@ -174,17 +174,17 @@ public void testLinkStopInsideArea() { */ @Test public void testLinkStopNearPlatformVertex() { - Coordinate platform[] = { + Coordinate[] platform = { new Coordinate(10, 60.002), new Coordinate(10.004, 60.002), new Coordinate(10.004, 60), new Coordinate(10, 60), }; // add entrance to every corner of the platform - int visibilityPoints[] = { 0, 1, 2, 3 }; + int[] visibilityPoints = { 0, 1, 2, 3 }; // place one stop inside the platform, very near of the bottom left corner - Coordinate stops[] = { new Coordinate(10.00000001, 60.00000001) }; + Coordinate[] stops = { new Coordinate(10.00000001, 60.00000001) }; Graph graph = prepareTest(platform, visibilityPoints, stops); linkStops(graph); @@ -200,17 +200,17 @@ public void testLinkStopNearPlatformVertex() { */ @Test public void testLinkTwoStopsInsideArea() { - Coordinate platform[] = { + Coordinate[] platform = { new Coordinate(10, 60.002), new Coordinate(10.004, 60.002), new Coordinate(10.004, 60), new Coordinate(10, 60), }; // entrance to every corner of the platform - int visibilityPoints[] = { 0, 1, 2, 3 }; + int[] visibilityPoints = { 0, 1, 2, 3 }; // place two stops inside the platform - Coordinate stops[] = { new Coordinate(10.001, 60.001), new Coordinate(10.003, 60.001) }; + Coordinate[] stops = { new Coordinate(10.001, 60.001), new Coordinate(10.003, 60.001) }; Graph graph = prepareTest(platform, visibilityPoints, stops); linkStops(graph); From a3c6ee17887d8b3be5a5d82423c6e36e2a32cfaa Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 6 Mar 2023 14:29:34 +0200 Subject: [PATCH 14/17] Remove some unused methods --- .../routing/linking/DisposableEdgeCollection.java | 4 ---- .../opentripplanner/street/model/vertex/Vertex.java | 10 ---------- 2 files changed, 14 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java b/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java index cf2af6e2041..a8c09e92081 100644 --- a/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java +++ b/src/main/java/org/opentripplanner/routing/linking/DisposableEdgeCollection.java @@ -33,10 +33,6 @@ public void addEdge(Edge edge) { this.edges.add(edge); } - public int edgeCount() { - return this.edges.size(); - } - /** * Removes all the edges in this collection from the graph. */ diff --git a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java index f370fe6b646..719601f621e 100644 --- a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java +++ b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java @@ -171,16 +171,6 @@ public Coordinate getCoordinate() { return new Coordinate(getX(), getY()); } - /** Get the bearing, in degrees, between this vertex and another coordinate. */ - public double azimuthTo(Coordinate other) { - return DirectionUtils.getAzimuth(getCoordinate(), other); - } - - /** Get the bearing, in degrees, between this vertex and another. */ - public double azimuthTo(Vertex other) { - return azimuthTo(other.getCoordinate()); - } - public List getIncomingStreetEdges() { List result = new ArrayList<>(); for (Edge out : this.getIncoming()) { From 40fd7795ae7a25936038fcc4a8eb9ae03cf10e8c Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Mon, 6 Mar 2023 14:46:53 +0200 Subject: [PATCH 15/17] Fix typo --- .../graph_builder/linking/LinkStopToPlatformTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java index ca573d4a172..757bd885b1a 100644 --- a/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/linking/LinkStopToPlatformTest.java @@ -190,7 +190,7 @@ public void testLinkStopNearPlatformVertex() { linkStops(graph); // stop links to a existing vertex with 2 edges - // selected vertex connects to oppiste corner with 2 new edges + // selected vertex connects to opposite corner with 2 new edges assertEquals(12, graph.getEdges().size()); } From cc54e96b131cbca0b3b8508a3e09df312ac62bb4 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Thu, 9 Mar 2023 14:28:07 +0200 Subject: [PATCH 16/17] Temporary hack to filter bad vertices There is a bug in graph serialisation. Quick fix until the actual bug is fixed --- .../routing/linking/VertexLinker.java | 6 ++++++ .../opentripplanner/street/model/vertex/Vertex.java | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index b5bb5994356..930ea1c3af3 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -569,6 +569,9 @@ public void addAreaVertex( int added = 0; for (IntersectionVertex v : visibilityVertices) { + if (!v.checkEdges()) { + continue; + } LineString newGeometry = GEOMETRY_FACTORY.createLineString( new Coordinate[] { nearestPoints[0], v.getCoordinate() } ); @@ -588,6 +591,9 @@ public void addAreaVertex( // area calculation to be done after stop linking if (added == 0) { for (IntersectionVertex v : visibilityVertices) { + if (!v.checkEdges()) { + continue; + } createSegments(newVertex, v, edgeList, areas, scope, tempEdges); } } diff --git a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java index 719601f621e..eaedbba51c5 100644 --- a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java +++ b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java @@ -130,6 +130,13 @@ public int getDegreeIn() { return incoming.length; } + public boolean checkEdges() { + if (outgoing == null || incoming == null) { + return false; + } + return true; + } + /** Get the longitude of the vertex */ public final double getX() { return getLon(); @@ -197,13 +204,13 @@ public List getOutgoingStreetEdges() { * Returns true if vertex is connected to another one by an edge */ public boolean isConnected(Vertex v) { - for (Edge e : this.getOutgoing()) { + for (Edge e : outgoing) { if (e.getToVertex() == v) { return true; } } - for (Edge e : v.getOutgoing()) { - if (e.getToVertex() == this) { + for (Edge e : incoming) { + if (e.getFromVertex() == v) { return true; } } From 05c07ae65378f0bcdef6030100ebc1ccff90d113 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Thu, 9 Mar 2023 14:35:33 +0200 Subject: [PATCH 17/17] Better name for edge validity test --- .../org/opentripplanner/routing/linking/VertexLinker.java | 4 ++-- .../java/org/opentripplanner/street/model/vertex/Vertex.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java index 930ea1c3af3..cbf759bc3a6 100644 --- a/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java +++ b/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java @@ -569,7 +569,7 @@ public void addAreaVertex( int added = 0; for (IntersectionVertex v : visibilityVertices) { - if (!v.checkEdges()) { + if (!v.hasEdges()) { continue; } LineString newGeometry = GEOMETRY_FACTORY.createLineString( @@ -591,7 +591,7 @@ public void addAreaVertex( // area calculation to be done after stop linking if (added == 0) { for (IntersectionVertex v : visibilityVertices) { - if (!v.checkEdges()) { + if (!v.hasEdges()) { continue; } createSegments(newVertex, v, edgeList, areas, scope, tempEdges); diff --git a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java index eaedbba51c5..890589337bd 100644 --- a/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java +++ b/src/main/java/org/opentripplanner/street/model/vertex/Vertex.java @@ -130,7 +130,7 @@ public int getDegreeIn() { return incoming.length; } - public boolean checkEdges() { + public boolean hasEdges() { if (outgoing == null || incoming == null) { return false; }