From 65c8380ecb0c5c2b4276a32c27cdb350668b9b10 Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Mon, 1 Jan 2024 22:36:26 -0500 Subject: [PATCH 01/13] extend NextRelation/FindInRelation to nodes This also makes `NextRelation` return a tuple of ID, role rather than just ID. According to https://www.lua.org/pil/5.1.html, I think that's not a breaking change. Motivation: When I naively try to create labels from OSM entities with the `place` tag, I occasionally get duplicates due to relations. For example, the city of Guelph has [relation 7486148](https://www.openstreetmap.org/relation/7486148), which has [node 36576733](https://www.openstreetmap.org/node/36576733) as its `label` member. They both have `place=city`, so my Lua script faithfully spits out two labels. This PR allows me to suppress the label from the node, which is a start. Ideally, I'd actually prefer to use the node, as it'll likely have a nicely human-curated location. For that, I'd need the relation to be able to interrogate its members. Would you be open to me extending this PR to add `NextMember`, `FindInMember` and `RestartMembers` functions that mirror the ones used by nodes/ways ? --- include/osm_lua_processing.h | 11 +++++++-- include/osm_store.h | 26 +++++++++++++++++----- src/osm_lua_processing.cpp | 43 +++++++++++++++++++++++++++++++----- src/pbf_processor.cpp | 35 ++++++++++++++++++++++++----- 4 files changed, 96 insertions(+), 19 deletions(-) diff --git a/include/osm_lua_processing.h b/include/osm_lua_processing.h index 54a939a4..a6c52862 100644 --- a/include/osm_lua_processing.h +++ b/include/osm_lua_processing.h @@ -171,7 +171,13 @@ class OsmLuaProcessing { void ZOrder(const double z); // Relation scan support - kaguya::optional NextRelation(); + + struct OptionalRelation { + bool done; + int id; + std::string role; + }; + OptionalRelation NextRelation(); void RestartRelations(); std::string FindInRelation(const std::string &key); void Accept(); @@ -213,6 +219,7 @@ class OsmLuaProcessing { polygonInited = false; multiPolygonInited = false; relationAccepted = false; + relationList.clear(); relationSubscript = -1; lastStoredGeometryId = 0; } @@ -235,7 +242,7 @@ class OsmLuaProcessing { bool isWay, isRelation, isClosed; ///< Way, node, relation? bool relationAccepted; // in scanRelation, whether we're using a non-MP relation - std::vector relationList; // in processWay, list of relations this way is in + std::vector> relationList; // in processNode/processWay, list of relations this entity is in, and its role int relationSubscript = -1; // in processWay, position in the relation list int32_t lon,latp; ///< Node coordinates diff --git a/include/osm_store.h b/include/osm_store.h index b8607543..edf9371d 100644 --- a/include/osm_store.h +++ b/include/osm_store.h @@ -81,14 +81,19 @@ class RelationScanStore { private: using tag_map_t = boost::container::flat_map; - std::map> relationsForWays; + std::map>> relationsForWays; + std::map>> relationsForNodes; std::map relationTags; mutable std::mutex mutex; public: - void relation_contains_way(WayID relid, WayID wayid) { + void relation_contains_way(WayID relid, WayID wayid, std::string role) { std::lock_guard lock(mutex); - relationsForWays[wayid].emplace_back(relid); + relationsForWays[wayid].emplace_back(std::make_pair(relid, role)); + } + void relation_contains_node(WayID relid, NodeID nodeId, std::string role) { + std::lock_guard lock(mutex); + relationsForNodes[nodeId].emplace_back(std::make_pair(relid, role)); } void store_relation_tags(WayID relid, const tag_map_t &tags) { std::lock_guard lock(mutex); @@ -97,9 +102,15 @@ class RelationScanStore { bool way_in_any_relations(WayID wayid) { return relationsForWays.find(wayid) != relationsForWays.end(); } - std::vector relations_for_way(WayID wayid) { + bool node_in_any_relations(NodeID nodeId) { + return relationsForNodes.find(nodeId) != relationsForNodes.end(); + } + std::vector> relations_for_way(WayID wayid) { return relationsForWays[wayid]; } + std::vector> relations_for_node(NodeID nodeId) { + return relationsForNodes[nodeId]; + } std::string get_relation_tag(WayID relid, const std::string &key) { auto it = relationTags.find(relid); if (it==relationTags.end()) return ""; @@ -211,10 +222,13 @@ class OSMStore void ensureUsedWaysInited(); using tag_map_t = boost::container::flat_map; - void relation_contains_way(WayID relid, WayID wayid) { scanned_relations.relation_contains_way(relid,wayid); } + void relation_contains_way(WayID relid, WayID wayid, std::string role) { scanned_relations.relation_contains_way(relid, wayid, role); } + void relation_contains_node(WayID relid, NodeID nodeId, std::string role) { scanned_relations.relation_contains_node(relid, nodeId, role); } void store_relation_tags(WayID relid, const tag_map_t &tags) { scanned_relations.store_relation_tags(relid,tags); } bool way_in_any_relations(WayID wayid) { return scanned_relations.way_in_any_relations(wayid); } - std::vector relations_for_way(WayID wayid) { return scanned_relations.relations_for_way(wayid); } + bool node_in_any_relations(NodeID nodeId) { return scanned_relations.node_in_any_relations(nodeId); } + std::vector> relations_for_way(WayID wayid) { return scanned_relations.relations_for_way(wayid); } + std::vector> relations_for_node(NodeID nodeId) { return scanned_relations.relations_for_node(nodeId); } std::string get_relation_tag(WayID relid, const std::string &key) { return scanned_relations.get_relation_tag(relid, key); } void clear(); diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index f94d6734..99130690 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -550,10 +550,39 @@ void OsmLuaProcessing::ZOrder(const double z) { } // Read scanned relations -kaguya::optional OsmLuaProcessing::NextRelation() { +// Kaguya doesn't support optional>, so we write a custom serializer +// to either return nil or a tuple. +template<> struct kaguya::lua_type_traits { + typedef OsmLuaProcessing::OptionalRelation get_type; + typedef const OsmLuaProcessing::OptionalRelation& push_type; + + static bool strictCheckType(lua_State* l, int index) + { + throw std::runtime_error("Lua code doesn't know how to send OptionalRelation"); + } + static bool checkType(lua_State* l, int index) + { + throw std::runtime_error("Lua code doesn't know how to send OptionalRelation"); + } + static get_type get(lua_State* l, int index) + { + throw std::runtime_error("Lua code doesn't know how to send OptionalRelation"); + } + static int push(lua_State* l, push_type s) + { + if (s.done) + return 0; + + lua_pushinteger(l, s.id); + lua_pushlstring(l, s.role.data(), s.role.size()); + return 2; + } +}; + +OsmLuaProcessing::OptionalRelation OsmLuaProcessing::NextRelation() { relationSubscript++; - if (relationSubscript >= relationList.size()) return kaguya::nullopt_t(); - return relationList[relationSubscript]; + if (relationSubscript >= relationList.size()) return { true }; + return { false, relationList[relationSubscript].first, relationList[relationSubscript].second }; } void OsmLuaProcessing::RestartRelations() { @@ -561,7 +590,7 @@ void OsmLuaProcessing::RestartRelations() { } std::string OsmLuaProcessing::FindInRelation(const std::string &key) { - return osmStore.get_relation_tag(relationList[relationSubscript], key); + return osmStore.get_relation_tag(relationList[relationSubscript].first, key); } // Record attribute name/type for vector_layers table @@ -603,6 +632,10 @@ void OsmLuaProcessing::setNode(NodeID id, LatpLon node, const tag_map_t &tags) { latp= node.latp; currentTags = &tags; + if (supportsReadingRelations && osmStore.node_in_any_relations(id)) { + relationList = osmStore.relations_for_node(id); + } + //Start Lua processing for node try { luaState["node_function"](this); @@ -634,8 +667,6 @@ bool OsmLuaProcessing::setWay(WayID wayId, LatpLonVec const &llVec, const tag_ma if (supportsReadingRelations && osmStore.way_in_any_relations(wayId)) { relationList = osmStore.relations_for_way(wayId); - } else { - relationList.clear(); } try { diff --git a/src/pbf_processor.cpp b/src/pbf_processor.cpp index 0cf0f1d9..0e446be0 100644 --- a/src/pbf_processor.cpp +++ b/src/pbf_processor.cpp @@ -167,6 +167,11 @@ bool PbfProcessor::ScanRelations(OsmLuaProcessing& output, PbfReader::PrimitiveG int typeKey = findStringPosition(pb, "type"); int mpKey = findStringPosition(pb, "multipolygon"); + int innerKey= findStringPosition(pb, "inner"); + int outerKey= findStringPosition(pb, "outer"); + int labelKey = findStringPosition(pb, "label"); + int adminCentreKey = findStringPosition(pb, "admin_centre"); + for (PbfReader::Relation pbfRelation : pg.relations()) { bool isMultiPolygon = relationIsType(pbfRelation, typeKey, mpKey); bool isAccepted = false; @@ -181,10 +186,30 @@ bool PbfProcessor::ScanRelations(OsmLuaProcessing& output, PbfReader::PrimitiveG } for (int n=0; n < pbfRelation.memids.size(); n++) { uint64_t lastID = pbfRelation.memids[n]; - if (pbfRelation.types[n] != PbfReader::Relation::MemberType::WAY) { continue; } - if (lastID >= pow(2,42)) throw std::runtime_error("Way ID in relation "+std::to_string(relid)+" negative or too large: "+std::to_string(lastID)); - osmStore.mark_way_used(static_cast(lastID)); - if (isAccepted) { osmStore.relation_contains_way(relid, lastID); } + + if (pbfRelation.types[n] == PbfReader::Relation::MemberType::NODE) { + if (isAccepted) { + std::string role; + if (pbfRelation.roles_sid[n] == labelKey) + role = "label"; + else if (pbfRelation.roles_sid[n] == adminCentreKey) + role = "admin_centre"; + + osmStore.relation_contains_node(relid, lastID, role); + } + } else if (pbfRelation.types[n] == PbfReader::Relation::MemberType::WAY) { + if (lastID >= pow(2,42)) throw std::runtime_error("Way ID in relation "+std::to_string(relid)+" negative or too large: "+std::to_string(lastID)); + osmStore.mark_way_used(static_cast(lastID)); + if (isAccepted) { + std::string role; + if (pbfRelation.roles_sid[n] == outerKey) + role = "outer"; + else if (pbfRelation.roles_sid[n] == innerKey) + role = "inner"; + + osmStore.relation_contains_way(relid, lastID, role); + } + } } } return true; @@ -495,7 +520,7 @@ int PbfProcessor::ReadPbfFile( } - std::vector all_phases = { ReadPhase::Nodes, ReadPhase::RelationScan, ReadPhase::Ways, ReadPhase::Relations }; + std::vector all_phases = { ReadPhase::RelationScan, ReadPhase::Nodes, ReadPhase::Ways, ReadPhase::Relations }; for(auto phase: all_phases) { uint effectiveShards = 1; From 3ce355a90d22466533b8a9c3ddbe23a012e366a1 Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Tue, 2 Jan 2024 00:41:59 -0500 Subject: [PATCH 02/13] don't truncate int --- include/osm_lua_processing.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/osm_lua_processing.h b/include/osm_lua_processing.h index a6c52862..15316cb6 100644 --- a/include/osm_lua_processing.h +++ b/include/osm_lua_processing.h @@ -174,7 +174,7 @@ class OsmLuaProcessing { struct OptionalRelation { bool done; - int id; + lua_Integer id; std::string role; }; OptionalRelation NextRelation(); From ac30cbd8c41b46d993791e00308f41aaa0e8179a Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Tue, 2 Jan 2024 00:46:42 -0500 Subject: [PATCH 03/13] add static_cast --- src/osm_lua_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index 99130690..ac8f04bb 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -582,7 +582,7 @@ template<> struct kaguya::lua_type_traits { OsmLuaProcessing::OptionalRelation OsmLuaProcessing::NextRelation() { relationSubscript++; if (relationSubscript >= relationList.size()) return { true }; - return { false, relationList[relationSubscript].first, relationList[relationSubscript].second }; + return { false, static_cast(relationList[relationSubscript].first), relationList[relationSubscript].second }; } void OsmLuaProcessing::RestartRelations() { From a401dadc4cab83fd78dca68aae69957aa6d42aad Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Tue, 2 Jan 2024 22:02:28 -0500 Subject: [PATCH 04/13] don't hardcode role names I dunno what I was thinking, this is much better. (And I see now that there are many more roles than these four.) --- src/pbf_processor.cpp | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/pbf_processor.cpp b/src/pbf_processor.cpp index 0e446be0..63a0c36e 100644 --- a/src/pbf_processor.cpp +++ b/src/pbf_processor.cpp @@ -167,11 +167,6 @@ bool PbfProcessor::ScanRelations(OsmLuaProcessing& output, PbfReader::PrimitiveG int typeKey = findStringPosition(pb, "type"); int mpKey = findStringPosition(pb, "multipolygon"); - int innerKey= findStringPosition(pb, "inner"); - int outerKey= findStringPosition(pb, "outer"); - int labelKey = findStringPosition(pb, "label"); - int adminCentreKey = findStringPosition(pb, "admin_centre"); - for (PbfReader::Relation pbfRelation : pg.relations()) { bool isMultiPolygon = relationIsType(pbfRelation, typeKey, mpKey); bool isAccepted = false; @@ -189,24 +184,16 @@ bool PbfProcessor::ScanRelations(OsmLuaProcessing& output, PbfReader::PrimitiveG if (pbfRelation.types[n] == PbfReader::Relation::MemberType::NODE) { if (isAccepted) { - std::string role; - if (pbfRelation.roles_sid[n] == labelKey) - role = "label"; - else if (pbfRelation.roles_sid[n] == adminCentreKey) - role = "admin_centre"; - + const auto& roleView = pb.stringTable[pbfRelation.roles_sid[n]]; + std::string role(roleView.data(), roleView.size()); osmStore.relation_contains_node(relid, lastID, role); } } else if (pbfRelation.types[n] == PbfReader::Relation::MemberType::WAY) { if (lastID >= pow(2,42)) throw std::runtime_error("Way ID in relation "+std::to_string(relid)+" negative or too large: "+std::to_string(lastID)); osmStore.mark_way_used(static_cast(lastID)); if (isAccepted) { - std::string role; - if (pbfRelation.roles_sid[n] == outerKey) - role = "outer"; - else if (pbfRelation.roles_sid[n] == innerKey) - role = "inner"; - + const auto& roleView = pb.stringTable[pbfRelation.roles_sid[n]]; + std::string role(roleView.data(), roleView.size()); osmStore.relation_contains_way(relid, lastID, role); } } From 94ea972be887ace1667fc3a71a8880815a99d2d7 Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Tue, 2 Jan 2024 23:11:42 -0500 Subject: [PATCH 05/13] extend LayerAsCentroid to take relation roles --- include/osm_lua_processing.h | 17 +++++++-- src/osm_lua_processing.cpp | 70 +++++++++++++++++++++++++++++++----- src/pbf_processor.cpp | 2 +- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/include/osm_lua_processing.h b/include/osm_lua_processing.h index 15316cb6..05e7d2c0 100644 --- a/include/osm_lua_processing.h +++ b/include/osm_lua_processing.h @@ -13,6 +13,7 @@ #include "shp_mem_tiles.h" #include "osm_mem_tiles.h" #include "helpers.h" +#include "pbf_reader.h" #include #include @@ -87,7 +88,15 @@ class OsmLuaProcessing { * (note that we store relations as ways with artificial IDs, and that * we use decrementing positive IDs to give a bit more space for way IDs) */ - void setRelation(int64_t relationId, WayVec const &outerWayVec, WayVec const &innerWayVec, const tag_map_t &tags, bool isNativeMP, bool isInnerOuter); + void setRelation( + const std::vector& stringTable, + const PbfReader::Relation& relation, + const WayVec& outerWayVec, + const WayVec& innerWayVec, + const tag_map_t& tags, + bool isNativeMP, + bool isInnerOuter + ); // ---- Metadata queries called from Lua @@ -158,7 +167,7 @@ class OsmLuaProcessing { // Add layer void Layer(const std::string &layerName, bool area); - void LayerAsCentroid(const std::string &layerName); + void LayerAsCentroid(const std::string &layerName, kaguya::VariadicArgType nodeSources); // Set attributes in a vector tile's Attributes table void Attribute(const std::string &key, const std::string &val); @@ -211,6 +220,8 @@ class OsmLuaProcessing { /// Internal: clear current cached state inline void reset() { outputs.clear(); + currentRelation = nullptr; + stringTable = nullptr; llVecPtr = nullptr; outerWayVecPtr = nullptr; innerWayVecPtr = nullptr; @@ -267,6 +278,8 @@ class OsmLuaProcessing { std::vector> outputs; // All output objects that have been created const boost::container::flat_map* currentTags; + const PbfReader::Relation* currentRelation; + const std::vector* stringTable; std::vector finalizeOutputs(); diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index ac8f04bb..ff690d12 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -5,7 +5,7 @@ #include "helpers.h" #include "coordinates_geom.h" #include "osm_mem_tiles.h" - +#include "node_store.h" using namespace std; @@ -442,16 +442,60 @@ void OsmLuaProcessing::Layer(const string &layerName, bool area) { } } -void OsmLuaProcessing::LayerAsCentroid(const string &layerName) { +// Emit a point. This function can be called for nodes, ways or relations. +// +// When called for a relation, it accepts a variadic list of relation roles whose +// node position should be used as the centroid. The first matching node is selected. +// +// As a fallback, or if no list is provided, we'll compute the geometric centroid +// of the relation. +void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::VariadicArgType relationRoles) { if (layers.layerMap.count(layerName) == 0) { throw out_of_range("ERROR: LayerAsCentroid(): a layer named as \"" + layerName + "\" doesn't exist."); } + // This will be non-zero if we ultimately used a node from a relation to + // label the point. + NodeID relationNode = 0; + uint layerMinZoom = layers.layers[layers.layerMap[layerName]].minzoom; AttributeSet attributes; Point geomp; try { - geomp = calculateCentroid(); + // If we're a relation, see if the user would prefer we use one of its members + // to label the point. + if (isRelation) { + for (auto needleRef : relationRoles) { + const std::string needle = needleRef.get(); + + // We do a linear search of the relation's members. This is not very efficient + // for relations like Tongass National Park (ID 6535292, 29,000 members), + // but in general, it's probably fine. + // + // I note that relation members seem to be sorted nodes first, then ways, + // then relations. I'm not sure if we can rely on that, so I don't + // short-circuit on the first non-node. + for (int i = 0; i < currentRelation->memids.size(); i++) { + if (currentRelation->types[i] != PbfReader::Relation::MemberType::NODE) + continue; + + const protozero::data_view role = stringTable->at(currentRelation->roles_sid[i]); + if (role.size() == needle.size() && 0 == memcmp(role.data(), needle.data(), role.size())) { + relationNode = currentRelation->memids[i]; + const auto ll = osmStore.nodes.at(relationNode); + geomp = Point(ll.lon, ll.latp); + break; + } + } + + if (relationNode != 0) + break; + } + } + + if (geom::is_empty(geomp)) + geomp = calculateCentroid(); + if(geom::is_empty(geomp)) { cerr << "Geometry is empty in OsmLuaProcessing::LayerAsCentroid (" << (isRelation ? "relation " : isWay ? "way " : "node ") << originalOsmID << ")" << endl; return; @@ -472,8 +516,10 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName) { // We don't do lazy centroids for relations - calculating their centroid // can be quite expensive, and there's not as many of them as there are // ways. - if (materializeGeometries || isRelation) { + if (materializeGeometries || (isRelation && relationNode == 0)) { id = osmMemTiles.storePoint(geomp); + } else if (relationNode != 0) { + id = USE_NODE_STORE | relationNode; } else if (!isRelation && !isWay) { // Sometimes people call LayerAsCentroid(...) on a node, because they're // writing a generic handler that doesn't know if it's a node or a way, @@ -702,11 +748,19 @@ bool OsmLuaProcessing::setWay(WayID wayId, LatpLonVec const &llVec, const tag_ma } // We are now processing a relation -void OsmLuaProcessing::setRelation(int64_t relationId, WayVec const &outerWayVec, WayVec const &innerWayVec, const tag_map_t &tags, - bool isNativeMP, // only OSM type=multipolygon - bool isInnerOuter) { // any OSM relation with "inner" and "outer" roles (e.g. type=multipolygon|boundary) +void OsmLuaProcessing::setRelation( + const std::vector& stringTable, + const PbfReader::Relation& relation, + const WayVec& outerWayVec, + const WayVec& innerWayVec, + const tag_map_t &tags, + bool isNativeMP, // only OSM type=multipolygon + bool isInnerOuter // any OSM relation with "inner" and "outer" roles (e.g. type=multipolygon|boundary) +) { reset(); - originalOsmID = relationId; + this->stringTable = &stringTable; + currentRelation = &relation; + originalOsmID = relation.id; isWay = true; isRelation = true; isClosed = isNativeMP || isInnerOuter; diff --git a/src/pbf_processor.cpp b/src/pbf_processor.cpp index 63a0c36e..62a87d29 100644 --- a/src/pbf_processor.cpp +++ b/src/pbf_processor.cpp @@ -259,7 +259,7 @@ bool PbfProcessor::ReadRelations( try { tag_map_t tags; readTags(pbfRelation, pb, tags); - output.setRelation(pbfRelation.id, outerWayVec, innerWayVec, tags, isMultiPolygon, isInnerOuter); + output.setRelation(pb.stringTable, pbfRelation, outerWayVec, innerWayVec, tags, isMultiPolygon, isInnerOuter); } catch (std::out_of_range &err) { // Relation is missing a member? From 154d3ab73e7a83f564e6cc60ea157fe0e29ce050 Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Tue, 2 Jan 2024 23:25:30 -0500 Subject: [PATCH 06/13] openmaptiles: don't pass false to LayerAsCentroid Ugh, I guess this could be considered a breaking change after all. If a user previously passed `LayerAsCentroid("layername", false)`, we'd ignore the false. With this change, the false causes us to fail, because we expect a string. I'd normally just say this was undefined behaviour, and those users deserve to be broken...but it's tricky, since this is a script from the official tilemaker repo. --- resources/process-openmaptiles.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/process-openmaptiles.lua b/resources/process-openmaptiles.lua index c7f74745..b648a089 100644 --- a/resources/process-openmaptiles.lua +++ b/resources/process-openmaptiles.lua @@ -566,7 +566,7 @@ function way_function(way) -- Set 'housenumber' if housenumber~="" then - way:LayerAsCentroid("housenumber", false) + way:LayerAsCentroid("housenumber") way:Attribute("housenumber", housenumber) end From 77fb64a989defe498e8ac47bd8205fbd61105890 Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Wed, 3 Jan 2024 20:02:33 -0500 Subject: [PATCH 07/13] port polylabel from mapbox These seems to work well and, at least for large polygons (city parks, national parks), is faster than Boost's centroid algorithm. That surprised me! I haven't benchmarked it on building polygons yet. There are several todos here around making it configurable, and making it play well with lazy geometries. Going to finish off the relation memory stuff then fix those. --- include/osm_lua_processing.h | 5 +- include/polylabel.h | 209 +++++++++++++++++++++++++++++++++++ src/osm_lua_processing.cpp | 45 ++++++-- 3 files changed, 251 insertions(+), 8 deletions(-) create mode 100644 include/polylabel.h diff --git a/include/osm_lua_processing.h b/include/osm_lua_processing.h index 05e7d2c0..612fff51 100644 --- a/include/osm_lua_processing.h +++ b/include/osm_lua_processing.h @@ -134,7 +134,10 @@ class OsmLuaProcessing { // Return centroid lat/lon std::vector Centroid(); - Point calculateCentroid(); + + enum class CentroidAlgorithm: char { Centroid = 0, Polylabel = 1 }; + const CentroidAlgorithm defaultCentroidAlgorithm() const { return CentroidAlgorithm::Polylabel; } + Point calculateCentroid(CentroidAlgorithm algorithm); enum class CorrectGeometryResult: char { Invalid = 0, Valid = 1, Corrected = 2 }; // ---- Requests from Lua to write this way/node to a vector tile's Layer diff --git a/include/polylabel.h b/include/polylabel.h new file mode 100644 index 00000000..5d3805eb --- /dev/null +++ b/include/polylabel.h @@ -0,0 +1,209 @@ +#pragma once + +// Original source from https://github.com/mapbox/polylabel, licensed +// under ISC. +// +// Adapted to use Boost Geometry instead of MapBox's geometry library. +// +// Changes: +// - Default precision changed from 1 to 0.00001. +// @mourner has some comments about what a reasonable precision value is +// for latitude/longitude coordinates, see +// https://github.com/mapbox/polylabel/issues/68#issuecomment-694906027 +// https://github.com/mapbox/polylabel/issues/103#issuecomment-1516623862 +// +// Possible future changes: +// - Port the change described in https://github.com/mapbox/polylabel/issues/33, +// implemented in Mapnik's Java renderer, to C++. +// - But see counterexample: https://github.com/mapbox/polylabel/pull/63 +// @Fil also proposes an alternative approach there. +// - Pick precision as a function of the input geometry. + +#include "geom.h" + +#include +#include +#include +#include + +namespace mapbox { + +namespace detail { + +// get squared distance from a point to a segment +double getSegDistSq(const Point& p, + const Point& a, + const Point& b) { + auto x = a.get<0>(); + auto y = a.get<1>(); + auto dx = b.get<0>() - x; + auto dy = b.get<1>() - y; + + if (dx != 0 || dy != 0) { + + auto t = ((p.get<0>() - x) * dx + (p.get<1>() - y) * dy) / (dx * dx + dy * dy); + + if (t > 1) { + x = b.get<0>(); + y = b.get<1>(); + + } else if (t > 0) { + x += dx * t; + y += dy * t; + } + } + + dx = p.get<0>() - x; + dy = p.get<1>() - y; + + return dx * dx + dy * dy; +} + +// signed distance from point to polygon outline (negative if point is outside) +auto pointToPolygonDist(const Point& point, const Polygon& polygon) { + bool inside = false; + auto minDistSq = std::numeric_limits::infinity(); + + { + const auto& ring = polygon.outer(); + for (std::size_t i = 0, len = ring.size(), j = len - 1; i < len; j = i++) { + const auto& a = ring[i]; + const auto& b = ring[j]; + + if ((a.get<1>() > point.get<1>()) != (b.get<1>() > point.get<1>()) && + (point.get<0>() < (b.get<0>() - a.get<0>()) * (point.get<1>() - a.get<1>()) / (b.get<1>() - a.get<1>()) + a.get<0>())) inside = !inside; + + minDistSq = std::min(minDistSq, getSegDistSq(point, a, b)); + } + } + + for (const auto& ring : polygon.inners()) { + for (std::size_t i = 0, len = ring.size(), j = len - 1; i < len; j = i++) { + const auto& a = ring[i]; + const auto& b = ring[j]; + + if ((a.get<1>() > point.get<1>()) != (b.get<1>() > point.get<1>()) && + (point.get<0>() < (b.get<0>() - a.get<0>()) * (point.get<1>() - a.get<1>()) / (b.get<1>() - a.get<1>()) + a.get<0>())) inside = !inside; + + minDistSq = std::min(minDistSq, getSegDistSq(point, a, b)); + } + } + + return (inside ? 1 : -1) * std::sqrt(minDistSq); +} + +struct Cell { + Cell(const Point& c_, double h_, const Polygon& polygon) + : c(c_), + h(h_), + d(pointToPolygonDist(c, polygon)), + max(d + h * std::sqrt(2)) + {} + + Point c; // cell center + double h; // half the cell size + double d; // distance from cell center to polygon + double max; // max distance to polygon within a cell +}; + +// get polygon centroid +Cell getCentroidCell(const Polygon& polygon) { + double area = 0; + double cx = 0, cy = 0; + const auto& ring = polygon.outer(); + + for (std::size_t i = 0, len = ring.size(), j = len - 1; i < len; j = i++) { + const Point& a = ring[i]; + const Point& b = ring[j]; + auto f = a.get<0>() * b.get<1>() - b.get<0>() * a.get<1>(); + cx += (a.get<0>() + b.get<0>()) * f; + cy += (a.get<1>() + b.get<1>()) * f; + area += f * 3; + } + + Point c { cx, cy }; + return Cell(area == 0 ? ring.at(0) : Point { cx / area, cy / area }, 0, polygon); +} + +} // namespace detail + +Point polylabel(const Polygon& polygon, double precision = 0.00001, bool debug = false) { + using namespace detail; + + // find the bounding box of the outer ring + Box envelope; + geom::envelope(polygon.outer(), envelope); + + const Point size { + envelope.max_corner().get<0>() - envelope.min_corner().get<0>(), + envelope.max_corner().get<1>() - envelope.min_corner().get<1>() + }; + + const double cellSize = std::min(size.get<0>(), size.get<1>()); + double h = cellSize / 2; + + // a priority queue of cells in order of their "potential" (max distance to polygon) + auto compareMax = [] (const Cell& a, const Cell& b) { + return a.max < b.max; + }; + using Queue = std::priority_queue, decltype(compareMax)>; + Queue cellQueue(compareMax); + + if (cellSize == 0) { + return envelope.min_corner(); + } + + // cover polygon with initial cells + + for (double x = envelope.min_corner().get<0>(); x < envelope.max_corner().get<0>(); x += cellSize) { + for (double y = envelope.min_corner().get<1>(); y < envelope.max_corner().get<1>(); y += cellSize) { + cellQueue.push(Cell({x + h, y + h}, h, polygon)); + } + } + + // take centroid as the first best guess + auto bestCell = getCentroidCell(polygon); + + // second guess: bounding box centroid + Cell bboxCell( + Point { + envelope.min_corner().get<0>() + size.get<0>() / 2.0, + envelope.min_corner().get<1>() + size.get<1>() / 2.0 + }, 0, polygon); + if (bboxCell.d > bestCell.d) { + bestCell = bboxCell; + } + + auto numProbes = cellQueue.size(); + while (!cellQueue.empty()) { + // pick the most promising cell from the queue + auto cell = cellQueue.top(); + cellQueue.pop(); + + // update the best cell if we found a better one + if (cell.d > bestCell.d) { + bestCell = cell; + if (debug) std::cout << "found best " << ::round(1e4 * cell.d) / 1e4 << " after " << numProbes << " probes" << std::endl; + } + + // do not drill down further if there's no chance of a better solution + if (cell.max - bestCell.d <= precision) continue; + + // split the cell into four cells + h = cell.h / 2; + cellQueue.push(Cell({cell.c.get<0>() - h, cell.c.get<1>() - h}, h, polygon)); + cellQueue.push(Cell({cell.c.get<0>() + h, cell.c.get<1>() - h}, h, polygon)); + cellQueue.push(Cell({cell.c.get<0>() - h, cell.c.get<1>() + h}, h, polygon)); + cellQueue.push(Cell({cell.c.get<0>() + h, cell.c.get<1>() + h}, h, polygon)); + numProbes += 4; + } + + if (debug) { + std::cout << "num probes: " << numProbes << std::endl; + std::cout << "best distance: " << bestCell.d << std::endl; + } + + return bestCell.c; +} + +} // namespace mapbox diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index ff690d12..74646c41 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -6,6 +6,7 @@ #include "coordinates_geom.h" #include "osm_mem_tiles.h" #include "node_store.h" +#include "polylabel.h" using namespace std; @@ -461,6 +462,7 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic uint layerMinZoom = layers.layers[layers.layerMap[layerName]].minzoom; AttributeSet attributes; Point geomp; + bool centroidFound = false; try { // If we're a relation, see if the user would prefer we use one of its members // to label the point. @@ -484,6 +486,7 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic relationNode = currentRelation->memids[i]; const auto ll = osmStore.nodes.at(relationNode); geomp = Point(ll.lon, ll.latp); + centroidFound = true; break; } } @@ -493,9 +496,12 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic } } - if (geom::is_empty(geomp)) - geomp = calculateCentroid(); + if (!centroidFound) + // TODO: make this configurable via the 2nd argument + geomp = calculateCentroid(CentroidAlgorithm::Polylabel); + // TODO: I think geom::is_empty always returns false for Points? + // See https://github.com/boostorg/geometry/blob/fa3623528ea27ba2c3c1327e4b67408a2b567038/include/boost/geometry/algorithms/is_empty.hpp#L103 if(geom::is_empty(geomp)) { cerr << "Geometry is empty in OsmLuaProcessing::LayerAsCentroid (" << (isRelation ? "relation " : isWay ? "way " : "node ") << originalOsmID << ")" << endl; return; @@ -526,6 +532,7 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic // e.g. POIs. id = USE_NODE_STORE | originalOsmID; } else { + // TODO: encode the centroid algorithm in the ID id = USE_WAY_STORE | originalOsmID; wayEmitted = true; } @@ -533,17 +540,40 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic outputs.push_back(std::make_pair(std::move(oo), attributes)); } -Point OsmLuaProcessing::calculateCentroid() { +Point OsmLuaProcessing::calculateCentroid(CentroidAlgorithm algorithm) { Point centroid; if (isRelation) { - Geometry tmp; + MultiPolygon tmp; tmp = multiPolygonCached(); - geom::centroid(tmp, centroid); + + if (algorithm == CentroidAlgorithm::Polylabel) { + int index = 0; + + // CONSIDER: pick precision intelligently + // Polylabel works on polygons, so for multipolygons we'll label the biggest outer. + double biggestSize = 0; + for (int i = 0; i < tmp.size(); i++) { + double size = geom::area(tmp[i]); + if (size > biggestSize) { + biggestSize = size; + index = i; + } + } + centroid = mapbox::polylabel(tmp[index]); + } else { + geom::centroid(tmp, centroid); + } return Point(centroid.x()*10000000.0, centroid.y()*10000000.0); } else if (isWay) { Polygon p; geom::assign_points(p, linestringCached()); - geom::centroid(p, centroid); + + if (algorithm == CentroidAlgorithm::Polylabel) { + // CONSIDER: pick precision intelligently + centroid = mapbox::polylabel(p); + } else { + geom::centroid(p, centroid); + } return Point(centroid.x()*10000000.0, centroid.y()*10000000.0); } else { return Point(lon, latp); @@ -551,7 +581,8 @@ Point OsmLuaProcessing::calculateCentroid() { } std::vector OsmLuaProcessing::Centroid() { - Point c = calculateCentroid(); + // TODO: make this configurable by a parameter + Point c = calculateCentroid(CentroidAlgorithm::Polylabel); return std::vector { latp2lat(c.y()/10000000.0), c.x()/10000000.0 }; } From 9ca08b3638298489ad454648d7452e851fd27c0b Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Wed, 3 Jan 2024 21:34:13 -0500 Subject: [PATCH 08/13] relation roles: track as short, not string --- CMakeLists.txt | 1 + Makefile | 7 +++++ include/osm_lua_processing.h | 2 +- include/osm_store.h | 27 ++++++++--------- include/relation_roles.h | 22 ++++++++++++++ src/osm_lua_processing.cpp | 19 +++++++----- src/pbf_processor.cpp | 4 +-- src/relation_roles.cpp | 57 ++++++++++++++++++++++++++++++++++++ test/relation_roles.test.cpp | 24 +++++++++++++++ 9 files changed, 138 insertions(+), 25 deletions(-) create mode 100644 include/relation_roles.h create mode 100644 src/relation_roles.cpp create mode 100644 test/relation_roles.test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 2109edcd..f866c00d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -90,6 +90,7 @@ file(GLOB tilemaker_src_files src/pbf_reader.cpp src/pmtiles.cpp src/pooled_string.cpp + src/relation_roles.cpp src/sharded_node_store.cpp src/sharded_way_store.cpp src/shared_data.cpp diff --git a/Makefile b/Makefile index 8cec77e5..de9644b5 100644 --- a/Makefile +++ b/Makefile @@ -114,6 +114,7 @@ tilemaker: \ src/pbf_reader.o \ src/pmtiles.o \ src/pooled_string.o \ + src/relation_roles.o \ src/sharded_node_store.o \ src/sharded_way_store.o \ src/shared_data.o \ @@ -133,6 +134,7 @@ test: \ test_deque_map \ test_pbf_reader \ test_pooled_string \ + test_relation_roles \ test_sorted_node_store \ test_sorted_way_store @@ -163,6 +165,11 @@ test_pooled_string: \ test/pooled_string.test.o $(CXX) $(CXXFLAGS) -o test.pooled_string $^ $(INC) $(LIB) $(LDFLAGS) && ./test.pooled_string +test_relation_roles: \ + src/relation_roles.o \ + test/relation_roles.test.o + $(CXX) $(CXXFLAGS) -o test.relation_roles $^ $(INC) $(LIB) $(LDFLAGS) && ./test.relation_roles + test_sorted_node_store: \ src/external/streamvbyte_decode.o \ src/external/streamvbyte_encode.o \ diff --git a/include/osm_lua_processing.h b/include/osm_lua_processing.h index 612fff51..878987f7 100644 --- a/include/osm_lua_processing.h +++ b/include/osm_lua_processing.h @@ -256,7 +256,7 @@ class OsmLuaProcessing { bool isWay, isRelation, isClosed; ///< Way, node, relation? bool relationAccepted; // in scanRelation, whether we're using a non-MP relation - std::vector> relationList; // in processNode/processWay, list of relations this entity is in, and its role + std::vector> relationList; // in processNode/processWay, list of relations this entity is in, and its role int relationSubscript = -1; // in processWay, position in the relation list int32_t lon,latp; ///< Node coordinates diff --git a/include/osm_store.h b/include/osm_store.h index edf9371d..2b378ebb 100644 --- a/include/osm_store.h +++ b/include/osm_store.h @@ -5,6 +5,7 @@ #include "geom.h" #include "coordinates.h" #include "mmap_allocator.h" +#include "relation_roles.h" #include #include @@ -81,19 +82,22 @@ class RelationScanStore { private: using tag_map_t = boost::container::flat_map; - std::map>> relationsForWays; - std::map>> relationsForNodes; + std::map>> relationsForWays; + std::map>> relationsForNodes; std::map relationTags; mutable std::mutex mutex; + RelationRoles relationRoles; public: void relation_contains_way(WayID relid, WayID wayid, std::string role) { + uint16_t roleId = relationRoles.getOrAddRole(role); std::lock_guard lock(mutex); - relationsForWays[wayid].emplace_back(std::make_pair(relid, role)); + relationsForWays[wayid].emplace_back(std::make_pair(relid, roleId)); } void relation_contains_node(WayID relid, NodeID nodeId, std::string role) { + uint16_t roleId = relationRoles.getOrAddRole(role); std::lock_guard lock(mutex); - relationsForNodes[nodeId].emplace_back(std::make_pair(relid, role)); + relationsForNodes[nodeId].emplace_back(std::make_pair(relid, roleId)); } void store_relation_tags(WayID relid, const tag_map_t &tags) { std::lock_guard lock(mutex); @@ -105,10 +109,11 @@ class RelationScanStore { bool node_in_any_relations(NodeID nodeId) { return relationsForNodes.find(nodeId) != relationsForNodes.end(); } - std::vector> relations_for_way(WayID wayid) { + std::string getRole(uint16_t roleId) const { return relationRoles.getRole(roleId); } + const std::vector>& relations_for_way(WayID wayid) { return relationsForWays[wayid]; } - std::vector> relations_for_node(NodeID nodeId) { + const std::vector>& relations_for_node(NodeID nodeId) { return relationsForNodes[nodeId]; } std::string get_relation_tag(WayID relid, const std::string &key) { @@ -187,6 +192,7 @@ class OSMStore public: NodeStore& nodes; WayStore& ways; + RelationScanStore scannedRelations; protected: bool use_compact_nodes = false; @@ -194,7 +200,6 @@ class OSMStore RelationStore relations; // unused UsedWays used_ways; - RelationScanStore scanned_relations; public: @@ -222,14 +227,6 @@ class OSMStore void ensureUsedWaysInited(); using tag_map_t = boost::container::flat_map; - void relation_contains_way(WayID relid, WayID wayid, std::string role) { scanned_relations.relation_contains_way(relid, wayid, role); } - void relation_contains_node(WayID relid, NodeID nodeId, std::string role) { scanned_relations.relation_contains_node(relid, nodeId, role); } - void store_relation_tags(WayID relid, const tag_map_t &tags) { scanned_relations.store_relation_tags(relid,tags); } - bool way_in_any_relations(WayID wayid) { return scanned_relations.way_in_any_relations(wayid); } - bool node_in_any_relations(NodeID nodeId) { return scanned_relations.node_in_any_relations(nodeId); } - std::vector> relations_for_way(WayID wayid) { return scanned_relations.relations_for_way(wayid); } - std::vector> relations_for_node(NodeID nodeId) { return scanned_relations.relations_for_node(nodeId); } - std::string get_relation_tag(WayID relid, const std::string &key) { return scanned_relations.get_relation_tag(relid, key); } void clear(); void reportSize() const; diff --git a/include/relation_roles.h b/include/relation_roles.h new file mode 100644 index 00000000..2a585407 --- /dev/null +++ b/include/relation_roles.h @@ -0,0 +1,22 @@ +#ifndef RELATION_ROLES_H +#define RELATION_ROLES_H + +#include +#include +#include +#include + +class RelationRoles { +public: + RelationRoles(); + uint16_t getOrAddRole(const std::string& role); + std::string getRole(uint16_t role) const; + +private: + std::vector popularRoleStrings; + std::vector rareRoleStrings; + std::mutex mutex; + boost::container::flat_map popularRoles; + boost::container::flat_map rareRoles; +}; +#endif diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index 74646c41..8beb29f6 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -659,7 +659,12 @@ template<> struct kaguya::lua_type_traits { OsmLuaProcessing::OptionalRelation OsmLuaProcessing::NextRelation() { relationSubscript++; if (relationSubscript >= relationList.size()) return { true }; - return { false, static_cast(relationList[relationSubscript].first), relationList[relationSubscript].second }; + + return { + false, + static_cast(relationList[relationSubscript].first), + osmStore.scannedRelations.getRole(relationList[relationSubscript].second) + }; } void OsmLuaProcessing::RestartRelations() { @@ -667,7 +672,7 @@ void OsmLuaProcessing::RestartRelations() { } std::string OsmLuaProcessing::FindInRelation(const std::string &key) { - return osmStore.get_relation_tag(relationList[relationSubscript].first, key); + return osmStore.scannedRelations.get_relation_tag(relationList[relationSubscript].first, key); } // Record attribute name/type for vector_layers table @@ -695,7 +700,7 @@ bool OsmLuaProcessing::scanRelation(WayID id, const tag_map_t &tags) { for (const auto& i : tags) { m[std::string(i.first.data(), i.first.size())] = std::string(i.second.data(), i.second.size()); } - osmStore.store_relation_tags(id, m); + osmStore.scannedRelations.store_relation_tags(id, m); return true; } @@ -709,8 +714,8 @@ void OsmLuaProcessing::setNode(NodeID id, LatpLon node, const tag_map_t &tags) { latp= node.latp; currentTags = &tags; - if (supportsReadingRelations && osmStore.node_in_any_relations(id)) { - relationList = osmStore.relations_for_node(id); + if (supportsReadingRelations && osmStore.scannedRelations.node_in_any_relations(id)) { + relationList = osmStore.scannedRelations.relations_for_node(id); } //Start Lua processing for node @@ -742,8 +747,8 @@ bool OsmLuaProcessing::setWay(WayID wayId, LatpLonVec const &llVec, const tag_ma innerWayVecPtr = nullptr; linestringInited = polygonInited = multiPolygonInited = false; - if (supportsReadingRelations && osmStore.way_in_any_relations(wayId)) { - relationList = osmStore.relations_for_way(wayId); + if (supportsReadingRelations && osmStore.scannedRelations.way_in_any_relations(wayId)) { + relationList = osmStore.scannedRelations.relations_for_way(wayId); } try { diff --git a/src/pbf_processor.cpp b/src/pbf_processor.cpp index 62a87d29..b89c3d0b 100644 --- a/src/pbf_processor.cpp +++ b/src/pbf_processor.cpp @@ -186,7 +186,7 @@ bool PbfProcessor::ScanRelations(OsmLuaProcessing& output, PbfReader::PrimitiveG if (isAccepted) { const auto& roleView = pb.stringTable[pbfRelation.roles_sid[n]]; std::string role(roleView.data(), roleView.size()); - osmStore.relation_contains_node(relid, lastID, role); + osmStore.scannedRelations.relation_contains_node(relid, lastID, role); } } else if (pbfRelation.types[n] == PbfReader::Relation::MemberType::WAY) { if (lastID >= pow(2,42)) throw std::runtime_error("Way ID in relation "+std::to_string(relid)+" negative or too large: "+std::to_string(lastID)); @@ -194,7 +194,7 @@ bool PbfProcessor::ScanRelations(OsmLuaProcessing& output, PbfReader::PrimitiveG if (isAccepted) { const auto& roleView = pb.stringTable[pbfRelation.roles_sid[n]]; std::string role(roleView.data(), roleView.size()); - osmStore.relation_contains_way(relid, lastID, role); + osmStore.scannedRelations.relation_contains_way(relid, lastID, role); } } } diff --git a/src/relation_roles.cpp b/src/relation_roles.cpp new file mode 100644 index 00000000..f9584b92 --- /dev/null +++ b/src/relation_roles.cpp @@ -0,0 +1,57 @@ +#include "relation_roles.h" + +RelationRoles::RelationRoles() { + // Computed in early 2024 from popular roles: https://gist.github.com/systemed/29ea4c8d797a20dcdffee8ba907d62ea + // This is just an optimization to avoid taking a lock in the common case. + // + // The list should be refreshed if the set of popular roles dramatically changes, + // but tilemaker will still be correct, just slower. + popularRoleStrings = { + "", + "1700","1800","1900","2700","2800","2900","3000","3100","3200","above","accessfrom", + "accessto","accessvia","across","address","admin_centre","alternative","associated", + "attached_to","backward","basket","both","branch_circuit","building","buildingpart", + "builidingpart","camera","claimed","connection","contains","crossing","destination", + "device","de_facto","east","edge","empty role","end","endpoint","entrance","entry", + "ex-camera","exit","extent","facility","force","forward","from","generator","give_way", + "graph","guidepost","hidden","highway","Hole","hole","house","inner","intersection", + "in_tunnel","junction","label","lable","landuse","lateral","left","line","location_hint", + "lower","main","main_stream","marker","member","memorial","mirror","negative", + "negative:entry","negative:exit","negative:parking","object","on_bridge","outer", + "outline","part","pedestrian","pin","pit_lane","platform","positive","positive:entry", + "positive:exit","positive:parking","priority","ridge","right","road_marking","road_sign", + "room","section","sector","shell","side_stream","sign","signal","start","stop","street", + "sub-relation","subarea","subbasin","substation","switch","target","tee","through","to", + "tomb","track","tracksection","traffic_sign","tributary","trunk_circuit","under","upper", + "via","visible","walk","ways","west" + }; + + for (const auto& s : popularRoleStrings) { + popularRoles[s] = popularRoles.size(); + } +} + +std::string RelationRoles::getRole(uint16_t role) const { + if (role < popularRoleStrings.size()) + return popularRoleStrings[role]; + + return rareRoleStrings[role - popularRoleStrings.size()]; +} + +uint16_t RelationRoles::getOrAddRole(const std::string& role) { + { + const auto& pos = popularRoles.find(role); + if (pos != popularRoles.end()) + return pos->second; + } + + std::lock_guard lock(mutex); + const auto& pos = rareRoles.find(role); + if (pos != rareRoles.end()) + return pos->second; + + uint16_t rv = popularRoleStrings.size() + rareRoleStrings.size(); + rareRoles[role] = rv; + rareRoleStrings.push_back(role); + return rv; +} diff --git a/test/relation_roles.test.cpp b/test/relation_roles.test.cpp new file mode 100644 index 00000000..2cf8c438 --- /dev/null +++ b/test/relation_roles.test.cpp @@ -0,0 +1,24 @@ +#include +#include "external/minunit.h" +#include "relation_roles.h" + +MU_TEST(test_relation_roles) { + RelationRoles rr; + + mu_check(rr.getRole(0) == ""); + mu_check(rr.getOrAddRole("inner") == rr.getOrAddRole("inner")); + mu_check(rr.getOrAddRole("never before seen") == rr.getOrAddRole("never before seen")); + mu_check(rr.getRole(rr.getOrAddRole("inner")) == "inner"); + mu_check(rr.getRole(rr.getOrAddRole("never before seen")) == "never before seen"); +} + +MU_TEST_SUITE(test_suite_relation_roles) { + MU_RUN_TEST(test_relation_roles); +} + +int main() { + MU_RUN_SUITE(test_suite_relation_roles); + MU_REPORT(); + return MU_EXIT_CODE; +} + From 0910d8a39a976327aec77b5ec1c3ae8383b202be Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Wed, 3 Jan 2024 21:47:13 -0500 Subject: [PATCH 09/13] ::Centroid() takes optional algorithm parameter --- include/osm_lua_processing.h | 5 +++-- src/osm_lua_processing.cpp | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/include/osm_lua_processing.h b/include/osm_lua_processing.h index 878987f7..c93288b8 100644 --- a/include/osm_lua_processing.h +++ b/include/osm_lua_processing.h @@ -133,10 +133,11 @@ class OsmLuaProcessing { double Length(); // Return centroid lat/lon - std::vector Centroid(); + std::vector Centroid(kaguya::VariadicArgType algorithm); enum class CentroidAlgorithm: char { Centroid = 0, Polylabel = 1 }; - const CentroidAlgorithm defaultCentroidAlgorithm() const { return CentroidAlgorithm::Polylabel; } + CentroidAlgorithm defaultCentroidAlgorithm() const { return CentroidAlgorithm::Polylabel; } + CentroidAlgorithm parseCentroidAlgorithm(const std::string& algorithm) const; Point calculateCentroid(CentroidAlgorithm algorithm); enum class CorrectGeometryResult: char { Invalid = 0, Valid = 1, Corrected = 2 }; diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index 8beb29f6..a4703a0d 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -580,9 +580,22 @@ Point OsmLuaProcessing::calculateCentroid(CentroidAlgorithm algorithm) { } } -std::vector OsmLuaProcessing::Centroid() { - // TODO: make this configurable by a parameter - Point c = calculateCentroid(CentroidAlgorithm::Polylabel); +OsmLuaProcessing::CentroidAlgorithm OsmLuaProcessing::parseCentroidAlgorithm(const std::string& algorithm) const { + if (algorithm == "polylabel") return OsmLuaProcessing::CentroidAlgorithm::Polylabel; + if (algorithm == "centroid") return OsmLuaProcessing::CentroidAlgorithm::Centroid; + + throw std::runtime_error("unknown centroid algorithm " + algorithm); +} + +std::vector OsmLuaProcessing::Centroid(kaguya::VariadicArgType algorithmArgs) { + CentroidAlgorithm algorithm = defaultCentroidAlgorithm(); + + for (auto needleRef : algorithmArgs) { + const std::string needle = needleRef.get(); + algorithm = parseCentroidAlgorithm(needle); + break; + } + Point c = calculateCentroid(algorithm); return std::vector { latp2lat(c.y()/10000000.0), c.x()/10000000.0 }; } From c74e4c1822a546bcbb31abeb4bbea6291bdba08d Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Wed, 3 Jan 2024 21:53:47 -0500 Subject: [PATCH 10/13] ::LayerAsCentroid(layer, algorithm, role, ...) You can now pass the preferred algorithm to use to LayerAsCentroid. Still to do: teach lazy geometries about which algorithm was used. --- src/osm_lua_processing.cpp | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index a4703a0d..5a8dce07 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -443,18 +443,28 @@ void OsmLuaProcessing::Layer(const string &layerName, bool area) { } } +// LayerAsCentroid(layerName, [centroid-algorithm, [role, [role, ...]]]) +// // Emit a point. This function can be called for nodes, ways or relations. // -// When called for a relation, it accepts a variadic list of relation roles whose -// node position should be used as the centroid. The first matching node is selected. +// When called for a 2D geometry, you can pass a preferred centroid algorithm +// in `centroid-algorithm`. Currently `polylabel` and `centroid` are supported. // -// As a fallback, or if no list is provided, we'll compute the geometric centroid -// of the relation. -void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::VariadicArgType relationRoles) { +// When called for a relation, you can pass a list of roles. The point of a node +// with that role will be used if available. +void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::VariadicArgType varargs) { if (layers.layerMap.count(layerName) == 0) { throw out_of_range("ERROR: LayerAsCentroid(): a layer named as \"" + layerName + "\" doesn't exist."); } + CentroidAlgorithm algorithm = defaultCentroidAlgorithm(); + + for (auto needleRef : varargs) { + const std::string needle = needleRef.get(); + algorithm = parseCentroidAlgorithm(needle); + break; + } + // This will be non-zero if we ultimately used a node from a relation to // label the point. NodeID relationNode = 0; @@ -467,7 +477,11 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic // If we're a relation, see if the user would prefer we use one of its members // to label the point. if (isRelation) { - for (auto needleRef : relationRoles) { + size_t i = 0; + for (auto needleRef : varargs) { + // Skip the first vararg, it's the algorithm. + if (i == 0) continue; + i++; const std::string needle = needleRef.get(); // We do a linear search of the relation's members. This is not very efficient @@ -497,8 +511,7 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic } if (!centroidFound) - // TODO: make this configurable via the 2nd argument - geomp = calculateCentroid(CentroidAlgorithm::Polylabel); + geomp = calculateCentroid(algorithm); // TODO: I think geom::is_empty always returns false for Points? // See https://github.com/boostorg/geometry/blob/fa3623528ea27ba2c3c1327e4b67408a2b567038/include/boost/geometry/algorithms/is_empty.hpp#L103 From 3f35ff62fee14f7d60e45288791df8b98f278cb9 Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Wed, 3 Jan 2024 22:00:44 -0500 Subject: [PATCH 11/13] always materialize centroids if polylabel This ensures that the user gets the same, correct behaviour both in --materialize-geometries and --lazy-geometries. We can extend support to materialize geometries, but this PR is already getting big (and it has conflicts with the other 2 PRs), so I'm leery of getting further out over my skis. --- src/osm_lua_processing.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index 5a8dce07..a108dd00 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -532,10 +532,16 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic } NodeID id = 0; - // We don't do lazy centroids for relations - calculating their centroid - // can be quite expensive, and there's not as many of them as there are - // ways. - if (materializeGeometries || (isRelation && relationNode == 0)) { + // We don't do lazy geometries for centroids in these cases: + // + // - --materialize-geometries is set + // - the geometry is a relation - calculating their centroid can be quite expensive, + // and there's not as many of them as there are ways + // - when the algorithm chosen is polylabel + // We can extend lazy geometries to this, it just needs some fiddling to + // express it in the ID and measure if there's a runtime impact in computing + // the polylabel twice. + if (materializeGeometries || (isRelation && relationNode == 0) || (isWay && algorithm != CentroidAlgorithm::Centroid)) { id = osmMemTiles.storePoint(geomp); } else if (relationNode != 0) { id = USE_NODE_STORE | relationNode; @@ -545,7 +551,6 @@ void OsmLuaProcessing::LayerAsCentroid(const string &layerName, kaguya::Variadic // e.g. POIs. id = USE_NODE_STORE | originalOsmID; } else { - // TODO: encode the centroid algorithm in the ID id = USE_WAY_STORE | originalOsmID; wayEmitted = true; } From 6b06eb554d1995d3cdae87d95537787b67e8e3cb Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Wed, 3 Jan 2024 23:12:37 -0500 Subject: [PATCH 12/13] handle multipolygons with 0 polygons --- src/osm_lua_processing.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/osm_lua_processing.cpp b/src/osm_lua_processing.cpp index a108dd00..d5a282a5 100644 --- a/src/osm_lua_processing.cpp +++ b/src/osm_lua_processing.cpp @@ -577,6 +577,9 @@ Point OsmLuaProcessing::calculateCentroid(CentroidAlgorithm algorithm) { index = i; } } + + if (tmp.size() == 0) + throw geom::centroid_exception(); centroid = mapbox::polylabel(tmp[index]); } else { geom::centroid(tmp, centroid); From 2f5ca20c9a54a9a66e75a74eefc71666101a93f3 Mon Sep 17 00:00:00 2001 From: Colin Dellow Date: Sat, 6 Jan 2024 18:24:15 -0500 Subject: [PATCH 13/13] look in relation for ISO3166 country code --- resources/process-openmaptiles.lua | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/resources/process-openmaptiles.lua b/resources/process-openmaptiles.lua index b648a089..3e4a9981 100644 --- a/resources/process-openmaptiles.lua +++ b/resources/process-openmaptiles.lua @@ -172,7 +172,17 @@ function node_function(node) node:MinZoom(mz) if rank then node:AttributeNumeric("rank", rank) end if capital then node:AttributeNumeric("capital", capital) end - if place=="country" then node:Attribute("iso_a2", node:Find("ISO3166-1:alpha2")) end + if place=="country" then + local iso_a2 = node:Find("ISO3166-1:alpha2") + while iso_a2 == "" do + local rel, role = node:NextRelation() + if not rel then break end + if role == 'label' then + iso_a2 = node:FindInRelation("ISO3166-1:alpha2") + end + end + node:Attribute("iso_a2", iso_a2) + end SetNameAttributes(node) return end