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

Fix layering order for "construction=*" highway categories #3646

Merged
merged 3 commits into from
Jan 24, 2019

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Jan 17, 2019

Related to #3579

Changes proposed in this pull request:

  • Use osm_id as the last way to determine rendering order for roads-fill, so that different highway=construction roads will always layer in the same order, at different zoom levels. This change will take effect immediately.
  • Add "construction=" values to the z-order table in openstreetmap-carto.lua, so that the different categories of highway=construction will render in proper order (eg construction=motorway on top, construction=service on bottom). This will take effect on database reload, and for new or edited features.

Explanation

Highway=construction is currently set to z = 10 in the z-order table, so that it will render below almost all highways and paths. However, the different types of construction=*, such as construction=motorway, construction=trunk_link, construction=service, all have the same z value, so they are laid in apparently random fashion. The layering order often changes from one zoom level to the next.

These two commits will eventually fix this problem.

The change to project.mml, adding osm_id as a final fall-back for ordering, will at least cause all highway=construction to rendering in a consistent way between zoom levels, effective immediately.

The improvement to rendering for different classifications of highway=construction is part of the lua transformations, and will take effect gradually for new and edited features, and then for old features when the database is next reloaded.

EDITED: the lua transformation changes have been removed for now, because a database reload is not yet planned

Test rendering with links to the example places:

EDITED: These were with the first commits

Before
A6 - New Trunk Interchange - Northern Ireland
https://www.openstreetmap.org/#map=16/54.7629/-6.4847

  • construction=trunk_link above construction=trunk
    z15 Before
    z15-new-trunk-links-master
    z16 Before
    z16-new-trunk-links-master

After z15
z15-new-trunk-links-after
After z16
z16-new-trunk-links-after

Murry's Island - Northern Ireland
https://www.openstreetmap.org/#map=15/54.7734/-6.5189

  • New trunk road crosses an under-construction minor road and an under-construction secondary road (right) and an under-construction tertiary road (left)
    z15 Before
    z15-murrays-island-construction-master
    z15 After
    z15-murrays-island-construction-after

Castledawson - new residential area
https://www.openstreetmap.org/#map=16/54.7692/-6.5512
z16 Before
z16-annaghmore-construction-master
z17 Before
z17-annaghmore-construction-master

After z16
z16-annaghmore-construction-after
After z17
z17-annaghmore-construction-after

EDITED: See new test renderings below

…ayers properly

This change is in openstreetmap-carto.lua, so it will take effect for new features and after database reload
This will cause roads to layer consistently between different zoom level, though this will only be visible for highway=construction

Also adjust uncategorized construciton z-order and modify z-order for minor highways
@imagico
Copy link
Collaborator

imagico commented Jan 17, 2019

Support for ordering by osm_id, regarding z_order (and therefore database related) changes see #3644 (comment).

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 17, 2019 via email

@imagico
Copy link
Collaborator

imagico commented Jan 17, 2019

Your choice of approach is perfectly fine IMO but it will have to wait until the next database reload. I don't think adding complexity to the road layers' SQL to circumvent this would be a good idea.

Just have a bit of patience here - the last database reload was in August - openstreetmap/chef#155 (comment) and for previous discussion about database reload patterns see #1286. While this is a problem that would be good to fix it is not a high impact problem that frequently causes serious problems for map users or mappers.

Pending database reload, we cannot yet make changes to the openstreetmap-carto.lua file
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 17, 2019 via email

Copy link
Collaborator

@pnorman pnorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL looks good in principle, but I haven't tested it.

I was going to comment on splitting up the construction changes, but I see @imagico has already done that.

@jeisenbe
Copy link
Collaborator Author

New tests with the current commit:
https://www.openstreetmap.org/#map=6/54.7629/-6.4847

  • layering order is now consistent between different zoom levels

z17
z17-osm_id-layers

z16
z16-osm_id-layers

z15
z15-osm_id-layers

@matthijsmelissen matthijsmelissen merged commit c8e6bc2 into gravitystorm:master Jan 24, 2019
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 24, 2019 via email

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

Successfully merging this pull request may close these issues.

4 participants