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

Do not render railway crossings if only tramways are involved (#4559). #4579

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion project.mml
Original file line number Diff line number Diff line change
Expand Up @@ -2220,7 +2220,6 @@ Layer:
way,
name,
COALESCE(
'railway_' || CASE WHEN railway IN ('level_crossing', 'crossing') AND way_area IS NULL THEN railway END,
'amenity_' || CASE WHEN amenity IN ('bench', 'waste_basket', 'waste_disposal') AND way_area IS NULL THEN amenity END,
'historic_' || CASE WHEN historic IN ('wayside_cross', 'wayside_shrine') AND way_area IS NULL THEN historic END,
'man_made_' || CASE WHEN man_made IN ('cross') AND way_area IS NULL THEN man_made END,
Expand Down Expand Up @@ -2278,6 +2277,30 @@ Layer:
properties:
cache-features: true
minzoom: 14
# Render railway crossings except if they concern tramways only
- id: railway-crossings
geometry: point
<<: *extents
Datasource:
<<: *osm2pgsql
table: &railway-crossing_sql |-
Copy link
Collaborator

Choose a reason for hiding this comment

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

The &railway-crossing_sql is unused and therefore should be removed.

(SELECT DISTINCT ON (crossing.way)
'railway_' || crossing.railway AS feature,
crossing.way AS way,
crossing.access AS access,
crossing.railway AS type,
crossing.railway AS railway,
track.railway AS int_lc_type
Comment on lines +2290 to +2293
Copy link
Collaborator

Choose a reason for hiding this comment

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

These appear to be unused so should be removed.

FROM planet_osm_point crossing
JOIN planet_osm_line track
ON ST_DWithin(crossing.way, track.way, 0.1) -- Assumes Mercator
WHERE crossing.railway IN ('crossing', 'level_crossing')
AND track.railway IS NOT NULL AND track.railway <> 'tram'
Copy link
Collaborator

Choose a reason for hiding this comment

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

That means all railway crossings that are wrongly mapped not on any kind of railway (tram or otherwise) are hidden and not visible to the map user. That is incompatible with our goal of providing constructive mapper feedback i think. We - as a general rule - do not hide features from rendering based on spatial analytics telling us that they are mapped incorrectly. If a mapper maps a railway crossing in the middle of nowhere we show that because the mapper either has a good reason for that or it is an error and should be corrected and showing it helps mappers in seeing such errors.

ORDER BY crossing.way
Copy link
Collaborator

Choose a reason for hiding this comment

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

Although this is not a newly introduced problem i would like to mention that we generally want to use explicit ordering in queries where the order of features returned matters (which is the case here because the symbols are blocking each other). It would therefore be advisable to order also by (a) priority based on railway type and (b) osm_id to always have a defined drawing order.

) AS railway_crossing_sql
properties:
cache-features: true
minzoom: 14
- id: text-low-priority
geometry: point
<<: *extents
Expand Down
3 changes: 2 additions & 1 deletion style/amenity-points.mss
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,8 @@
}
}

#amenity-low-priority {
#amenity-low-priority,
#railway-crossings {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since there is no overlap in style rules between the amenity-low-priority and railway-crossings layers the style rules for those should be separate.

[feature = 'man_made_cross'][zoom >= 16],
[feature = 'historic_wayside_cross'][zoom >= 16] {
marker-file: url('symbols/man_made/cross.svg');
Expand Down