Skip to content
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

Move control over road rendering order away from db import #4819

Open
imagico opened this issue May 24, 2023 · 0 comments
Open

Move control over road rendering order away from db import #4819

imagico opened this issue May 24, 2023 · 0 comments

Comments

@imagico
Copy link
Collaborator

imagico commented May 24, 2023

This topic came up in context of the move to the osm2pgsql flex backend (#4431) - but it is in fact unrelated so i am opening a new issue.

Background

The natural priority order of the road classes, i.e. which road types are drawn above which other road types in the absence of other tags like layer or tunnel/bridge, is currently defined through a z_order column in the database, which is set during database import by osm2pgsql based on a lookup table defined in LUA:

local roads_info = {
highway = {
motorway = {z = 380, roads = true},
trunk = {z = 370, roads = true},
primary = {z = 360, roads = true},
secondary = {z = 350, roads = true},
tertiary = {z = 340, roads = false},
residential = {z = 330, roads = false},
unclassified = {z = 330, roads = false},
road = {z = 330, roads = false},
living_street = {z = 320, roads = false},
pedestrian = {z = 310, roads = false},
raceway = {z = 300, roads = false},
motorway_link = {z = 240, roads = true},
trunk_link = {z = 230, roads = true},
primary_link = {z = 220, roads = true},
secondary_link = {z = 210, roads = true},
tertiary_link = {z = 200, roads = false},
service = {z = 150, roads = false},
track = {z = 110, roads = false},
path = {z = 100, roads = false},
footway = {z = 100, roads = false},
bridleway = {z = 100, roads = false},
cycleway = {z = 100, roads = false},
steps = {z = 90, roads = false},
platform = {z = 90, roads = false}
},

But it has not always been this way, between 2014 (#626) and 2017 (#2732) we used an inline lookup table in the road queries instead. The reasoning for that was:

Currently, rendering order of road rendering within one layer is handled
by the z_order column, which comes from osm2pgsql. As such, we have
little control over road rendering without reloading the database.
This PR moves control over the rendering order to the SQL query.

This adds complexity to the SQL queries, but increases customizability,
and simplifies the roads.mms code.

The main reasons for changing this back were:

  • we were not actually much using the customizability advantage
  • the additional complexity made changes to the road layers more difficult
  • the lookup table needed to be duplicated across several queries, making it awkward and error prone to maintain
  • osm2pgsql meanwhile allowed customization of the z_order via LUA (previously this was hardcoded within osm2pgsql)

However, the primary reasons for moving away from defining the drawing order during database import still apply. The road drawing order is inherently a styling decision. Hence it is unfortunate if this styling decision is fixed in the rendering database during import.

Another thing is that meanwhile dealing with the construction roads is moved to the database import stage as well (#4075) - adding additional constraints and side effects to the z_order values.

Options

There are mainly three options if we want to move the drawing order definition back to where it can be customized by the map designers without a database reload:

  • inlining a lookup table into the SQL queries again. This would come with the issues already mentioned so i would not recommend that.
  • separating the lookup table into an SQL view.
  • separating the lookup table into an SQL function.
  • storing the lookup table in the database.

Suggestion

My suggestion would be to go with one of the last two options. In either case the approach would be similar to the one we currently use for the indices - that means defining the drawing order in some yaml file that gets processed by a python script into SQL code to be run on the database.

For handling the construction roads i would suggest - on the fly at render time - recasting the construction value into the road class and having a yes/no construction column. This would not be unlike what we do right now for link roads.

Reasoning

The benefits of this would be:

  • it would be easier to introduce new road types or to change the drawing order both for us and for derivative styles (supporting our goals of adaptability and maintainability).
  • it would make testing changes to the road drawing order easier because testing could be done without reloading the test database.
  • setting the z_order values would be more intuitive and with less side effects to consider.
  • it would create a precedent for better modularization of our SQL code and provide a clear path to do de-duplication of SQL code elsewhere in the style. I am in particular thinking about Symbol and label prioritization is not in sync with the starting zoom levels #3880 here.

The main arguments against this are likely:

  • potential performance implications, since anything you do during database import of course does not need to be done at render time.
  • the additional need to install functions/lookup table when deploying the style.

Complexity wise it would be gaining some additional SQL complexity (though clearly modularized when going with a function or a database table) while loosing the LUA complexity of filling the z_order column.

To me it seems doubtful that for a simple feature-string-to-value lookup performance is a significant issue. In addition two remarks from what i wrote on #4431 (comment) already:

I don't think we should optimize for the ease of deployment if that comes at the cost of map design flexibility and ease of maintenance of the style. I also doubt that for the typical deployment installing a few views or functions is more of a difficulty than having to rebuild the database every time we make a z_order change.

So far on the SQL code level of this project (which - after the MSS code - is the second most important level where design work happens and that most map designers need to deal with) we are inlining all code in project.mml (with the exception of use of YAML features to de-duplicate completely identical queries). I don't think this is sustainable any more. In particular resolving #3880 in a scalable and future-proof way would not be possible under that paradigm. Same applies most likely for ground unit rendering (#4340, #4346). So introducing use of views, functions or lookup tables or a combination of such seems inevitable if we want to make progress on the larger issues this project faces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant