-
-
Notifications
You must be signed in to change notification settings - Fork 185
[FEATURE] Add relation_members geometry type for processing OSM relation members #1427
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
b25ad5e
a6c4313
6b2ab52
e187082
dcefcb6
2fb0a10
bae3ff7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,9 @@ target/ | |
| .settings | ||
| bin/ | ||
|
|
||
| # cursor | ||
| .cursor/ | ||
|
|
||
| TODO | ||
|
|
||
| data/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # Ignore everything in this directory | ||
| * | ||
| # Except this file | ||
| !.gitignore | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,23 @@ public class OsmReader implements Closeable, MemoryEstimator.HasEstimate { | |
| private final Object waysInMultipolygonLock = new Object(); | ||
| // ~7GB | ||
| private LongLongMultimap.Replaceable multipolygonWayGeometries; | ||
| // for relation_members: track relations that need member processing | ||
| private Roaring64Bitmap relationsForMemberProcessing = new Roaring64Bitmap(); | ||
| private final Object relationsForMemberProcessingLock = new Object(); | ||
| // for relation_members: track ways that are members of relations we care about | ||
| private Roaring64Bitmap waysInRelationMembers = new Roaring64Bitmap(); | ||
| private final Object waysInRelationMembersLock = new Object(); | ||
| // for relation_members: store way geometries (node IDs) for member ways | ||
| private LongLongMultimap.Replaceable relationMembersWayGeometries; | ||
| // for relation_members: store way tags for member ways | ||
| private LongObjectHashMap<Map<String, Object>> relationMembersWayTags = Hppc.newLongObjectHashMap(); | ||
| private final Object relationMembersWayTagsLock = new Object(); | ||
| // for relation_members: track nodes that are members of relations we care about | ||
| private Roaring64Bitmap nodesInRelationMembers = new Roaring64Bitmap(); | ||
| private final Object nodesInRelationMembersLock = new Object(); | ||
| // for relation_members: store node tags for member nodes | ||
| private LongObjectHashMap<Map<String, Object>> relationMembersNodeTags = Hppc.newLongObjectHashMap(); | ||
| private final Object relationMembersNodeTagsLock = new Object(); | ||
| // keep track of data needed to encode/decode role strings into a long | ||
| private final ObjectIntHashMap<String> roleIds = new ObjectIntHashMap<>(); | ||
| private final IntObjectHashMap<String> roleIdsReverse = new IntObjectHashMap<>(); | ||
|
|
@@ -130,6 +147,8 @@ public OsmReader(String name, Supplier<OsmBlockSource> osmSourceProvider, LongLo | |
| "relations", pass1Phaser::relations | ||
| )); | ||
| this.multipolygonWayGeometries = multipolygonGeometries; | ||
| // Initialize relation members way geometries storage (similar to multipolygons) | ||
| this.relationMembersWayGeometries = LongLongMultimap.newInMemoryReplaceableMultimap(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -296,6 +315,32 @@ void processPass1Blocks(Iterable<? extends Iterable<? extends OsmElement>> block | |
| } | ||
| } | ||
| } | ||
| // Track relations that need member processing (relation_members geometry) | ||
| // Note: RelationMembersInfo is in a different module, so we check by class name | ||
| List<OsmRelationInfo> infos = relationInfo.get(relation.id()) != null ? | ||
| List.of(relationInfo.get(relation.id())) : null; | ||
| if (infos != null && !infos.isEmpty()) { | ||
| for (OsmRelationInfo info : infos) { | ||
| String className = info.getClass().getName(); | ||
| if (className.contains("RelationMembersInfo")) { | ||
| synchronized (relationsForMemberProcessingLock) { | ||
| relationsForMemberProcessing.add(relation.id()); | ||
| } | ||
| synchronized (waysInRelationMembersLock) { | ||
| synchronized (nodesInRelationMembersLock) { | ||
| for (var member : relation.members()) { | ||
| if (member.type() == OsmElement.Type.WAY) { | ||
| waysInRelationMembers.add(member.ref()); | ||
| } else if (member.type() == OsmElement.Type.NODE) { | ||
| nodesInRelationMembers.add(member.ref()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems very similar to the above processing in the Can we merge this processing into the block above ? It seems like the only extra functionality that we need would be to also track nodes that are in relations where preprocessOsmRelation returned something. Then we can add relationInfo to NodeSourceFeature as well so that pass 2 node processing also has access to what way a node is a member of. It seems like that should be sufficient for custommap to implement the same functionality?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see 2fb0a10. Is that what you had in mind? |
||
| } | ||
| } | ||
| PASS1_BLOCKS.inc(); | ||
|
|
@@ -346,9 +391,45 @@ public void pass2(FeatureGroup writer, PlanetilerConfig config) { | |
| try (var renderer = createFeatureRenderer(writer, config, next)) { | ||
| var phaser = pass2Phaser.forWorker(); | ||
| var relationHandler = relationDistributor.forThread(relation -> { | ||
| var feature = processRelationPass2(relation, nodeLocations); | ||
| if (feature != null) { | ||
| render(featureCollectors, renderer, relation, feature); | ||
| // Process as multipolygon if applicable (independent check) | ||
| if (isMultipolygon(relation)) { | ||
| List<RelationMember<OsmRelationInfo>> parentRelations = getRelationMembershipForWay(relation.id()); | ||
| SourceFeature multipolygonFeature = new MultipolygonSourceFeature(relation, nodeLocations, parentRelations); | ||
| render(featureCollectors, renderer, relation, multipolygonFeature); | ||
| } | ||
| // Process as relation_members if applicable (independent check - can be both) | ||
| if (relationsForMemberProcessing.contains(relation.id())) { | ||
| List<RelationMember<OsmRelationInfo>> parentRelations = getRelationMembershipForWay(relation.id()); | ||
| RelationMemberDataProvider dataProvider = new RelationMemberDataProvider() { | ||
| @Override | ||
| public LongArrayList getWayGeometry(long wayId) { | ||
| return relationMembersWayGeometries != null ? relationMembersWayGeometries.get(wayId) : null; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> getWayTags(long wayId) { | ||
| return relationMembersWayTags != null ? relationMembersWayTags.get(wayId) : null; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> getNodeTags(long nodeId) { | ||
| return relationMembersNodeTags != null ? relationMembersNodeTags.get(nodeId) : null; | ||
| } | ||
|
|
||
| @Override | ||
| public org.locationtech.jts.geom.Coordinate getNodeCoordinate(long nodeId) { | ||
| long encoded = nodeLocationDb.get(nodeId); | ||
| if (encoded == LongLongMap.MISSING_VALUE) { | ||
| return null; | ||
| } | ||
| return new org.locationtech.jts.geom.CoordinateXY( | ||
| GeoUtils.decodeWorldX(encoded), | ||
| GeoUtils.decodeWorldY(encoded) | ||
| ); | ||
| } | ||
| }; | ||
| SourceFeature relationMembersFeature = new RelationSourceFeature(relation, parentRelations, dataProvider); | ||
| render(featureCollectors, renderer, relation, relationMembersFeature); | ||
| } | ||
| rels.inc(); | ||
| }); | ||
|
|
@@ -500,6 +581,12 @@ private FeatureRenderer createFeatureRenderer(FeatureGroup writer, PlanetilerCon | |
|
|
||
| SourceFeature processNodePass2(OsmElement.Node node) { | ||
| // nodes are simple because they already contain their location | ||
| // Store node tags if this node is a member of a relation we care about | ||
| if (nodesInRelationMembers.contains(node.id())) { | ||
| synchronized (relationMembersNodeTagsLock) { | ||
| relationMembersNodeTags.put(node.id(), node.tags()); | ||
| } | ||
| } | ||
| return new NodeSourceFeature(node); | ||
| } | ||
|
|
||
|
|
@@ -514,6 +601,15 @@ SourceFeature processWayPass2(OsmElement.Way way, NodeLocationProvider nodeLocat | |
| multipolygonWayGeometries.replaceValues(way.id(), nodes); | ||
| } | ||
| } | ||
| // Store way geometry and tags if this way is a member of a relation we care about | ||
| if (waysInRelationMembers.contains(way.id())) { | ||
| synchronized (this) { | ||
| relationMembersWayGeometries.replaceValues(way.id(), nodes); | ||
| } | ||
| synchronized (relationMembersWayTagsLock) { | ||
| relationMembersWayTags.put(way.id(), way.tags()); | ||
| } | ||
| } | ||
| boolean closed = nodes.size() > 1 && nodes.get(0) == nodes.get(nodes.size() - 1); | ||
| // area tag used to differentiate between whether a closed way should be treated as a polygon or linestring | ||
| String area = way.getString("area"); | ||
|
|
@@ -527,9 +623,41 @@ SourceFeature processRelationPass2(OsmElement.Relation rel, NodeLocationProvider | |
| if (isMultipolygon(rel)) { | ||
| List<RelationMember<OsmRelationInfo>> parentRelations = getRelationMembershipForWay(rel.id()); | ||
| return new MultipolygonSourceFeature(rel, nodeLocations, parentRelations); | ||
| } else { | ||
| return null; | ||
| } | ||
| if (relationsForMemberProcessing.contains(rel.id())) { | ||
| // This relation needs member processing (relation_members geometry) | ||
| List<RelationMember<OsmRelationInfo>> parentRelations = getRelationMembershipForWay(rel.id()); | ||
| RelationMemberDataProvider dataProvider = new RelationMemberDataProvider() { | ||
| @Override | ||
| public LongArrayList getWayGeometry(long wayId) { | ||
| return relationMembersWayGeometries != null ? relationMembersWayGeometries.get(wayId) : null; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> getWayTags(long wayId) { | ||
| return relationMembersWayTags != null ? relationMembersWayTags.get(wayId) : null; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> getNodeTags(long nodeId) { | ||
| return relationMembersNodeTags != null ? relationMembersNodeTags.get(nodeId) : null; | ||
| } | ||
|
|
||
| @Override | ||
| public org.locationtech.jts.geom.Coordinate getNodeCoordinate(long nodeId) { | ||
| long encoded = nodeLocationDb.get(nodeId); | ||
| if (encoded == LongLongMap.MISSING_VALUE) { | ||
| return null; | ||
| } | ||
| return new org.locationtech.jts.geom.CoordinateXY( | ||
| GeoUtils.decodeWorldX(encoded), | ||
| GeoUtils.decodeWorldY(encoded) | ||
| ); | ||
| } | ||
| }; | ||
| return new RelationSourceFeature(rel, parentRelations, dataProvider); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private List<RelationMember<OsmRelationInfo>> getRelationMembershipForWay(long wayId) { | ||
|
|
@@ -583,6 +711,10 @@ public long estimateMemoryUsageBytes() { | |
| long size = 0; | ||
| size += waysInMultipolygon == null ? 0 : waysInMultipolygon.serializedSizeInBytes(); | ||
| // multipolygonWayGeometries is reported separately | ||
| size += waysInRelationMembers == null ? 0 : waysInRelationMembers.serializedSizeInBytes(); | ||
| size += nodesInRelationMembers == null ? 0 : nodesInRelationMembers.serializedSizeInBytes(); | ||
| size += estimateSize(relationMembersWayTags); | ||
| size += estimateSize(relationMembersNodeTags); | ||
| size += estimateSize(wayToRelations); | ||
| size += estimateSize(relationToParentRelations); | ||
| size += estimateSize(relationInfo); | ||
|
|
@@ -599,9 +731,17 @@ public void close() throws IOException { | |
| multipolygonWayGeometries.close(); | ||
| multipolygonWayGeometries = null; | ||
| } | ||
| if (relationMembersWayGeometries != null) { | ||
| relationMembersWayGeometries.close(); | ||
| relationMembersWayGeometries = null; | ||
| } | ||
| wayToRelations = null; | ||
| relationToParentRelations = null; | ||
| waysInMultipolygon = null; | ||
| waysInRelationMembers = null; | ||
| nodesInRelationMembers = null; | ||
| relationMembersWayTags = null; | ||
| relationMembersNodeTags = null; | ||
| relationInfo = null; | ||
| nodeLocationDb.close(); | ||
| roleIds.release(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package com.onthegomap.planetiler.reader.osm; | ||
|
|
||
| import com.carrotsearch.hppc.LongArrayList; | ||
| import java.util.Map; | ||
|
|
||
| /** | ||
| * Provides access to stored member data for relation_members processing. | ||
| * Used to retrieve way geometries, way tags, and node tags for relation members. | ||
| */ | ||
| public interface RelationMemberDataProvider { | ||
|
|
||
| /** | ||
| * Gets the node IDs for a way member. | ||
| * @param wayId the way ID | ||
| * @return the node IDs, or null if not found | ||
| */ | ||
| LongArrayList getWayGeometry(long wayId); | ||
|
|
||
| /** | ||
| * Gets the tags for a way member. | ||
| * @param wayId the way ID | ||
| * @return the tags, or null if not found | ||
| */ | ||
| Map<String, Object> getWayTags(long wayId); | ||
|
|
||
| /** | ||
| * Gets the tags for a node member. | ||
| * @param nodeId the node ID | ||
| * @return the tags, or null if not found | ||
| */ | ||
| Map<String, Object> getNodeTags(long nodeId); | ||
|
|
||
| /** | ||
| * Gets the coordinate for a node. | ||
| * @param nodeId the node ID | ||
| * @return the coordinate, or null if not found | ||
| */ | ||
| org.locationtech.jts.geom.Coordinate getNodeCoordinate(long nodeId); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The root .gitignore already excludes data directory, so I don't think we need this
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of the
.gitignorefile with the specific contents is to ensure the existence oftmpdir=data/tmp(temp directory).If the
data/tmpdirectory does not exist, and planetiler is run with docker, the directory will be created from within the container and most likely it will be owned byroot.I should have made it clear in the comment...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit dcefcb6 replaces this with a modification of
scripts/test-release.sh.