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

Reorder layers to get all the road layers together #4464

Closed
wants to merge 5 commits into from

Conversation

pitdicker
Copy link
Contributor

@pitdicker pitdicker commented Sep 9, 2021

The intention of this PR is to get all road layers together. This is a first step of working on the road styling complexity. Next steps for other PRs would be to unify the road layers.

I would like to ask for review on this first step. Whether it is possible to reorder the layers like this impacts the ability to simplify the road styling.

Layers before this pr

  • bridge (polygon instead of way)
  • buildings
  • tunnels
    • ::casing [zoom >= 12]
    • ::bridges_and_tunnels_background [zoom >= 12]
    • ::halo [zoom < 12]
    • ::fill
  • landuse-overlay (military area hatch)
  • tourism-boundary (zoo, theme-park)
  • barriers
  • cliffs (cliff, ridge, arete, embankment)
  • ferry-routes
  • turning-circle-casings
  • highway-area-casing
  • roads-casing
    • ::casing
  • highway-area-fill
  • roads-fill
    • ::halo [zoom < 12]
    • ::fill
  • turning-circle-fill
  • aerialways
  • roads-low-zoom
    • ::halo
    • ::fill
  • waterway-bridges (aqueducts)
  • bridges
    • ::casing [zoom >= 12]
    • ::bridges_and_tunnels_background [zoom >= 12]
    • ::halo [zoom < 12]
    • ::fill
  • guideways
  • entrances
  • aeroway (runway & taxiway)

The layers landuse-overlay, tourism-boundary, barriers, cliffs and ferry-routes should move before tunnels.
To be moved after the road layers are aerialways, waterway-bridges and entrances.
Is it acceptable for all these layers to be moved out of the way?

Moving some layers before tunnels

While tunnels are physically underneath under basically all objects, they should often be rendered on top. As said in #3025 (comment): "rendering underground roads under features on the ground would make them invisible."

Something to consider is that underground roads are currently even rendered on top of buildings. Is for each of the following layers that feature really important enough to be rendered on top?

landuse-overlay

This is only used for the military area hatch. Plenty of layers are already drawn on top of this one. I see no problem in moving it up a bit, to just on top of buildings. That would put in on top of only landcovers and buildings, which seems about right to me. A tunnel underneath a military area (is that an existing combination?) would be rendered on top of the hatch.

tourism-boundary

Used for zoos and theme-parks. The boundary is already covered by roads. This makes sense for entrances, and is a bit unfortune when there are park roads running close along the boundary. I don't think moving tunnels to be on top of it really changes rendering for the worse. And again, probably a rare combination.

barriers

Whether to render barriers above or below roads is an issue: #528, #2394, #3872. highway-area-fill should render on top of roads-casing, roads-casing on top of barriers, and barriers on top of highway-area-fill.

Without touching that issue... A barrier on this layer doesn't obstruct the tunnel. It can be equally reasonable to draw the road on top, as it is to draw the barrier on top. Rendering roads is currently considered more important than rendering barriers, and I would like to extend that to tunnels.

cliffs

Cliff, ridge, arete and embankment are terrain features, defined in the landcover MSS. In my opinion, it makes sense to move this layer to right after landcover-area-symbols and icesheet-outlines.

Compared to now, that would cause a couple of water-related features such as a dam, weir or pier to be drawn on top of them (should be rare, maybe those can exist near a cliff?). Buildings and barriers would also be drawn on top. I consider this an improvement.

Tunnels, which are the issue here, would also be drawn on top. I can't really call this better or worse.

ferry-routes

At low zoom ferry routes are rendered underneath roads. At higher zoom tunnels would move underneath the ferry route.I don't think it matters much.

Layers to move after roads

waterway-bridges

The layer waterway-bridges with aquaducts should remain on top of roads. Is there a problem when it is also on top of bridges? Conceptually I can imagine bridges should be drawn on top of aquaducts. I can't help but imagine a bridge over an aquaduct to be rare, and not worth the simplifications it would stand in the way.
Edit: Moved to be after guideways.

aerialways

Moving aerialways with cable_cars etc. on top of all road-related features and after waterway-bridges aeroways (edit) seems like nothing but an improvement.

entrances

Having the layer with building entrances on top of buildings and roads is logical. I see no problem in moving it a bit further. For now I have put it after aerialways, but can imagine it would even be good to put it around aminity-points. Edit: This layer doesn't have to move, as aeroway doesn't have to be merged into the road layers after all. But I put it to just before amenities, as that seems to just be better.

@pitdicker
Copy link
Contributor Author

I would like to try continue the work in #2869 to reduce the complexity of the road styles. What seems to me to be the biggest problem is that is hard to divide the work into managable, reviewable portions.

My current plan of attack is:

  • Start from scratch, but work in the same direction.
  • Reorder road layers. All the non-road layers should be either before or after.
  • Probably split out low zoom styling (zoom < 12). Styling at those zoom levels is different enough that there is little shared with the higher zooms.
  • Merge the layers, likely one by one per pr. I hope this makes it possible to reason about the impact of the changes in layering. This is pretty much guaranteed to temporary make the MSS more complex.
  • Refactor the MSS style. Once much is in one layer with attachments it should be relatively easy to move things around in the MSS without altering cartography.

Problems I expect to hit :-):

  • Little knowledge of the history of the current style. But I can read up on previous PRs and issues.
  • No knowledge of the OSM database and of SQL.
  • CartoCSS seems doable.
  • Let's hope I have enough time to see things through to the end.

