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

Combine road lines and areas to one layer #2046

Open
pnorman opened this issue Feb 16, 2016 · 42 comments
Open

Combine road lines and areas to one layer #2046

pnorman opened this issue Feb 16, 2016 · 42 comments
Labels
layering Issues related to the layering structure of the style roads

Comments

@pnorman
Copy link
Collaborator

pnorman commented Feb 16, 2016

From #1975 (comment)

Doing this will reduce the number of layers and allow future MSS simplifications

@pnorman pnorman added the roads label Feb 16, 2016
@pnorman pnorman self-assigned this Feb 17, 2016
@pnorman
Copy link
Collaborator Author

pnorman commented Feb 17, 2016

@math1985

Right now the layers we have are

  • tunnels, class tunnels-fill tunnels-casing access directions
  • barrier-like features and landuse overlay
  • ferry-routes
  • turning-circle-casing
  • highway-area-casing
  • roads-casing, class roads-casing
  • highway-area-fill
  • roads-fill, class roads-fill access directions
  • turning-circle-fill
  • aerialways
  • roads-low-zoom
  • waterway-bridges,
  • bridges, class bridges-fill bridges-casing access directions

Entries in bold are versions of the large 6kb road query.

I've successfully brought highway-area-fill into roads-fill. On initial inspection, roads-casing is the same as roads-fill, so there will be two layers returning the same results.

Would it then be possible to combine them into one layer?

@matthijsmelissen
Copy link
Collaborator

Great work!

Yes, roads-casing and roads-fill can be merged without problem. Merging highway-area-fill and roads-fill was the hard part. For tunnels and bridges, roads-casing and roads-fill are in fact already merged.

It probably also makes sense then to bring highway-area-casing into roads-casing in the same way.

To merge tunnels with the main road query, we have two options:

  1. Include ferry-routes and turning-circle-casing into the main road query. This will lead to an even huger query, but with less SQL in total as we don't need a separate tunnels query. As a side effect, it would also improve the rendering of turning circles on layers/bridges.
  2. Render ferry-routes and turning-circle-casing under tunnels. It is an option, but it would probably not look great.

To merge bridges with the main road query, we need to get rid of the layers turning-circle-fill, aerialways, roads-low-zoom, and waterway-bridges.

I think aerialwayscan be moved above bridges without problem. This seems a good idea even independent from the other changes, as most aerialways go indeed over bridges.

The layer roads-low-zoom can also be moved above bridges without problem, as it is only used in layers up to z9, while turning-circle-fill and waterway-bridges are only used in higher zoomlevels

For the remaining layers turning-circle-fill and waterway-bridges, we have two options again.

  1. Include turning-circle-fill and waterway-bridges into the main road query. That would lead again to a huge query, but with less total SQL. It might also improve rendering a bit.
  2. Render turning circles and waterway-bridges above bridges. The first would probably not look great, the second will improve the rendering in some places, and make it worse in others.

@pnorman
Copy link
Collaborator Author

pnorman commented Feb 18, 2016

It probably also makes sense then to bring highway-area-casing into roads-casing in the same way.

Right now if you have road 1 with layer 1 and road 2 with layer 2, you'd get

  • road 1 casing
  • road 2 casing
  • road 1 fill
  • road 2 fill

If you used attachments instead of having two layers, you'd get

  • road 1 casing
  • road 1 fill
  • road 2 casing
  • road 2 fill

I guess this works, since it's already being done for tunnels and bridges?

  1. Render ferry-routes and turning-circle-casing under tunnels. It is an option, but it would probably not look great.

Ferries might be okay. As it is, they're not designed to work well over tunnels.
image

I'll try moving ferries before tunnels without doing any refactoring, since that should let me know how it works. New York City is the only test area I have for this - overlapping ferries and tunnels are not the most common.

I think aerialways can be moved above bridges without problem. This seems a good idea even independent from the other changes, as most aerialways go indeed over bridges.

I'll split this into its own PR, as it stands by itself.


I don't want to include either ferries or waterway-bridges in the road/rail query. Turning circles, maybe - that will require some thought.

Mapbox Streets Vector Tiles end up using a tunnel, road, and bridge layer. That would have the advantage it'd make it easier to place stuff between tunnels and roads. But getting down to one query for all roads except low zoom has a huge advantage for reducing duplication, and should open up some MSS simplifications.

@mboeringa
Copy link

If you used attachments instead of having two layers, you'd get

road 1 casing
road 1 fill
road 2 casing
road 2 fill

I guess this works, since it's already being done for tunnels and bridges?

