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 aeroways to their own layer #2841

Merged
merged 1 commit into from
Sep 17, 2017
Merged

Conversation

pnorman
Copy link
Collaborator

@pnorman pnorman commented Sep 17, 2017

More progress on #2046. The cartographic changes are minimal and involve some strange real-world situations.

This is done as part of road layer simplification, but also fixes a few layering bugs showing up where there are aeroway bridges, and some casing width problems with invisible bridges.

There are no changes to the typical airport

image
(left v4.2.0 on osmf servers, right this PR)

The weirder situations are bridges
image
17/47.92228/-122.27635

image
16/33.6224/-84.4291

Where there is a road crossing a runway (e.g. Gibralter) the runway will remain rendered on top of the road.

There is one case of a runway tagged as a tunnel in OSM where the runway goes through a mountain in North Korea, and that case will remain unhandled and rendered like a normal runway.

The current rendering has bugs with bridge casing widths for aeroways, and these are fixed by using the same road casing constructions as highways.

The code changes are worth explaining, because they're the real reason for this PR

  • Aeroways are a special case of roads where we assume they're always on top of any roads (layer-wise). This assumption combined with the z_order rendering them on top means they can be their own layer. Most styles wouldn't bother with aeroway bridges

  • Fixing the casing width bugs brought up the MSS code length, otherwise it would be shorter. Because a lot of the new code is repetitive line-width stuff (e.g. [zoom >= 16] { line-width: 24 + 2*@major-casing-width-z16; }) I find it simpler

  • Using an attachment is a technique I mentioned in 2016 and is a better solution than duplicate layers for ground level and bridges.

tl;dr: Moves aeroways to their own layers, modernizes aeroway code, fixes aeroway bridge bugs.

This is done as part of road layer simplification, but also fixes
a few layering bugs showing up where there are aeroway bridges,
and some casing width problems with invisible bridges.
@pnorman
Copy link
Collaborator Author

pnorman commented Sep 17, 2017

Just to be clear, I think moving aeroways into the roads query was a mistake at the time, but none of us realized it.

@matthijsmelissen
Copy link
Collaborator

matthijsmelissen commented Sep 17, 2017

By moving aeroways to their own layer, we are removing the functionality where aeroways can be drawn below roads (for example for aeroways in tunnels, or for aeroways with a bridge over them). I'm not aware of any real-life example of this (apart from the North-Korea example), so unless somebody provides counterevidence, I think that's a safe thing to do. The opposite, roads on top of aeroways, occurs quite frequently but this we can still handle.

@matthijsmelissen
Copy link
Collaborator

Ok, apparently Phoenix has a sky train crossing a taxiway: https://osm.org/go/TlVi8S3el--?m= (pictures at http://photos.wikimapia.org/p/00/01/67/24/78_big.jpg). The current PR will break layering there. I have no strong opinion on whether we want to accept that or not.

@kocio-pl
Copy link
Collaborator

It won't be too visible because both colors are dark, so I would accept that if there are more general benefits.

@pnorman
Copy link
Collaborator Author

pnorman commented Sep 17, 2017

Ok, apparently Phoenix has a sky train crossing a taxiway: https://osm.org/go/TlVi8S3el--?m= (pictures at http://photos.wikimapia.org/p/00/01/67/24/78_big.jpg). The current PR will break layering there. I have no strong opinion on whether we want to accept that or not.

I'm fine with breaking that layering. Looking at the other layers on osm.org, they all either get that wrong or the other crossing at https://www.openstreetmap.org/?mlat=33.43478&mlon=-112.00423#map=18/33.43478/-112.00423 where the order is reversed.

In fact, if you look at bbbike map compare you can see a selection of 8 maps, all of which get it wrong in some way or another.

@pnorman pnorman merged commit 791a019 into gravitystorm:master Sep 17, 2017
@pnorman pnorman deleted the aeroway_split branch September 17, 2017 08:00
@pnorman
Copy link
Collaborator Author

pnorman commented Sep 17, 2017

Whoops - I thought this had an accepted review. Since we just did a release and none of the comments were objections, I'm going to leave it committed unless someone particularly wants to revert it.

@matthijsmelissen
Copy link
Collaborator

I think this change should be reviewed by somebody first. I don't think snybody had time to look at the PR on a code level. The roads code is quite complex and changes might easily have unexpected side-effects.

@matthijsmelissen
Copy link
Collaborator

Ok, fine for now.

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.

3 participants