The best way to help this out would be for someone to pair up with me and we could work on it together. This would also be a good chance for someone to learn more about complex CartoCSS. The new roads code is simpler, but roads are still one of our biggest areas.

@pnorman Is this still an offer? Would you be willing to review and/or mentor me? Or can we maybe even divide up the work between us?

@pitdicker pitdicker changed the title Roads refactor Reorder layers to get all the road layers together Sep 9, 2021
@pnorman pnorman self-requested a review September 9, 2021 19:20
@pnorman
Copy link
Collaborator

pnorman commented Sep 11, 2021

@pnorman Is this still an offer? Would you be willing to review and/or mentor me? Or can we maybe even divide up the work between us?

Yes, although my schedule tends to be busy.

@pitdicker
Copy link
Contributor Author

Yes, although my schedule tends to be busy.

Great! And I can understand, don't feel pressure from me.

@pitdicker
Copy link
Contributor Author

Fixed merge conflict, and made some changes to the layers that got moved after the road layers. See edits in first post.

@imagico
Copy link
Collaborator

imagico commented Sep 13, 2021

My reading of this at the moment is that you would like (for reasons you have not yet gotten into details about - but i assume they are technical and not cartographic in nature) to render all road features of this style in a compact block within the layer stack and to add this as a firm technical constraint for future style development (no non-road feature shall ever come between the road features in terms of drawing order).

To accomplish that you try to rationalize moving each and everyone of the non-road layers in between the road layers currently out of there - either above or below. Now don't get me wrong - for some of the layers that move is likely to be a good choice cartographically. The problem is however that your motivation is purely technical in nature, you want technically to get these layers out of the way independent of how good or bad that is from a cartographic perspective.

It seems self evident to me that from a cartographic perspective it can be necessary to place non-road features in between road features in terms of drawing order. There is for example also the possibility to use compositioning (comp-op) to selective layer different features above other features in different ways (see #3854 for where we are considering something like that). Even if we should decide in this style to not do so the need for that is likely to arise in derivative style projects and us making decisions that make this impossible is not a good idea IMO.

So my suggestion would be: Whatever you exactly try to accomplish here in the end - work on a way to do that which is compatible to having non-road features in between the road features. If and when this is accomplished then we can look at which of the layers currently interleaved with the road layers makes sense to be moved from a cartographic perspective and which are better to be kept there.

@pitdicker
Copy link
Contributor Author

@imagico That is an excellend description of the situation here.

The problem is however that your motivation is purely technical in nature, you want technically to get these layers out of the way independent of how good or bad that is from a cartographic perspective.

It seems self evident to me that from a cartographic perspective it can be necessary to place non-road features in between road features in terms of drawing order.

I appreciate your comments, they are what I hoped for when opening this PR. Now I know the approach has to be a bit different.

You have a good point: simplifications should not stand in the way of current and future cartographic wishes. On the other hand: flexibility has a cost. The complexity in the roads styling currently makes it hard to work on a number of open issues. And currently there are four layers with a huge and mostly identical 77-line SQL query.

So my suggestion would be: Whatever you exactly try to accomplish here in the end - work on a way to do that which is compatible to having non-road features in between the road features.

An essential part of what would make the styling more maintainable is to merge some layers so that related features are grouped together in the mss. Surely we don't need to preserve all the current 11 layers? I think the following devision would give enough flexibility while allowing some refactoring:

  • tunnels
  • (room for other layers)
  • roads (combination of turning-circle-casings, highway-area-casing, roads-casing, highway-area-fill, roads-fill and turning-circle-fill)
  • (room for other layers)
  • bridges
  • (room for other layers)
  • road overlays (now only guideways)

@imagico What would be your opinion on this?

Closing as his PR should not be merged.

In the first post there were three layers of which I thought moving them would be an improvement. Maybe I'll open a new PR for those. But probably not, as I don't have concrete examples of places that would benefit.

@pitdicker pitdicker closed this Sep 13, 2021
@imagico
Copy link
Collaborator

imagico commented Sep 13, 2021

An essential part of what would make the styling more maintainable is to merge some layers so that related features are grouped together in the mss.

I am not sure that would be the case. The current MSS code already has style rules that apply across different layers. So MSS complexity is not dependent on the number of layers really. I completely agree that removing code duplication, especially of complex SQL queries, across multiple layers would be beneficial. But i don't think that is the core issue.

The main problem with our roads rendering is that it has serious complexity making it difficult to maintain but at the same time it is seriously limited cartographically and many of the things we might like to do (like getting proper layering (including tunnels and bridges) for highway areas - see #2046, though we have also considered going the opposite route with #3872) are impossible without adding a lot of additional complexity.

I am honestly not sure what the best route is with the current tool set. I have experimented with a radical take on the subject and that indeed offers great flexibility - but debugging any SQL changes is hard in that setup. However there are also components of that (like moving the line width logic from MSS code to a separate YAML file - and from there via script to SQL - either in the form of CASE statements or using additional styling data tables) which we could think about adopting here without taking the radical step.

And the technique @pnorman showed in #2046 (comment) - which i adopted in my changes - can also be used to integrate other features within the road feature stack in a single layer if desirable. So don't take my comment as discouraging work on the roads in a this can't work kind of way. As i said my suggestion is just to work on a way to do that which is compatible to having non-road features in between the road features.

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