Am I right to understand that this would mean that two road sections interconnected but with different layer=x tag, would no longer visually merge their fills, like what you see happening when normal roads connect to tunnels (e.g. here: http://www.openstreetmap.org/#map=18/40.70733/-74.01535)? This would lead to many undesirable artefacts of apparently unconnected roads near bridges and viaducts. I think this would be undesirable in that case. It might actually be solvable by no longer rendering the casing "rounded" at the end point of lines, but instead to cut them straight, and have the rounded fill extend it allowing visual connection.

@matthijsmelissen
Copy link
Collaborator

I guess this works, since it's already being done for tunnels and bridges?

No, actually this wouldn't work, good point.

For tunnels, this is solved by keeping the fill visible, as in @mboeringa's link. This is good for tunnels, but wouldn't work for regular roads.

For bridges, this is solved by having casing straight instead of rounded. This leads to gaps in casing for rounded roads, for example here. It's the same issue as #470. I don't think introducing such gaps would look great.

@pnorman
Copy link
Collaborator Author

pnorman commented Mar 31, 2016

I've been working on this in a slightly different context, and ran into an issue I can't seem to get around with line-caps. We have line-cap: butt on bridges to let the fill cover the bridge casing at the end of bridges. The problem I run into when I have all roads in one layer is a way with layer=0 joined to a way with layer=1, neither with bridge. There is typically then a way with layer=1 bridge=yes, but this one does not take part in the bug.

image

I cannot see a way around it. Using line-cap: butt would cause problems at every dead-end street.

The current code does not have this problem because the order is road layer 0 casing, road layer 1 casing, road layer 0 fill, road layer 1 fill.

Using one layer would change it to road layer 0 casing, road layer 0 fill, road layer 1 casing, road layer 1 fill. This is the same order that is used in the current code for tunnels and bridges. This means that there is a bug with bridges that have a dead end, but these are very rare.

image

@matthijsmelissen
Copy link
Collaborator

True. However, our ordering means that we don't really use the layer tag for non-bridge/tunnel roads. If two non-bridge/tunnel roads with a different layer cross, we don't show which way is on top by the layers. The only way the layer tag has an effect if when two roads with different colour cross: then the intersection gets the colour of the road on top. However, that's a fairly subtle effect compared to using casing to show which road is on top, so I think we can say in practice we ignore the layer tag for non-bridge/tunnel roads. That's in line with the wiki by the way, as it specifies that a layer-tag needs a tunnel or bridge tag,

As we basically ignore the layer tag now for non-bridge/tunnel roads, I think a new solution could also ignore the layer tag for these roads. We could for example rewrite the layer tag to 0 in the query if there is no tunnel or bridge tag. That would avoid this problem, wouldn't it?

@gravitystorm
Copy link
Owner

That's in line with the wiki by the way, as it specifies that a layer-tag needs a tunnel or bridge tag,

Then the wiki is wrong (again, frustratingly). Layer tags are useful for roads that are vertically separated yet parallel, e.g. North Street and the M8 through Glasgow:

20452
jpeg image 620 x 414 pixels

(North street is on the left of the photo, running parallel to the sunken motorway but neither of them are bridges nor tunnels. Since at many zoom levels the roads are drawn wider than reality, the drawn roads overlap, and which ends up on top can be controlled by the layer tags. Although separate highway tags in this situation, it's easy to see they might have the same highway tag elsewhere in the world).

I would like to see the layer information used for the stacking of roads, and use bridge/tunnel tags only for decoration. That makes it most robust, and avoids the potential for people adding fake bridge/tunnel tags to situations where the layer tags are being ignored.

@matthijsmelissen
Copy link
Collaborator

North Street and the M8 through Glasgow:

These streets don't seem to be crossing or overlapping, so I'm not sure if a layer tag makes sense here.

@gravitystorm
Copy link
Owner

I added more details under the photo, but you can see in the map extract that they are overlapping.

@dieterdreist
Copy link

sent from a phone

Am 31.03.2016 um 10:13 schrieb math1985 notifications@github.com:

That's in line with the wiki by the way, as it specifies that a layer-tag needs a tunnel or bridge tag,

the wiki doesn't say this, it says:
"With some exceptions, layer=* on ways should be used only in combination with one of tunnel=, bridge=, highway=steps, highway=elevator, covered=* or indoor=yes."

@pnorman
Copy link
Collaborator Author

pnorman commented Mar 31, 2016

So

  • Getting the ends right cannot be done with a single roads layer
  • Some maps use subtle or no casings which hides the problem
  • Other maps make the assumption that layer and bridge go together
  • With vector tiles there are some tricks, but they're not applicable here
  • The solution is to draw a casing for everything first, then render casing/fill as normal. The initial casing gets round end-caps while the normal casing gets butt end-caps and the normal fill gets round end-caps.

Taking this a bit farther,

  • We don't actually need to draw a casing everywhere, just way end points as circles

@matthijsmelissen
Copy link
Collaborator

Sounds good, I think this should work.

@matthijsmelissen
Copy link
Collaborator

Upon closed thought, I think it actually does not work. Consider the situation where two roads meet under an angle of 90 degrees., one road on layer 1 and the other road on layer 0. The top (normal, butt-end) casing of the road on layer 1 will now be extended too far into the intersection.

@pnorman
Copy link
Collaborator Author

pnorman commented Mar 31, 2016

Upon closed thought, I think it actually does not work. Consider the situation where two roads meet under an angle of 90 degrees., one road on layer 1 and the other road on layer 0. The top (normal, butt-end) casing of the road on layer 1 will now be extended too far into the intersection.

I initially thought you were wrong, so I drew the layers below, except put the bottom casing as a different color.

image

And we currently see this problem with bridges

image

Doing an initial bottom casing would fill in the missing casing on the left, but the casing at the right protruding into the fill is the problem we're talking about.

image

Because we don't have topology, I think this inherently unsolvable while maintaining casings separating roads on different layers.

@matthijsmelissen
Copy link
Collaborator

And yet another problem with this approach: dead-end, or sharply bent, streets on top of tunnels. Real-life example Your proposed approach would drop the round casing from this dead-end way, making it look like the service road leads to the tunnel.

Also inherently unsolvable without topology.

@mboeringa
Copy link

Because we don't have topology, I think this inherently unsolvable while maintaining casings separating roads on different layers.

+ 1, at some point, people should stop pixel peeking and accept this is not a hand-drawn map.

In my ArcGIS Renderer, I am facing similar minor issues. But the overall map look, even laser-printed, is OK, so I don't bother.

@pnorman
Copy link
Collaborator Author

pnorman commented Apr 1, 2016

A minor technical detail: Even though end points involve less rendering, I've come to prefer rendering the entire road to just the end point because that way everything involves line-* properties and some MSS can be combined.

The query I had for road ends was

(WITH
  bbox_roads AS (SELECT way, z_order, class FROM roads WHERE way && !bbox!),
  end_nodes AS (SELECT unnest(ARRAY[ST_PointN(way, 1), ST_PointN(way,ST_NPoints(way))]) AS way, z_order, class FROM bbox_roads)
SELECT DISTINCT ON (way)
    way,
    class
  FROM end_nodes
  ORDER BY way, z_order DESC -- take the topmost most important road
) AS road_ends

@StyXman
Copy link
Contributor

StyXman commented Apr 3, 2016

I noticed the n-plication of very similar 9and sometimes identical) queries on the layers. All those queries are run independently, (over)loading the database (and its connections). What would actually be needed to solve this is two different things: a) support in the styling/layering code to reference a single query and b) support in mapnik to actually execute it as such.

