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.

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

Namer for applying street names to nearby sidewalks #5774

Merged
merged 32 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
7fe1f3e
Make more classes package private
leonardehrenfried Mar 19, 2024
debb6eb
Use instance method for figuring out bogus name
leonardehrenfried Mar 20, 2024
f2d5e4a
Improve debugging of edge names
leonardehrenfried Mar 20, 2024
ea94f83
Simplify builder chains
leonardehrenfried Mar 20, 2024
c25c12a
Add first draft of edge naming
leonardehrenfried Mar 21, 2024
bac74c9
Finetune sidewalk naming
leonardehrenfried Mar 25, 2024
22e102a
Finetune sidewalk naming
leonardehrenfried Mar 25, 2024
1f106b1
Add documentation
leonardehrenfried Mar 26, 2024
1d83a07
Fine-tune sidewalk naming algorithm
leonardehrenfried Mar 27, 2024
ea1c169
Update tests and docs
leonardehrenfried Mar 27, 2024
daac600
Improve documentation
leonardehrenfried Mar 27, 2024
4d67575
Split up methods, add documentation
leonardehrenfried Mar 27, 2024
8670790
Add test, docs
leonardehrenfried Mar 28, 2024
4b6a27e
Sort by keys before comparing
leonardehrenfried Mar 28, 2024
f596432
Sort order for JSON files
leonardehrenfried Mar 28, 2024
a312d2a
Revert set conversion
leonardehrenfried Mar 28, 2024
1c66f06
Apply review feedback about documentation
leonardehrenfried Apr 10, 2024
c53e838
Fix more typos
leonardehrenfried Apr 12, 2024
70585de
Remove extra expandBy
leonardehrenfried Apr 16, 2024
3056805
Use better name for variable
leonardehrenfried Apr 16, 2024
de0e382
Compute UTM CRS only once
leonardehrenfried Apr 16, 2024
17279ba
Merge remote-tracking branch 'upstream/dev-2.x' into sidewalk-naming
leonardehrenfried Apr 16, 2024
1d749c3
Always record edges in pairs in the custom namer
leonardehrenfried Apr 17, 2024
4013d70
Remove unneeded operation
leonardehrenfried May 6, 2024
e28a69c
Improve performance of Portland custom namer
leonardehrenfried May 6, 2024
f995865
Improve computation of center point
leonardehrenfried May 6, 2024
e9a0722
Improve if/else logic
leonardehrenfried May 6, 2024
8a88c99
Update comment
leonardehrenfried May 6, 2024
6467149
Merge remote-tracking branch 'upstream/dev-2.x' into sidewalk-naming
leonardehrenfried May 6, 2024
2b2d365
Clarify hasName/hasNoName/isExplicitlyUnnamed
leonardehrenfried May 8, 2024
dbd8f2f
Remove separate section about OSM naming
leonardehrenfried May 13, 2024
60c4ee9
Incorporate review feedback
leonardehrenfried May 14, 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
10 changes: 9 additions & 1 deletion docs/BuildConfiguration.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Sections follow that describe particular settings in more depth.
| maxTransferDuration | `duration` | Transfers up to this duration with the default walk speed value will be pre-calculated and included in the Graph. | *Optional* | `"PT30M"` | 2.1 |
| [multiThreadElevationCalculations](#multiThreadElevationCalculations) | `boolean` | Configuring multi-threading during elevation calculations. | *Optional* | `false` | 2.0 |
| [osmCacheDataInMem](#osmCacheDataInMem) | `boolean` | If OSM data should be cached in memory during processing. | *Optional* | `false` | 2.0 |
| osmNaming | `string` | A custom OSM namer to use. | *Optional* | | 2.0 |
| [osmNaming](#osmNaming) | `enum` | A custom OSM namer to use. | *Optional* | `"default"` | 1.5 |
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
| platformEntriesLinking | `boolean` | Link unconnected entries to public transport platforms. | *Optional* | `false` | 2.0 |
| [readCachedElevations](#readCachedElevations) | `boolean` | Whether to read cached elevation data. | *Optional* | `true` | 2.0 |
| staticBikeParkAndRide | `boolean` | Whether we should create bike P+R stations from OSM data. | *Optional* | `false` | 1.5 |
Expand Down Expand Up @@ -536,6 +536,14 @@ deployment depending on your infrastructure. Set the parameter to `true` to cach
data, and to `false` to read the stream from the source each time.


<h3 id="osmNaming">osmNaming</h3>

**Since version:** `1.5` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"default"`
**Path:** /
**Enum values:** `default` | `portland` | `sidewalks`

A custom OSM namer to use.

<h3 id="readCachedElevations">readCachedElevations</h3>

**Since version:** `2.0` ∙ **Type:** `boolean` ∙ **Cardinality:** `Optional` ∙ **Default value:** `true`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.opentripplanner.apis.gtfs;

import com.bedatadriven.jackson.datatype.jts.JtsModule;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import graphql.language.StringValue;
Expand All @@ -16,15 +15,15 @@
import java.time.format.DateTimeFormatter;
import javax.annotation.Nonnull;
import org.locationtech.jts.geom.Geometry;
import org.opentripplanner.framework.geometry.GeometryUtils;
import org.opentripplanner.framework.graphql.scalar.DurationScalarFactory;
import org.opentripplanner.framework.json.ObjectMappers;
import org.opentripplanner.framework.model.Grams;
import org.opentripplanner.framework.time.OffsetDateTimeParser;

public class GraphQLScalars {

private static final ObjectMapper geoJsonMapper = new ObjectMapper()
.registerModule(new JtsModule(GeometryUtils.getGeometryFactory()));
private static final ObjectMapper geoJsonMapper = ObjectMappers.geoJson();

public static GraphQLScalarType DURATION_SCALAR = DurationScalarFactory.createDurationScalar();

public static final GraphQLScalarType POLYLINE_SCALAR = GraphQLScalarType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.opentripplanner.service.vehiclerental.street.StreetVehicleRentalLink;
import org.opentripplanner.street.model.edge.AreaEdge;
import org.opentripplanner.street.model.edge.BoardingLocationToStopLink;
import org.opentripplanner.street.model.edge.Edge;
import org.opentripplanner.street.model.edge.ElevatorHopEdge;
import org.opentripplanner.street.model.edge.EscalatorEdge;
import org.opentripplanner.street.model.edge.PathwayEdge;
Expand Down Expand Up @@ -49,6 +50,15 @@ public class DebugStyleSpec {
1,
List.of(new ZoomStop(15, 0.2f), new ZoomStop(MAX_ZOOM, 3))
);
private static final Class<Edge>[] EDGES_TO_DISPLAY = new Class[] {
StreetEdge.class,
AreaEdge.class,
EscalatorEdge.class,
PathwayEdge.class,
ElevatorHopEdge.class,
TemporaryPartialStreetEdge.class,
TemporaryFreeEdge.class,
};

static StyleSpec build(
VectorSourceLayer regularStops,
Expand All @@ -73,19 +83,20 @@ static StyleSpec build(
.typeLine()
.vectorSourceLayer(edges)
.lineColor(MAGENTA)
.edgeFilter(
StreetEdge.class,
AreaEdge.class,
EscalatorEdge.class,
PathwayEdge.class,
ElevatorHopEdge.class,
TemporaryPartialStreetEdge.class,
TemporaryFreeEdge.class
)
.edgeFilter(EDGES_TO_DISPLAY)
.lineWidth(LINE_WIDTH)
.minZoom(13)
.maxZoom(MAX_ZOOM)
.intiallyHidden(),
StyleBuilder
.ofId("edge-name")
.typeSymbol()
.lineText("name")
.vectorSourceLayer(edges)
.edgeFilter(EDGES_TO_DISPLAY)
.minZoom(17)
.maxZoom(MAX_ZOOM)
.intiallyHidden(),
StyleBuilder
.ofId("link")
.typeLine()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.stream.Stream;
import org.opentripplanner.apis.vectortiles.model.ZoomDependentNumber.ZoomStop;
import org.opentripplanner.framework.collection.ListUtils;
import org.opentripplanner.framework.json.ObjectMappers;
import org.opentripplanner.street.model.edge.Edge;
Expand Down Expand Up @@ -41,6 +42,7 @@ public enum LayerType {
Line,
Raster,
Fill,
Symbol,
}

private StyleBuilder(String id) {
Expand Down Expand Up @@ -94,11 +96,35 @@ public StyleBuilder typeFill() {
return this;
}

public StyleBuilder typeSymbol() {
type(LayerType.Symbol);
return this;
}

private StyleBuilder type(LayerType type) {
props.put(TYPE, type.name().toLowerCase());
return this;
}

public StyleBuilder lineText(String name) {
layout.put("symbol-placement", "line");
layout.put("symbol-spacing", 500);
layout.put("text-field", "{%s}".formatted(name));
layout.put("text-font", List.of("KlokanTech Noto Sans Regular"));
layout.put(
"text-size",
new ZoomDependentNumber(14, List.of(new ZoomStop(14, 12), new ZoomStop(20, 14))).toJson()
);
layout.put("text-max-width", 5);
layout.put("text-keep-upright", true);
layout.put("text-rotation-alignment", "map");
paint.put("text-color", "#000");
paint.put("text-halo-color", "#fff");
paint.put("text-halo-blur", 4);
paint.put("text-halo-width", 3);
return this;
}

public StyleBuilder circleColor(String color) {
paint.put("circle-color", validateColor(color));
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,9 @@ public Map<String, TileSource> sources() {
public List<JsonNode> layers() {
return layers;
}

@JsonSerialize
public String glyphs() {
return "https://cdn.jsdelivr.net/gh/klokantech/klokantech-gl-fonts@master/{fontstack}/{range}.pbf";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ public static Geometry convertGeoJsonToJtsGeometry(GeoJsonObject geoJsonGeom)
}

/**
* Extract individual line string from a mult-line string.
* Extract individual line strings from a multi-line string.
*/
public static List<LineString> getLineStrings(MultiLineString mls) {
var ret = new ArrayList<LineString>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,16 @@ public static double distance(Coordinate from, Coordinate to) {
return distance(from.y, from.x, to.y, to.x);
}

/**
* @see SphericalDistanceLibrary#fastDistance(double, double, double, double)
*/
public static double fastDistance(Coordinate from, Coordinate to) {
return fastDistance(from.y, from.x, to.y, to.x);
}

/**
* @see SphericalDistanceLibrary#fastDistance(double, double, double, double)
*/
public static double fastDistance(Coordinate from, Coordinate to, double cosLat) {
double dLat = toRadians(from.y - to.y);
double dLon = toRadians(from.x - to.x) * cosLat;
Expand Down Expand Up @@ -105,8 +111,8 @@ public static double distance(double lat1, double lon1, double lat2, double lon2
}

/**
* Compute an (approximated) distance between two points, with a known cos(lat). Be careful, this
* is approximated and never check for the validity of input cos(lat).
* Compute an (approximated) distance in meters between two points, with a known cos(lat).
* Be careful, this is approximated and never checks for the validity of input cos(lat).
*/
public static double fastDistance(double lat1, double lon1, double lat2, double lon2) {
return fastDistance(lat1, lon1, lat2, lon2, RADIUS_OF_EARTH_IN_M);
Expand All @@ -131,8 +137,8 @@ public static double distance(double lat1, double lon1, double lat2, double lon2
}

/**
* Approximated, fast and under-estimated equirectangular distance between two points. Works only
* for small delta lat/lon, fall-back on exact distance if not the case. See:
* Approximated, fast and under-estimated equirectangular distance in meters between two points.
* Works only for small delta lat/lon, fall-back on exact distance if not the case. See:
* http://www.movable-type.co.uk/scripts/latlong.html
*/
public static double fastDistance(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package org.opentripplanner.framework.json;

import com.bedatadriven.jackson.datatype.jts.JtsModule;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.opentripplanner.framework.geometry.GeometryUtils;

public class ObjectMappers {

Expand All @@ -13,4 +15,11 @@ public static ObjectMapper ignoringExtraFields() {
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
return mapper;
}

/**
* Returns a mapper that can serialize JTS geometries into GeoJSON.
*/
public static ObjectMapper geoJson() {
return new ObjectMapper().registerModule(new JtsModule(GeometryUtils.getGeometryFactory()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.opentripplanner.framework.collection.MapUtils;

/** Basic union-find data structure with path compression */
public class DisjointSet<T> {
class DisjointSet<T> {

TIntList sets = new TIntArrayList();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ public Map<Vertex, Double> elevationDataOutput() {
return elevationData;
}

private record StreetEdgePair(StreetEdge main, StreetEdge back) {}

private void build() {
var parkingProcessor = new ParkingProcessor(
graph,
Expand Down Expand Up @@ -390,8 +388,10 @@ private void buildBasicGraph() {
geometry
);

StreetEdge street = streets.main;
StreetEdge backStreet = streets.back;
params.edgeNamer().recordEdges(way, streets);

StreetEdge street = streets.main();
StreetEdge backStreet = streets.back();
normalizer.applyWayProperties(street, backStreet, wayData, way);

applyEdgesToTurnRestrictions(way, startNode, endNode, street, backStreet);
Expand Down Expand Up @@ -541,14 +541,10 @@ private StreetEdge getEdgeForStreet(
.withRoundabout(way.isRoundabout())
.withSlopeOverride(way.getOsmProvider().getWayPropertySet().getSlopeOverride(way))
.withStairs(way.isSteps())
.withWheelchairAccessible(way.isWheelchairAccessible());

if (!way.hasTag("name") && !way.hasTag("ref")) {
seb.withBogusName(true);
}
.withWheelchairAccessible(way.isWheelchairAccessible())
.withBogusName(way.hasNoName());

StreetEdge street = seb.buildAndConnect();
params.edgeNamer().recordEdge(way, street);

return street;
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import org.opentripplanner.framework.geometry.GeometryUtils;
import org.opentripplanner.openstreetmap.model.OSMNode;

public class Ring {
class Ring {

private final LinearRing shell;
private final List<Ring> holes = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package org.opentripplanner.graph_builder.module.osm;

import java.util.ArrayList;
import org.opentripplanner.street.model.edge.StreetEdge;

public record StreetEdgePair(StreetEdge main, StreetEdge back) {
/**
* Return the non-null elements of this pair as an Iterable.
*/
public Iterable<StreetEdge> asIterable() {
var ret = new ArrayList<StreetEdge>(2);
if (main != null) {
ret.add(main);
}
if (back != null) {
ret.add(back);
}
return ret;
}

/**
* Select one of the edges contained in this pair that is not null. No particular algorithm is
* guaranteed, and it may change in the future.
*/
public StreetEdge pickAny() {
if (main != null) {
return main;
} else if (back != null) {
return back;
}
throw new IllegalStateException(
"%s must not contain two null elements".formatted(getClass().getSimpleName())
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
* number of edges for an area wouldn't be determined by the nodes. The current approach can lead
* to an excessive number of edges, or to no edges at all if maxAreaNodes is surpassed.
*/
public class WalkableAreaBuilder {
class WalkableAreaBuilder {

private final DataImportIssueStore issueStore;

Expand Down Expand Up @@ -400,7 +400,7 @@ private void pruneAreaEdges(
Set<Edge> edges,
Set<Edge> edgesToKeep
) {
if (edges.size() == 0) return;
if (edges.isEmpty()) return;
StreetMode mode;
StreetEdge firstEdge = (StreetEdge) edges.iterator().next();

Expand Down Expand Up @@ -496,7 +496,7 @@ private Set<AreaEdge> createSegments(
}
// do we need to recurse?
if (intersects.size() == 1) {
Area area = intersects.get(0);
Area area = intersects.getFirst();
OSMWithTags areaEntity = area.parent;

StreetTraversalPermission areaPermissions = areaEntity.overridePermissions(
Expand Down Expand Up @@ -531,15 +531,10 @@ private Set<AreaEdge> createSegments(
.withPermission(areaPermissions)
.withBack(false)
.withArea(edgeList)
.withCarSpeed(carSpeed);

if (!areaEntity.hasTag("name") && !areaEntity.hasTag("ref")) {
streetEdgeBuilder.withBogusName(true);
}

streetEdgeBuilder.withWheelchairAccessible(areaEntity.isWheelchairAccessible());

streetEdgeBuilder.withLink(areaEntity.isLink());
.withCarSpeed(carSpeed)
.withBogusName(areaEntity.hasNoName())
.withWheelchairAccessible(areaEntity.isWheelchairAccessible())
.withLink(areaEntity.isLink());

label =
"way (area) " +
Expand All @@ -559,15 +554,10 @@ private Set<AreaEdge> createSegments(
.withPermission(areaPermissions)
.withBack(true)
.withArea(edgeList)
.withCarSpeed(carSpeed);

if (!areaEntity.hasTag("name") && !areaEntity.hasTag("ref")) {
backStreetEdgeBuilder.withBogusName(true);
}

backStreetEdgeBuilder.withWheelchairAccessible(areaEntity.isWheelchairAccessible());

backStreetEdgeBuilder.withLink(areaEntity.isLink());
.withCarSpeed(carSpeed)
.withBogusName(areaEntity.hasNoName())
.withWheelchairAccessible(areaEntity.isWheelchairAccessible())
.withLink(areaEntity.isLink());

if (!wayPropertiesCache.containsKey(areaEntity)) {
WayProperties wayData = areaEntity
Expand Down
Loading
Loading