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 stream, ditch and drain tunnels at z14 #3938

Closed
wants to merge 1 commit into from

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Oct 19, 2019

Fixes #3676

Changes proposed in this pull request:

  • Do not render stream, ditch and drain tunnels at z14

Explanation:

  • Currently stream, ditch and drain waterway tunnels are rendered the same as above-ground waterways at z14
  • At z15 and above they have a special tunnel rendering.
  • This commit removes the rendering at z14, since at this level intermittent streams are no longer rendered. It would not make sense to render stream culverts at z14 but not intermittent streams.

Test rendering:

https:/www.openstreetmap.org/#map=15/36.9649/-121.9659
Before
z15-rodeo-creek-same

z14-rodeo-creek-before

After
z14-rodeo-creek-after-no-tunnel

https:/www.openstreetmap.org/#map=15/37.1896/-122.2085
Before
z14-sempervirens-stream-tunnel-before

z15-sempervirens-stream-tunnel-before

After
z14-sempervirens-stream-tunnel-after

Currently stream, ditch and drain waterway tunnels are rendered the same as above-ground waterways at z14
At z15 and above they have a special tunnel rendering. This commit removes the rendering at z14
@kocio-pl
Copy link
Collaborator

Before and after images in the wild looks the same to me, probably the line is too short here for the changes to be seen.

Otherwise it makes sense to me.

@jeisenbe
Copy link
Collaborator Author

I've fixed the images for the second example, they were the same previously. The difference is subtle for both examples, because even a 100 meter long tunnel is quite short at this zoom level.

@imagico
Copy link
Collaborator

imagico commented Oct 21, 2019

I have my doubts about this. Removing selectively rendering of waterway tunnels removes display continuity of waterways along their route - they'd disappear and re-appear potentially at unconnected points.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Oct 21, 2019 via email

@imagico
Copy link
Collaborator

imagico commented Oct 21, 2019

I would also be happy to change this to show the tunnel style at z14
instead, except that intermittent streams are not being rendering at
z14. It would be strange to not show intermittent streams at this
level, since they can actually be seen, but to show tunnels which are
not visible.

I don't really see this connection. The intermittent nature of a stream is completely independent of it running in a tunnel or not.

The main purpose of stream display at the lower zoom levels is showing the drainage network. For this it makes sense to start by rendering streams uniformly and continuously and not interrupt their display if there is a tunnel. I don't really mind much showing the tunnels distinctly but IMO rendering would be much cleaner if they are shown like normal waterways at the lower zoom levels.

The example shown in #3676 is not very good since it is showing incorrect use of waterway=stream + tunnel=yes.

@jeisenbe
Copy link
Collaborator Author

The example shown in #3676 is not very good since it is showing incorrect use of waterway=stream + tunnel=yes.

There are waterway=drain and waterway=ditch features in long tunnels in some places, and they are not part of the natural drainage network. Should these be shown at z14?

@Adamant36
Copy link
Contributor

This waterway here is probably miss-tagged due to my ignorance at the time, but imagine it wasn't. In my opinion there's zero reason to show it going through the retail area at z14 as it just adds clutter and get's in the way of the parking lot being rendered. Plus, it makes it seem like it's over ground when it's not. Is Anyone going to be looking for a long tunnel full of water (or not) like that one at z14 on this map? My guess is probably not. Nothing would be taken away from it not being displayed I don't think either. If it wasn't displayed it would be more cohesive visually because it would be clearer where it goes under ground and comes out again.

z17
z17
z14
z14

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 5, 2019

@imagico, there are some waterway=drain and waterway=ditch features in long tunnels in some places, and they are not part of the natural drainage network. Should these be shown at z14?

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 8, 2020

@imagico please see question above: #3938 (comment)

@imagico
Copy link
Collaborator

imagico commented Jan 9, 2020

I would need to see specific cases of long ditch/drain tunnels that cannot be reasonably rendered as normal waterways at lower zoom levels without causing major confusion. Keep in mind we have no way of distinguishing between long and short tunnels in rendering because waterways can be split arbitrarily.

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

Successfully merging this pull request may close these issues.

Inconsistent tunnel=yes rendering for streams, drains and ditches
4 participants