It could also be interesting to be able to reuse a previous query for a certain (meta)tile for rendering the tiles in the next zoom level (thinking of a pyramid rendering scheme). I guess I'm simply calling for another renderer altogether.

For the moment, I can only think of creating more _parts and then referencing them in the .yaml, but that doesn't solve the performance part of the problem.

@pnorman
Copy link
Collaborator Author

pnorman commented Apr 4, 2016

I did a clean implementation of roads rendering from scratch

layer definitions
MSS

The road styling is simpler, combining some classifications, but it does handle the way as highway and railway case. It ends up being a lot cleaner than other implementations, and has a few features I'd like to implement in osm-carto. It doesn't yet have road text or shields.

  • Not counting low zoom, it has two layers for all roads/rail, and the layers have identical SQL
    • Having identical SQL means I only need to include it once in the YAML
  • Cascading is used to get a minimum of duplication in the MSS
  • The MSS is 326 lines, while osm-carto roads.mss is 2375 lines, not including text or access styles. Part of this is we have more distinct appearances for roads, but a big part is having fewer layers

Given the huge reduction in code size and maintenance burden we could, I'm convinced that going forward is a good idea, even if some edge-cases with layering and casing look worse.

@matthijsmelissen
Copy link
Collaborator

Looks promising.

Of course the decrease in code size alone doesn't tell too much, as is for a large part achieved by removing functionality and by moving complexity to the .lua file.

I hacked the code a bit to get it to work with a standard rendering database. In my set-up, your code renders all roads on top of bridges. Does it do the same for you or is my set-up to blame?

Apart from that I don't immediately see major issues in Luxembourg.

@pnorman
Copy link
Collaborator Author

pnorman commented Sep 17, 2017

In my set-up, your code renders all roads on top of bridges. Does it do the same for you or is my set-up to blame?

Just to note that this is probably an issue with the hacking you did on the code, in particular, something related to enum types and sorting.


After doing a bit of editing of the SQL queries, I think how I'm going to tackle this is to reimplement the roads code, rather than trying to change it bit by bit.

@matthijsmelissen
Copy link
Collaborator

