Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH# 74: Snap to streets #148

Merged
merged 10 commits into from
May 25, 2024
Merged

GH# 74: Snap to streets #148

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
df460df
Expand SnapPreventionEdgeFilter to exclude footways.
rsarathy May 24, 2024
30ac66b
Incorporate SnapPreventionEdgeFilter into PtLocationSnapper.
rsarathy May 24, 2024
d3cf495
Keep switch-statement indentation consistent with rest of GH codebase.
rsarathy May 24, 2024
d6184c3
Checks that SnapPreventionEdgeFilter.accept() excludes footways succe…
rsarathy May 24, 2024
9bce96c
Add a makeConnectingSnapFilter() factory method.
rsarathy May 25, 2024
6a1969e
Revert PtLocationSnapper, SnapPreventionEdgeFilter changes will happe…
rsarathy May 25, 2024
0fbb869
Reorganize PtRouterImpl imports
rsarathy May 25, 2024
804de09
Call makeConnectingSnapFilter() in PtRouterImpl ctor.
rsarathy May 25, 2024
7282d44
Add snap_preventions to configs.
rsarathy May 25, 2024
077fcdd
Revert OSM file name change in config-local.yml
rsarathy May 25, 2024
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
20 changes: 13 additions & 7 deletions core/src/main/java/com/graphhopper/routing/util/SnapPreventionEdgeFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import java.util.List;

