Skip to content

Commit

Permalink
Method-based tile feature ids (#924)
Browse files Browse the repository at this point in the history
By default, `sourceFeature.featureIdFromElement()` is using `sourceFeature.id()`, but it allows overiding it in sub-classes, such as `OsmElement`.

For OSM elements, the feature id encodes the origin of a vector tile feature - an OSM node, way, or relation.
It uses the formula
`{vector tile feature id} = {osm id} * 10 + {1 for node, 2 for way, 3 for relation}`.
See #824

- A default implementation for  `featureIdFromElement()` added to `sourceFeature`
- A `OsmElement.type()` implemented in its sub-classes to provide the OSM element type.
- `OsmElement.featureIdFromElement()` uses `type()` to implemented the above formula.
- `source.featureIdFromElement()` is used instead of `source.id()` when converting a source element to a tile feature in `FeatureCollector`, `OpenMapTilesMapping`, and `OsmFeature`.
- Additional constructors to avoid the use of `withId()` in tests
  • Loading branch information
zstadler authored Jun 28, 2024
1 parent 7c86699 commit f8a8f8d
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static void main(String[] args) {
if (inputs.size() % 1_000_000 == 0) {
logger.log();
}
inputs.add(new SourceFeature(element.tags(), "", "", null, element.id()) {
inputs.add(new SourceFeature(element.tags(), "", "", null, element.featureIdFromElement()) {
@Override
public Geometry latLonGeometry() {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public Iterator<Feature> iterator() {
* @return a feature that can be configured further.
*/
public Feature geometry(String layer, Geometry geometry) {
Feature feature = new Feature(layer, geometry, source);
Feature feature = new Feature(layer, geometry, source.featureIdFromElement());
output.add(feature);
return feature;
}
Expand All @@ -82,8 +82,8 @@ public Feature point(String layer) {
}
return geometry(layer, source.worldGeometry());
} catch (GeometryException e) {
e.log(stats, "feature_point", "Error getting point geometry for " + source.id());
return new Feature(layer, EMPTY_GEOM, source);
e.log(stats, "feature_point", "Error getting point geometry for " + source.featureIdFromElement());
return new Feature(layer, EMPTY_GEOM, source.featureIdFromElement());
}
}

Expand All @@ -102,8 +102,8 @@ public Feature line(String layer) {
try {
return geometry(layer, source.line());
} catch (GeometryException e) {
e.log(stats, "feature_line", "Error constructing line for " + source.id());
return new Feature(layer, EMPTY_GEOM, source);
e.log(stats, "feature_line", "Error constructing line for " + source.featureIdFromElement());
return new Feature(layer, EMPTY_GEOM, source.featureIdFromElement());
}
}

Expand All @@ -122,8 +122,8 @@ public Feature polygon(String layer) {
try {
return geometry(layer, source.polygon());
} catch (GeometryException e) {
e.log(stats, "feature_polygon", "Error constructing polygon for " + source.id());
return new Feature(layer, EMPTY_GEOM, source);
e.log(stats, "feature_polygon", "Error constructing polygon for " + source.featureIdFromElement());
return new Feature(layer, EMPTY_GEOM, source.featureIdFromElement());
}
}

Expand All @@ -137,8 +137,8 @@ public Feature centroid(String layer) {
try {
return geometry(layer, source.centroid());
} catch (GeometryException e) {
e.log(stats, "feature_centroid", "Error getting centroid for " + source.id());
return new Feature(layer, EMPTY_GEOM, source);
e.log(stats, "feature_centroid", "Error getting centroid for " + source.featureIdFromElement());
return new Feature(layer, EMPTY_GEOM, source.featureIdFromElement());
}
}

Expand All @@ -154,8 +154,8 @@ public Feature centroidIfConvex(String layer) {
try {
return geometry(layer, source.centroidIfConvex());
} catch (GeometryException e) {
e.log(stats, "feature_centroid_if_convex", "Error constructing centroid if convex for " + source.id());
return new Feature(layer, EMPTY_GEOM, source);
e.log(stats, "feature_centroid_if_convex", "Error constructing centroid if convex for " + source.featureIdFromElement());
return new Feature(layer, EMPTY_GEOM, source.featureIdFromElement());
}
}

Expand All @@ -170,8 +170,8 @@ public Feature pointOnSurface(String layer) {
try {
return geometry(layer, source.pointOnSurface());
} catch (GeometryException e) {
e.log(stats, "feature_point_on_surface", "Error constructing point on surface for " + source.id());
return new Feature(layer, EMPTY_GEOM, source);
e.log(stats, "feature_point_on_surface", "Error constructing point on surface for " + source.featureIdFromElement());
return new Feature(layer, EMPTY_GEOM, source.featureIdFromElement());
}
}

Expand All @@ -192,8 +192,8 @@ public Feature innermostPoint(String layer, double tolerance) {
try {
return geometry(layer, source.innermostPoint(tolerance));
} catch (GeometryException e) {
e.log(stats, "feature_innermost_point", "Error constructing innermost point for " + source.id());
return new Feature(layer, EMPTY_GEOM, source);
e.log(stats, "feature_innermost_point", "Error constructing innermost point for " + source.featureIdFromElement());
return new Feature(layer, EMPTY_GEOM, source.featureIdFromElement());
}
}

Expand Down Expand Up @@ -277,21 +277,11 @@ public final class Feature {

private String numPointsAttr = null;

private Feature(String layer, Geometry geom, SourceFeature source) {
private Feature(String layer, Geometry geom, long id) {
this.layer = layer;
this.geom = geom;
this.geometryType = GeometryType.typeOf(geom);
if (source instanceof OsmSourceFeature osmSourceFeature) {
long osmId = osmSourceFeature.originalElement().id();
this.id = switch (osmSourceFeature.originalElement()) {
case OsmElement.Node node -> node.id() * 10 + 1;
case OsmElement.Way way -> way.id() * 10 + 2;
case OsmElement.Relation relation -> relation.id() * 10 + 3;
default -> osmId * 10;
};
} else {
this.id = source.id();
}
this.id = id;
if (geometryType == GeometryType.POINT) {
minPixelSizeAtMaxZoom = 0;
defaultMinPixelSize = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ public int cost() {
return 1;
}

@Override
public Type type() {
return isPoint() ? Type.NODE : Type.WAY;
}

@Override
public Map<String, Object> tags() {
return tags();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,10 @@ public final long id() {
return id;
}

/** By default, the feature id is taken as-is from the input data source id. */
public long featureIdFromElement() {
return id;
}

/** Returns true if this element has any OSM relation info. */
public boolean hasRelationInfo() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,24 @@ public interface OsmElement extends WithTags {

int cost();

Type type();

enum Type {
NODE,
WAY,
RELATION
/** The specific numeric values, as returned by ordinal(),
* are used for encoding the OSM element type. */
OTHER, // 0
NODE, // 1
WAY, // 2
RELATION // 3
}

/** Encode OSM element id and type in a feature id.
* The OSM element type is encoded in the least significant digit and
* the OSM element id is stored in the higher significance digits.
* */
default long featureIdFromElement() {
int encodedType = type().ordinal();
return (id() * 10) + encodedType;
}

/** An un-handled element read from the .osm.pbf file (i.e. file header). */
Expand All @@ -41,6 +55,11 @@ record Other(
public int cost() {
return 1 + tags.size() + (info == null ? 0 : Info.COST);
}

@Override
public Type type() {
return Type.OTHER;
}
}

/** A point on the earth's surface. */
Expand Down Expand Up @@ -117,6 +136,11 @@ public int cost() {
return 1 + tags.size() + (info == null ? 0 : Info.COST);
}

@Override
public Type type() {
return Type.NODE;
}

@Override
public boolean equals(Object obj) {
if (obj == this) {
Expand Down Expand Up @@ -170,6 +194,11 @@ public Way(long id, Map<String, Object> tags, LongArrayList nodes) {
public int cost() {
return 1 + tags.size() + nodes.size() + (info == null ? 0 : Info.COST);
}

@Override
public Type type() {
return Type.WAY;
}
}

/** An ordered list of nodes, ways, and other relations. */
Expand Down Expand Up @@ -199,6 +228,11 @@ public int cost() {
return 1 + tags.size() + members.size() * 3 + (info == null ? 0 : Info.COST);
}

@Override
public Type type() {
return Type.RELATION;
}

/**
* A node, way, or relation contained in a relation with an optional "role" to clarify the purpose of each member.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ private abstract class OsmFeature extends SourceFeature implements OsmSourceFeat

public OsmFeature(OsmElement elem, boolean point, boolean line, boolean polygon,
List<RelationMember<OsmRelationInfo>> relationInfo) {
super(elem.tags(), name, null, relationInfo, elem.id());
super(elem.tags(), name, null, relationInfo, elem.featureIdFromElement());
this.originalElement = elem;
this.point = point;
this.line = line;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,7 @@ void testOsmPoint() throws Exception {
feature(newPoint(128, 128), Map.of(
"attr", "value",
"name", "name value"
)).withId(11)
), 11)
)
), results.tiles);
}
Expand Down Expand Up @@ -964,7 +964,7 @@ void testOsmLine() throws Exception {
feature(newLineString(128, 128, 192, 192), Map.of(
"attr", "value",
"name", "name value"
)).withId(32)
), 32)
)
), results.tiles);
}
Expand Down Expand Up @@ -1089,7 +1089,7 @@ record TestRelationInfo(long id, String name) implements OsmRelationInfo {}
"attr", "value",
"name", "name value",
"relname", "rel name"
)).withId(173)
), 173)
)
), results.tiles);
}
Expand Down Expand Up @@ -1151,20 +1151,21 @@ record TestRelationInfo(long id, String name) implements OsmRelationInfo {}

@Test
void testPreprocessOsmNodesAndWays() throws Exception {
Set<Long> nodes1 = new HashSet<>();
HashMap<Long, Long> nodes1 = new HashMap<>();
Set<Long> nodes2 = new HashSet<>();
var profile = new Profile.NullProfile() {
@Override
public void preprocessOsmNode(OsmElement.Node node) {
if (node.hasTag("a", "b")) {
nodes1.add(node.id());
nodes1.put(node.id(), node.featureIdFromElement());
}
}

@Override
public void preprocessOsmWay(OsmElement.Way way) {
if (nodes1.contains(way.nodes().get(0))) {
nodes2.add(way.nodes().get(0));
Long featureId = nodes1.get(way.nodes().get(0));
if (featureId != null) {
nodes2.add(featureId);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ public static Map<TileCoord, List<ComparableFeature>> getTileMap(ReadableTileArc
};
var decoded = VectorTile.decode(bytes).stream()
.map(
feature -> feature(decodeSilently(feature.geometry()), feature.layer(), feature.attrs()).withId(feature.id()))
feature -> feature(decodeSilently(feature.geometry()), feature.layer(), feature.attrs(), feature.id()))
.toList();
tiles.put(tile.coord(), decoded);
}
Expand Down Expand Up @@ -501,10 +501,19 @@ ComparableFeature withId(long id) {
}
}


public static ComparableFeature feature(Geometry geom, String layer, Map<String, Object> attrs, long id) {
return new ComparableFeature(new NormGeometry(geom), layer, attrs, id);
}

public static ComparableFeature feature(Geometry geom, String layer, Map<String, Object> attrs) {
return new ComparableFeature(new NormGeometry(geom), layer, attrs);
}

public static ComparableFeature feature(Geometry geom, Map<String, Object> attrs, long id) {
return new ComparableFeature(new NormGeometry(geom), null, attrs, id);
}

public static ComparableFeature feature(Geometry geom, Map<String, Object> attrs) {
return new ComparableFeature(new NormGeometry(geom), null, attrs);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void integrationTest(@TempDir Path tmpDir) throws Exception {
"public_transport", "stop_position",
"@type", "node",
"@version", 4L,
"@id", 1699777833L
"@id", 16997778331L
), GeoUtils.WORLD_LAT_LON_BOUNDS, 1, Point.class);
TestUtils
.assertNumFeatures(mbtiles, "osm", 12, Map.of(
Expand All @@ -120,7 +120,7 @@ void integrationTest(@TempDir Path tmpDir) throws Exception {
"wikidata", "Q45240",
"@type", "relation",
"@version", 9L,
"@id", 5986438L
"@id", 59864383L
), GeoUtils.WORLD_LAT_LON_BOUNDS, 1, Polygon.class);
TestUtils
.assertNumFeatures(mbtiles, "osm", 12, Map.of(
Expand All @@ -131,7 +131,7 @@ void integrationTest(@TempDir Path tmpDir) throws Exception {
"lanes", "2",
"@type", "way",
"@version", 5L,
"@id", 166009791L
"@id", 1660097912L
), GeoUtils.WORLD_LAT_LON_BOUNDS, 1, LineString.class);
}
}
Expand Down

0 comments on commit f8a8f8d

Please sign in to comment.