I suppose you meant roads below waterway bridges, or did you actually mean roads above waterway bridges? My bet is you won't find the latter, as the entire point of waterway bridges, at least in the Netherlands, is to allow tall ships to pass through unhindered.

@matthijsmelissen
Copy link
Collaborator

matthijsmelissen commented Sep 18, 2017

Ok, never say never: here a canal near Birmingham crossing the River Tame, with a motorway bridge passing above: https://www.openstreetmap.org/way/115858978

@pnorman
Copy link
Collaborator Author

pnorman commented Sep 18, 2017

I suppose you meant roads below waterway bridges, or did you actually mean roads above waterway bridges?

Nope, I meant roads above waterway bridges. I'd expect roads below to be the common case.

If we move the waterway bridge layer above the roads layers then we'd be able to merge fill and tunnels, and get the common case right. We'd get the road over waterway bridge case wrong

@matthijsmelissen
Copy link
Collaborator

Ok, then the Birmingham example will do.

@pnorman
Copy link
Collaborator Author

pnorman commented Sep 18, 2017

Looking at it and three bridges I suspect we're currently getting waterway bridges over road bridges wrong. Anyways, the next step is to put my head down and write some MSS

@matthijsmelissen
Copy link
Collaborator

Looking at it and three bridges I suspect we're currently getting waterway bridges over road bridges wrong.

Correct.

@pnorman
Copy link
Collaborator Author

pnorman commented Sep 19, 2017

After work I did a bunch more CartoCSS, and now have the skeleton of something that works: https://github.com/pnorman/openstreetmap-carto/tree/roads_rewrite/master

  • roads only
  • lines and turning circles, no areas
  • no tunnels

The general goal is to keep all the code for a road type reasonably close together

@sommerluk
Copy link
Collaborator

+                CASE lines.highway -- Use the most important type for cases where the TC is at an intersection. TODO: Should lines.layer be in here?
+                  WHEN 'tertiary' THEN 6
+                  WHEN 'unclassified' THEN 5
+                  WHEN 'residential' THEN 4
+                  WHEN 'living_street' THEN 3
+                  WHEN 'service' THEN 2
+                  WHEN 'track' THEN 1
+                END DESC,

Is it possible to use simply z_order from highway instead this?

@pnorman
Copy link
Collaborator Author

pnorman commented Sep 19, 2017

Is it possible to use simply z_order from highway instead this?

Yep, good catch.

Once I do rail styling I'll open a PR so we can do a more detailed review. My main concern at this point is the general approach, which consists of

  • One query, unioning lines, areas, and TC points
  • impacts of what is effectively moving anything between tunnels and bridges either before or after all roads
  • the general form of the MSS, which is
#transportation {
  [class selector] {
    ::casing {
      [geom type] {
        styling
      }
    }
  }
}

@matthijsmelissen
Copy link
Collaborator

matthijsmelissen commented Sep 6, 2019

I think we should have all areas beneath all roads (in general, area features should render below line features). For example, a tunnel should render on top of an area (oitherwise the tunnel would be invisible). Therefore, having two layers would suffice: one layer for area features, and one layer for road features. There is no need for merging the line and area layers.

I agree with the mentioned issues, but the solution proposed in this ticket is not the way forward.

@matthijsmelissen
Copy link
Collaborator

Re-reading this, I see this ticket is not about merhing lines and areas together, but merging all lines and all areas. That's still useful so I'm re-opening this.

matthijsmelissen pushed a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Sep 6, 2019
This PR moves highway areas below all linear road types (resolves gravitystorm#3281).

In particular, this change has the following effects:

* Render highway areas below tunnels (resolves gravitystorm#529). This prevents the current
  situation where tunnels are invisible if there happens to be a highway area
  above them.
* Render highway areas below line/area barriers, ferry routes, tourism
  boundaries, cliffs, landuse-overlay, and turning circles. I think these
  changes are mainly neutral or positive.
* Render highway areas below line/area barriers. This is probably the most
  controversial aspect (but necessary for the other changes). See screenshot
  below.

* This PR is a necessary condition for merging the roads-casing and roads-fill
  layers (part of gravitystorm#2046), which would greatly simplify our code.
* This PR is a necessary condition for rendering buildings above highway area
  (gravitystorm#688).

* Promoting linear highways over areas makes life easier for other data
  consumers, that already tend to have poor support for highway areas. For
  example:
  * Transport and wikipedia do not render road areas
  * Humanitarian, bicycle, mapbox, streets, OSM bright renders linear roads on
  top of areas, like in this proposal
@pnorman pnorman removed their assignment Mar 8, 2020
@imagico imagico added the layering Issues related to the layering structure of the style label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
layering Issues related to the layering structure of the style roads
Projects
None yet
Development

No branches or pull requests

9 participants