From b0cd6fb66d5f32a969834f0f70a965d4f40e4f30 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Thu, 28 Nov 2024 11:33:34 +0100 Subject: [PATCH 1/2] Boundary lines in shortbread have no names Do not add the names from boundary ways to the output. Some ways have names but use is inconsistent and generally not useful in this context. --- themes/shortbread_v1/topics/boundaries.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/themes/shortbread_v1/topics/boundaries.lua b/themes/shortbread_v1/topics/boundaries.lua index 95d3e44..9d8aefd 100644 --- a/themes/shortbread_v1/topics/boundaries.lua +++ b/themes/shortbread_v1/topics/boundaries.lua @@ -11,7 +11,7 @@ themepark:add_table{ name = 'boundaries', ids_type = 'way', geom = 'linestring', - columns = themepark:columns('core/name', { + columns = themepark:columns({ { column = 'admin_level', type = 'int' }, { column = 'maritime', type = 'bool' }, { column = 'disputed', type = 'bool' }, @@ -71,7 +71,7 @@ themepark:add_proc('way', function(object, data) disputed = info.disputed or (t.disputed and t.disputed == 'yes'), geom = object:as_linestring() } - themepark.themes.core.add_name(a, object) + themepark:insert('boundaries', a, t) end) From 51c341d52ab5a00a035b960c51fe356bdf2213c4 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Thu, 28 Nov 2024 11:22:51 +0100 Subject: [PATCH 2/2] Fix shortbread boundary processing in regards to disputed boundaries The way disputed boundaries are implemented was not correct. Generally boundaries should be processed as if disputed boundaries do not exists. But then boundary lines are flagged as disputed if they are created from ways * that have the tag disputed=yes, or * that are in a relation tagged boundary=disputed with no admin_level set or an admin_level smaller or equal to the admin_level that the boundary line has (which is the smallest of any boundary relation the way is a member of) Note that the relation that marks a way as disputed is not the same relation that marks a way as part of some specific admin boundary. The first is tagged boundary=disputed, the second is tagged boundary=administrative. This is based on the work in #10 but rewrites the code to be (hopefully) easier to understand. Note that this only fixes the version for shortbread_v1, the version for shortbread_v1_gen also needs updating which will come later. --- themes/shortbread_v1/topics/boundaries.lua | 84 ++++++++++++++-------- 1 file changed, 53 insertions(+), 31 deletions(-) diff --git a/themes/shortbread_v1/topics/boundaries.lua b/themes/shortbread_v1/topics/boundaries.lua index 9d8aefd..46bff10 100644 --- a/themes/shortbread_v1/topics/boundaries.lua +++ b/themes/shortbread_v1/topics/boundaries.lua @@ -12,7 +12,7 @@ themepark:add_table{ ids_type = 'way', geom = 'linestring', columns = themepark:columns({ - { column = 'admin_level', type = 'int' }, + { column = 'admin_level', type = 'int', not_null = true }, { column = 'maritime', type = 'bool' }, { column = 'disputed', type = 'bool' }, }), @@ -29,27 +29,33 @@ themepark:add_table{ } } -local rinfos = {} +-- --------------------------------------------------------------------------- +-- Storage of information from boundary relations for use by boundary ways +-- (two-stage processing). + +-- Minimum admin level of all relations that reference a way id +local min_admin_level = {} + +-- Minimum admin level of all relations tagged boundary=disputed that +-- reference a way id +local min_disputed_admin_level = {} -- --------------------------------------------------------------------------- --- Check if this looks like a boundary and return admin_level as number --- Return nil if this is not a valid boundary. -local function get_admin_level(tags) - local type = tags.type +-- Shortbread is only interested in level 2 and level 4 admin boundaries. +local function is_admin_boundary(tags) + return (tags.type == 'boundary' or tags.type == 'multipolygon') + and tags.boundary == 'administrative' + and (tags.admin_level == '2' or tags.admin_level == '4') +end - if type == 'boundary' or type == 'multipolygon' then - local boundary = tags.boundary - if boundary == 'administrative' or boundary == 'disputed' then - return tonumber(tags.admin_level) - end +-- Get numerical admin level from string, default to 1 if invalid +local function get_admin_level(value) + if not value or not string.match(value, '^[1-9][0-9]?$') then + return 1 end -end --- Check the (numeric) admin level. Change this depending on which admin --- levels you want to process. Shortbread only shows 2 and 4. -local function valid_admin_level(level) - return level == 2 or level == 4 + return tonumber(value) end -- --------------------------------------------------------------------------- @@ -59,16 +65,25 @@ themepark:add_proc('way', function(object, data) return end - local info = rinfos[object.id] - if not info then + local admin_level = min_admin_level[object.id] + if not admin_level then return end local t = object.tags + + -- Set disputed flag either from disputed tag on the way... + local disputed = (t.disputed == 'yes') + + -- .. or from a parent relation with boundary=disputed + if min_disputed_admin_level[object.id] and min_disputed_admin_level[object.id] <= admin_level then + disputed = true + end + local a = { - admin_level = info.admin_level, + admin_level = admin_level, maritime = (t.maritime and (t.maritime == 'yes' or t.natural == 'coastline')), - disputed = info.disputed or (t.disputed and t.disputed == 'yes'), + disputed = disputed, geom = object:as_linestring() } @@ -76,28 +91,35 @@ themepark:add_proc('way', function(object, data) end) themepark:add_proc('select_relation_members', function(relation) - if valid_admin_level(get_admin_level(relation.tags)) then + if is_admin_boundary(relation.tags) then return { ways = osm2pgsql.way_member_ids(relation) } end end) themepark:add_proc('relation', function(object, data) - local t = object.tags + if is_admin_boundary(object.tags) then + local admin_level = tonumber(object.tags.admin_level) - local admin_level = get_admin_level(t) + for _, id in ipairs(osm2pgsql.way_member_ids(object)) do + if not min_admin_level[id] or min_admin_level[id] > admin_level then + min_admin_level[id] = admin_level + end + end - if not valid_admin_level(admin_level) then return end - for _, member in ipairs(object.members) do - if member.type == 'w' then - if not rinfos[member.ref] then - rinfos[member.ref] = { admin_level = admin_level } - elseif rinfos[member.ref].admin_level > admin_level then - rinfos[member.ref].admin_level = admin_level + if object.tags.boundary == 'disputed' then + -- Ways in relations tagged boundary=disputed are flagged as disputed + -- if either the relation doesn't have an admin_level tag or the + -- admin_level tag is <= the admin level the way got from the + -- boundary=administrative relation(s). + local admin_level = get_admin_level(object.tags.admin_level) + + for _, id in ipairs(osm2pgsql.way_member_ids(object)) do + if not min_disputed_admin_level[id] or min_disputed_admin_level[id] > admin_level then + min_disputed_admin_level[id] = admin_level end - rinfos[member.ref].disputed = (t.boundary == 'disputed') end end end)