From 4460a91f95459e588c013592c136fe61eaa905f0 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sun, 25 Apr 2021 20:30:04 +0200 Subject: [PATCH 1/3] Refactoring: Remove unused parameter from filter_tags()/check_key() After the multi code is gone, the 'strict' parameter is always false. --- src/tagtransform-c.cpp | 68 ++++++++++++++++++---------------------- src/tagtransform-c.hpp | 4 +-- src/tagtransform-lua.cpp | 2 +- src/tagtransform-lua.hpp | 2 +- src/tagtransform.hpp | 3 +- 5 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/tagtransform-c.cpp b/src/tagtransform-c.cpp index b63f2e3f3..896504d4a 100644 --- a/src/tagtransform-c.cpp +++ b/src/tagtransform-c.cpp @@ -101,8 +101,7 @@ std::unique_ptr c_tagtransform_t::clone() const } bool c_tagtransform_t::check_key(std::vector const &infos, - char const *k, bool *filter, int *flags, - bool strict) + char const *k, bool *filter, int *flags) { //go through the actual tags found on the item and keep the ones in the export list for (auto const &info : infos) { @@ -119,29 +118,26 @@ bool c_tagtransform_t::check_key(std::vector const &infos, } // if we didn't find any tags that we wanted to export - // and we aren't strictly adhering to the list - if (!strict) { - if (m_options->hstore_mode != hstore_column::none) { - /* ... but if hstore_match_only is set then don't take this + if (m_options->hstore_mode != hstore_column::none) { + /* ... but if hstore_match_only is set then don't take this as a reason for keeping the object */ - if (!m_options->hstore_match_only) { - *filter = false; - } - /* with hstore, copy all tags... */ - return true; + if (!m_options->hstore_match_only) { + *filter = false; } + /* with hstore, copy all tags... */ + return true; + } - if (!m_options->hstore_columns.empty()) { - /* does this column match any of the hstore column prefixes? */ - for (auto const &column : m_options->hstore_columns) { - if (boost::starts_with(k, column)) { - /* ... but if hstore_match_only is set then don't take this + if (!m_options->hstore_columns.empty()) { + /* does this column match any of the hstore column prefixes? */ + for (auto const &column : m_options->hstore_columns) { + if (boost::starts_with(k, column)) { + /* ... but if hstore_match_only is set then don't take this as a reason for keeping the object */ - if (!m_options->hstore_match_only) { - *filter = false; - } - return true; + if (!m_options->hstore_match_only) { + *filter = false; } + return true; } } } @@ -150,7 +146,7 @@ bool c_tagtransform_t::check_key(std::vector const &infos, } bool c_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, - int *roads, taglist_t &out_tags, bool strict) + int *roads, taglist_t &out_tags) { //assume we dont like this set of tags bool filter = true; @@ -170,27 +166,25 @@ bool c_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, for (auto const &item : o.tags()) { char const *const k = item.key(); char const *const v = item.value(); - //if we want to do more than the export list says - if (!strict) { - if (o.type() == osmium::item_type::relation && - std::strcmp("type", k) == 0) { - out_tags.add_tag(k, v); + + if (o.type() == osmium::item_type::relation && + std::strcmp("type", k) == 0) { + out_tags.add_tag(k, v); + continue; + } + /* Allow named islands to appear as polygons */ + if (std::strcmp("natural", k) == 0 && + std::strcmp("coastline", v) == 0) { + add_area_tag = 1; + + /* Discard natural=coastline tags (we render these from a shapefile instead) */ + if (!m_options->keep_coastlines) { continue; } - /* Allow named islands to appear as polygons */ - if (std::strcmp("natural", k) == 0 && - std::strcmp("coastline", v) == 0) { - add_area_tag = 1; - - /* Discard natural=coastline tags (we render these from a shapefile instead) */ - if (!m_options->keep_coastlines) { - continue; - } - } } //go through the actual tags found on the item and keep the ones in the export list - if (check_key(infos, k, &filter, &flags, strict)) { + if (check_key(infos, k, &filter, &flags)) { out_tags.add_tag(k, v); } } diff --git a/src/tagtransform-c.hpp b/src/tagtransform-c.hpp index 204f2c64d..b42dd2aba 100644 --- a/src/tagtransform-c.hpp +++ b/src/tagtransform-c.hpp @@ -21,7 +21,7 @@ class c_tagtransform_t : public tagtransform_t std::unique_ptr clone() const override; bool filter_tags(osmium::OSMObject const &o, int *polygon, int *roads, - taglist_t &out_tags, bool strict = false) override; + taglist_t &out_tags) override; bool filter_rel_member_tags(taglist_t const &rel_tags, osmium::memory::Buffer const &members, @@ -32,7 +32,7 @@ class c_tagtransform_t : public tagtransform_t private: bool check_key(std::vector const &infos, char const *k, - bool *filter, int *flags, bool strict); + bool *filter, int *flags); options_t const *m_options; export_list m_export_list; diff --git a/src/tagtransform-lua.cpp b/src/tagtransform-lua.cpp index 7cd7034c1..20224b963 100644 --- a/src/tagtransform-lua.cpp +++ b/src/tagtransform-lua.cpp @@ -64,7 +64,7 @@ void lua_tagtransform_t::check_lua_function_exists(std::string const &func_name) } bool lua_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, - int *roads, taglist_t &out_tags, bool) + int *roads, taglist_t &out_tags) { switch (o.type()) { case osmium::item_type::node: diff --git a/src/tagtransform-lua.hpp b/src/tagtransform-lua.hpp index 2c5658eeb..1d2312587 100644 --- a/src/tagtransform-lua.hpp +++ b/src/tagtransform-lua.hpp @@ -30,7 +30,7 @@ class lua_tagtransform_t : public tagtransform_t std::unique_ptr clone() const override; bool filter_tags(osmium::OSMObject const &o, int *polygon, int *roads, - taglist_t &out_tags, bool strict = false) override; + taglist_t &out_tags) override; bool filter_rel_member_tags(taglist_t const &rel_tags, osmium::memory::Buffer const &members, diff --git a/src/tagtransform.hpp b/src/tagtransform.hpp index 0aaf4631e..eca8b0091 100644 --- a/src/tagtransform.hpp +++ b/src/tagtransform.hpp @@ -30,8 +30,7 @@ class tagtransform_t virtual std::unique_ptr clone() const = 0; virtual bool filter_tags(osmium::OSMObject const &o, int *polygon, - int *roads, taglist_t &out_tags, - bool strict = false) = 0; + int *roads, taglist_t &out_tags) = 0; virtual bool filter_rel_member_tags(taglist_t const &rel_tags, osmium::memory::Buffer const &members, From 2e0d8304def64a5dcac24b5e2923df153ea026c8 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sun, 25 Apr 2021 20:40:52 +0200 Subject: [PATCH 2/3] Refactoring: Remove parameter from filter_rel_member_tags() The parameter 'allow_typeless' is always false. --- src/tagtransform-c.cpp | 28 +++++++++++++--------------- src/tagtransform-c.hpp | 3 +-- src/tagtransform-lua.cpp | 2 +- src/tagtransform-lua.hpp | 3 +-- src/tagtransform.hpp | 3 +-- 5 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/tagtransform-c.cpp b/src/tagtransform-c.cpp index 896504d4a..6db926616 100644 --- a/src/tagtransform-c.cpp +++ b/src/tagtransform-c.cpp @@ -217,26 +217,24 @@ bool c_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, bool c_tagtransform_t::filter_rel_member_tags( taglist_t const &rel_tags, osmium::memory::Buffer const &, rolelist_t const &, int *make_boundary, int *make_polygon, int *roads, - taglist_t &out_tags, bool allow_typeless) + taglist_t &out_tags) { - //if it has a relation figure out what kind it is std::string const *type = rel_tags.get("type"); + if (!type) { + return true; + } + bool is_route = false; bool is_boundary = false; bool is_multipolygon = false; - if (type) { - //what kind of relation is it - if (*type == "route") { - is_route = true; - } else if (*type == "boundary") { - is_boundary = true; - } else if (*type == "multipolygon") { - is_multipolygon = true; - } else if (!allow_typeless) { - return true; - } - } //you didnt have a type and it was required - else if (!allow_typeless) { + + if (*type == "route") { + is_route = true; + } else if (*type == "boundary") { + is_boundary = true; + } else if (*type == "multipolygon") { + is_multipolygon = true; + } else { return true; } diff --git a/src/tagtransform-c.hpp b/src/tagtransform-c.hpp index b42dd2aba..1da97b66e 100644 --- a/src/tagtransform-c.hpp +++ b/src/tagtransform-c.hpp @@ -27,8 +27,7 @@ class c_tagtransform_t : public tagtransform_t osmium::memory::Buffer const &members, rolelist_t const &member_roles, int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags, - bool allow_typeless = false) override; + int *roads, taglist_t &out_tags) override; private: bool check_key(std::vector const &infos, char const *k, diff --git a/src/tagtransform-lua.cpp b/src/tagtransform-lua.cpp index 20224b963..3acf2e31c 100644 --- a/src/tagtransform-lua.cpp +++ b/src/tagtransform-lua.cpp @@ -151,7 +151,7 @@ bool lua_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, bool lua_tagtransform_t::filter_rel_member_tags( taglist_t const &rel_tags, osmium::memory::Buffer const &members, rolelist_t const &member_roles, int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags, bool) + int *roads, taglist_t &out_tags) { size_t const num_members = member_roles.size(); lua_getglobal(L, m_rel_mem_func.c_str()); diff --git a/src/tagtransform-lua.hpp b/src/tagtransform-lua.hpp index 1d2312587..70f34fe09 100644 --- a/src/tagtransform-lua.hpp +++ b/src/tagtransform-lua.hpp @@ -36,8 +36,7 @@ class lua_tagtransform_t : public tagtransform_t osmium::memory::Buffer const &members, rolelist_t const &member_roles, int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags, - bool allow_typeless = false) override; + int *roads, taglist_t &out_tags) override; private: void open_style(); diff --git a/src/tagtransform.hpp b/src/tagtransform.hpp index eca8b0091..2933f455e 100644 --- a/src/tagtransform.hpp +++ b/src/tagtransform.hpp @@ -36,8 +36,7 @@ class tagtransform_t osmium::memory::Buffer const &members, rolelist_t const &member_roles, int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags, - bool allow_typeless = false) = 0; + int *roads, taglist_t &out_tags) = 0; }; #endif // OSM2PGSQL_TAGTRANSFORM_HPP From c3647a223cdffb8d36ecd2876678f9e9df626587 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sun, 25 Apr 2021 21:06:52 +0200 Subject: [PATCH 3/3] Refactoring: Change a bunch of int variables/parameters to bool They are only used as boolean values anyway. --- src/output-pgsql.cpp | 14 +++++----- src/tagtransform-c.cpp | 58 ++++++++++++++++++++-------------------- src/tagtransform-c.hpp | 6 ++--- src/tagtransform-lua.cpp | 8 +++--- src/tagtransform-lua.hpp | 6 ++--- src/tagtransform.hpp | 8 +++--- 6 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/output-pgsql.cpp b/src/output-pgsql.cpp index 7f239ccbc..05f6698e9 100644 --- a/src/output-pgsql.cpp +++ b/src/output-pgsql.cpp @@ -84,8 +84,8 @@ void output_pgsql_t::pending_way(osmid_t id) pgsql_delete_way_from_output(id); taglist_t outtags; - int polygon = 0; - int roads = 0; + bool polygon = false; + bool roads = false; auto &way = buffer.get(0); if (!m_tagtransform->filter_tags(way, &polygon, &roads, outtags)) { auto nnodes = m_mid->nodes_get_list(&(way.nodes())); @@ -149,8 +149,8 @@ void output_pgsql_t::node_add(osmium::Node const &node) void output_pgsql_t::way_add(osmium::Way *way) { - int polygon = 0; - int roads = 0; + bool polygon = false; + bool roads = false; taglist_t outtags; /* Check whether the way is: (1) Exportable, (2) Maybe a polygon */ @@ -181,9 +181,9 @@ void output_pgsql_t::pgsql_process_relation(osmium::Relation const &rel) return; } - int roads = 0; - int make_polygon = 0; - int make_boundary = 0; + bool roads = false; + bool make_polygon = false; + bool make_boundary = false; taglist_t outtags; // If it's a route relation make_boundary and make_polygon will be false diff --git a/src/tagtransform-c.cpp b/src/tagtransform-c.cpp index 6db926616..0f8f2a46f 100644 --- a/src/tagtransform-c.cpp +++ b/src/tagtransform-c.cpp @@ -23,25 +23,25 @@ static const struct { int offset; char const *highway; - int roads; -} layers[] = {{1, "proposed", 0}, {2, "construction", 0}, - {10, "steps", 0}, {10, "cycleway", 0}, - {10, "bridleway", 0}, {10, "footway", 0}, - {10, "path", 0}, {11, "track", 0}, - {15, "service", 0}, - - {24, "tertiary_link", 0}, {25, "secondary_link", 1}, - {27, "primary_link", 1}, {28, "trunk_link", 1}, - {29, "motorway_link", 1}, - - {30, "raceway", 0}, {31, "pedestrian", 0}, - {32, "living_street", 0}, {33, "road", 0}, - {33, "unclassified", 0}, {33, "residential", 0}, - {34, "tertiary", 0}, {36, "secondary", 1}, - {37, "primary", 1}, {38, "trunk", 1}, - {39, "motorway", 1}}; - -void add_z_order(taglist_t &tags, int *roads) + bool roads; +} layers[] = {{1, "proposed", false}, {2, "construction", false}, + {10, "steps", false}, {10, "cycleway", false}, + {10, "bridleway", false}, {10, "footway", false}, + {10, "path", false}, {11, "track", false}, + {15, "service", false}, + + {24, "tertiary_link", false}, {25, "secondary_link", true}, + {27, "primary_link", true}, {28, "trunk_link", true}, + {29, "motorway_link", true}, + + {30, "raceway", false}, {31, "pedestrian", false}, + {32, "living_street", false}, {33, "road", false}, + {33, "unclassified", false}, {33, "residential", false}, + {34, "tertiary", false}, {36, "secondary", true}, + {37, "primary", true}, {38, "trunk", true}, + {39, "motorway", true}}; + +void add_z_order(taglist_t &tags, bool *roads) { std::string const *const layer = tags.get("layer"); std::string const *const highway = tags.get("highway"); @@ -54,7 +54,7 @@ void add_z_order(taglist_t &tags, int *roads) int l = layer ? (int)strtol(layer->c_str(), nullptr, 10) : 0; z_order = 100 * l; - *roads = 0; + *roads = false; if (highway) { for (const auto &layer : layers) { @@ -68,11 +68,11 @@ void add_z_order(taglist_t &tags, int *roads) if (railway && !railway->empty()) { z_order += 35; - *roads = 1; + *roads = true; } /* Administrative boundaries are rendered at low zooms so we prefer to use the roads table */ if (boundary && *boundary == "administrative") { - *roads = 1; + *roads = true; } if (bridge) { @@ -145,8 +145,8 @@ bool c_tagtransform_t::check_key(std::vector const &infos, return false; } -bool c_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, - int *roads, taglist_t &out_tags) +bool c_tagtransform_t::filter_tags(osmium::OSMObject const &o, bool *polygon, + bool *roads, taglist_t &out_tags) { //assume we dont like this set of tags bool filter = true; @@ -196,7 +196,7 @@ bool c_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, if (add_area_tag) { /* If we need to force this as a polygon, append an area tag */ out_tags.add_tag_if_not_exists("area", "yes"); - *polygon = 1; + *polygon = true; } else { auto const *area = o.tags()["area"]; if (area) { @@ -216,7 +216,7 @@ bool c_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, bool c_tagtransform_t::filter_rel_member_tags( taglist_t const &rel_tags, osmium::memory::Buffer const &, - rolelist_t const &, int *make_boundary, int *make_polygon, int *roads, + rolelist_t const &, bool *make_boundary, bool *make_polygon, bool *roads, taglist_t &out_tags) { std::string const *type = rel_tags.get("type"); @@ -323,12 +323,12 @@ bool c_tagtransform_t::filter_rel_member_tags( - Linear features will end up in the line and roads tables (useful for admin boundaries) - Polygon features also go into the polygon table (useful for national_forests) The edges of the polygon also get treated as linear fetaures allowing these to be rendered seperately. */ - *make_boundary = 1; + *make_boundary = true; } else if (is_multipolygon && out_tags.contains("boundary")) { /* Treat type=multipolygon exactly like type=boundary if it has a boundary tag. */ - *make_boundary = 1; + *make_boundary = true; } else if (is_multipolygon) { - *make_polygon = 1; + *make_polygon = true; } add_z_order(out_tags, roads); diff --git a/src/tagtransform-c.hpp b/src/tagtransform-c.hpp index 1da97b66e..e3ca8cf3b 100644 --- a/src/tagtransform-c.hpp +++ b/src/tagtransform-c.hpp @@ -20,14 +20,14 @@ class c_tagtransform_t : public tagtransform_t std::unique_ptr clone() const override; - bool filter_tags(osmium::OSMObject const &o, int *polygon, int *roads, + bool filter_tags(osmium::OSMObject const &o, bool *polygon, bool *roads, taglist_t &out_tags) override; bool filter_rel_member_tags(taglist_t const &rel_tags, osmium::memory::Buffer const &members, rolelist_t const &member_roles, - int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags) override; + bool *make_boundary, bool *make_polygon, + bool *roads, taglist_t &out_tags) override; private: bool check_key(std::vector const &infos, char const *k, diff --git a/src/tagtransform-lua.cpp b/src/tagtransform-lua.cpp index 3acf2e31c..2928afa83 100644 --- a/src/tagtransform-lua.cpp +++ b/src/tagtransform-lua.cpp @@ -63,8 +63,8 @@ void lua_tagtransform_t::check_lua_function_exists(std::string const &func_name) lua_pop(L, 1); } -bool lua_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, - int *roads, taglist_t &out_tags) +bool lua_tagtransform_t::filter_tags(osmium::OSMObject const &o, bool *polygon, + bool *roads, taglist_t &out_tags) { switch (o.type()) { case osmium::item_type::node: @@ -150,8 +150,8 @@ bool lua_tagtransform_t::filter_tags(osmium::OSMObject const &o, int *polygon, bool lua_tagtransform_t::filter_rel_member_tags( taglist_t const &rel_tags, osmium::memory::Buffer const &members, - rolelist_t const &member_roles, int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags) + rolelist_t const &member_roles, bool *make_boundary, bool *make_polygon, + bool *roads, taglist_t &out_tags) { size_t const num_members = member_roles.size(); lua_getglobal(L, m_rel_mem_func.c_str()); diff --git a/src/tagtransform-lua.hpp b/src/tagtransform-lua.hpp index 70f34fe09..1fb448ee4 100644 --- a/src/tagtransform-lua.hpp +++ b/src/tagtransform-lua.hpp @@ -29,14 +29,14 @@ class lua_tagtransform_t : public tagtransform_t std::unique_ptr clone() const override; - bool filter_tags(osmium::OSMObject const &o, int *polygon, int *roads, + bool filter_tags(osmium::OSMObject const &o, bool *polygon, bool *roads, taglist_t &out_tags) override; bool filter_rel_member_tags(taglist_t const &rel_tags, osmium::memory::Buffer const &members, rolelist_t const &member_roles, - int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags) override; + bool *make_boundary, bool *make_polygon, + bool *roads, taglist_t &out_tags) override; private: void open_style(); diff --git a/src/tagtransform.hpp b/src/tagtransform.hpp index 2933f455e..146ac3ec0 100644 --- a/src/tagtransform.hpp +++ b/src/tagtransform.hpp @@ -29,14 +29,14 @@ class tagtransform_t virtual std::unique_ptr clone() const = 0; - virtual bool filter_tags(osmium::OSMObject const &o, int *polygon, - int *roads, taglist_t &out_tags) = 0; + virtual bool filter_tags(osmium::OSMObject const &o, bool *polygon, + bool *roads, taglist_t &out_tags) = 0; virtual bool filter_rel_member_tags(taglist_t const &rel_tags, osmium::memory::Buffer const &members, rolelist_t const &member_roles, - int *make_boundary, int *make_polygon, - int *roads, taglist_t &out_tags) = 0; + bool *make_boundary, bool *make_polygon, + bool *roads, taglist_t &out_tags) = 0; }; #endif // OSM2PGSQL_TAGTRANSFORM_HPP