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

Jump to bottom

Add ways from highway relations to road network #2807

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add ways from highway relations to road network #2807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7911835
Add ways from highway relations to road network
easbar Apr 26, 2023
8a5995d
Ignore inner polygons
easbar Apr 26, 2023
3912106
Fix test
easbar Apr 26, 2023
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
1 change: 1 addition & 0 deletions core/src/main/java/com/graphhopper/reader/osm/OSMReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ public void readGraph() throws IOException {
.setElevationProvider(eleProvider)
.setWayFilter(this::acceptWay)
.setSplitNodeFilter(this::isBarrierNode)
.setRelationFilterForWays(osmParsers.getRelationFilterForWays())
.setWayPreprocessor(this::preprocessWay)
.setRelationPreprocessor(this::preprocessRelations)
.setRelationProcessor(this::processRelation)
Expand Down
95 changes: 88 additions & 7 deletions core/src/main/java/com/graphhopper/reader/osm/WaySegmentParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
package com.graphhopper.reader.osm;

import com.carrotsearch.hppc.cursors.LongCursor;
import com.graphhopper.coll.GHLongIntBTree;
import com.graphhopper.coll.LongIntMap;
import com.graphhopper.reader.ReaderElement;
import com.graphhopper.reader.ReaderNode;
import com.graphhopper.reader.ReaderRelation;
Expand Down Expand Up @@ -68,29 +70,35 @@ public class WaySegmentParser {

private final ElevationProvider eleProvider;
private final Predicate<ReaderWay> wayFilter;
private final Predicate<ReaderRelation> relationFilterForWays;
private final Predicate<ReaderNode> splitNodeFilter;
private final WayPreprocessor wayPreprocessor;
private final Consumer<ReaderRelation> relationPreprocessor;
private final RelationProcessor relationProcessor;
private final EdgeHandler edgeHandler;
private final int workerThreads;

private final List<ReaderRelation> relationsWithWays;
private final LongIntMap relationWithWaysIndexByOSMWayId;
private final OSMNodeData nodeData;
private Date timestamp;

private WaySegmentParser(PointAccess nodeAccess, Directory directory, ElevationProvider eleProvider,
Predicate<ReaderWay> wayFilter, Predicate<ReaderNode> splitNodeFilter, WayPreprocessor wayPreprocessor,
Predicate<ReaderWay> wayFilter, Predicate<ReaderRelation> relationFilterForWays,
Predicate<ReaderNode> splitNodeFilter, WayPreprocessor wayPreprocessor,
Consumer<ReaderRelation> relationPreprocessor, RelationProcessor relationProcessor,
EdgeHandler edgeHandler, int workerThreads) {
this.eleProvider = eleProvider;
this.wayFilter = wayFilter;
this.relationFilterForWays = relationFilterForWays;
this.splitNodeFilter = splitNodeFilter;
this.wayPreprocessor = wayPreprocessor;
this.relationPreprocessor = relationPreprocessor;
this.relationProcessor = relationProcessor;
this.edgeHandler = edgeHandler;
this.workerThreads = workerThreads;

relationsWithWays = new ArrayList<>();
relationWithWaysIndexByOSMWayId = new GHLongIntBTree(200);
this.nodeData = new OSMNodeData(nodeAccess, directory);
}

Expand All @@ -101,6 +109,13 @@ public void readOSM(File osmFile) {
if (nodeData.getNodeCount() > 0)
throw new IllegalStateException("You can only run way segment parser once");

LOGGER.info("Start reading OSM file: '" + osmFile + "'");
LOGGER.info("pass0 - start");
StopWatch sw0 = StopWatch.started();
if (relationFilterForWays != null)
readOSM(osmFile, new Pass0Handler(), new SkipOptions(true, true, false));
LOGGER.info("pass0 - finished, took: {}", sw0.stop().getTimeString());

LOGGER.info("Start reading OSM file: '" + osmFile + "'");
LOGGER.info("pass1 - start");
StopWatch sw1 = StopWatch.started();
Expand All @@ -119,9 +134,10 @@ public void readOSM(File osmFile) {
nodeData.release();

LOGGER.info("Finished reading OSM file." +
" pass0: " + (int) sw0.getSeconds() + "s, " +
" pass1: " + (int) sw1.getSeconds() + "s, " +
" pass2: " + (int) sw2.getSeconds() + "s, " +
" total: " + (int) (sw1.getSeconds() + sw2.getSeconds()) + "s");
" total: " + (int) (sw0.getSeconds() + sw1.getSeconds() + sw2.getSeconds()) + "s");
}

/**
Expand All @@ -131,6 +147,51 @@ public Date getTimeStamp() {
return timestamp;
}

private class Pass0Handler implements ReaderElementHandler {
private boolean handledRelations;
private long acceptedRelations = 0;
private long relationsCounter = 0;

@Override
public void handleRelation(ReaderRelation relation) {
if (!handledRelations) {
LOGGER.info("pass0 - start reading OSM relations");
handledRelations = true;
}

if (++relationsCounter % 1_000_000 == 0)
LOGGER.info("pass0 - processed relations: " + nf(relationsCounter) + ", " + Helper.getMemInfo());

if (!relationFilterForWays.test(relation))
return;

long acceptedRelationsBefore = acceptedRelations;
for (ReaderRelation.Member member : relation.getMembers()) {
if (member.getType() != ReaderElement.Type.WAY)
continue;
if ("inner".equals(member.getRole()))
// in many cases the inner polygon won't be connected to the road network, which
// wouldn't even be a big deal, because of the subnetwork 'removal', but sometimes
// it is connected at one point only which can lead to strange detours
continue;
// add relation only if there is at least one accepted way member
if (acceptedRelationsBefore == acceptedRelations) {
relationsWithWays.add(relation);
acceptedRelations++;
}
// note that we simply ignore the possibility of ways being contained in multiple highway relations
// and use the tags of the last such relation for each way
relationWithWaysIndexByOSMWayId.put(member.getRef(), relationsWithWays.size() - 1);
}
}

@Override
public void onFinish() {
LOGGER.info("pass0 - finished, processed relations: " + nf(relationsCounter) + ", accepted relations with ways: " +
nf(acceptedRelations) + ", " + Helper.getMemInfo());
}
}

private class Pass1Handler implements ReaderElementHandler {
private boolean handledWays;
private boolean handledRelations;
Expand All @@ -151,7 +212,7 @@ public void handleWay(ReaderWay way) {
LOGGER.info("pass1 - processed ways: " + nf(wayCounter) + ", accepted ways: " + nf(acceptedWays) +
", way nodes: " + nf(nodeData.getNodeCount()) + ", " + Helper.getMemInfo());

if (!wayFilter.test(way))
if (!wayFilter.test(way) && relationWithWaysIndexByOSMWayId.get(way.getId()) == -1)
return;
acceptedWays++;

Expand Down Expand Up @@ -256,8 +317,16 @@ public void handleWay(ReaderWay way) {
if (++wayCounter % 10_000_000 == 0)
LOGGER.info("pass2 - processed ways: " + nf(wayCounter) + ", " + Helper.getMemInfo());

if (!wayFilter.test(way))
return;
if (!wayFilter.test(way)) {
int relationWithWaysIndex = relationWithWaysIndexByOSMWayId.get(way.getId());
if (relationWithWaysIndex == -1)
return;
// We only accept this way because it is part of an accepted relation. So we use the
// tags of this relation. Typical common example are ways without tags that are members
// of a relation carrying the highway=pedestrian tag.
ReaderRelation relation = relationsWithWays.get(relationWithWaysIndex);
way.setTags(relation.getTags());
}
List<SegmentNode> segment = new ArrayList<>(way.getNodes().size());
for (LongCursor node : way.getNodes())
segment.add(new SegmentNode(node.value, nodeData.getId(node.value), nodeData.getTags(node.value)));
Expand Down Expand Up @@ -425,6 +494,7 @@ public static class Builder {
private Directory directory = new RAMDirectory();
private ElevationProvider elevationProvider = ElevationProvider.NOOP;
private Predicate<ReaderWay> wayFilter = way -> true;
private Predicate<ReaderRelation> relationFilterForWays = null;
private Predicate<ReaderNode> splitNodeFilter = node -> false;
private WayPreprocessor wayPreprocessor = (way, supplier) -> {
};
Expand Down Expand Up @@ -468,6 +538,16 @@ public Builder setWayFilter(Predicate<ReaderWay> wayFilter) {
return this;
}

/**
* @param relationFilterForWays return true for OSM relations for which the corresponding member ways
* should be considered as part of the road network and false otherwise
* when set to null (default) pass0 will be skipped entirely
*/
public Builder setRelationFilterForWays(Predicate<ReaderRelation> relationFilterForWays) {
this.relationFilterForWays = relationFilterForWays;
return this;
}

/**
* @param splitNodeFilter return true if the given OSM node should be duplicated to create an artificial edge
*/
Expand Down Expand Up @@ -518,7 +598,8 @@ public Builder setWorkerThreads(int workerThreads) {

public WaySegmentParser build() {
return new WaySegmentParser(
nodeAccess, directory, elevationProvider, wayFilter, splitNodeFilter, wayPreprocessor, relationPreprocessor, relationProcessor,
nodeAccess, directory, elevationProvider, wayFilter, relationFilterForWays,
splitNodeFilter, wayPreprocessor, relationPreprocessor, relationProcessor,
edgeHandler, workerThreads
);
}
Expand Down
14 changes: 13 additions & 1 deletion core/src/main/java/com/graphhopper/routing/util/OSMParsers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@
import com.graphhopper.reader.ReaderRelation;
import com.graphhopper.reader.ReaderWay;
import com.graphhopper.reader.osm.RestrictionTagParser;
import com.graphhopper.routing.ev.EncodedValue;
import com.graphhopper.routing.ev.EdgeIntAccess;
import com.graphhopper.routing.ev.EncodedValue;
import com.graphhopper.routing.util.parsers.RelationTagParser;
import com.graphhopper.routing.util.parsers.TagParser;
import com.graphhopper.storage.IntsRef;

import java.util.ArrayList;
import java.util.List;
import java.util.function.Function;
import java.util.function.Predicate;

public class OSMParsers {
private final List<String> ignoredHighways;
Expand Down Expand Up @@ -87,6 +88,17 @@ else if ("platform".equals(way.getTag("railway")))
return false;
}

public Predicate<ReaderRelation> getRelationFilterForWays() {
if (ignoredHighways.contains("pedestrian") && ignoredHighways.contains("footway"))
// if are both ignored we can skip pass0 altogether
return null;
Comment on lines +92 to +94
Copy link
Member Author

Choose a reason for hiding this comment

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

The 0th pass is skipped entirely if pedestrian and footway are both added to the list of ignored highways, because >80% of these highway relations use one of these two highway values. Then again, there are also around 10k relations tagged as service,tertiary,platform,bus_stop.... I'm not sure if we really need them for motorized-only routing (i.e. when pedestrian+footway are excluded). In case they are not excluded we would accept all highway relations anyway with this PR.

else
return relation -> {
String highway = relation.getTag("highway");
return highway != null && !ignoredHighways.contains(highway);
};
}

public IntsRef handleRelationTags(ReaderRelation relation, IntsRef relFlags) {
for (RelationTagParser relParser : relationTagParsers) {
relParser.handleRelationTags(relFlags, relation);
Expand Down
12 changes: 6 additions & 6 deletions web/src/test/java/com/graphhopper/application/resources/RouteResourceLeipzigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ void testNoErrors() {

@ParameterizedTest
@CsvSource(value = {
"86,-1,algorithm=" + DIJKSTRA_BI,
"110,-1,algorithm=" + ASTAR_BI,
"30834,1,ch.disable=true&algorithm=" + DIJKSTRA,
"21137,1,ch.disable=true&algorithm=" + ASTAR,
"14800,1,ch.disable=true&algorithm=" + DIJKSTRA_BI,
"10536,1,ch.disable=true&algorithm=" + ASTAR_BI
"76,-1,algorithm=" + DIJKSTRA_BI,
"91,-1,algorithm=" + ASTAR_BI,
"31030,1,ch.disable=true&algorithm=" + DIJKSTRA,
"21347,1,ch.disable=true&algorithm=" + ASTAR,
"14870,1,ch.disable=true&algorithm=" + DIJKSTRA_BI,
"10592,1,ch.disable=true&algorithm=" + ASTAR_BI
})
void testTimeout(int expectedVisitedNodes, int timeout, String args) {
{
Expand Down