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

feat: Render roller_coaster=track ✨🎢 #4666

Merged
merged 17 commits into from May 30, 2023

Conversation

tjur0
Copy link
Contributor

@tjur0 tjur0 commented Aug 28, 2022

Fixes #3596

Changes proposed in this pull request:

  • Render roller_coaster=track

I based the design after typical rollercoaster track. I made sure that the track is not too wide, so small roller coasters and tracks next to each other render properly. While also making sure that it cannot be confused with railways.

Test rendering with links to the example places:

Before
Maverick:
before

After
Maverick zoom 18:
Maverick1_Z18

Cedar Point Ohio USA zoom 16:
CedarPoint1_Z18

Gemini zoom 18:
Gemini1_Z18

Millennium Force zoom 19:
MillenniumForce2_Z19

Millennium Force zoom 19:
MillenniumForce_Z19

Millennium Force zoom 18:
MillenniumForce3_Z18

@Eiim
Copy link

Eiim commented Aug 28, 2022

The tunnel rendering seems a bit too subtle. Also, in your last screenshot, some overlapping tracks conflict strangely - not sure if that's fixable or not. Definitely like the general design though, and roller coaster rendering is overdue.
image

@tjur0
Copy link
Contributor Author

tjur0 commented Aug 28, 2022

Yea, I’m not quite sure how that happens, but I think this is caused if the layers don’t change and it’s the same way, see: way/106610418
I agree that tunnels could use some work, I will fix that asap.

openstreetmap-carto.lua Outdated Show resolved Hide resolved
openstreetmap-carto.style Outdated Show resolved Hide resolved
project.mml Outdated Show resolved Hide resolved
project.mml Outdated Show resolved Hide resolved
project.mml Outdated Show resolved Hide resolved
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.

I haven't reviewed the cartography, but there's a few technical issues first.

I think you want to use attachments to get the layering correct for overlapping ways, but am not positive without checking.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

I have not tested this yet, but i would suggest to reconsider rendering of the name tag.

In contrast to roads/paths a roller coaster track does not usually have a local and locally verifiable name. The name that gets tagged will almost always be inferred from the name of the overall structure. And for tagging the name of the overall structure attraction=roller_coaster is clearly the more established method.

Also note cases of roller coaster tracks tagged with railway tags at the moment show that line labels of those often work poorly because they often end up overlapping other parts of the track, badly affecting readability in general:

https://www.openstreetmap.org/#map=18/49.03685/9.05351

I would suggest also to check and make a conscious choice about the results when a way is double tagged with roller_coaster=track and railway=* - which is a common occurrence at the moment.

project.mml Outdated Show resolved Hide resolved
project.mml Outdated Show resolved Hide resolved
style/roads.mss Outdated Show resolved Hide resolved
@tjur0
Copy link
Contributor Author

tjur0 commented Aug 29, 2022

Thank you all for the feedback 👍, I have removed the name rendering as I agree it looks cluttered. I chose not to render roller_coaster=track if there is a railway=*, to avoid double rendering. And I adjusted the tunnels and widths at z 20 and 19. I’m not sure how I can solve the extra table issue, so suggestions are welcome.

Example of double tagged and single tagged roller coasters:

Toverland, The Netherlands
zoom 18
Toverland1_Z18
(the indoor coaster looks weird because it is not tagged with covered='yes')

Improved tunnels:
zoom 18
MillenniumForce5_Z18

@tjur0 tjur0 requested review from pnorman and imagico and removed request for pnorman and imagico August 29, 2022 19:58
@tjur0 tjur0 changed the title feat: Add roller coaster rendering feat: Render roller_coaster=track ✨🎢 Aug 30, 2022
@Eiim
Copy link

Eiim commented Aug 31, 2022

I chose not to render roller_coaster=track if there is a railway=*, to avoid double rendering.