import static com.graphhopper.routing.ev.RoadClass.FOOTWAY;
import static com.graphhopper.routing.ev.RoadClass.MOTORWAY;
import static com.graphhopper.routing.ev.RoadClass.TRUNK;
import static com.graphhopper.routing.ev.RoadEnvironment.*;
Expand All @@ -18,7 +19,7 @@ public class SnapPreventionEdgeFilter implements EdgeFilter {
private final EnumEncodedValue<RoadClass> rcEnc;
private final EdgeFilter filter;
private boolean avoidMotorway = false, avoidTrunk;
private boolean avoidTunnel, avoidBridge, avoidFerry, avoidFord;
private boolean avoidTunnel, avoidBridge, avoidFerry, avoidFord, avoidFootway;

public SnapPreventionEdgeFilter(EdgeFilter filter, EnumEncodedValue<RoadClass> rcEnc,
EnumEncodedValue<RoadEnvironment> reEnc, List<String> snapPreventions) {
Expand All @@ -27,12 +28,16 @@ public SnapPreventionEdgeFilter(EdgeFilter filter, EnumEncodedValue<RoadClass> r
this.rcEnc = rcEnc;

for (String roadClassOrRoadEnv : snapPreventions) {
if ("motorway".equals(roadClassOrRoadEnv)) {
avoidMotorway = true;
continue;
} else if ("trunk".equals(roadClassOrRoadEnv)) {
avoidTrunk = true;
continue;
switch (roadClassOrRoadEnv) {
case "motorway":
avoidMotorway = true;
continue;
case "trunk":
avoidTrunk = true;
continue;
case "footway":
avoidFootway = true;
continue;
}

RoadEnvironment rc = RoadEnvironment.find(roadClassOrRoadEnv);
Expand All @@ -52,6 +57,7 @@ else if (rc == FORD)
@Override
public boolean accept(EdgeIteratorState edgeState) {
return filter.accept(edgeState)
&& !(avoidFootway && edgeState.get(rcEnc) == FOOTWAY)
&& !(avoidMotorway && edgeState.get(rcEnc) == MOTORWAY)
&& !(avoidTrunk && edgeState.get(rcEnc) == TRUNK)
&& !(avoidTunnel && edgeState.get(reEnc) == TUNNEL)
Expand Down
9 changes: 7 additions & 2 deletions core/src/test/java/com/graphhopper/routing/util/SnapPreventionEdgeFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ public void accept() {
EncodingManager em = new EncodingManager.Builder().build();
EnumEncodedValue<RoadClass> rcEnc = em.getEnumEncodedValue(RoadClass.KEY, RoadClass.class);
EnumEncodedValue<RoadEnvironment> reEnc = em.getEnumEncodedValue(RoadEnvironment.KEY, RoadEnvironment.class);
SnapPreventionEdgeFilter filter = new SnapPreventionEdgeFilter(trueFilter, rcEnc, reEnc, Arrays.asList("motorway", "ferry"));
SnapPreventionEdgeFilter filter = new SnapPreventionEdgeFilter(
trueFilter, rcEnc, reEnc,
Arrays.asList("motorway", "ferry", "footway"));

IntsRef intsRef = em.createEdgeFlags();
assertTrue(filter.accept(GHUtility.createMockedEdgeIteratorState(1, intsRef)));
Expand All @@ -33,5 +35,8 @@ public void accept() {
assertTrue(filter.accept(GHUtility.createMockedEdgeIteratorState(1, intsRef)));
rcEnc.setEnum(false, intsRef, RoadClass.MOTORWAY);
assertFalse(filter.accept(GHUtility.createMockedEdgeIteratorState(1, intsRef)));
rcEnc.setEnum(false, intsRef, RoadClass.FOOTWAY);
assertFalse(filter.accept(
GHUtility.createMockedEdgeIteratorState(1, intsRef)));
}
}
}
16 changes: 15 additions & 1 deletion reader-gtfs/src/main/java/com/graphhopper/gtfs/PtLocationSnapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@
import com.carrotsearch.hppc.cursors.IntCursor;
import com.conveyal.gtfs.GTFSFeed;
import com.conveyal.gtfs.model.Stop;
import com.graphhopper.routing.ev.EnumEncodedValue;
import com.graphhopper.routing.ev.RoadClass;
import com.graphhopper.routing.ev.RoadEnvironment;
import com.graphhopper.routing.querygraph.QueryGraph;
import com.graphhopper.routing.util.EdgeFilter;
import com.graphhopper.routing.util.EncodingManager;
import com.graphhopper.routing.util.SnapPreventionEdgeFilter;
import com.graphhopper.storage.GraphHopperStorage;
import com.graphhopper.storage.index.LocationIndex;
import com.graphhopper.storage.index.Snap;
Expand All @@ -14,6 +19,7 @@
import com.graphhopper.util.shapes.GHPoint;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -47,11 +53,19 @@ public Result snapAll(List<GHLocation> locations, List<EdgeFilter> snapFilters)
PointList points = new PointList(2, false);
ArrayList<Snap> pointSnaps = new ArrayList<>();
ArrayList<Supplier<Label.NodeId>> allSnaps = new ArrayList<>();
EncodingManager encodingManager =
graphHopperStorage.getEncodingManager();
final EnumEncodedValue<RoadClass> roadClassEnc = encodingManager.getEnumEncodedValue(RoadClass.KEY, RoadClass.class);
final EnumEncodedValue<RoadEnvironment> roadEnvEnc = encodingManager.getEnumEncodedValue(RoadEnvironment.KEY, RoadEnvironment.class);
for (int i = 0; i < locations.size(); i++) {
GHLocation location = locations.get(i);
if (location instanceof GHPointLocation) {
GHPoint point = ((GHPointLocation) location).ghPoint;
final Snap closest = locationIndex.findClosest(point.lat, point.lon, snapFilters.get(i));
EdgeFilter mergedFilter = new SnapPreventionEdgeFilter(
snapFilters.get(i), roadClassEnc, roadEnvEnc,
Collections.singletonList("footway"));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we still want to snap to sidewalks for public transit directions with walking as the connecting mode? Probably, right? If so, this should test for the connecting mode. Not currently relevant but matters when we add a transit/no bike directions mode or want to merge changes upstream.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Let me try and implement this at the config-level, using a snap_preventions field underneath our bike2 profile.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, this is now handled via our configs. We can specify snap_preventions as a comma-separated list.

final Snap closest = locationIndex.findClosest(
point.lat, point.lon, mergedFilter);
if (!closest.isValid()) {
IntHashSet result = new IntHashSet();
gtfsStorage.getStopIndex().findEdgeIdsInNeighborhood(point.lat, point.lon, 0, result::add);
Expand Down
Loading