As most (if not all - can anyone come up with another real reason for double tagging? Perhaps there's a semantic argument to be made that a rollercoaster track is a type of rail track?) roller_coaster=track+railway=* taggings seem to be tagging for the renderer, it would probably be more correct to render roller_coaster rather than railway in those cases. However, from a mapper feedback perspective, rendering the "wrong" tag might get people to switch to the "correct" tagging quicker.

@tjur0
Copy link
Contributor Author

tjur0 commented Sep 4, 2022

can anyone come up with another real reason for double tagging?

I’ve thought about this the last past days, but I can’t think of a good reason why you can double tag a rollercoaster and a railway. You are right that roller coaster track is a type of rail track, however it is not a railway. This Is because the functions are different. A roller coaster only gets build for entertainment purposes. While a railway is a road that transports people or goods, except for some exceptions like educational railways.
These different functions lead to different design requirements for the roller coaster track, for example the roller coaster trains need to wrap around the track etc.
The reason that I chose not to render roller_coaster=track is there is railway=* are:

  • Less spread-out git diffs
  • Changes the least amount of stuff on the map
  • Gives an upgrade path for the roller coasters
  • If I do it the other way around, I’m afraid the double tagging remains

The last few days I have been experimenting with the correct background layer, I have added an line-cap:round background that makes shure there are no gaps between two track pieces. A problem with this is that where there is a looping, and the track makes an almost 180 degree turn the round fill-in is undesirable and looks weird. I need some more time and experimenting to get that right.

Comment on lines +1033 to +1034
CASE WHEN (tunnel = 'yes' OR tunnel = 'building_passage' OR covered = 'yes' OR tags->'indoor' = 'yes') THEN 'yes' ELSE 'no' END AS tunnel,
CASE WHEN (bridge = 'yes' OR bridge = 'covered' OR bridge = 'viaduct') THEN 'yes' ELSE 'no' END AS bridge
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had overseen this before - this extends the tunnel rendering to also cover indoor=yes and limits the bridge values compared to those supported for highway/railway, which is:

WHERE bridge IN ('yes', 'boardwalk', 'cantilever', 'covered', 'low_water_crossing', 'movable', 'trestle', 'viaduct')

@imagico
Copy link
Collaborator

imagico commented Oct 21, 2022

@matkoniecz - i think it is fine in general - especially compared to the monorail rendering, which is occasionally used right now as tagging for the renderer.

The bridge rendering with the changed black casing is however - as already hinted - somewhat too heavy. I'd suggest to narrow the bridge casing width (half of the current seems to work quite well, maybe a bit more at the highest zoom levels) and limit the bridge casing display to z16+.

@tjur0
Copy link
Contributor Author

tjur0 commented Oct 21, 2022

I have adjusted the bridge casing widths.

On roads a bridge is used when there is extra structure to hold up the road. With roller coasters this structure is always there and therefore you could say that roller_coaster=track has an implied bridge=yes.

I’m currently content with the rendering. But to comply with your statements above an option could be to remove the bridge=yes completely or always show it. What do the other maintainers think of this?

Like pnorman pointed out earlier there is not a real solution for this problem it is going to be a compromise anyhow. I feel like I have balanced the meaning of the secondary tags and rendered the data as is as good as possible.

In my opinion it is wrong to render covered sections always below normal sections. Let’s say there is a covered lift hill high up in the air, the track below the covered lift hill is not covered because of the height difference. And therefore, should be below the tunnel section.

But I could do something like this for the render order:
• Tunnel: Layer * 3
• Normal: Layer * 3 + 1
• Bridge: Layer * 3 + 2
Then it would respect the layer and tunnel/bridge render order.

@pnorman pnorman self-requested a review November 30, 2022 19:01
@pnorman pnorman requested a review from imagico January 8, 2023 22:34
@01110229
Copy link

Any updates ongoing?

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

My view here is essentially unchanged since i last reviewed this. The bridge casing seems fine now - because the signature of the roller coaster track is fairly heavy it is a bit difficult to recognize at times, but this is something that can be adjusted later as well.

I still think it would be best if the layering logic matches that of roads. After having looked at various roller coaster tracks in the database it is clear that there is no consensus among mappers about using layer/bridge/tunnel tagging on these with different semantics than for roads - hence providing feedback on this that is consistent with that for roads/railways seems prudent. If this is the goal, putting roller coaster tracks into the road layers would probably be more convenient than separate layers - even if interleaving between roads and roller coaster tracks is rarely necessary.

But this is not a big deal for me - if others are fine with the current approach i won't object.

I also still think the two things i remarked on the code (interpreting bridge and tunnel the same way as for roads, using @bridge-casing instead of a color literal) would be good to change. Yes,cases where bridge=* and covered=yes are used in combination are an open question - but this is the case for roads as well, nothing specific to roller coasters IMO.

But likewise - if others are fine with the change as is i would be ok with that.

@tjur0
Copy link
Contributor Author

tjur0 commented Mar 14, 2023

I'm happy to consider implementing changes to the rendering if there is a consensus that something needs to be changed.

In my opinion, adding road logic to the mapping of roller coaster tracks could be problematic since it may require the use of bridge=yes, which could potentially create confusion when it comes to verifying the actual bridge structure. As in contrast with roads there is not a clear point where an overlapping track “bridge” starts and ends. That in turn may incentivize adding bridge=yes to all parts of the track, which could make bridge=yes on roller coaster track meaningless.

I understand the importance of finding a solution that works well for everyone. I believe that the current rendering system is flexible enough to accommodate the forming of a consensus. Not having the extra tag bridge=* and covered=yes and adding the layering heuristics could inhibit the formation of a new consensus.

I would like to hear some more opinions on this matter.

@arminkollascheck
Copy link

Thanks you for working on this topic.

I have the same opinion as you @tjur0 . On roller coasters you should not have to mark everthing as bridge=yes (what makes it indead meaningless). To add bridge=yes only to parts where it passing over a other roller coaster track is even more problematic. As you said, there would not be a clear end or start of this bridge, a bad idea. Using just layers is better imo.

@imagico
In the rules of Mapping it reads "Bridge=* tags should not be used just because tracks overlap. There should be an actual bridge structure underneath the track." (https://wiki.openstreetmap.org/wiki/Tag:roller_coaster%3Dtrack)

But as you also said, since many mappers map for the openstreetmap carto map (even it should not be goal to map for a map layout), there is much chaos in mapping roller coasters like having bridge=yes everywhere or railway=..... It even gets changed many times on the same roller coaster over the time.

@imagico
Copy link
Collaborator

imagico commented Mar 19, 2023

My (certainly limited) studying of mapping of roller coaster tracks indicates that the semantics of bridge=yes and layer=* of roller coaster tracks matches that of roads where it is applied, the main difference seems to be that it is applied less (i.e. there are quite a lot of tracks which completely lack this kind of information). Yes, there are a few cases where mappers either tag a whole track with bridge=yes because they are assuming that tag to mean feature is above ground level rather than an actual bridge (in the sense of how the tag is used on roads) or cases where no bridge tagging is used at all, not even for the actual places where the track bridges over itself or a road or something else. But these are clearly in the minority or represent cases of coarse, preliminary, non-detailed mapping.

The key here is to consider our goal to support mappers in consistent use of tags. For that the bridge vs. non-bridge difference is not an issue, both this change and the traditional road rendering provide intuitive feedback. The handling of layer=* on non-bridge features is the problem here. On that this change would provide fundamentally different feedback indicating different meaning of the layer=* tag on roller coasters. Mapper consensus on layer=* on non-bridge features is that it describes the relative vertical relationship between non-connecting features. Where features connect on a shared node different layer values are not considered to disconnect the features. For roads this is pretty fundamental to allow data users to correctly interpret the tagging and most map styles rely on this.

@tjur0
Copy link
Contributor Author

tjur0 commented Mar 19, 2023

I can’t wrap my head around that you keep comparing roller coaster tracks to roads. Maybe that is my mistake from implementing many road features.

From the start I took the water slide rendering as inspiration because IMO the closes thing to roller coaster tracks are water slides. And those also layer correctly without additional tags.
https://www.openstreetmap.org/way/936392687
I do not see the issue of having this different implementation of the layer tag.

@tjur0 tjur0 requested review from imagico and removed request for pnorman March 19, 2023 12:57
@imagico
Copy link
Collaborator

imagico commented Mar 19, 2023

I can’t wrap my head around that you keep comparing roller coaster tracks to roads.

I can see that and therefore i tried to explain in detail how - as far as i can see - the semantics of layer/bridge/tunnel tags on roller coaster tracks and on roads are the same for mappers and that there does not seem to be a consensus on a different meaning of those on roller coaster tracks. I am open to the possibility that this is a wrong impression but i would need evidence for that. Someone would need to explain to me the distinct meaning of those tags for roller coaster tracks.

From the start I took the water slide rendering as inspiration because IMO the closes thing to roller coaster tracks are water slides.

I see. Two things to keep in mind here:

  • Water slides were added in Water slide #3346 without consensus (and initially even without any explicit layering - see Layer not respected in attraction=water_slide #3413). There was no conscious decision back then regarding how this supports or goes against our goal of providing constructive mapper feedback. If such a discussion had happened back then the arguments brought up here could well have already been brought up back then.
  • The situation with water slides is a bit different because those contain junctions/switches much less frequently than roller coaster tracks (in other words: these tend to be topologically simple).

Again - i am not categorically against the approach chosen, my aim is for us to make a conscious and well informed choice here taking the different arguments into account.

@arminkollascheck
Copy link

arminkollascheck commented Mar 20, 2023

I looked at some parks in the US and Europe and I found no evidence that the bridge tag is often used for roller coasters/crossing roller coaster tracks. Most of the time the layers get used. Obviouly this is not a complete overlook.

https://www.openstreetmap.org/edit#map=18/38.51458/-90.67468 [no bridge tag, different layers]
https://www.openstreetmap.org/edit#map=17/42.03964/-72.61536 [no bridge tag, different layers]
https://www.openstreetmap.org/edit#map=17/40.13800/-74.43518 [mapped as monorail]
https://www.openstreetmap.org/edit#map=18/34.42291/-118.59733 [no bridge tag, but also no layers]
https://www.openstreetmap.org/edit#map=16/48.2656/7.7222 [some mapped as railway, some not but with layers]
https://www.openstreetmap.org/edit#map=16/53.0238/9.8836 [bridge tag gets used]
https://www.openstreetmap.org/edit#map=16/53.0238/9.8836 [as railway, only tunnel tag gets used]
https://www.openstreetmap.org/edit#map=17/52.98740/-1.89116 [as railway, only layers, sometimes bridge tag]

@jespervd2
Copy link

Is this mergeable? It looks so cool!😁

@arminkollascheck
Copy link

arminkollascheck commented May 29, 2023

Also wonder what is still needed now for a merge? Would be cool to have it included.

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.

Add rendering for roller_coaster
10 participants